diff --git a/app/user.go b/app/user.go index f2dcaa3530..b0217969e6 100644 --- a/app/user.go +++ b/app/user.go @@ -376,7 +376,14 @@ func (a *App) CreateOAuthUser(c *request.Context, service string, userData io.Re if userByEmail.AuthService == "" { return nil, model.NewAppError("CreateOAuthUser", "api.user.create_oauth_user.already_attached.app_error", map[string]interface{}{"Service": service, "Auth": model.USER_AUTH_SERVICE_EMAIL}, "email="+user.Email, http.StatusBadRequest) } - return nil, model.NewAppError("CreateOAuthUser", "api.user.create_oauth_user.already_attached.app_error", map[string]interface{}{"Service": service, "Auth": userByEmail.AuthService}, "email="+user.Email, http.StatusBadRequest) + if provider.IsSameUser(userByEmail, user) { + 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 + mlog.Warn("Error attempting to update user AuthData", mlog.Err(err)) + } + return userByEmail, nil + } + return nil, model.NewAppError("CreateOAuthUser", "api.user.create_oauth_user.already_attached.app_error", map[string]interface{}{"Service": service, "Auth": userByEmail.AuthService}, "email="+user.Email+" authData="+*user.AuthData, http.StatusBadRequest) } user.EmailVerified = true diff --git a/app/user_test.go b/app/user_test.go index 97c0f1c7d7..7ab4a8e1cb 100644 --- a/app/user_test.go +++ b/app/user_test.go @@ -15,10 +15,12 @@ import ( "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/mattermost/mattermost-server/v5/app/request" "github.com/mattermost/mattermost-server/v5/einterfaces" + "github.com/mattermost/mattermost-server/v5/einterfaces/mocks" "github.com/mattermost/mattermost-server/v5/model" oauthgitlab "github.com/mattermost/mattermost-server/v5/model/gitlab" "github.com/mattermost/mattermost-server/v5/store" @@ -82,20 +84,51 @@ func TestCreateOAuthUser(t *testing.T) { *cfg.GitLabSettings.Enable = true }) - glUser := oauthgitlab.GitLabUser{Id: 42, Username: "o" + model.NewId(), Email: model.NewId() + "@simulator.amazonses.com", Name: "Joram Wilander"} + t.Run("create user successfully", func(t *testing.T) { + glUser := oauthgitlab.GitLabUser{Id: 42, Username: "o" + model.NewId(), Email: model.NewId() + "@simulator.amazonses.com", Name: "Joram Wilander"} + json := glUser.ToJson() - json := glUser.ToJson() - user, err := th.App.CreateOAuthUser(th.Context, model.USER_AUTH_SERVICE_GITLAB, strings.NewReader(json), th.BasicTeam.Id, nil) - require.Nil(t, err) + user, err := th.App.CreateOAuthUser(th.Context, model.USER_AUTH_SERVICE_GITLAB, strings.NewReader(json), th.BasicTeam.Id, nil) + require.Nil(t, err) - require.Equal(t, glUser.Username, user.Username, "usernames didn't match") + require.Equal(t, glUser.Username, user.Username, "usernames didn't match") - th.App.PermanentDeleteUser(th.Context, user) + th.App.PermanentDeleteUser(th.Context, user) + }) - *th.App.Config().TeamSettings.EnableUserCreation = false + t.Run("user exists, update authdata successfully", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + *cfg.Office365Settings.Enable = true + }) - _, err = th.App.CreateOAuthUser(th.Context, model.USER_AUTH_SERVICE_GITLAB, strings.NewReader(json), th.BasicTeam.Id, nil) - require.NotNil(t, err, "should have failed - user creation disabled") + dbUser := th.BasicUser + + // mock oAuth Provider, return data + mockUser := &model.User{Id: "abcdef", AuthData: model.NewString("e7110007-64be-43d8-9840-4a7e9c26b710"), Email: dbUser.Email} + providerMock := &mocks.OauthProvider{} + providerMock.On("IsSameUser", mock.Anything, mock.Anything).Return(true) + providerMock.On("GetUserFromJson", mock.Anything, mock.Anything).Return(mockUser, nil) + einterfaces.RegisterOauthProvider(model.SERVICE_OFFICE365, providerMock) + + // Update user to be OAuth, formatting to match Office365 OAuth data + s, er2 := th.App.Srv().Store.User().UpdateAuthData(dbUser.Id, model.SERVICE_OFFICE365, model.NewString("e711000764be43d898404a7e9c26b710"), "", false) + assert.NoError(t, er2) + assert.Equal(t, dbUser.Id, s) + + // data passed doesn't matter as return is mocked + _, err := th.App.CreateOAuthUser(th.Context, model.SERVICE_OFFICE365, strings.NewReader("{}"), th.BasicTeam.Id, nil) + assert.Nil(t, err) + u, er := th.App.Srv().Store.User().GetByEmail(dbUser.Email) + assert.NoError(t, er) + // make sure authdata is updated + assert.Equal(t, "e7110007-64be-43d8-9840-4a7e9c26b710", *u.AuthData) + }) + + t.Run("user creation disabled", func(t *testing.T) { + *th.App.Config().TeamSettings.EnableUserCreation = false + _, err := th.App.CreateOAuthUser(th.Context, model.USER_AUTH_SERVICE_GITLAB, strings.NewReader("{}"), th.BasicTeam.Id, nil) + require.NotNil(t, err, "should have failed - user creation disabled") + }) } func TestCreateProfileImage(t *testing.T) { diff --git a/einterfaces/mocks/OauthProvider.go b/einterfaces/mocks/OauthProvider.go index ce6df85b47..4cf4b20114 100644 --- a/einterfaces/mocks/OauthProvider.go +++ b/einterfaces/mocks/OauthProvider.go @@ -84,3 +84,17 @@ func (_m *OauthProvider) GetUserFromJson(data io.Reader, tokenUser *model.User) return r0, r1 } + +// IsSameUser provides a mock function with given fields: dbUser, oAuthUser +func (_m *OauthProvider) IsSameUser(dbUser *model.User, oAuthUser *model.User) bool { + ret := _m.Called(dbUser, oAuthUser) + + var r0 bool + if rf, ok := ret.Get(0).(func(*model.User, *model.User) bool); ok { + r0 = rf(dbUser, oAuthUser) + } else { + r0 = ret.Get(0).(bool) + } + + return r0 +} diff --git a/einterfaces/oauthproviders.go b/einterfaces/oauthproviders.go index def190c78b..af3335f5dd 100644 --- a/einterfaces/oauthproviders.go +++ b/einterfaces/oauthproviders.go @@ -13,6 +13,7 @@ type OauthProvider interface { GetUserFromJson(data io.Reader, tokenUser *model.User) (*model.User, error) GetSSOSettings(config *model.Config, service string) (*model.SSOSettings, error) GetUserFromIdToken(idToken string) (*model.User, error) + IsSameUser(dbUser, oAuthUser *model.User) bool } var oauthProviders = make(map[string]OauthProvider) diff --git a/model/gitlab/gitlab.go b/model/gitlab/gitlab.go index 19639f0f70..ac90a481a9 100644 --- a/model/gitlab/gitlab.go +++ b/model/gitlab/gitlab.go @@ -109,3 +109,7 @@ func (m *GitLabProvider) GetSSOSettings(config *model.Config, service string) (* func (m *GitLabProvider) GetUserFromIdToken(idToken string) (*model.User, error) { return nil, nil } + +func (m *GitLabProvider) IsSameUser(dbUser, oauthUser *model.User) bool { + return dbUser.AuthData == oauthUser.AuthData +} diff --git a/web/oauth_test.go b/web/oauth_test.go index 672c7901a0..325f78d98d 100644 --- a/web/oauth_test.go +++ b/web/oauth_test.go @@ -618,6 +618,10 @@ func (m *MattermostTestProvider) GetUserFromIdToken(token string) (*model.User, return nil, nil } +func (m *MattermostTestProvider) IsSameUser(dbUser, oauthUser *model.User) bool { + return dbUser.AuthData == oauthUser.AuthData +} + func GenerateTestAppName() string { return "fakeoauthapp" + model.NewRandomString(10) }