mirrors/mostlymatter
1
0
Форкнуть 0
Код Задачи Запросы на слияние Действия Пакеты Проекты Релизы Вики Активность

[MM-25780] Fix incorrect session length when logging in through mobile using SSO (#14874)

Просмотр исходного кода 
* Pass device ID

* dont use device id as way of detecting

* fix spelling mistake

* update layers

* fix test

* fix linting

* save schema

* put columns in correct place

* fix linting

* update

* upgrade go change

* use props

* fix stuff

* update session tests

* address PR comments

* address PR comments
Этот коммит содержится в:
Hossein Ahmadian-Yazdi
2020-06-30 10:34:05 -04:00
коммит произвёл GitHub
родитель df943fbf91
Коммит 4c50c7c59b
12 изменённых файлов: 119 добавлений и 19 удалений
Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

2
api4/user.go
Просмотреть файл

@@ -1564,7 +1564,7 @@ func login(c *Context, w http.ResponseWriter, r *http.Request) {
c.LogAuditWithUserId(user.Id, "authenticated")
err = c.App.DoLogin(w, r, user, deviceId)
err = c.App.DoLogin(w, r, user, deviceId, false, false, false)
if err != nil {
c.Err = err
return

4
app/app_iface.go
Просмотреть файл

@@ -457,7 +457,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) *model.AppError
DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, deviceId string, isMobile, isOAuth, 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)
@@ -579,7 +579,7 @@ type AppIface interface {
GetOAuthAppsByCreator(userId string, page, perPage int) ([]*model.OAuthApp, *model.AppError)
GetOAuthCodeRedirect(userId string, authRequest *model.AuthorizeRequest) (string, *model.AppError)
GetOAuthImplicitRedirect(userId string, authRequest *model.AuthorizeRequest) (string, *model.AppError)
GetOAuthLoginEndpoint(w http.ResponseWriter, r *http.Request, service, teamId, action, redirectTo, loginHint string) (string, *model.AppError)
GetOAuthLoginEndpoint(w http.ResponseWriter, r *http.Request, service, teamId, action, redirectTo, loginHint string, isMobile bool) (string, *model.AppError)
GetOAuthSignupEndpoint(w http.ResponseWriter, r *http.Request, service, teamId string) (string, *model.AppError)
GetOAuthStateToken(token string) (*model.Token, *model.AppError)
GetOpenGraphMetadata(requestURL string) *opengraph.OpenGraph

12
app/login.go
Просмотреть файл

@@ -6,6 +6,7 @@ package app
import (
"fmt"
"net/http"
"strconv"
"strings"
"time"
@@ -110,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) *model.AppError {
func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, deviceId string, isMobile, isOAuth, isSaml bool) *model.AppError {
if pluginsEnvironment := a.GetPluginsEnvironment(); pluginsEnvironment != nil {
var rejectionReason string
pluginContext := a.PluginContext()
@@ -124,7 +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: false}
session := &model.Session{UserId: user.Id, Roles: user.GetRawRoles(), DeviceId: deviceId, IsOAuth: isOAuth, Props: map[string]string{
model.USER_AUTH_SERVICE_IS_MOBILE: strconv.FormatBool(isMobile),
model.USER_AUTH_SERVICE_IS_SAML: strconv.FormatBool(isSaml),
}}
session.GenerateCSRF()
if len(deviceId) > 0 {
@@ -135,6 +139,10 @@ func (a *App) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User,
err.StatusCode = http.StatusInternalServerError
return err
}
} else if isMobile {
session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthMobileInDays)
} else if isOAuth || isSaml {
session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthSSOInDays)
} else {
session.SetExpireInDays(*a.Config().ServiceSettings.SessionLengthWebInDays)
}

4
app/oauth.go
Просмотреть файл

@@ -350,7 +350,7 @@ func (a *App) newSessionUpdateToken(appName string, accessData *model.AccessData
return accessRsp, nil
}
func (a *App) GetOAuthLoginEndpoint(w http.ResponseWriter, r *http.Request, service, teamId, action, redirectTo, loginHint string) (string, *model.AppError) {
func (a *App) GetOAuthLoginEndpoint(w http.ResponseWriter, r *http.Request, service, teamId, action, redirectTo, loginHint string, isMobile bool) (string, *model.AppError) {
stateProps := map[string]string{}
stateProps["action"] = action
if len(teamId) != 0 {
@@ -361,6 +361,8 @@ func (a *App) GetOAuthLoginEndpoint(w http.ResponseWriter, r *http.Request, serv
stateProps["redirect_to"] = redirectTo
}
stateProps[model.USER_AUTH_SERVICE_IS_MOBILE] = strconv.FormatBool(isMobile)
authUrl, err := a.GetAuthorizationCode(w, r, service, stateProps, loginHint)
if err != nil {
return "", err

8
app/opentracing_layer.go
Просмотреть файл

@@ -3088,7 +3088,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) *model.AppError {
func (a *OpenTracingAppLayer) DoLogin(w http.ResponseWriter, r *http.Request, user *model.User, deviceId string, isMobile bool, isOAuth bool, isSaml bool) *model.AppError {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.DoLogin")
@@ -3100,7 +3100,7 @@ func (a *OpenTracingAppLayer) DoLogin(w http.ResponseWriter, r *http.Request, us
}()
defer span.Finish()
resultVar0 := a.app.DoLogin(w, r, user, deviceId)
resultVar0 := a.app.DoLogin(w, r, user, deviceId, isMobile, isOAuth, isSaml)
if resultVar0 != nil {
span.LogFields(spanlog.Error(resultVar0))
@@ -6215,7 +6215,7 @@ func (a *OpenTracingAppLayer) GetOAuthImplicitRedirect(userId string, authReques
return resultVar0, resultVar1
}
func (a *OpenTracingAppLayer) GetOAuthLoginEndpoint(w http.ResponseWriter, r *http.Request, service string, teamId string, action string, redirectTo string, loginHint string) (string, *model.AppError) {
func (a *OpenTracingAppLayer) GetOAuthLoginEndpoint(w http.ResponseWriter, r *http.Request, service string, teamId string, action string, redirectTo string, loginHint string, isMobile bool) (string, *model.AppError) {
origCtx := a.ctx
span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.GetOAuthLoginEndpoint")
@@ -6227,7 +6227,7 @@ func (a *OpenTracingAppLayer) GetOAuthLoginEndpoint(w http.ResponseWriter, r *ht
}()
defer span.Finish()
resultVar0, resultVar1 := a.app.GetOAuthLoginEndpoint(w, r, service, teamId, action, redirectTo, loginHint)
resultVar0, resultVar1 := a.app.GetOAuthLoginEndpoint(w, r, service, teamId, action, redirectTo, loginHint, isMobile)
if resultVar1 != nil {
span.LogFields(spanlog.Error(resultVar1))

6
app/plugin_hooks_test.go
Просмотреть файл

@@ -696,7 +696,7 @@ func TestUserWillLogIn_Blocked(t *testing.T) {
r := &http.Request{}
w := httptest.NewRecorder()
err = th.App.DoLogin(w, r, th.BasicUser, "")
err = th.App.DoLogin(w, r, th.BasicUser, "", false, false, false)
assert.Contains(t, err.Id, "Login rejected by plugin", "Expected Login rejected by plugin, got %s", err.Id)
}
@@ -735,7 +735,7 @@ func TestUserWillLogInIn_Passed(t *testing.T) {
r := &http.Request{}
w := httptest.NewRecorder()
err = th.App.DoLogin(w, r, th.BasicUser, "")
err = th.App.DoLogin(w, r, th.BasicUser, "", false, false, false)
assert.Nil(t, err, "Expected nil, got %s", err)
assert.Equal(t, th.App.Session().UserId, th.BasicUser.Id)
@@ -776,7 +776,7 @@ func TestUserHasLoggedIn(t *testing.T) {
r := &http.Request{}
w := httptest.NewRecorder()
err = th.App.DoLogin(w, r, th.BasicUser, "")
err = th.App.DoLogin(w, r, th.BasicUser, "", false, false, false)
assert.Nil(t, err, "Expected nil, got %s", err)

2
app/session.go
Просмотреть файл

@@ -348,7 +348,7 @@ func (a *App) GetSessionLengthInMillis(session *model.Session) int64 {
var days int
if session.IsMobileApp() {
days = *a.Config().ServiceSettings.SessionLengthMobileInDays
} else if session.IsOAuth {
} else if session.IsSSOLogin() {
days = *a.Config().ServiceSettings.SessionLengthSSOInDays
} else {
days = *a.Config().ServiceSettings.SessionLengthWebInDays

42
app/session_test.go
Просмотреть файл

@@ -206,6 +206,35 @@ func TestApp_GetSessionLengthInMillis(t *testing.T) {
require.Equal(t, dayMillis*3, sessionLength)
})
t.Run("get session length mobile when isMobile in props is set", func(t *testing.T) {
session := &model.Session{
UserId: model.NewId(),
Props: map[string]string{
model.USER_AUTH_SERVICE_IS_MOBILE: "true",
},
}
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 mobile when isMobile in props is set and takes priority over saml", func(t *testing.T) {
session := &model.Session{
UserId: model.NewId(),
Props: map[string]string{
model.USER_AUTH_SERVICE_IS_MOBILE: "true",
model.USER_AUTH_SERVICE_IS_SAML: "true",
},
}
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(),
@@ -218,6 +247,19 @@ func TestApp_GetSessionLengthInMillis(t *testing.T) {
require.Equal(t, dayMillis*2, sessionLength)
})
t.Run("get session length SSO using props", func(t *testing.T) {
session := &model.Session{
UserId: model.NewId(),
Props: map[string]string{
model.USER_AUTH_SERVICE_IS_SAML: "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(),

2
model/saml.go
Просмотреть файл

@@ -13,6 +13,8 @@ import (
const (
USER_AUTH_SERVICE_SAML = "saml"
USER_AUTH_SERVICE_SAML_TEXT = "SAML"
USER_AUTH_SERVICE_IS_SAML = "isSaml"
USER_AUTH_SERVICE_IS_MOBILE = "isMobile"
)
type SamlAuthRequest struct {

35
model/session.go
Просмотреть файл

@@ -6,7 +6,10 @@ package model
import (
"encoding/json"
"io"
"strconv"
"strings"
"github.com/mattermost/mattermost-server/v5/mlog"
)
const (
@@ -140,7 +143,37 @@ func (me *Session) GetTeamByTeamId(teamId string) *TeamMember {
}
func (me *Session) IsMobileApp() bool {
return len(me.DeviceId) > 0
return len(me.DeviceId) > 0 || me.IsMobile()
}
func (me *Session) IsMobile() bool {
val, ok := me.Props[USER_AUTH_SERVICE_IS_MOBILE]
if !ok {
return false
}
isMobile, err := strconv.ParseBool(val)
if err != nil {
mlog.Error("Error parsing boolean property from Session", mlog.Err(err))
return false
}
return isMobile
}
func (me *Session) IsSaml() bool {
val, ok := me.Props[USER_AUTH_SERVICE_IS_SAML]
if !ok {
return false
}
isSaml, err := strconv.ParseBool(val)
if err != nil {
mlog.Error("Error parsing boolean property from Session", mlog.Err(err))
return false
}
return isSaml
}
func (me *Session) IsSSOLogin() bool {
return me.IsOAuth || me.IsSaml()
}
func (me *Session) GetUserRoles() []string {

11
web/oauth.go
Просмотреть файл

@@ -7,6 +7,7 @@ import (
"net/http"
"net/url"
"path/filepath"
"strconv"
"strings"
"github.com/mattermost/mattermost-server/v5/app"
@@ -301,7 +302,11 @@ func completeOAuth(c *Context, w http.ResponseWriter, r *http.Request) {
} else if action == model.OAUTH_ACTION_SSO_TO_EMAIL {
redirectUrl = app.GetProtocol(r) + "://" + r.Host + "/claim?email=" + url.QueryEscape(props["email"])
} else {
err = c.App.DoLogin(w, r, user, "")
isMobile, parseErr := strconv.ParseBool(props[model.USER_AUTH_SERVICE_IS_MOBILE])
if parseErr != nil {
mlog.Error("Error parsing boolean property from props", mlog.Err(parseErr))
}
err = c.App.DoLogin(w, r, user, "", isMobile, true, false)
if err != nil {
err.Translate(c.App.T)
c.Err = err
@@ -343,7 +348,7 @@ func loginWithOAuth(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
authUrl, err := c.App.GetOAuthLoginEndpoint(w, r, c.Params.Service, teamId, model.OAUTH_ACTION_LOGIN, redirectTo, loginHint)
authUrl, err := c.App.GetOAuthLoginEndpoint(w, r, c.Params.Service, teamId, model.OAUTH_ACTION_LOGIN, redirectTo, loginHint, false)
if err != nil {
c.Err = err
return
@@ -364,7 +369,7 @@ func mobileLoginWithOAuth(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
authUrl, err := c.App.GetOAuthLoginEndpoint(w, r, c.Params.Service, teamId, model.OAUTH_ACTION_MOBILE, "", "")
authUrl, err := c.App.GetOAuthLoginEndpoint(w, r, c.Params.Service, teamId, model.OAUTH_ACTION_MOBILE, "", "", true)
if err != nil {
c.Err = err
return

10
web/saml.go
Просмотреть файл

@@ -6,6 +6,7 @@ package web
import (
b64 "encoding/base64"
"net/http"
"strconv"
"strings"
"github.com/mattermost/mattermost-server/v5/audit"
@@ -32,6 +33,7 @@ func loginWithSaml(c *Context, w http.ResponseWriter, r *http.Request) {
return
}
action := r.URL.Query().Get("action")
isMobile := action == model.OAUTH_ACTION_MOBILE
redirectTo := r.URL.Query().Get("redirect_to")
relayProps := map[string]string{}
relayState := ""
@@ -48,6 +50,8 @@ func loginWithSaml(c *Context, w http.ResponseWriter, r *http.Request) {
relayProps["redirect_to"] = redirectTo
}
relayProps[model.USER_AUTH_SERVICE_IS_MOBILE] = strconv.FormatBool(isMobile)
if len(relayProps) > 0 {
relayState = b64.StdEncoding.EncodeToString([]byte(model.MapToJson(relayProps)))
}
@@ -142,7 +146,11 @@ func completeSaml(c *Context, w http.ResponseWriter, r *http.Request) {
auditRec.AddMeta("obtained_user_id", user.Id)
c.LogAuditWithUserId(user.Id, "obtained user")
err = c.App.DoLogin(w, r, user, "")
isMobile, parseErr := strconv.ParseBool(relayProps[model.USER_AUTH_SERVICE_IS_MOBILE])
if parseErr != nil {
mlog.Error("Error parsing boolean property from relay props", mlog.Err(parseErr))
}
err = c.App.DoLogin(w, r, user, "", isMobile, false, true)
if err != nil {
c.Err = err
return