From 24957f5e224edbfad7e8bbfc47dd8e20ab927bbb Mon Sep 17 00:00:00 2001 From: Christopher Poile Date: Tue, 10 Feb 2026 10:10:27 -0500 Subject: [PATCH] [MM-63393] Add support for preferred_username claims (#30852) * rebased all prev commits into one (see commit desc) add UsePreferredUsername support to gitlab; tests resort en.json update an out of date comment webapp i18n simplify username logic new arguments needed in tests debug statements -- revert * merge conflicts * fix i18n --------- Co-authored-by: Mattermost Build --- server/channels/app/oauth.go | 43 ++- .../app/oauthproviders/gitlab/gitlab.go | 30 +- .../app/oauthproviders/gitlab/gitlab_test.go | 320 ++++++++++++++++++ server/channels/app/user.go | 30 +- server/channels/app/user_test.go | 4 +- server/channels/web/oauth_test.go | 2 +- server/einterfaces/mocks/OAuthProvider.go | 18 +- server/einterfaces/oauthproviders.go | 8 +- server/i18n/en.json | 4 + server/public/model/config.go | 50 +-- .../admin_console/admin_definition.tsx | 27 ++ webapp/channels/src/i18n/en.json | 2 + 12 files changed, 469 insertions(+), 69 deletions(-) create mode 100644 server/channels/app/oauthproviders/gitlab/gitlab_test.go diff --git a/server/channels/app/oauth.go b/server/channels/app/oauth.go index 4767ac8b527..ecfe2118908 100644 --- a/server/channels/app/oauth.go +++ b/server/channels/app/oauth.go @@ -740,6 +740,7 @@ func (a *App) getSSOProvider(service string) (einterfaces.OAuthProvider, *model. return provider, nil } +// TODO: merge conflict, needs teamID string func (a *App) LoginByOAuth(rctx request.CTX, service string, userData io.Reader, inviteToken string, inviteId string, tokenUser *model.User) (*model.User, *model.AppError) { provider, e := a.getSSOProvider(service) if e != nil { @@ -752,10 +753,16 @@ func (a *App) LoginByOAuth(rctx request.CTX, service string, userData io.Reader, map[string]any{"Service": service}, "", http.StatusBadRequest) } - authUser, err1 := provider.GetUserFromJSON(rctx, bytes.NewReader(buf.Bytes()), tokenUser) - if err1 != nil { + settings, err := provider.GetSSOSettings(rctx, a.Config(), service) + if err != nil { + return nil, model.NewAppError("LoginByOAuth", "api.user.oauth.get_settings.app_error", + map[string]any{"Service": service}, "", http.StatusBadRequest).Wrap(err) + } + + authUser, err := provider.GetUserFromJSON(rctx, bytes.NewReader(buf.Bytes()), tokenUser, settings) + if err != nil { return nil, model.NewAppError("LoginByOAuth", "api.user.login_by_oauth.parse.app_error", - map[string]any{"Service": service}, "", http.StatusBadRequest).Wrap(err1) + map[string]any{"Service": service}, "", http.StatusBadRequest).Wrap(err) } if *authUser.AuthData == "" { @@ -763,12 +770,12 @@ func (a *App) LoginByOAuth(rctx request.CTX, service string, userData io.Reader, map[string]any{"Service": service}, "", http.StatusBadRequest) } - user, err := a.GetUserByAuth(model.NewPointer(*authUser.AuthData), service) - if err != nil { - if err.Id == MissingAuthAccountError { - user, err = a.CreateOAuthUser(rctx, service, bytes.NewReader(buf.Bytes()), inviteToken, inviteId, tokenUser) + user, appErr := a.GetUserByAuth(model.NewPointer(*authUser.AuthData), service) + if appErr != nil { + if appErr.Id == MissingAuthAccountError { + user, appErr = a.CreateOAuthUser(rctx, service, bytes.NewReader(buf.Bytes()), inviteToken, inviteId, tokenUser) } else { - return nil, err + return nil, appErr } } else { // OAuth doesn't run through CheckUserPreflightAuthenticationCriteria, so prevent bot login @@ -778,17 +785,17 @@ func (a *App) LoginByOAuth(rctx request.CTX, service string, userData io.Reader, return nil, model.NewAppError("loginByOAuth", "api.user.login_by_oauth.bot_login_forbidden.app_error", nil, "", http.StatusForbidden) } - if err = a.UpdateOAuthUserAttrs(rctx, bytes.NewReader(buf.Bytes()), user, provider, service, tokenUser); err != nil { - return nil, err + if appErr = a.UpdateOAuthUserAttrs(rctx, bytes.NewReader(buf.Bytes()), user, provider, service, tokenUser); appErr != nil { + return nil, appErr } - if err = a.AddUserToTeamByInviteIfNeeded(rctx, user, inviteToken, inviteId); err != nil { - rctx.Logger().Warn("Failed to add user to team", mlog.Err(err)) + if appErr = a.AddUserToTeamByInviteIfNeeded(rctx, user, inviteToken, inviteId); appErr != nil { + rctx.Logger().Warn("Failed to add user to team", mlog.Err(appErr)) } } - if err != nil { - return nil, err + if appErr != nil { + return nil, appErr } return user, nil @@ -860,7 +867,13 @@ func (a *App) CompleteSwitchWithOAuth(rctx request.CTX, service string, userData return nil, model.NewAppError("CompleteSwitchWithOAuth", "api.user.complete_switch_with_oauth.blank_email.app_error", nil, "", http.StatusBadRequest) } - ssoUser, err1 := provider.GetUserFromJSON(rctx, userData, tokenUser) + settings, err := provider.GetSSOSettings(rctx, a.Config(), service) + if err != nil { + return nil, model.NewAppError("CompleteSwitchWithOAuth", "api.user.oauth.get_settings.app_error", + map[string]any{"Service": service}, "", http.StatusBadRequest).Wrap(err) + } + + ssoUser, err1 := provider.GetUserFromJSON(rctx, userData, tokenUser, settings) if err1 != nil { return nil, model.NewAppError("CompleteSwitchWithOAuth", "api.user.complete_switch_with_oauth.parse.app_error", map[string]any{"Service": service}, "", http.StatusBadRequest).Wrap(err1) diff --git a/server/channels/app/oauthproviders/gitlab/gitlab.go b/server/channels/app/oauthproviders/gitlab/gitlab.go index 2d2ca73e870..d3835ae77b3 100644 --- a/server/channels/app/oauthproviders/gitlab/gitlab.go +++ b/server/channels/app/oauthproviders/gitlab/gitlab.go @@ -20,11 +20,12 @@ type GitLabProvider struct { } type GitLabUser struct { - Id int64 `json:"id"` - Username string `json:"username"` - Login string `json:"login"` - Email string `json:"email"` - Name string `json:"name"` + Id int64 `json:"id"` + Username string `json:"username"` + Login string `json:"login"` + Email string `json:"email"` + Name string `json:"name"` + PreferredUsername string `json:"preferred_username"` } func init() { @@ -32,13 +33,22 @@ func init() { einterfaces.RegisterOAuthProvider(model.UserAuthServiceGitlab, provider) } -func userFromGitLabUser(logger mlog.LoggerIFace, glu *GitLabUser) *model.User { +func userFromGitLabUser(logger mlog.LoggerIFace, glu *GitLabUser, settings *model.SSOSettings) *model.User { user := &model.User{} - username := glu.Username - if username == "" { + + // set username in order of preference + var username string + if settings != nil && model.SafeDereference(settings.UsePreferredUsername) && glu.PreferredUsername != "" { + // to maintain consistency with other providers, we split the username by @ (if present) and + // take the first part (but only for the preferred username) + username = strings.Split(glu.PreferredUsername, "@")[0] + } else if glu.Username != "" { + username = glu.Username + } else { username = glu.Login } user.Username = model.CleanUsername(logger, username) + splitName := strings.Split(glu.Name, " ") if len(splitName) == 2 { user.FirstName = splitName[0] @@ -84,7 +94,7 @@ func (glu *GitLabUser) getAuthData() string { return strconv.FormatInt(glu.Id, 10) } -func (gp *GitLabProvider) GetUserFromJSON(rctx request.CTX, data io.Reader, tokenUser *model.User) (*model.User, error) { +func (gp *GitLabProvider) GetUserFromJSON(rctx request.CTX, data io.Reader, tokenUser *model.User, settings *model.SSOSettings) (*model.User, error) { glu, err := gitLabUserFromJSON(data) if err != nil { return nil, err @@ -93,7 +103,7 @@ func (gp *GitLabProvider) GetUserFromJSON(rctx request.CTX, data io.Reader, toke return nil, err } - return userFromGitLabUser(rctx.Logger(), glu), nil + return userFromGitLabUser(rctx.Logger(), glu, settings), nil } func (gp *GitLabProvider) GetSSOSettings(_ request.CTX, config *model.Config, service string) (*model.SSOSettings, error) { diff --git a/server/channels/app/oauthproviders/gitlab/gitlab_test.go b/server/channels/app/oauthproviders/gitlab/gitlab_test.go new file mode 100644 index 00000000000..76c782c77f3 --- /dev/null +++ b/server/channels/app/oauthproviders/gitlab/gitlab_test.go @@ -0,0 +1,320 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +package oauthgitlab + +import ( + "bytes" + "encoding/json" + "strconv" + "strings" + "testing" + + "github.com/mattermost/mattermost/server/public/model" + "github.com/mattermost/mattermost/server/public/shared/mlog" + "github.com/mattermost/mattermost/server/public/shared/request" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestGitLabUserFromJSON(t *testing.T) { + rctx := request.TestContext(t) + glu := GitLabUser{ + Id: 12345, + Username: "testuser", + Login: "testlogin", + Email: "test@example.com", + Name: "Test User", + } + + provider := &GitLabProvider{} + + t.Run("valid gitlab user", func(t *testing.T) { + b, err := json.Marshal(glu) + require.NoError(t, err) + + userJSON, err := provider.GetUserFromJSON(rctx, bytes.NewReader(b), nil, nil) + require.NoError(t, err) + // We get AuthData via GetUserFromJSON which calls userFromGitLabUser which calls getAuthData + require.Equal(t, strconv.FormatInt(glu.Id, 10), *userJSON.AuthData) + + // Check GetAuthDataFromJSON indirectly by ensuring GetUserFromJSON worked, + // as GetAuthDataFromJSON is not directly exported or used elsewhere in a testable way without duplicating logic. + // It relies on gitLabUserFromJSON and glu.IsValid, which are tested separately. + }) + + t.Run("empty body should fail validation", func(t *testing.T) { + // GetUserFromJSON should return an error because IsValid fails on the empty user + _, err := provider.GetUserFromJSON(rctx, strings.NewReader("{}"), nil, nil) + require.Error(t, err) + assert.Contains(t, err.Error(), "user id can't be 0") + }) + + t.Run("invalid json", func(t *testing.T) { + _, err := provider.GetUserFromJSON(rctx, strings.NewReader("invalid json"), nil, nil) + require.Error(t, err) + }) +} + +func TestGitLabUserIsValid(t *testing.T) { + testCases := []struct { + description string + user GitLabUser + isValid bool + expectedErr string + }{ + {"valid user", GitLabUser{Id: 1, Email: "test@example.com"}, true, ""}, + {"zero id", GitLabUser{Id: 0, Email: "test@example.com"}, false, "user id can't be 0"}, + {"empty email", GitLabUser{Id: 1, Email: ""}, false, "user e-mail should not be empty"}, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + err := tc.user.IsValid() + if tc.isValid { + require.NoError(t, err) + } else { + require.Error(t, err) + assert.Equal(t, tc.expectedErr, err.Error()) + } + }) + } +} + +func TestGitLabUserGetAuthData(t *testing.T) { + glu := GitLabUser{Id: 98765} + assert.Equal(t, "98765", glu.getAuthData()) +} + +func TestUserFromGitLabUser(t *testing.T) { + logger := mlog.CreateConsoleTestLogger(t) + + testCases := []struct { + description string + gitlabUser GitLabUser + usePreferredUsername bool + expectedUsername string + expectedFirstName string + expectedLastName string + expectedEmail string + expectedAuthData string + }{ + { + description: "Username from PreferredUsername when UsePreferredUsername=true", + gitlabUser: GitLabUser{ + Id: 1, + Username: "gitlab.user", + Login: "gitlab.login", + Email: "test@example.com", + Name: "First Last", + PreferredUsername: "preferred.user", + }, + usePreferredUsername: true, + expectedUsername: "preferred.user", + expectedFirstName: "First", + expectedLastName: "Last", + expectedEmail: "test@example.com", + expectedAuthData: "1", + }, + { + description: "Username from Username when UsePreferredUsername=false even if PreferredUsername exists", + gitlabUser: GitLabUser{ + Id: 2, + Username: "gitlab.user", + Login: "gitlab.login", + Email: "test2@example.com", + Name: "First Last", + PreferredUsername: "preferred.user", + }, + usePreferredUsername: false, + expectedUsername: "gitlab.user", + expectedFirstName: "First", + expectedLastName: "Last", + expectedEmail: "test2@example.com", + expectedAuthData: "2", + }, + { + description: "Username from Username when PreferredUsername is empty and UsePreferredUsername=true", + gitlabUser: GitLabUser{ + Id: 3, + Username: "gitlab.user.only", + Login: "gitlab.login", + Email: "test3@example.com", + Name: "Another User", + PreferredUsername: "", + }, + usePreferredUsername: true, + expectedUsername: "gitlab.user.only", + expectedFirstName: "Another", + expectedLastName: "User", + expectedEmail: "test3@example.com", + expectedAuthData: "3", + }, + { + description: "Username from PreferredUsername when Username is empty and UsePreferredUsername=true", + gitlabUser: GitLabUser{ + Id: 5, + Username: "", + Login: "gitlab.login.only", + Email: "test5@example.com", + Name: "Login User", + PreferredUsername: "preferred.user.again", + }, + usePreferredUsername: true, + expectedUsername: "preferred.user.again", + expectedFirstName: "Login", + expectedLastName: "User", + expectedEmail: "test5@example.com", + expectedAuthData: "5", + }, + { + description: "Username from Login when Username is empty and UsePreferredUsername=false, even if PreferredUsername exists", + gitlabUser: GitLabUser{ + Id: 6, + Username: "", + Login: "gitlab.login.only.again", + Email: "test6@example.com", + Name: "Login User", + PreferredUsername: "preferred.user.ignored", + }, + usePreferredUsername: false, + expectedUsername: "gitlab.login.only.again", + expectedFirstName: "Login", + expectedLastName: "User", + expectedEmail: "test6@example.com", + expectedAuthData: "6", + }, + { + description: "Username from Login when Username and PreferredUsername are empty and UsePreferredUsername=true", + gitlabUser: GitLabUser{ + Id: 7, + Username: "", + Login: "the.login", + Email: "test7@example.com", + Name: "Just Login", + PreferredUsername: "", + }, + usePreferredUsername: true, + expectedUsername: "the.login", + expectedFirstName: "Just", + expectedLastName: "Login", + expectedEmail: "test7@example.com", + expectedAuthData: "7", + }, + { + description: "Name splitting: single name", + gitlabUser: GitLabUser{ + Id: 9, + Username: "testuser9", + Email: "test9@example.com", + Name: "Mononym", + }, + usePreferredUsername: false, + expectedUsername: "testuser9", + expectedFirstName: "Mononym", + expectedLastName: "", + expectedEmail: "test9@example.com", + expectedAuthData: "9", + }, + { + description: "Name splitting: multiple last names", + gitlabUser: GitLabUser{ + Id: 10, + Username: "testuser10", + Email: "test10@example.com", + Name: "First Middle Van Der Lastname", + }, + usePreferredUsername: false, + expectedUsername: "testuser10", + expectedFirstName: "First", + expectedLastName: "Middle Van Der Lastname", + expectedEmail: "test10@example.com", + expectedAuthData: "10", + }, + { + description: "Email lowercasing", + gitlabUser: GitLabUser{ + Id: 11, + Username: "testuser11", + Email: "TEST11@EXAMPLE.COM", + Name: "Test User", + }, + usePreferredUsername: false, + expectedUsername: "testuser11", + expectedFirstName: "Test", + expectedLastName: "User", + expectedEmail: "test11@example.com", + expectedAuthData: "11", + }, + { + description: "Username needing cleaning when UsePreferredUsername=true", + gitlabUser: GitLabUser{ + Id: 12, + Username: "gitlab.user", + Login: "gitlab.login", + Email: "test12@example.com", + Name: "Needs Clean", + PreferredUsername: "preferred@@user!!", + }, + usePreferredUsername: true, + expectedUsername: "preferred", // Cleaned + expectedFirstName: "Needs", + expectedLastName: "Clean", + expectedEmail: "test12@example.com", + expectedAuthData: "12", + }, + { + description: "Username needing cleaning when UsePreferredUsername=false", + gitlabUser: GitLabUser{ + Id: 13, + Username: "gitlab@@user!!", + Login: "gitlab.login", + Email: "test13@example.com", + Name: "Needs Clean", + PreferredUsername: "preferred.user", + }, + usePreferredUsername: false, + expectedUsername: "gitlab--user", // Cleaned + expectedFirstName: "Needs", + expectedLastName: "Clean", + expectedEmail: "test13@example.com", + expectedAuthData: "13", + }, + { + description: "Login needing cleaning when UsePreferredUsername=false and Username is empty", + gitlabUser: GitLabUser{ + Id: 14, + Username: "", + Login: "gitlab@@login!!", + Email: "test14@example.com", + Name: "Needs Clean", + PreferredUsername: "preferred.user", + }, + usePreferredUsername: false, + expectedUsername: "gitlab--login", // Cleaned + expectedFirstName: "Needs", + expectedLastName: "Clean", + expectedEmail: "test14@example.com", + expectedAuthData: "14", + }, + } + + for _, tc := range testCases { + t.Run(tc.description, func(t *testing.T) { + settings := &model.SSOSettings{ + UsePreferredUsername: model.NewPointer(tc.usePreferredUsername), + } + + user := userFromGitLabUser(logger, &tc.gitlabUser, settings) + + require.NotNil(t, user) + assert.Equal(t, tc.expectedUsername, user.Username) + assert.Equal(t, tc.expectedFirstName, user.FirstName) + assert.Equal(t, tc.expectedLastName, user.LastName) + assert.Equal(t, tc.expectedEmail, user.Email) + require.NotNil(t, user.AuthData) + assert.Equal(t, tc.expectedAuthData, *user.AuthData) + assert.Equal(t, model.UserAuthServiceGitlab, user.AuthService) + }) + } +} diff --git a/server/channels/app/user.go b/server/channels/app/user.go index 3fc77da2c9b..dc9e7277657 100644 --- a/server/channels/app/user.go +++ b/server/channels/app/user.go @@ -441,9 +441,15 @@ func (a *App) CreateOAuthUser(rctx request.CTX, service string, userData io.Read if e != nil { return nil, e } - user, err1 := provider.GetUserFromJSON(rctx, userData, tokenUser) - if err1 != nil { - return nil, model.NewAppError("CreateOAuthUser", "api.user.create_oauth_user.create.app_error", map[string]any{"Service": service}, "", http.StatusInternalServerError).Wrap(err1) + + settings, err := provider.GetSSOSettings(rctx, a.Config(), service) + if err != nil { + return nil, model.NewAppError("CreateOAuthUser", "api.user.oauth.get_settings.app_error", map[string]any{"Service": service}, "", http.StatusInternalServerError).Wrap(err) + } + + user, err := provider.GetUserFromJSON(rctx, userData, tokenUser, settings) + if err != nil { + return nil, model.NewAppError("CreateOAuthUser", "api.user.create_oauth_user.create.app_error", map[string]any{"Service": service}, "", http.StatusInternalServerError).Wrap(err) } if user.AuthService == "" { user.AuthService = service @@ -469,7 +475,7 @@ func (a *App) CreateOAuthUser(rctx request.CTX, service string, userData io.Read return nil, model.NewAppError("CreateOAuthUser", "api.user.create_oauth_user.already_attached.app_error", map[string]any{"Service": service, "Auth": model.UserAuthServiceEmail}, "email="+user.Email, http.StatusBadRequest) } if provider.IsSameUser(rctx, userByEmail, user) { - if _, err := a.Srv().Store().User().UpdateAuthData(userByEmail.Id, user.AuthService, user.AuthData, "", false); err != nil { + if _, err = a.Srv().Store().User().UpdateAuthData(userByEmail.Id, user.AuthService, user.AuthData, "", false); err != nil { // if the user is not updated, write a warning to the log, but don't prevent user login rctx.Logger().Warn("Error attempting to update user AuthData", mlog.Err(err)) } @@ -480,13 +486,13 @@ func (a *App) CreateOAuthUser(rctx request.CTX, service string, userData io.Read user.EmailVerified = true - ruser, err := a.CreateUser(rctx, user) - if err != nil { - return nil, err + ruser, appErr := a.CreateUser(rctx, user) + if appErr != nil { + return nil, appErr } - if err = a.AddUserToTeamByInviteIfNeeded(rctx, ruser, inviteToken, inviteId); err != nil { - rctx.Logger().Warn("Failed to add user to team", mlog.Err(err)) + if appErr = a.AddUserToTeamByInviteIfNeeded(rctx, ruser, inviteToken, inviteId); appErr != nil { + rctx.Logger().Warn("Failed to add user to team", mlog.Err(appErr)) } return ruser, nil @@ -2449,7 +2455,11 @@ func (a *App) AutocompleteUsersInTeam(rctx request.CTX, teamID string, term stri } func (a *App) UpdateOAuthUserAttrs(rctx request.CTX, userData io.Reader, user *model.User, provider einterfaces.OAuthProvider, service string, tokenUser *model.User) *model.AppError { - oauthUser, err1 := provider.GetUserFromJSON(rctx, userData, tokenUser) + settings, err := provider.GetSSOSettings(rctx, a.Config(), service) + if err != nil { + return model.NewAppError("UpdateOAuthUserAttrs", "api.user.oauth.get_settings.app_error", map[string]any{"Service": service}, "", http.StatusInternalServerError).Wrap(err) + } + oauthUser, err1 := provider.GetUserFromJSON(rctx, userData, tokenUser, settings) if err1 != nil { return model.NewAppError("UpdateOAuthUserAttrs", "api.user.update_oauth_user_attrs.get_user.app_error", map[string]any{"Service": service}, "", http.StatusBadRequest).Wrap(err1) } diff --git a/server/channels/app/user_test.go b/server/channels/app/user_test.go index 9a6520d93eb..07656ced9fe 100644 --- a/server/channels/app/user_test.go +++ b/server/channels/app/user_test.go @@ -71,9 +71,11 @@ func TestCreateOAuthUser(t *testing.T) { // mock oAuth Provider, return data mockUser := &model.User{Id: "abcdef", AuthData: model.NewPointer("e7110007-64be-43d8-9840-4a7e9c26b710"), Email: dbUser.Email} + mockSSOSettings := &model.SSOSettings{} providerMock := &mocks.OAuthProvider{} providerMock.On("IsSameUser", mock.AnythingOfType("*request.Context"), mock.Anything, mock.Anything).Return(true) - providerMock.On("GetUserFromJSON", mock.AnythingOfType("*request.Context"), mock.Anything, mock.Anything).Return(mockUser, nil) + providerMock.On("GetUserFromJSON", mock.AnythingOfType("*request.Context"), mock.Anything, mock.Anything, mock.Anything).Return(mockUser, nil) + providerMock.On("GetSSOSettings", mock.AnythingOfType("*request.Context"), mock.Anything, mock.Anything).Return(mockSSOSettings, nil) einterfaces.RegisterOAuthProvider(model.ServiceOffice365, providerMock) // Update user to be OAuth, formatting to match Office365 OAuth data diff --git a/server/channels/web/oauth_test.go b/server/channels/web/oauth_test.go index 08c63b3fecf..f63eb4d623f 100644 --- a/server/channels/web/oauth_test.go +++ b/server/channels/web/oauth_test.go @@ -701,7 +701,7 @@ func closeBody(r *http.Response) { type MattermostTestProvider struct { } -func (m *MattermostTestProvider) GetUserFromJSON(_ request.CTX, data io.Reader, tokenUser *model.User) (*model.User, error) { +func (m *MattermostTestProvider) GetUserFromJSON(_ request.CTX, data io.Reader, tokenUser *model.User, settings *model.SSOSettings) (*model.User, error) { var user model.User if err := json.NewDecoder(data).Decode(&user); err != nil { return nil, err diff --git a/server/einterfaces/mocks/OAuthProvider.go b/server/einterfaces/mocks/OAuthProvider.go index d808116d7e2..e48f3929d3b 100644 --- a/server/einterfaces/mocks/OAuthProvider.go +++ b/server/einterfaces/mocks/OAuthProvider.go @@ -78,9 +78,9 @@ func (_m *OAuthProvider) GetUserFromIdToken(rctx request.CTX, idToken string) (* return r0, r1 } -// GetUserFromJSON provides a mock function with given fields: rctx, data, tokenUser -func (_m *OAuthProvider) GetUserFromJSON(rctx request.CTX, data io.Reader, tokenUser *model.User) (*model.User, error) { - ret := _m.Called(rctx, data, tokenUser) +// GetUserFromJSON provides a mock function with given fields: c, data, tokenUser, settings +func (_m *OAuthProvider) GetUserFromJSON(rctx request.CTX, data io.Reader, tokenUser *model.User, settings *model.SSOSettings) (*model.User, error) { + ret := _m.Called(rctx, data, tokenUser, settings) if len(ret) == 0 { panic("no return value specified for GetUserFromJSON") @@ -88,19 +88,19 @@ func (_m *OAuthProvider) GetUserFromJSON(rctx request.CTX, data io.Reader, token var r0 *model.User var r1 error - if rf, ok := ret.Get(0).(func(request.CTX, io.Reader, *model.User) (*model.User, error)); ok { - return rf(rctx, data, tokenUser) + if rf, ok := ret.Get(0).(func(request.CTX, io.Reader, *model.User, *model.SSOSettings) (*model.User, error)); ok { + return rf(rctx, data, tokenUser, settings) } - if rf, ok := ret.Get(0).(func(request.CTX, io.Reader, *model.User) *model.User); ok { - r0 = rf(rctx, data, tokenUser) + if rf, ok := ret.Get(0).(func(request.CTX, io.Reader, *model.User, *model.SSOSettings) *model.User); ok { + r0 = rf(rctx, data, tokenUser, settings) } else { if ret.Get(0) != nil { r0 = ret.Get(0).(*model.User) } } - if rf, ok := ret.Get(1).(func(request.CTX, io.Reader, *model.User) error); ok { - r1 = rf(rctx, data, tokenUser) + if rf, ok := ret.Get(1).(func(request.CTX, io.Reader, *model.User, *model.SSOSettings) error); ok { + r1 = rf(rctx, data, tokenUser, settings) } else { r1 = ret.Error(1) } diff --git a/server/einterfaces/oauthproviders.go b/server/einterfaces/oauthproviders.go index c8297a48c2c..365ef79ef1a 100644 --- a/server/einterfaces/oauthproviders.go +++ b/server/einterfaces/oauthproviders.go @@ -11,10 +11,10 @@ import ( ) type OAuthProvider interface { - GetUserFromJSON(rctx request.CTX, data io.Reader, tokenUser *model.User) (*model.User, error) - GetSSOSettings(rctx request.CTX, config *model.Config, service string) (*model.SSOSettings, error) - GetUserFromIdToken(rctx request.CTX, idToken string) (*model.User, error) - IsSameUser(rctx request.CTX, dbUser, oAuthUser *model.User) bool + GetUserFromJSON(c request.CTX, data io.Reader, tokenUser *model.User, settings *model.SSOSettings) (*model.User, error) + GetSSOSettings(c request.CTX, config *model.Config, service string) (*model.SSOSettings, error) + GetUserFromIdToken(c request.CTX, idToken string) (*model.User, error) + IsSameUser(c request.CTX, dbUser, oAuthUser *model.User) bool } var oauthProviders = make(map[string]OAuthProvider) diff --git a/server/i18n/en.json b/server/i18n/en.json index 817f95dd412..8e90a137f93 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -4534,6 +4534,10 @@ "id": "api.user.login_with_desktop_token.not_oauth_or_saml_user.app_error", "translation": "User is not an OAuth or SAML user." }, + { + "id": "api.user.oauth.get_settings.app_error", + "translation": "Failed to get {{.Service}} settings" + }, { "id": "api.user.oauth_to_email.context.app_error", "translation": "Update password failed because context user_id did not match provided user's id." diff --git a/server/public/model/config.go b/server/public/model/config.go index 562d4d04fad..7ad5bf7b6b1 100644 --- a/server/public/model/config.go +++ b/server/public/model/config.go @@ -1266,16 +1266,17 @@ func (s *AnalyticsSettings) SetDefaults() { } type SSOSettings struct { - Enable *bool `access:"authentication_openid"` - Secret *string `access:"authentication_openid"` // telemetry: none - Id *string `access:"authentication_openid"` // telemetry: none - Scope *string `access:"authentication_openid"` // telemetry: none - AuthEndpoint *string `access:"authentication_openid"` // telemetry: none - TokenEndpoint *string `access:"authentication_openid"` // telemetry: none - UserAPIEndpoint *string `access:"authentication_openid"` // telemetry: none - DiscoveryEndpoint *string `access:"authentication_openid"` // telemetry: none - ButtonText *string `access:"authentication_openid"` // telemetry: none - ButtonColor *string `access:"authentication_openid"` // telemetry: none + Enable *bool `access:"authentication_openid"` + Secret *string `access:"authentication_openid"` // telemetry: none + Id *string `access:"authentication_openid"` // telemetry: none + Scope *string `access:"authentication_openid"` // telemetry: none + AuthEndpoint *string `access:"authentication_openid"` // telemetry: none + TokenEndpoint *string `access:"authentication_openid"` // telemetry: none + UserAPIEndpoint *string `access:"authentication_openid"` // telemetry: none + DiscoveryEndpoint *string `access:"authentication_openid"` // telemetry: none + ButtonText *string `access:"authentication_openid"` // telemetry: none + ButtonColor *string `access:"authentication_openid"` // telemetry: none + UsePreferredUsername *bool `access:"authentication_openid"` // telemetry: none } func (s *SSOSettings) setDefaults(scope, authEndpoint, tokenEndpoint, userAPIEndpoint, buttonColor string) { @@ -1318,18 +1319,24 @@ func (s *SSOSettings) setDefaults(scope, authEndpoint, tokenEndpoint, userAPIEnd if s.ButtonColor == nil { s.ButtonColor = NewPointer(buttonColor) } + + // Note: Preferred username is not supported for Google. + if s.UsePreferredUsername == nil { + s.UsePreferredUsername = NewPointer(false) + } } type Office365Settings struct { - Enable *bool `access:"authentication_openid"` - Secret *string `access:"authentication_openid"` // telemetry: none - Id *string `access:"authentication_openid"` // telemetry: none - Scope *string `access:"authentication_openid"` - AuthEndpoint *string `access:"authentication_openid"` // telemetry: none - TokenEndpoint *string `access:"authentication_openid"` // telemetry: none - UserAPIEndpoint *string `access:"authentication_openid"` // telemetry: none - DiscoveryEndpoint *string `access:"authentication_openid"` // telemetry: none - DirectoryId *string `access:"authentication_openid"` // telemetry: none + Enable *bool `access:"authentication_openid"` + Secret *string `access:"authentication_openid"` // telemetry: none + Id *string `access:"authentication_openid"` // telemetry: none + Scope *string `access:"authentication_openid"` + AuthEndpoint *string `access:"authentication_openid"` // telemetry: none + TokenEndpoint *string `access:"authentication_openid"` // telemetry: none + UserAPIEndpoint *string `access:"authentication_openid"` // telemetry: none + DiscoveryEndpoint *string `access:"authentication_openid"` // telemetry: none + DirectoryId *string `access:"authentication_openid"` // telemetry: none + UsePreferredUsername *bool `access:"authentication_openid"` // telemetry: none } func (s *Office365Settings) setDefaults() { @@ -1368,6 +1375,10 @@ func (s *Office365Settings) setDefaults() { if s.DirectoryId == nil { s.DirectoryId = NewPointer("") } + + if s.UsePreferredUsername == nil { + s.UsePreferredUsername = NewPointer(false) + } } func (s *Office365Settings) SSOSettings() *SSOSettings { @@ -1380,6 +1391,7 @@ func (s *Office365Settings) SSOSettings() *SSOSettings { ssoSettings.AuthEndpoint = s.AuthEndpoint ssoSettings.TokenEndpoint = s.TokenEndpoint ssoSettings.UserAPIEndpoint = s.UserAPIEndpoint + ssoSettings.UsePreferredUsername = s.UsePreferredUsername return &ssoSettings } diff --git a/webapp/channels/src/components/admin_console/admin_definition.tsx b/webapp/channels/src/components/admin_console/admin_definition.tsx index 1690dc7c9ce..f23d93537cc 100644 --- a/webapp/channels/src/components/admin_console/admin_definition.tsx +++ b/webapp/channels/src/components/admin_console/admin_definition.tsx @@ -5054,6 +5054,33 @@ const AdminDefinition: AdminDefinitionType = { isHidden: it.any(it.not(it.stateEquals('openidType', Constants.OPENID_SERVICE)), it.licensedForCloudStarter), isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.OPENID)), }, + { + type: 'bool', + key: 'GitLabSettings.UsePreferredUsername', + label: defineMessage({id: 'admin.openid.usePreferredUsernameTitle', defaultMessage: 'Use Preferred Username:'}), + help_text: defineMessage({id: 'admin.openid.usePreferredUsernameDescription', defaultMessage: 'When true, use the `preferred_username` claim as the Mattermost username for the user. The scope must include `profile` and `openid` to use this feature.'}), + help_text_markdown: true, + isHidden: it.not(it.stateEquals('openidType', Constants.GITLAB_SERVICE)), + isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.OPENID)), + }, + { + type: 'bool', + key: 'Office365Settings.UsePreferredUsername', + label: defineMessage({id: 'admin.openid.usePreferredUsernameTitle', defaultMessage: 'Use Preferred Username:'}), + help_text: defineMessage({id: 'admin.openid.usePreferredUsernameDescription', defaultMessage: 'When true, use the `preferred_username` claim as the Mattermost username for the user. The scope must include `profile` and `openid` to use this feature.'}), + help_text_markdown: true, + isHidden: it.not(it.stateEquals('openidType', Constants.OFFICE365_SERVICE)), + isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.OPENID)), + }, + { + type: 'bool', + key: 'OpenIdSettings.UsePreferredUsername', + label: defineMessage({id: 'admin.openid.usePreferredUsernameTitle', defaultMessage: 'Use Preferred Username:'}), + help_text: defineMessage({id: 'admin.openid.usePreferredUsernameDescription', defaultMessage: 'When true, use the `preferred_username` claim as the Mattermost username for the user. The scope must include `profile` and `openid` to use this feature.'}), + help_text_markdown: true, + isHidden: it.not(it.stateEquals('openidType', Constants.OPENID_SERVICE)), + isDisabled: it.not(it.userHasWritePermissionOnResource(RESOURCE_KEYS.AUTHENTICATION.OPENID)), + }, { type: 'custom', key: 'OpenIDCustomFeatureDiscovery', diff --git a/webapp/channels/src/i18n/en.json b/webapp/channels/src/i18n/en.json index 711dadd9b5b..9871699e17f 100644 --- a/webapp/channels/src/i18n/en.json +++ b/webapp/channels/src/i18n/en.json @@ -1868,6 +1868,8 @@ "admin.openid.off": "Do not allow sign-in via an OpenID provider.", "admin.openid.office365": "Entra ID", "admin.openid.select": "Select service provider:", + "admin.openid.usePreferredUsernameDescription": "When true, use the `preferred_username` claim as the Mattermost username for the user. The scope must include `profile` and `openid` to use this feature.", + "admin.openid.usePreferredUsernameTitle": "Use Preferred Username:", "admin.openIdConvert.help": "Learn more", "admin.openIdConvert.message": "You can now convert your OAuth2.0 configuration to OpenID Connect.", "admin.openIdConvert.text": "Convert to OpenID Connect",