From 627dd650d6026468689d22180d5646064c3bd55a Mon Sep 17 00:00:00 2001 From: mkraft Date: Tue, 15 Feb 2022 12:50:56 -0500 Subject: [PATCH] Revert "MM-41211: Adds ability to set SessionLengthSSOInDays to a decimal. (#19396)" (#19563) This reverts commit 739845103059c1472d15cfe9bcb745698600e30c. --- api4/user.go | 2 +- app/app_iface.go | 4 ++-- app/login.go | 8 +++---- app/oauth.go | 2 +- app/oauth_test.go | 4 ++-- app/opentracing/opentracing_layer.go | 6 ++--- app/session.go | 20 ++++++++-------- app/users/session.go | 8 +++---- app/users/session_test.go | 4 ++-- i18n/en.json | 4 ---- model/builtin.go | 9 ++++--- model/config.go | 20 ++-------------- model/session.go | 36 ++++++++++++++-------------- services/upgrader/upgrader.go | 1 - web/handlers_test.go | 2 +- 15 files changed, 54 insertions(+), 76 deletions(-) diff --git a/api4/user.go b/api4/user.go index 1463ef5880..ab0084aed4 100644 --- a/api4/user.go +++ b/api4/user.go @@ -2080,7 +2080,7 @@ func attachDeviceId(c *Context, w http.ResponseWriter, r *http.Request) { } c.App.ClearSessionCacheForUser(c.AppContext.Session().UserId) - c.App.SetSessionExpireInHours(c.AppContext.Session(), *c.App.Config().ServiceSettings.SessionLengthMobileInHours()) + c.App.SetSessionExpireInDays(c.AppContext.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 97bbffe4ec..fc2402b912 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -280,10 +280,10 @@ type AppIface interface { SessionHasPermissionToManageBot(session model.Session, botUserId string) *model.AppError // SessionIsRegistered determines if a specific session has been registered SessionIsRegistered(session model.Session) bool - // SetSessionExpireInHours sets the session's expiry the specified number of days + // 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. - SetSessionExpireInHours(session *model.Session, hours int) + SetSessionExpireInDays(session *model.Session, days int) // SetStatusDoNotDisturbTimed takes endtime in unix epoch format in UTC // and sets status of given userId to dnd which will be restored back after endtime SetStatusDoNotDisturbTimed(userId string, endtime int64) diff --git a/app/login.go b/app/login.go index 0ca1392dd9..255595d7cb 100644 --- a/app/login.go +++ b/app/login.go @@ -178,7 +178,7 @@ func (a *App) DoLogin(c *request.Context, w http.ResponseWriter, r *http.Request session.GenerateCSRF() if deviceID != "" { - a.ch.srv.userService.SetSessionExpireInHours(session, *a.Config().ServiceSettings.SessionLengthMobileInHours()) + a.ch.srv.userService.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 { @@ -186,11 +186,11 @@ func (a *App) DoLogin(c *request.Context, w http.ResponseWriter, r *http.Request return err } } else if isMobile { - a.ch.srv.userService.SetSessionExpireInHours(session, *a.Config().ServiceSettings.SessionLengthMobileInHours()) + a.ch.srv.userService.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthMobileInDays) } else if isOAuthUser || isSaml { - a.ch.srv.userService.SetSessionExpireInHours(session, *a.Config().ServiceSettings.SessionLengthSSOInHours()) + a.ch.srv.userService.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthSSOInDays) } else { - a.ch.srv.userService.SetSessionExpireInHours(session, *a.Config().ServiceSettings.SessionLengthWebInHours()) + a.ch.srv.userService.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthWebInDays) } ua := uasurfer.Parse(r.UserAgent()) diff --git a/app/oauth.go b/app/oauth.go index a0d458272f..9e3bbc9dd9 100644 --- a/app/oauth.go +++ b/app/oauth.go @@ -371,7 +371,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() - a.ch.srv.userService.SetSessionExpireInHours(session, *a.Config().ServiceSettings.SessionLengthSSOInHours()) + a.ch.srv.userService.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthSSOInDays) session.AddProp(model.SessionPropPlatform, appName) session.AddProp(model.SessionPropOs, "OAuth2") session.AddProp(model.SessionPropBrowser, "OAuth2") diff --git a/app/oauth_test.go b/app/oauth_test.go index 1d7807fa3b..35c2904b94 100644 --- a/app/oauth_test.go +++ b/app/oauth_test.go @@ -79,7 +79,7 @@ func TestOAuthRevokeAccessToken(t *testing.T) { session.UserId = model.NewId() session.Token = model.NewId() session.Roles = model.SystemUserRoleId - th.App.SetSessionExpireInHours(session, 24) + th.App.SetSessionExpireInDays(session, 1) var err *model.AppError session, err = th.App.CreateSession(session) @@ -111,7 +111,7 @@ func TestOAuthDeleteApp(t *testing.T) { session.Token = model.NewId() session.Roles = model.SystemUserRoleId session.IsOAuth = true - th.App.ch.srv.userService.SetSessionExpireInHours(session, 24) + th.App.ch.srv.userService.SetSessionExpireInDays(session, 1) session, _ = th.App.CreateSession(session) diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 6d7bc81657..8244925a84 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -15038,9 +15038,9 @@ func (a *OpenTracingAppLayer) SetSearchEngine(se *searchengine.Broker) { a.app.SetSearchEngine(se) } -func (a *OpenTracingAppLayer) SetSessionExpireInHours(session *model.Session, hours int) { +func (a *OpenTracingAppLayer) SetSessionExpireInDays(session *model.Session, days int) { origCtx := a.ctx - span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SetSessionExpireInHours") + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.SetSessionExpireInDays") a.ctx = newCtx a.app.Srv().Store.SetContext(newCtx) @@ -15050,7 +15050,7 @@ func (a *OpenTracingAppLayer) SetSessionExpireInHours(session *model.Session, ho }() defer span.Finish() - a.app.SetSessionExpireInHours(session, hours) + a.app.SetSessionExpireInDays(session, days) } func (a *OpenTracingAppLayer) SetStatusAwayIfNeeded(userID string, manual bool) { diff --git a/app/session.go b/app/session.go index 0a1fbfcf3e..2029fd0414 100644 --- a/app/session.go +++ b/app/session.go @@ -239,7 +239,7 @@ func (a *App) UpdateLastActivityAtIfNeeded(session model.Session) { a.UpdateWebConnUserActivity(session, now) - if now-session.LastActivityAt < model.SessionActivityTimeoutMiliseconds { + if now-session.LastActivityAt < model.SessionActivityTimeout { return } @@ -321,22 +321,22 @@ func (a *App) GetSessionLengthInMillis(session *model.Session) int64 { return 0 } - var hours int + var days int if session.IsMobileApp() { - hours = *a.Config().ServiceSettings.SessionLengthMobileInHours() + days = *a.Config().ServiceSettings.SessionLengthMobileInDays } else if session.IsSSOLogin() { - hours = *a.Config().ServiceSettings.SessionLengthSSOInHours() + days = *a.Config().ServiceSettings.SessionLengthSSOInDays } else { - hours = *a.Config().ServiceSettings.SessionLengthWebInHours() + days = *a.Config().ServiceSettings.SessionLengthWebInDays } - return int64(hours * 60 * 60 * 1000) + return int64(days * 24 * 60 * 60 * 1000) } -// SetSessionExpireInHours sets the session's expiry the specified number of days +// 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) SetSessionExpireInHours(session *model.Session, hours int) { - a.ch.srv.userService.SetSessionExpireInHours(session, hours) +func (a *App) SetSessionExpireInDays(session *model.Session, days int) { + a.ch.srv.userService.SetSessionExpireInDays(session, days) } func (a *App) CreateUserAccessToken(token *model.UserAccessToken) (*model.UserAccessToken, *model.AppError) { @@ -425,7 +425,7 @@ func (a *App) createSessionForUserAccessToken(tokenString string) (*model.Sessio } else { session.AddProp(model.SessionPropIsGuest, "false") } - a.ch.srv.userService.SetSessionExpireInHours(session, model.SessionUserAccessTokenExpiryHours) + a.ch.srv.userService.SetSessionExpireInDays(session, model.SessionUserAccessTokenExpiry) session, nErr = a.Srv().Store.Session().Save(session) if nErr != nil { diff --git a/app/users/session.go b/app/users/session.go index 92c0cdf51f..de5570131c 100644 --- a/app/users/session.go +++ b/app/users/session.go @@ -199,14 +199,14 @@ func (us *UserService) RevokeAccessToken(token string) error { return nil } -// SetSessionExpireInHours sets the session's expiry the specified number of days +// 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 (us *UserService) SetSessionExpireInHours(session *model.Session, hours int) { +func (us *UserService) SetSessionExpireInDays(session *model.Session, days int) { if session.CreateAt == 0 || *us.config().ServiceSettings.ExtendSessionLengthWithActivity { - session.ExpiresAt = model.GetMillis() + (1000 * 60 * 60 * int64(hours)) + session.ExpiresAt = model.GetMillis() + (1000 * 60 * 60 * 24 * int64(days)) } else { - session.ExpiresAt = session.CreateAt + (1000 * 60 * 60 * int64(hours)) + session.ExpiresAt = session.CreateAt + (1000 * 60 * 60 * 24 * int64(days)) } } diff --git a/app/users/session_test.go b/app/users/session_test.go index 14971aea48..cee66b9af1 100644 --- a/app/users/session_test.go +++ b/app/users/session_test.go @@ -91,7 +91,7 @@ func TestSetSessionExpireInDays(t *testing.T) { CreateAt: create, ExpiresAt: model.GetMillis() + dayInMillis, } - th.service.SetSessionExpireInHours(session, tt.days*24) + th.service.SetSessionExpireInDays(session, tt.days) // must be within 5 seconds of expected time. require.GreaterOrEqual(t, session.ExpiresAt, tt.want-grace) @@ -112,7 +112,7 @@ func TestOAuthRevokeAccessToken(t *testing.T) { session.UserId = model.NewId() session.Token = model.NewId() session.Roles = model.SystemUserRoleId - th.service.SetSessionExpireInHours(session, 24) + th.service.SetSessionExpireInDays(session, 1) session, _ = th.service.CreateSession(session) err = th.service.RevokeAccessToken(session.Token) diff --git a/i18n/en.json b/i18n/en.json index 4bc143c2cc..ea324d8f1c 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -8271,10 +8271,6 @@ "id": "model.config.is_valid.saml_username_attribute.app_error", "translation": "Invalid Username attribute. Must be set." }, - { - "id": "model.config.is_valid.session_length_sso_in_days.app_error", - "translation": " " - }, { "id": "model.config.is_valid.site_url.app_error", "translation": "Site URL must be a valid URL and start with http:// or https://." diff --git a/model/builtin.go b/model/builtin.go index 2d20b88c96..38e01f8b2c 100644 --- a/model/builtin.go +++ b/model/builtin.go @@ -3,8 +3,7 @@ package model -func NewBool(b bool) *bool { return &b } -func NewInt(n int) *int { return &n } -func NewInt64(n int64) *int64 { return &n } -func NewString(s string) *string { return &s } -func NewFloat32(s float32) *float32 { return &s } +func NewBool(b bool) *bool { return &b } +func NewInt(n int) *int { return &n } +func NewInt64(n int64) *int64 { return &n } +func NewString(s string) *string { return &s } diff --git a/model/config.go b/model/config.go index dd0dd616bf..d70b06b5b8 100644 --- a/model/config.go +++ b/model/config.go @@ -320,7 +320,7 @@ type ServiceSettings struct { ExtendSessionLengthWithActivity *bool `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` SessionLengthWebInDays *int `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` SessionLengthMobileInDays *int `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` - SessionLengthSSOInDays *float32 `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` + SessionLengthSSOInDays *int `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` SessionCacheInMinutes *int `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` SessionIdleTimeoutInMinutes *int `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` WebsocketSecurePort *int `access:"write_restrictable,cloud_restrictable"` // telemetry: none @@ -368,18 +368,6 @@ type ServiceSettings struct { ManagedResourcePaths *string `access:"environment_web_server,write_restrictable,cloud_restrictable"` } -func (s *ServiceSettings) SessionLengthWebInHours() *int { - return NewInt(*s.SessionLengthWebInDays * 24) -} - -func (s *ServiceSettings) SessionLengthMobileInHours() *int { - return NewInt(*s.SessionLengthMobileInDays * 24) -} - -func (s *ServiceSettings) SessionLengthSSOInHours() *int { - return NewInt(int(*s.SessionLengthSSOInDays * 24)) -} - func (s *ServiceSettings) SetDefaults(isUpdate bool) { if s.EnableEmailInvitations == nil { // If the site URL is also not present then assume this is a clean install @@ -610,7 +598,7 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) { } if s.SessionLengthSSOInDays == nil { - s.SessionLengthSSOInDays = NewFloat32(30) + s.SessionLengthSSOInDays = NewInt(30) } if s.SessionCacheInMinutes == nil { @@ -3596,10 +3584,6 @@ func (s *ServiceSettings) isValid() *AppError { return NewAppError("Config.IsValid", "model.config.is_valid.collapsed_threads.app_error", nil, "", http.StatusBadRequest) } - if *s.SessionLengthSSOInHours() < 1 { - return NewAppError("Config.IsValid", "model.config.is_valid.session_length_sso_in_days.app_error", nil, "", http.StatusBadRequest) - } - return nil } diff --git a/model/session.go b/model/session.go index fc3699f47c..36955583ef 100644 --- a/model/session.go +++ b/model/session.go @@ -12,24 +12,24 @@ import ( ) const ( - SessionCookieToken = "MMAUTHTOKEN" - SessionCookieUser = "MMUSERID" - SessionCookieCsrf = "MMCSRF" - SessionCookieCloudUrl = "MMCLOUDURL" - SessionCacheSize = 35000 - SessionPropPlatform = "platform" - SessionPropOs = "os" - SessionPropBrowser = "browser" - SessionPropType = "type" - SessionPropUserAccessTokenId = "user_access_token_id" - SessionPropIsBot = "is_bot" - SessionPropIsBotValue = "true" - SessionTypeUserAccessToken = "UserAccessToken" - SessionTypeCloudKey = "CloudKey" - SessionTypeRemoteclusterToken = "RemoteClusterToken" - SessionPropIsGuest = "is_guest" - SessionActivityTimeoutMiliseconds = 1000 * 60 * 5 // 5 minutes - SessionUserAccessTokenExpiryHours = 100 * 365 * 24 // 100 years + SessionCookieToken = "MMAUTHTOKEN" + SessionCookieUser = "MMUSERID" + SessionCookieCsrf = "MMCSRF" + SessionCookieCloudUrl = "MMCLOUDURL" + SessionCacheSize = 35000 + SessionPropPlatform = "platform" + SessionPropOs = "os" + SessionPropBrowser = "browser" + SessionPropType = "type" + SessionPropUserAccessTokenId = "user_access_token_id" + SessionPropIsBot = "is_bot" + SessionPropIsBotValue = "true" + SessionTypeUserAccessToken = "UserAccessToken" + SessionTypeCloudKey = "CloudKey" + SessionTypeRemoteclusterToken = "RemoteClusterToken" + SessionPropIsGuest = "is_guest" + SessionActivityTimeout = 1000 * 60 * 5 // 5 minutes + SessionUserAccessTokenExpiry = 100 * 365 // 100 years ) //msgp StringMap diff --git a/services/upgrader/upgrader.go b/services/upgrader/upgrader.go index 7411b31d94..8218998c60 100644 --- a/services/upgrader/upgrader.go +++ b/services/upgrader/upgrader.go @@ -1,6 +1,5 @@ // Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. // See LICENSE.txt for license information. -//go:build !linux // +build !linux package upgrader diff --git a/web/handlers_test.go b/web/handlers_test.go index 808a882f64..2d93dd9db4 100644 --- a/web/handlers_test.go +++ b/web/handlers_test.go @@ -131,7 +131,7 @@ func TestHandlerServeCSRFToken(t *testing.T) { IsOAuth: false, } session.GenerateCSRF() - th.App.SetSessionExpireInHours(session, 24) + th.App.SetSessionExpireInDays(session, 1) session, err := th.App.CreateSession(session) if err != nil { t.Errorf("Expected nil, got %s", err)