mirror of
https://github.com/mattermost/mattermost.git
synced 2025-02-25 18:55:24 -06:00
[MM-58560] Configurable session revocation during password resets (#27286)
* [MM-58560] Allow for configurable session revocation during password reset * Missing i18n additions * Update Settings Wording * Update Settings Wording #2 * Update default_config.ts for Session Termination --------- Co-authored-by: Mattermost Build <build@mattermost.com>
This commit is contained in:
@@ -133,6 +133,7 @@ const defaultServerConfig: AdminConfig = {
|
||||
CorsDebug: false,
|
||||
AllowCookiesForSubdomains: false,
|
||||
ExtendSessionLengthWithActivity: true,
|
||||
TerminateSessionsOnPasswordChange: false,
|
||||
SessionLengthWebInDays: 30,
|
||||
SessionLengthWebInHours: 720,
|
||||
SessionLengthMobileInDays: 30,
|
||||
|
||||
@@ -1453,6 +1453,31 @@ func (a *App) UpdatePassword(rctx request.CTX, user *model.User, newPassword str
|
||||
|
||||
a.InvalidateCacheForUser(user.Id)
|
||||
|
||||
if *a.Config().ServiceSettings.TerminateSessionsOnPasswordChange {
|
||||
// Get currently active sessions if request is user-initiated to retain it
|
||||
currentSession := ""
|
||||
if rctx.Session() != nil && rctx.Session().UserId == user.Id {
|
||||
currentSession = rctx.Session().Id
|
||||
}
|
||||
|
||||
sessions, err := a.GetSessions(rctx, user.Id)
|
||||
if err != nil {
|
||||
return model.NewAppError("UpdatePassword", "api.user.update_password.failed.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
|
||||
}
|
||||
|
||||
// Revoke all but current session
|
||||
for _, session := range sessions {
|
||||
if session.Id == currentSession {
|
||||
continue
|
||||
}
|
||||
|
||||
err := a.RevokeSessionById(rctx, session.Id)
|
||||
if err != nil {
|
||||
return model.NewAppError("UpdatePassword", "api.user.update_password.failed.app_error", nil, "", http.StatusInternalServerError).Wrap(err)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
|
||||
@@ -1210,6 +1210,153 @@ func TestInvalidatePasswordRecoveryTokens(t *testing.T) {
|
||||
})
|
||||
}
|
||||
|
||||
func TestPasswordChangeSessionTermination(t *testing.T) {
|
||||
th := Setup(t).InitBasic()
|
||||
defer th.TearDown()
|
||||
|
||||
t.Run("user-initiated password change with termination enabled", func(t *testing.T) {
|
||||
th.App.UpdateConfig(func(c *model.Config) {
|
||||
*c.ServiceSettings.TerminateSessionsOnPasswordChange = true
|
||||
})
|
||||
|
||||
session, err := th.App.CreateSession(th.Context, &model.Session{
|
||||
UserId: th.BasicUser2.Id,
|
||||
Roles: model.SystemUserRoleId,
|
||||
})
|
||||
require.Nil(t, err)
|
||||
|
||||
session2, err := th.App.CreateSession(th.Context, &model.Session{
|
||||
UserId: th.BasicUser2.Id,
|
||||
Roles: model.SystemUserRoleId,
|
||||
})
|
||||
require.Nil(t, err)
|
||||
|
||||
th.Context.Session().UserId = th.BasicUser2.Id
|
||||
th.Context.Session().Id = session.Id
|
||||
|
||||
err = th.App.UpdatePassword(th.Context, th.BasicUser2, "Password2")
|
||||
require.Nil(t, err)
|
||||
|
||||
session, err = th.App.GetSession(session.Token)
|
||||
require.Nil(t, err)
|
||||
require.False(t, session.IsExpired())
|
||||
|
||||
session2, err = th.App.GetSession(session2.Token)
|
||||
require.Error(t, err)
|
||||
require.Nil(t, session2)
|
||||
|
||||
// Cleanup
|
||||
err = th.App.UpdatePassword(th.Context, th.BasicUser2, "Password1")
|
||||
require.Nil(t, err)
|
||||
th.Context.Session().UserId = ""
|
||||
th.Context.Session().Id = ""
|
||||
})
|
||||
|
||||
t.Run("user-initiated password change with termination disabled", func(t *testing.T) {
|
||||
th.App.UpdateConfig(func(c *model.Config) {
|
||||
*c.ServiceSettings.TerminateSessionsOnPasswordChange = false
|
||||
})
|
||||
|
||||
session, err := th.App.CreateSession(th.Context, &model.Session{
|
||||
UserId: th.BasicUser2.Id,
|
||||
Roles: model.SystemUserRoleId,
|
||||
})
|
||||
require.Nil(t, err)
|
||||
|
||||
session2, err := th.App.CreateSession(th.Context, &model.Session{
|
||||
UserId: th.BasicUser2.Id,
|
||||
Roles: model.SystemUserRoleId,
|
||||
})
|
||||
require.Nil(t, err)
|
||||
|
||||
th.Context.Session().UserId = th.BasicUser2.Id
|
||||
th.Context.Session().Id = session.Id
|
||||
|
||||
err = th.App.UpdatePassword(th.Context, th.BasicUser2, "Password2")
|
||||
require.Nil(t, err)
|
||||
|
||||
session, err = th.App.GetSession(session.Token)
|
||||
require.Nil(t, err)
|
||||
require.False(t, session.IsExpired())
|
||||
|
||||
session2, err = th.App.GetSession(session2.Token)
|
||||
require.Nil(t, err)
|
||||
require.False(t, session2.IsExpired())
|
||||
|
||||
// Cleanup
|
||||
err = th.App.UpdatePassword(th.Context, th.BasicUser2, "Password1")
|
||||
require.Nil(t, err)
|
||||
th.Context.Session().UserId = ""
|
||||
th.Context.Session().Id = ""
|
||||
})
|
||||
|
||||
t.Run("admin-initiated password change with termination enabled", func(t *testing.T) {
|
||||
th.App.UpdateConfig(func(c *model.Config) {
|
||||
*c.ServiceSettings.TerminateSessionsOnPasswordChange = true
|
||||
})
|
||||
|
||||
session, err := th.App.CreateSession(th.Context, &model.Session{
|
||||
UserId: th.BasicUser2.Id,
|
||||
Roles: model.SystemUserRoleId,
|
||||
})
|
||||
require.Nil(t, err)
|
||||
|
||||
session2, err := th.App.CreateSession(th.Context, &model.Session{
|
||||
UserId: th.BasicUser2.Id,
|
||||
Roles: model.SystemUserRoleId,
|
||||
})
|
||||
require.Nil(t, err)
|
||||
|
||||
err = th.App.UpdatePassword(th.Context, th.BasicUser2, "Password2")
|
||||
require.Nil(t, err)
|
||||
|
||||
session, err = th.App.GetSession(session.Token)
|
||||
require.Error(t, err)
|
||||
require.Nil(t, session)
|
||||
|
||||
session2, err = th.App.GetSession(session2.Token)
|
||||
require.Error(t, err)
|
||||
require.Nil(t, session2)
|
||||
|
||||
// Cleanup
|
||||
err = th.App.UpdatePassword(th.Context, th.BasicUser2, "Password1")
|
||||
require.Nil(t, err)
|
||||
})
|
||||
|
||||
t.Run("admin-initiated password change with termination disabled", func(t *testing.T) {
|
||||
th.App.UpdateConfig(func(c *model.Config) {
|
||||
*c.ServiceSettings.TerminateSessionsOnPasswordChange = false
|
||||
})
|
||||
|
||||
session, err := th.App.CreateSession(th.Context, &model.Session{
|
||||
UserId: th.BasicUser2.Id,
|
||||
Roles: model.SystemUserRoleId,
|
||||
})
|
||||
require.Nil(t, err)
|
||||
|
||||
session2, err := th.App.CreateSession(th.Context, &model.Session{
|
||||
UserId: th.BasicUser2.Id,
|
||||
Roles: model.SystemUserRoleId,
|
||||
})
|
||||
require.Nil(t, err)
|
||||
|
||||
err = th.App.UpdatePassword(th.Context, th.BasicUser2, "Password2")
|
||||
require.Nil(t, err)
|
||||
|
||||
session, err = th.App.GetSession(session.Token)
|
||||
require.Nil(t, err)
|
||||
require.False(t, session.IsExpired())
|
||||
|
||||
session2, err = th.App.GetSession(session2.Token)
|
||||
require.Nil(t, err)
|
||||
require.False(t, session2.IsExpired())
|
||||
|
||||
// Cleanup
|
||||
err = th.App.UpdatePassword(th.Context, th.BasicUser2, "Password1")
|
||||
require.Nil(t, err)
|
||||
})
|
||||
}
|
||||
|
||||
func TestGetViewUsersRestrictions(t *testing.T) {
|
||||
th := Setup(t).InitBasic()
|
||||
defer th.TearDown()
|
||||
|
||||
@@ -430,6 +430,7 @@ func (ts *TelemetryService) trackConfig() {
|
||||
"forward_80_to_443": *cfg.ServiceSettings.Forward80To443,
|
||||
"maximum_login_attempts": *cfg.ServiceSettings.MaximumLoginAttempts,
|
||||
"extend_session_length_with_activity": *cfg.ServiceSettings.ExtendSessionLengthWithActivity,
|
||||
"terminate_sessions_on_password_change": *cfg.ServiceSettings.TerminateSessionsOnPasswordChange,
|
||||
"session_length_web_in_hours": *cfg.ServiceSettings.SessionLengthWebInHours,
|
||||
"session_length_mobile_in_hours": *cfg.ServiceSettings.SessionLengthMobileInHours,
|
||||
"session_length_sso_in_hours": *cfg.ServiceSettings.SessionLengthSSOInHours,
|
||||
|
||||
@@ -341,6 +341,7 @@ type ServiceSettings struct {
|
||||
CorsDebug *bool `access:"integrations_cors,write_restrictable,cloud_restrictable"`
|
||||
AllowCookiesForSubdomains *bool `access:"write_restrictable,cloud_restrictable"`
|
||||
ExtendSessionLengthWithActivity *bool `access:"environment_session_lengths,write_restrictable,cloud_restrictable"`
|
||||
TerminateSessionsOnPasswordChange *bool `access:"environment_session_lengths,write_restrictable,cloud_restrictable"`
|
||||
|
||||
// Deprecated
|
||||
SessionLengthWebInDays *int `access:"environment_session_lengths,write_restrictable,cloud_restrictable"` // telemetry: none
|
||||
@@ -631,6 +632,11 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) {
|
||||
s.ExtendSessionLengthWithActivity = NewBool(!isUpdate)
|
||||
}
|
||||
|
||||
// Must be manually enabled for existing installations.
|
||||
if s.TerminateSessionsOnPasswordChange == nil {
|
||||
s.TerminateSessionsOnPasswordChange = NewBool(!isUpdate)
|
||||
}
|
||||
|
||||
if s.SessionLengthWebInDays == nil {
|
||||
if isUpdate {
|
||||
s.SessionLengthWebInDays = NewInt(180)
|
||||
|
||||
@@ -16,6 +16,7 @@ import TextSetting from './text_setting';
|
||||
|
||||
interface State extends BaseState {
|
||||
extendSessionLengthWithActivity: ServiceSettings['ExtendSessionLengthWithActivity'];
|
||||
terminateSessionsOnPasswordChange: ServiceSettings['TerminateSessionsOnPasswordChange'];
|
||||
sessionLengthWebInHours: ServiceSettings['SessionLengthWebInHours'];
|
||||
sessionLengthMobileInHours: ServiceSettings['SessionLengthMobileInHours'];
|
||||
sessionLengthSSOInHours: ServiceSettings['SessionLengthSSOInHours'];
|
||||
@@ -39,6 +40,8 @@ const messages = defineMessages({
|
||||
sessionIdleTimeout: {id: 'admin.service.sessionIdleTimeout', defaultMessage: 'Session Idle Timeout (minutes):'},
|
||||
extendSessionLengthActivity_label: {id: 'admin.service.extendSessionLengthActivity.label', defaultMessage: 'Extend session length with activity: '},
|
||||
extendSessionLengthActivity_helpText: {id: 'admin.service.extendSessionLengthActivity.helpText', defaultMessage: 'When true, sessions will be automatically extended when the user is active in their Mattermost client. Users sessions will only expire if they are not active in their Mattermost client for the entire duration of the session lengths defined in the fields below. When false, sessions will not extend with activity in Mattermost. User sessions will immediately expire at the end of the session length or idle timeouts defined below. '},
|
||||
terminateSessionsOnPasswordChange_label: {id: 'admin.service.terminateSessionsOnPasswordChange.label', defaultMessage: 'Terminate Sessions on Password Change: '},
|
||||
terminateSessionsOnPasswordChange_helpText: {id: 'admin.service.terminateSessionsOnPasswordChange.helpText', defaultMessage: 'When true, all sessions of a user will expire if their password is changed by themselves or an administrator.'},
|
||||
webSessionHours: {id: 'admin.service.webSessionHours', defaultMessage: 'Session Length AD/LDAP and Email (hours):'},
|
||||
mobileSessionHours: {id: 'admin.service.mobileSessionHours', defaultMessage: 'Session Length Mobile (hours):'},
|
||||
ssoSessionHours: {id: 'admin.service.ssoSessionHours', defaultMessage: 'Session Length SSO (hours):'},
|
||||
@@ -73,6 +76,7 @@ export default class SessionLengthSettings extends AdminSettings<Props, State> {
|
||||
const MINIMUM_IDLE_TIMEOUT = 5;
|
||||
|
||||
config.ServiceSettings.ExtendSessionLengthWithActivity = this.state.extendSessionLengthWithActivity;
|
||||
config.ServiceSettings.TerminateSessionsOnPasswordChange = this.state.terminateSessionsOnPasswordChange;
|
||||
config.ServiceSettings.SessionLengthWebInHours = this.parseIntNonZero(this.state.sessionLengthWebInHours);
|
||||
config.ServiceSettings.SessionLengthMobileInHours = this.parseIntNonZero(this.state.sessionLengthMobileInHours);
|
||||
config.ServiceSettings.SessionLengthSSOInHours = this.parseIntNonZero(this.state.sessionLengthSSOInHours);
|
||||
@@ -85,6 +89,7 @@ export default class SessionLengthSettings extends AdminSettings<Props, State> {
|
||||
getStateFromConfig(config: AdminConfig) {
|
||||
return {
|
||||
extendSessionLengthWithActivity: config.ServiceSettings.ExtendSessionLengthWithActivity,
|
||||
terminateSessionsOnPasswordChange: config.ServiceSettings.TerminateSessionsOnPasswordChange,
|
||||
sessionLengthWebInHours: config.ServiceSettings.SessionLengthWebInHours,
|
||||
sessionLengthMobileInHours: config.ServiceSettings.SessionLengthMobileInHours,
|
||||
sessionLengthSSOInHours: config.ServiceSettings.SessionLengthSSOInHours,
|
||||
@@ -140,6 +145,15 @@ export default class SessionLengthSettings extends AdminSettings<Props, State> {
|
||||
setByEnv={this.isSetByEnv('ServiceSettings.ExtendSessionLengthWithActivity')}
|
||||
disabled={this.props.isDisabled}
|
||||
/>
|
||||
<BooleanSetting
|
||||
id='terminateSessionsOnPasswordChange'
|
||||
label={<FormattedMessage {...messages.terminateSessionsOnPasswordChange_label}/>}
|
||||
helpText={<FormattedMessage {...messages.terminateSessionsOnPasswordChange_helpText}/>}
|
||||
value={this.state.terminateSessionsOnPasswordChange}
|
||||
onChange={this.handleChange}
|
||||
setByEnv={this.isSetByEnv('ServiceSettings.TerminateSessionsOnPasswordChange')}
|
||||
disabled={this.props.isDisabled}
|
||||
/>
|
||||
<TextSetting
|
||||
id='sessionLengthWebInHours'
|
||||
label={<FormattedMessage {...messages.webSessionHours}/>}
|
||||
|
||||
@@ -2285,6 +2285,8 @@
|
||||
"admin.service.ssoSessionHours": "Session Length SSO (hours):",
|
||||
"admin.service.ssoSessionHoursDesc": "The number of hours from the last time a user entered their credentials to the expiry of the user's session. If the authentication method is SAML or GitLab, the user may automatically be logged back in to Mattermost if they are already logged in to SAML or GitLab. After changing this setting, the setting will take effect after the next time the user enters their credentials.",
|
||||
"admin.service.ssoSessionHoursDesc.extendLength": "Set the number of hours from the last activity in Mattermost to the expiry of the user’s session for SSO authentication, such as SAML, GitLab and OAuth 2.0. If the authentication method is SAML or GitLab, the user may automatically be logged back in to Mattermost if they are already logged in to SAML or GitLab. After changing this setting, the setting will take effect after the next time the user enters their credentials.",
|
||||
"admin.service.terminateSessionsOnPasswordChange.helpText": "When true, all sessions of a user will expire if their password is changed by themselves or an administrator. If password change is initiated by user, their current session is not terminated",
|
||||
"admin.service.terminateSessionsOnPasswordChange.label": "Terminate Sessions on Password Change: ",
|
||||
"admin.service.testingDescription": "When true, /test slash command is enabled to load test accounts, data and text formatting. Changing this requires a server restart before taking effect.",
|
||||
"admin.service.testingTitle": "Enable Testing Commands: ",
|
||||
"admin.service.testSiteURL": "Test Live URL",
|
||||
|
||||
@@ -331,6 +331,7 @@ export type ServiceSettings = {
|
||||
CorsDebug: boolean;
|
||||
AllowCookiesForSubdomains: boolean;
|
||||
ExtendSessionLengthWithActivity: boolean;
|
||||
TerminateSessionsOnPasswordChange: boolean;
|
||||
SessionLengthWebInDays: number;
|
||||
SessionLengthWebInHours: number;
|
||||
SessionLengthMobileInDays: number;
|
||||
|
||||
Reference in New Issue
Block a user