Unverified Commit 773ab352 authored by Farhan Munshi's avatar Farhan Munshi Committed by GitHub
Browse files

[MM-27623] Add new session prop for oauth (#15221) (#15226)

* Add new session prop for oauth

* Make it isOAuthUser to differentiate better

* Fix up caps

* Fix tests

* Add tests for IsOAuthUser
parent dfe1f42a
......@@ -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)
......
......@@ -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 {
session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthMobileInDays)
} else if isOAuth || isSaml {
} else if isOAuthUser || isSaml {
session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthSSOInDays)
} else {
session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthWebInDays)
......
......@@ -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))
......
......@@ -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)
......
......@@ -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 {
......
......@@ -172,8 +172,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 {
......
......@@ -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())
})
}
}
......@@ -306,7 +306,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
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment