MM-23935 extend session expiry on user activity (#14275)

* MM-23935 extend session expiry on user activity - if user types anything before a session expires the session will be extended to now + session length - ensures new session expiries are not written to DB too frequently - new session store func for updating session ExpiresAt - session length defaults for mobile and web/ldap changed from 180 days to 30 days
2020-05-06 15:41:10 -04:00
--- a/app/app_iface.go
+++ b/app/app_iface.go
@@ -121,6 +121,10 @@ type AppIface interface {
 	// attributes of the attachment structure. The Slack attachment structure is
 	// documented here: https://api.slack.com/docs/attachments
 	ProcessSlackAttachments(attachments []*model.SlackAttachment) []*model.SlackAttachment
+	// ExtendSessionExpiryIfNeeded extends Session.ExpiresAt based on session lengths in config.
+	// A new ExpiresAt is only written if enough time has elapsed since last update.
+	// Returns true only if the session was extended.
+	ExtendSessionExpiryIfNeeded(session *model.Session) bool
 	// FillInPostProps should be invoked before saving posts to fill in properties such as
 	// channel_mentions.
 	//
@@ -183,6 +187,9 @@ type AppIface interface {
 	GetSanitizedConfig() *model.Config
 	// GetSchemeRolesForChannel Checks if a channel or its team has an override scheme for channel roles and returns the scheme roles or default channel roles.
 	GetSchemeRolesForChannel(channelId string) (guestRoleName string, userRoleName string, adminRoleName string, err *model.AppError)
+	// GetSessionLengthInMillis returns the session length, in milliseconds,
+	// based on the type of session (Mobile, SSO, Web/LDAP).
+	GetSessionLengthInMillis(session *model.Session) int64
 	// GetTeamGroupUsers returns the users who are associated to the team via GroupTeams and GroupMembers.
 	GetTeamGroupUsers(teamID string) ([]*model.User, *model.AppError)
 	// GetTeamSchemeChannelRoles Checks if a team has an override scheme and returns the scheme channel role names or default channel role names.
--- a/app/diagnostics.go
+++ b/app/diagnostics.go
@@ -306,6 +306,7 @@ func (a *App) trackConfig() {
 		"uses_letsencrypt":                                        *cfg.ServiceSettings.UseLetsEncrypt,
 		"forward_80_to_443":                                       *cfg.ServiceSettings.Forward80To443,
 		"maximum_login_attempts":                                  *cfg.ServiceSettings.MaximumLoginAttempts,
+		"extend_session_length_with_activity":                     *cfg.ServiceSettings.ExtendSessionLengthWithActivity,
 		"session_length_web_in_days":                              *cfg.ServiceSettings.SessionLengthWebInDays,
 		"session_length_mobile_in_days":                           *cfg.ServiceSettings.SessionLengthMobileInDays,
 		"session_length_sso_in_days":                              *cfg.ServiceSettings.SessionLengthSSOInDays,
--- a/app/opentracing_layer.go
+++ b/app/opentracing_layer.go
@@ -3337,6 +3337,23 @@ func (a *OpenTracingAppLayer) ExportPermissions(w io.Writer) error {
 	return resultVar0
 }

+func (a *OpenTracingAppLayer) ExtendSessionExpiryIfNeeded(session *model.Session) bool {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.ExtendSessionExpiryIfNeeded")
+
+	a.ctx = newCtx
+	a.app.Srv().Store.SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store.SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0 := a.app.ExtendSessionExpiryIfNeeded(session)
+
+	return resultVar0
+}
+
 func (a *OpenTracingAppLayer) FetchSamlMetadataFromIdp(url string) ([]byte, *model.AppError) {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.FetchSamlMetadataFromIdp")
@@ -7407,6 +7424,23 @@ func (a *OpenTracingAppLayer) GetSessionById(sessionId string) (*model.Session,
 	return resultVar0, resultVar1
 }

+func (a *OpenTracingAppLayer) GetSessionLengthInMillis(session *model.Session) int64 {
+	origCtx := a.ctx
+	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetSessionLengthInMillis")
+
+	a.ctx = newCtx
+	a.app.Srv().Store.SetContext(newCtx)
+	defer func() {
+		a.app.Srv().Store.SetContext(origCtx)
+		a.ctx = origCtx
+	}()
+
+	defer span.Finish()
+	resultVar0 := a.app.GetSessionLengthInMillis(session)
+
+	return resultVar0
+}
+
 func (a *OpenTracingAppLayer) GetSessions(userId string) ([]*model.Session, *model.AppError) {
 	origCtx := a.ctx
 	span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetSessions")
--- a/app/session.go
+++ b/app/session.go
@@ -4,8 +4,10 @@
 package app

 import (
+	"math"
 	"net/http"

+	"github.com/mattermost/mattermost-server/v5/audit"
 	"github.com/mattermost/mattermost-server/v5/mlog"
 	"github.com/mattermost/mattermost-server/v5/model"
 )
@@ -74,7 +76,8 @@ func (a *App) GetSession(token string) (*model.Session, *model.AppError) {

 	if *a.Config().ServiceSettings.SessionIdleTimeoutInMinutes > 0 &&
 		!session.IsOAuth &&
-		session.Props[model.SESSION_PROP_TYPE] != model.SESSION_TYPE_USER_ACCESS_TOKEN {
+		session.Props[model.SESSION_PROP_TYPE] != model.SESSION_TYPE_USER_ACCESS_TOKEN &&
+		!*a.Config().ServiceSettings.ExtendSessionLengthWithActivity {

 		timeout := int64(*a.Config().ServiceSettings.SessionIdleTimeoutInMinutes) * 1000 * 60
 		if (model.GetMillis() - session.LastActivityAt) > timeout {
@@ -284,6 +287,72 @@ func (a *App) UpdateLastActivityAtIfNeeded(session model.Session) {
 	a.AddSessionToCache(&session)
 }

+// ExtendSessionExpiryIfNeeded extends Session.ExpiresAt based on session lengths in config.
+// A new ExpiresAt is only written if enough time has elapsed since last update.
+// Returns true only if the session was extended.
+func (a *App) ExtendSessionExpiryIfNeeded(session *model.Session) bool {
+	if session == nil || session.IsExpired() {
+		return false
+	}
+
+	sessionLength := a.GetSessionLengthInMillis(session)
+
+	// Only extend the expiry if the lessor of 1% or 1 day has elapsed within the
+	// current session duration.
+	threshold := int64(math.Min(float64(sessionLength)*0.01, float64(24*60*60*1000)))
+	// Minimum session length is 1 day as of this writing, therefore a minimum ~14 minutes threshold.
+	// However we'll add a sanity check here in case that changes. Minimum 5 minute threshold,
+	// meaning we won't write a new expiry more than every 5 minutes.
+	if threshold < 5*60*1000 {
+		threshold = 5 * 60 * 1000
+	}
+
+	now := model.GetMillis()
+	elapsed := now - (session.ExpiresAt - sessionLength)
+	if elapsed < threshold {
+		return false
+	}
+
+	auditRec := a.MakeAuditRecord("extendSessionExpiry", audit.Fail)
+	defer a.LogAuditRec(auditRec, nil)
+	auditRec.AddMeta("session", session)
+
+	newExpiry := now + sessionLength
+	if err := a.Srv().Store.Session().UpdateExpiresAt(session.Id, 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.AddSessionToCache(session)
+
+	auditRec.Success()
+	auditRec.AddMeta("extended_session", session)
+	return true
+}
+
+// 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 {
+	if session == nil {
+		return 0
+	}
+
+	var days int
+	if session.IsMobileApp() {
+		days = *a.Config().ServiceSettings.SessionLengthMobileInDays
+	} else if session.IsOAuth {
+		days = *a.Config().ServiceSettings.SessionLengthSSOInDays
+	} else {
+		days = *a.Config().ServiceSettings.SessionLengthWebInDays
+	}
+	return int64(days * 24 * 60 * 60 * 1000)
+}
+
 func (a *App) CreateUserAccessToken(token *model.UserAccessToken) (*model.UserAccessToken, *model.AppError) {

 	user, err := a.Srv().Store.User().Get(token.UserId)
--- a/app/session_test.go
+++ b/app/session_test.go
@@ -4,12 +4,12 @@
 package app

 import (
+	"fmt"
 	"testing"

+	"github.com/mattermost/mattermost-server/v5/model"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
-
-	"github.com/mattermost/mattermost-server/v5/model"
 )

 func TestCache(t *testing.T) {
@@ -58,6 +58,7 @@ func TestGetSessionIdleTimeoutInMinutes(t *testing.T) {

 	th.App.SetLicense(model.NewTestLicense("compliance"))
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SessionIdleTimeoutInMinutes = 5 })
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ExtendSessionLengthWithActivity = false })

 	rsession, err := th.App.GetSession(session.Token)
 	require.Nil(t, err)
@@ -177,3 +178,129 @@ func TestUpdateSessionOnPromoteDemote(t *testing.T) {
 		assert.Equal(t, "true", rsession.Props[model.SESSION_PROP_IS_GUEST])
 	})
 }
+
+const hourMillis int64 = 60 * 60 * 1000
+const dayMillis int64 = 24 * hourMillis
+
+func TestApp_GetSessionLengthInMillis(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SessionLengthMobileInDays = 3 })
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SessionLengthSSOInDays = 2 })
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SessionLengthWebInDays = 1 })
+
+	t.Run("get session length mobile", func(t *testing.T) {
+		session := &model.Session{
+			UserId:   model.NewId(),
+			DeviceId: model.NewId(),
+		}
+		session, err := th.App.CreateSession(session)
+		require.Nil(t, err)
+
+		sessionLength := th.App.GetSessionLengthInMillis(session)
+		require.Equal(t, dayMillis*3, sessionLength)
+	})
+
+	t.Run("get session length SSO", func(t *testing.T) {
+		session := &model.Session{
+			UserId:  model.NewId(),
+			IsOAuth: true,
+		}
+		session, err := th.App.CreateSession(session)
+		require.Nil(t, err)
+
+		sessionLength := th.App.GetSessionLengthInMillis(session)
+		require.Equal(t, dayMillis*2, sessionLength)
+	})
+
+	t.Run("get session length web/LDAP", func(t *testing.T) {
+		session := &model.Session{
+			UserId: model.NewId(),
+		}
+		session, err := th.App.CreateSession(session)
+		require.Nil(t, err)
+
+		sessionLength := th.App.GetSessionLengthInMillis(session)
+		require.Equal(t, dayMillis*1, sessionLength)
+	})
+}
+
+func TestApp_ExtendExpiryIfNeeded(t *testing.T) {
+	th := Setup(t).InitBasic()
+	defer th.TearDown()
+
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.ExtendSessionLengthWithActivity = true })
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SessionLengthMobileInDays = 3 })
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SessionLengthSSOInDays = 2 })
+	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.ServiceSettings.SessionLengthWebInDays = 1 })
+
+	t.Run("expired session should not be extended", func(t *testing.T) {
+		expires := model.GetMillis() - hourMillis
+		session := &model.Session{
+			UserId:    model.NewId(),
+			ExpiresAt: expires,
+		}
+		session, err := th.App.CreateSession(session)
+		require.Nil(t, err)
+
+		ok := th.App.ExtendSessionExpiryIfNeeded(session)
+
+		require.False(t, ok)
+		require.Equal(t, expires, session.ExpiresAt)
+		require.True(t, session.IsExpired())
+	})
+
+	t.Run("session within threshold should not be extended", func(t *testing.T) {
+		session := &model.Session{
+			UserId: model.NewId(),
+		}
+		session, err := th.App.CreateSession(session)
+		require.Nil(t, err)
+
+		expires := model.GetMillis() + th.App.GetSessionLengthInMillis(session)
+		session.ExpiresAt = expires
+
+		ok := th.App.ExtendSessionExpiryIfNeeded(session)
+
+		require.False(t, ok)
+		require.Equal(t, expires, session.ExpiresAt)
+		require.False(t, session.IsExpired())
+	})
+
+	var tests = []struct {
+		name    string
+		session *model.Session
+	}{
+		{name: "mobile", session: &model.Session{UserId: model.NewId(), DeviceId: model.NewId(), Token: model.NewId()}},
+		{name: "SSO", session: &model.Session{UserId: model.NewId(), IsOAuth: true, Token: model.NewId()}},
+		{name: "web/LDAP", session: &model.Session{UserId: model.NewId(), Token: model.NewId()}},
+	}
+	for _, test := range tests {
+		t.Run(fmt.Sprintf("%s session beyond threshold should update ExpiresAt", test.name), func(t *testing.T) {
+			session, err := th.App.CreateSession(test.session)
+			require.Nil(t, err)
+
+			expires := model.GetMillis() + th.App.GetSessionLengthInMillis(session) - hourMillis
+			session.ExpiresAt = expires
+
+			ok := th.App.ExtendSessionExpiryIfNeeded(session)
+
+			require.True(t, ok)
+			require.Greater(t, session.ExpiresAt, expires)
+			require.False(t, session.IsExpired())
+
+			// check cache was updated
+			ts, ok := th.App.Srv().sessionCache.Get(session.Token)
+			require.True(t, ok)
+			cachedSession := ts.(*model.Session)
+			require.Equal(t, session.ExpiresAt, cachedSession.ExpiresAt)
+
+			// check database was updated.
+			storedSession, err := th.App.Srv().Store.Session().Get(session.Token)
+			require.Nil(t, err)
+			require.Equal(t, session.ExpiresAt, storedSession.ExpiresAt)
+		})
+	}
+
+}