Unverified Commit dd2f8185 authored by Doug Lauder's avatar Doug Lauder Committed by GitHub
Browse files

MM-27184 cherry pick (deprecate model.SetExpireInDays to fix mobile logout bug)

parent d02e4861
......@@ -36,7 +36,6 @@ imports/imports.go
# Folders
_obj
_test
.vscode
testfiles
# Architecture specific extensions/prefixes
......@@ -73,6 +72,10 @@ Session.vim
.netrwhist
*~
# VSCode project files
.vscode
*.code-workspace
# Gogland project files
mattermost-server.iml
......
......@@ -1854,7 +1854,7 @@ func attachDeviceId(c *Context, w http.ResponseWriter, r *http.Request) {
}
c.App.ClearSessionCacheForUser(c.App.Session().UserId)
c.App.Session().SetExpireInDays(*c.App.Config().ServiceSettings.SessionLengthMobileInDays)
c.App.SetSessionExpireInDays(c.App.Session(), *c.App.Config().ServiceSettings.SessionLengthMobileInDays)
maxAge := *c.App.Config().ServiceSettings.SessionLengthMobileInDays * 60 * 60 * 24
......
......@@ -279,6 +279,10 @@ type AppIface interface {
SetBotIconImage(botUserId string, file io.ReadSeeker) *model.AppError
// SetBotIconImageFromMultiPartFile sets LHS icon for a bot.
SetBotIconImageFromMultiPartFile(botUserId string, imageData *multipart.FileHeader) *model.AppError
// SetSessionExpireInDays sets the session's expiry the specified number of days
// relative to either the session creation date or the current time, depending
// on the `ExtendSessionOnActivity` config setting.
SetSessionExpireInDays(session *model.Session, days int)
// SetStatusLastActivityAt sets the last activity at for a user on the local app server and updates
// status to away if needed. Used by the WS to set status to away if an 'online' device disconnects
// while an 'away' device is still connected
......
......@@ -133,7 +133,7 @@ func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User,
session.GenerateCSRF()
if len(deviceId) > 0 {
session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthMobileInDays)
a.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthMobileInDays)
// A special case where we logout of all other sessions with the same Id
if err := a.RevokeSessionsForDeviceId(user.Id, deviceId, ""); err != nil {
......@@ -141,11 +141,11 @@ func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User,
return err
}
} else if isMobile {
session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthMobileInDays)
a.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthMobileInDays)
} else if isOAuthUser || isSaml {
session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthSSOInDays)
a.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthSSOInDays)
} else {
session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthWebInDays)
a.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthWebInDays)
}
ua := uasurfer.Parse(r.UserAgent())
......
......@@ -366,7 +366,7 @@ func (a *App) newSession(appName string, user *model.User) (*model.Session, *mod
// Set new token an session
session := &model.Session{UserId: user.Id, Roles: user.Roles, IsOAuth: true}
session.GenerateCSRF()
session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthSSOInDays)
a.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthSSOInDays)
session.AddProp(model.SESSION_PROP_PLATFORM, appName)
session.AddProp(model.SESSION_PROP_OS, "OAuth2")
session.AddProp(model.SESSION_PROP_BROWSER, "OAuth2")
......
......@@ -77,7 +77,7 @@ func TestOAuthRevokeAccessToken(t *testing.T) {
session.UserId = model.NewId()
session.Token = model.NewId()
session.Roles = model.SYSTEM_USER_ROLE_ID
session.SetExpireInDays(1)
th.App.SetSessionExpireInDays(session, 1)
session, _ = th.App.CreateSession(session)
err = th.App.RevokeAccessToken(session.Token)
......@@ -119,7 +119,7 @@ func TestOAuthDeleteApp(t *testing.T) {
session.Token = model.NewId()
session.Roles = model.SYSTEM_USER_ROLE_ID
session.IsOAuth = true
session.SetExpireInDays(1)
th.App.SetSessionExpireInDays(session, 1)
session, _ = th.App.CreateSession(session)
......
......@@ -13218,6 +13218,21 @@ func (a *OpenTracingAppLayer) SetSearchEngine(se *searchengine.Broker) {
a.app.SetSearchEngine(se)
}
func (a *OpenTracingAppLayer) SetSessionExpireInDays(session *model.Session, days int) {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SetSessionExpireInDays")
a.ctx = newCtx
a.app.Srv().Store.SetContext(newCtx)
defer func() {
a.app.Srv().Store.SetContext(origCtx)
a.ctx = origCtx
}()
defer span.Finish()
a.app.SetSessionExpireInDays(session, days)
}
func (a *OpenTracingAppLayer) SetStatusAwayIfNeeded(userId string, manual bool) {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SetStatusAwayIfNeeded")
......
......@@ -346,6 +346,9 @@ func (a *App) ExtendSessionExpiryIfNeeded(session *model.Session) bool {
session.ExpiresAt = newExpiry
a.AddSessionToCache(session)
mlog.Debug("Session extended", mlog.String("user_id", session.UserId), mlog.String("session_id", session.Id),
mlog.Int64("newExpiry", newExpiry), mlog.Int64("session_length", sessionLength))
auditRec.Success()
auditRec.AddMeta("extended_session", session)
return true
......@@ -369,6 +372,17 @@ func (a *App) GetSessionLengthInMillis(session *model.Session) int64 {
return int64(days * 24 * 60 * 60 * 1000)
}
// SetSessionExpireInDays sets the session's expiry the specified number of days
// relative to either the session creation date or the current time, depending
// on the `ExtendSessionOnActivity` config setting.
func (a *App) SetSessionExpireInDays(session *model.Session, days int) {
if session.CreateAt == 0 || *a.Config().ServiceSettings.ExtendSessionLengthWithActivity {
session.ExpiresAt = model.GetMillis() + (1000 * 60 * 60 * 24 * int64(days))
} else {
session.ExpiresAt = session.CreateAt + (1000 * 60 * 60 * 24 * int64(days))
}
}
func (a *App) CreateUserAccessToken(token *model.UserAccessToken) (*model.UserAccessToken, *model.AppError) {
user, err := a.Srv().Store.User().Get(token.UserId)
......@@ -438,7 +452,7 @@ func (a *App) createSessionForUserAccessToken(tokenString string) (*model.Sessio
} else {
session.AddProp(model.SESSION_PROP_IS_GUEST, "false")
}
session.SetExpireInDays(model.SESSION_USER_ACCESS_TOKEN_EXPIRY)
a.SetSessionExpireInDays(session, model.SESSION_USER_ACCESS_TOKEN_EXPIRY)
session, nErr := a.Srv().Store.Session().Save(session)
if nErr != nil {
......
......@@ -352,3 +352,55 @@ func TestApp_ExtendExpiryIfNeeded(t *testing.T) {
}
}
const (
dayInMillis = 86400000
grace = 5 * 1000
thirtyDays = dayInMillis * 30
)
func TestApp_SetSessionExpireInDays(t *testing.T) {
th := Setup(t)
defer th.TearDown()
now := model.GetMillis()
createAt := now - (dayInMillis * 20)
tests := []struct {
name string
extend bool
create bool
days int
want int64
}{
{name: "zero days, extend", extend: true, create: true, days: 0, want: now},
{name: "zero days, extend", extend: true, create: false, days: 0, want: now},
{name: "zero days, no extend", extend: false, create: true, days: 0, want: createAt},
{name: "zero days, no extend", extend: false, create: false, days: 0, want: now},
{name: "thirty days, extend", extend: true, create: true, days: 30, want: now + thirtyDays},
{name: "thirty days, extend", extend: true, create: false, days: 30, want: now + thirtyDays},
{name: "thirty days, no extend", extend: false, create: true, days: 30, want: createAt + thirtyDays},
{name: "thirty days, no extend", extend: false, create: false, days: 30, want: now + thirtyDays},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
th.App.UpdateConfig(func(cfg *model.Config) {
*cfg.ServiceSettings.ExtendSessionLengthWithActivity = tt.extend
})
var create int64
if tt.create {
create = createAt
}
session := &model.Session{
CreateAt: create,
ExpiresAt: model.GetMillis() + dayInMillis,
}
th.App.SetSessionExpireInDays(session, tt.days)
// must be within 5 seconds of expected time.
require.GreaterOrEqual(t, session.ExpiresAt, tt.want-grace)
require.LessOrEqual(t, session.ExpiresAt, tt.want+grace)
})
}
}
......@@ -115,6 +115,9 @@ func (me *Session) IsExpired() bool {
return false
}
// Deprecated: SetExpireInDays is deprecated and should not be used.
// Use (*App).SetSessionExpireInDays instead which handles the
// cases where the new ExpiresAt is not relative to CreateAt.
func (me *Session) SetExpireInDays(days int) {
if me.CreateAt == 0 {
me.ExpiresAt = GetMillis() + (1000 * 60 * 60 * 24 * int64(days))
......
......@@ -128,7 +128,7 @@ func TestHandlerServeCSRFToken(t *testing.T) {
IsOAuth: false,
}
session.GenerateCSRF()
session.SetExpireInDays(1)
th.App.SetSessionExpireInDays(session, 1)
session, err := th.App.CreateSession(session)
if err != nil {
t.Errorf("Expected nil, got %s", err)
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment