diff --git a/.gitignore b/.gitignore index 022b1dcfa7..04d6a57ff3 100644 --- a/.gitignore +++ b/.gitignore @@ -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 diff --git a/api4/user.go b/api4/user.go index 3326115551..b2d14d1b67 100644 --- a/api4/user.go +++ b/api4/user.go @@ -1863,7 +1863,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 diff --git a/app/app_iface.go b/app/app_iface.go index 9a403e5133..da6ddecafe 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -275,6 +275,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 diff --git a/app/login.go b/app/login.go index 135fb29d47..2795f95777 100644 --- a/app/login.go +++ b/app/login.go @@ -132,7 +132,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 { @@ -140,11 +140,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 isOAuth || 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()) diff --git a/app/oauth.go b/app/oauth.go index 25d6196cc5..1514ffe9ac 100644 --- a/app/oauth.go +++ b/app/oauth.go @@ -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") diff --git a/app/oauth_test.go b/app/oauth_test.go index f69a01aad9..60e28db747 100644 --- a/app/oauth_test.go +++ b/app/oauth_test.go @@ -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) diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index f83cfa08fb..34e4252a47 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -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") diff --git a/app/session.go b/app/session.go index 54a7fca821..6da7c61589 100644 --- a/app/session.go +++ b/app/session.go @@ -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) @@ -444,7 +458,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 { diff --git a/app/session_test.go b/app/session_test.go index 801eeb0e54..7aee85d249 100644 --- a/app/session_test.go +++ b/app/session_test.go @@ -350,3 +350,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) + }) + } +} diff --git a/model/session.go b/model/session.go index 1cbb276062..06f3e003b5 100644 --- a/model/session.go +++ b/model/session.go @@ -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)) diff --git a/web/handlers_test.go b/web/handlers_test.go index e569d14389..bbf8a0f3e1 100644 --- a/web/handlers_test.go +++ b/web/handlers_test.go @@ -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)