From 8d72cabee88debacaf5ecf0a602cf27334894aa1 Mon Sep 17 00:00:00 2001 From: Ben Schumacher Date: Tue, 15 Mar 2022 17:38:09 +0100 Subject: [PATCH] Update session cache if a plugin extends a session (#19178) --- app/plugin_api.go | 11 ++- .../test_sessions_plugin/main.go | 70 +++++++++++++++++++ app/session.go | 16 +---- app/users/session.go | 14 ++++ 4 files changed, 95 insertions(+), 16 deletions(-) create mode 100644 app/plugin_api_tests/test_sessions_plugin/main.go diff --git a/app/plugin_api.go b/app/plugin_api.go index 2e1356cac2..892e2104a3 100644 --- a/app/plugin_api.go +++ b/app/plugin_api.go @@ -289,7 +289,16 @@ func (api *PluginAPI) CreateSession(session *model.Session) (*model.Session, *mo } func (api *PluginAPI) ExtendSessionExpiry(sessionID string, expiresAt int64) *model.AppError { - return api.app.extendSessionExpiry(sessionID, expiresAt) + session, err := api.app.ch.srv.userService.GetSessionByID(sessionID) + if err != nil { + return model.NewAppError("extendSessionExpiry", "app.session.get_sessions.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + if err := api.app.ch.srv.userService.ExtendSessionExpiry(session, expiresAt); err != nil { + return model.NewAppError("extendSessionExpiry", "app.session.extend_session_expiry.app_error", nil, err.Error(), http.StatusInternalServerError) + } + + return nil } func (api *PluginAPI) RevokeSession(sessionID string) *model.AppError { diff --git a/app/plugin_api_tests/test_sessions_plugin/main.go b/app/plugin_api_tests/test_sessions_plugin/main.go new file mode 100644 index 0000000000..52ec7678d0 --- /dev/null +++ b/app/plugin_api_tests/test_sessions_plugin/main.go @@ -0,0 +1,70 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package main + +import ( + "fmt" + "time" + + "github.com/mattermost/mattermost-server/v6/app/plugin_api_tests" + "github.com/mattermost/mattermost-server/v6/model" + "github.com/mattermost/mattermost-server/v6/plugin" +) + +type MyPlugin struct { + plugin.MattermostPlugin + configuration plugin_api_tests.BasicConfig +} + +func (p *MyPlugin) OnConfigurationChange() error { + if err := p.API.LoadPluginConfiguration(&p.configuration); err != nil { + return err + } + return nil +} + +func (p *MyPlugin) MessageWillBePosted(_ *plugin.Context, _ *model.Post) (*model.Post, string) { + firstExpiry := time.Now().Add(time.Minute) + session := &model.Session{ + UserId: p.configuration.BasicUser2Id, + ExpiresAt: model.GetMillisForTime(firstExpiry), + } + session, appErr := p.API.CreateSession(session) + if appErr != nil { + return nil, appErr.Error() + } + + newExpiry := firstExpiry.Add(time.Minute) + appErr = p.API.ExtendSessionExpiry(session.Id, model.GetMillisForTime(newExpiry)) + if appErr != nil { + return nil, appErr.Error() + } + + rSession, appErr := p.API.GetSession(session.Id) + if appErr != nil { + return nil, appErr.Error() + } + if rSession.ExpiresAt != model.GetMillisForTime(firstExpiry.Add(time.Minute)) { + return nil, fmt.Sprintf("ExpiresAt not equal, expected: %v, got: %v", model.GetMillisForTime(firstExpiry.Add(time.Minute)), rSession.ExpiresAt) + } + + appErr = p.API.RevokeSession(session.Id) + if appErr != nil { + return nil, appErr.Error() + } + + rSession, appErr = p.API.GetSession(session.Id) + if appErr == nil { + return nil, "Session should not be found" + } + if rSession != nil { + return nil, "Returned session should be nil" + } + + return nil, "OK" +} + +func main() { + plugin.ClientMain(&MyPlugin{}) +} diff --git a/app/session.go b/app/session.go index fd7f70e12c..1d3a1833bb 100644 --- a/app/session.go +++ b/app/session.go @@ -286,18 +286,12 @@ func (a *App) ExtendSessionExpiryIfNeeded(session *model.Session) bool { auditRec.AddMeta("session", session) newExpiry := now + sessionLength - if err := a.Srv().Store.Session().UpdateExpiresAt(session.Id, newExpiry); err != nil { + if err := a.ch.srv.userService.ExtendSessionExpiry(session, newExpiry); err != nil { mlog.Error("Failed to update ExpiresAt", mlog.String("user_id", session.UserId), mlog.String("session_id", session.Id), mlog.Err(err)) auditRec.AddMeta("err", err.Error()) return false } - // Update local cache. No need to invalidate cache for cluster as the session cache timeout - // ensures each node will get an extended expiry within the next 10 minutes. - // Worst case is another node may generate a redundant expiry update. - session.ExpiresAt = newExpiry - a.ch.srv.userService.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)) @@ -306,14 +300,6 @@ func (a *App) ExtendSessionExpiryIfNeeded(session *model.Session) bool { return true } -func (a *App) extendSessionExpiry(sessionID string, newExpiry int64) *model.AppError { - if err := a.Srv().Store.Session().UpdateExpiresAt(sessionID, newExpiry); err != nil { - return model.NewAppError("ExtendSessionExpiry", "app.session.extend_session_expiry.app_error", nil, err.Error(), http.StatusInternalServerError) - } - - return nil -} - // GetSessionLengthInMillis returns the session length, in milliseconds, // based on the type of session (Mobile, SSO, Web/LDAP). func (a *App) GetSessionLengthInMillis(session *model.Session) int64 { diff --git a/app/users/session.go b/app/users/session.go index de5570131c..f21a083b5e 100644 --- a/app/users/session.go +++ b/app/users/session.go @@ -210,6 +210,20 @@ func (us *UserService) SetSessionExpireInDays(session *model.Session, days int) } } +func (us *UserService) ExtendSessionExpiry(session *model.Session, newExpiry int64) error { + if err := us.sessionStore.UpdateExpiresAt(session.Id, newExpiry); err != nil { + return err + } + + // Update local cache. No need to invalidate cache for cluster as the session cache timeout + // ensures each node will get an extended expiry within the next 10 minutes. + // Worst case is another node may generate a redundant expiry update. + session.ExpiresAt = newExpiry + us.AddSessionToCache(session) + + return nil +} + func (us *UserService) UpdateSessionsIsGuest(userID string, isGuest bool) error { sessions, err := us.GetSessions(userID) if err != nil {