diff --git a/app/app_iface.go b/app/app_iface.go index da6ddecafe..7326cc1ce2 100644 --- a/app/app_iface.go +++ b/app/app_iface.go @@ -461,7 +461,7 @@ type AppIface interface { DoEmojisPermissionsMigration() DoGuestRolesCreationMigration() DoLocalRequest(rawURL string, body []byte) (*http.Response, *model.AppError) - DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, deviceId string, isMobile, isOAuth, isSaml bool) *model.AppError + DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, deviceId string, isMobile, isOAuthUser, isSaml bool) *model.AppError DoPostAction(postId, actionId, userId, selectedOption string) (string, *model.AppError) DoPostActionWithCookie(postId, actionId, userId, selectedOption string, cookie *model.PostActionCookie) (string, *model.AppError) DoUploadFile(now time.Time, rawTeamId string, rawChannelId string, rawUserId string, rawFilename string, data []byte) (*model.FileInfo, *model.AppError) diff --git a/app/login.go b/app/login.go index 2795f95777..f9577f1142 100644 --- a/app/login.go +++ b/app/login.go @@ -111,7 +111,7 @@ func (a *App) GetUserForLogin(id, loginId string) (*model.User, *model.AppError) return nil, model.NewAppError("GetUserForLogin", "store.sql_user.get_for_login.app_error", nil, "", http.StatusBadRequest) } -func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, deviceId string, isMobile, isOAuth, isSaml bool) *model.AppError { +func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, deviceId string, isMobile, isOAuthUser, isSaml bool) *model.AppError { if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil { var rejectionReason string pluginContext := a.PluginContext() @@ -125,9 +125,10 @@ func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, } } - session := &model.Session{UserId: user.Id, Roles: user.GetRawRoles(), DeviceId: deviceId, IsOAuth: isOAuth, Props: map[string]string{ + session := &model.Session{UserId: user.Id, Roles: user.GetRawRoles(), DeviceId: deviceId, IsOAuth: false, Props: map[string]string{ model.USER_AUTH_SERVICE_IS_MOBILE: strconv.FormatBool(isMobile), model.USER_AUTH_SERVICE_IS_SAML: strconv.FormatBool(isSaml), + model.USER_AUTH_SERVICE_IS_OAUTH: strconv.FormatBool(isOAuthUser), }} session.GenerateCSRF() @@ -141,7 +142,7 @@ func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, } } else if isMobile { a.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthMobileInDays) - } else if isOAuth || isSaml { + } else if isOAuthUser || isSaml { a.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthSSOInDays) } else { a.SetSessionExpireInDays(session, *a.Config().ServiceSettings.SessionLengthWebInDays) diff --git a/app/opentracing/opentracing_layer.go b/app/opentracing/opentracing_layer.go index 34e4252a47..bfd7c30707 100644 --- a/app/opentracing/opentracing_layer.go +++ b/app/opentracing/opentracing_layer.go @@ -3111,7 +3111,7 @@ func (a *OpenTracingAppLayer) DoLocalRequest(rawURL string, body []byte) (*http. return resultVar0, resultVar1 } -func (a *OpenTracingAppLayer) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, deviceId string, isMobile bool, isOAuth bool, isSaml bool) *model.AppError { +func (a *OpenTracingAppLayer) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, deviceId string, isMobile bool, isOAuthUser bool, isSaml bool) *model.AppError { origCtx := a.ctx span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.DoLogin") @@ -3123,7 +3123,7 @@ func (a *OpenTracingAppLayer) DoLogin(w http.ResponseWriter, r *http.Request, us }() defer span.Finish() - resultVar0 := a.app.DoLogin(w, r, user, deviceId, isMobile, isOAuth, isSaml) + resultVar0 := a.app.DoLogin(w, r, user, deviceId, isMobile, isOAuthUser, isSaml) if resultVar0 != nil { span.LogFields(spanlog.Error(resultVar0)) diff --git a/app/session_test.go b/app/session_test.go index 7aee85d249..d4d819fbac 100644 --- a/app/session_test.go +++ b/app/session_test.go @@ -237,8 +237,10 @@ func TestApp_GetSessionLengthInMillis(t *testing.T) { t.Run("get session length SSO", func(t *testing.T) { session := &model.Session{ - UserId: model.NewId(), - IsOAuth: true, + UserId: model.NewId(), + Props: map[string]string{ + model.USER_AUTH_SERVICE_IS_OAUTH: "true", + }, } session, err := th.App.CreateSession(session) require.Nil(t, err) diff --git a/model/saml.go b/model/saml.go index 59ac2acc34..feaf325aeb 100644 --- a/model/saml.go +++ b/model/saml.go @@ -15,6 +15,7 @@ const ( USER_AUTH_SERVICE_SAML_TEXT = "SAML" USER_AUTH_SERVICE_IS_SAML = "isSaml" USER_AUTH_SERVICE_IS_MOBILE = "isMobile" + USER_AUTH_SERVICE_IS_OAUTH = "isOAuthUser" ) type SamlAuthRequest struct { diff --git a/model/session.go b/model/session.go index 06f3e003b5..5c53eceb6e 100644 --- a/model/session.go +++ b/model/session.go @@ -175,8 +175,21 @@ func (me *Session) IsSaml() bool { return isSaml } +func (me *Session) IsOAuthUser() bool { + val, ok := me.Props[USER_AUTH_SERVICE_IS_OAUTH] + if !ok { + return false + } + isOAuthUser, err := strconv.ParseBool(val) + if err != nil { + mlog.Error("Error parsing boolean property from Session", mlog.Err(err)) + return false + } + return isOAuthUser +} + func (me *Session) IsSSOLogin() bool { - return me.IsOAuth || me.IsSaml() + return me.IsOAuthUser() || me.IsSaml() } func (me *Session) GetUserRoles() []string { diff --git a/model/session_test.go b/model/session_test.go index 5ac99e5cc9..9f12a51514 100644 --- a/model/session_test.go +++ b/model/session_test.go @@ -4,6 +4,7 @@ package model import ( + "strconv" "strings" "testing" "time" @@ -72,3 +73,23 @@ func TestSessionCSRF(t *testing.T) { assert.NotEmpty(t, token2) assert.Equal(t, token, token2) } + +func TestSessionIsOAuthUser(t *testing.T) { + testCases := []struct { + Description string + Session Session + isOAuthUser bool + }{ + {"False on empty props", Session{}, false}, + {"True when key is set to true", Session{Props: StringMap{USER_AUTH_SERVICE_IS_OAUTH: strconv.FormatBool(true)}}, true}, + {"False when key is set to false", Session{Props: StringMap{USER_AUTH_SERVICE_IS_OAUTH: strconv.FormatBool(false)}}, false}, + {"Not affected by Session.IsOauth being true", Session{IsOAuth: true}, false}, + {"Not affected by Session.IsOauth being false", Session{IsOAuth: false, Props: StringMap{USER_AUTH_SERVICE_IS_OAUTH: strconv.FormatBool(true)}}, true}, + } + + for _, tc := range testCases { + t.Run(tc.Description, func(t *testing.T) { + require.Equal(t, tc.isOAuthUser, tc.Session.IsOAuthUser()) + }) + } +} diff --git a/web/oauth.go b/web/oauth.go index 797dfd8f77..1c68191662 100644 --- a/web/oauth.go +++ b/web/oauth.go @@ -310,7 +310,7 @@ func completeOAuth(c *Context, w http.ResponseWriter, r *http.Request) { if parseErr != nil { mlog.Error("Error parsing boolean property from props", mlog.Err(parseErr)) } - err = c.App.DoLogin(w, r, user, "", isMobile, true, false) + err = c.App.DoLogin(w, r, user, "", isMobile, false, false) if err != nil { err.Translate(c.App.T) c.Err = err