check authdata for different format (#17577)

* check authdata for different format

* add mocks, fix lint

* rename function

* update auth-data with new format

* update assert

Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

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

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) {

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
+}

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)

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
+}

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)
 }