Updating user attributes on oauth login (#5324)

Moving update function to app package
Fixing duplicate userID on create user test

2 files changed, 196 insertions, 1 deletions

diff --git a/app/user.go b/app/user.go
index 9f428089b..53e389947 100644
--- a/app/user.go
+++ b/app/user.go
@@ -27,6 +27,7 @@ import (
 	"github.com/golang/freetype"
 	"github.com/mattermost/platform/einterfaces"
 	"github.com/mattermost/platform/model"
+	"github.com/mattermost/platform/store"
 	"github.com/mattermost/platform/utils"
 )
 
@@ -1253,3 +1254,45 @@ func AutocompleteUsersInTeam(teamId string, term string, searchOptions map[strin
 
 	return autocomplete, nil
 }
+
+func UpdateOAuthUserAttrs(userData io.Reader, user *model.User, provider einterfaces.OauthProvider, service string, siteURL string) *model.AppError {
+	oauthUser := provider.GetUserFromJson(userData)
+
+	if oauthUser == nil {
+		return model.NewLocAppError("UpdateOAuthUserAttrs", "api.user.update_oauth_user_attrs.get_user.app_error", map[string]interface{}{"Service": service}, "")
+	}
+
+	userAttrsChanged := false
+
+	if oauthUser.Username != user.Username {
+		if existingUser, _ := GetUserByUsername(oauthUser.Username); existingUser == nil {
+			user.Username = oauthUser.Username
+			userAttrsChanged = true
+		}
+	}
+
+	if oauthUser.GetFullName() != user.GetFullName() {
+		user.FirstName = oauthUser.FirstName
+		user.LastName = oauthUser.LastName
+		userAttrsChanged = true
+	}
+
+	if oauthUser.Email != user.Email {
+		if existingUser, _ := GetUserByEmail(oauthUser.Email); existingUser == nil {
+			user.Email = oauthUser.Email
+			userAttrsChanged = true
+		}
+	}
+
+	if userAttrsChanged {
+		var result store.StoreResult
+		if result = <-Srv.Store.User().Update(user, true); result.Err != nil {
+			return result.Err
+		}
+
+		user = result.Data.([2]*model.User)[0]
+		InvalidateCacheForUser(user.Id)
+	}
+
+	return nil
+}
diff --git a/app/user_test.go b/app/user_test.go
index 0dba86241..177f5a638 100644
--- a/app/user_test.go
+++ b/app/user_test.go
@@ -4,9 +4,14 @@
 package app
 
 import (
+	"bytes"
+	"encoding/json"
+	"math/rand"
 	"strings"
 	"testing"
+	"time"
 
+	"github.com/mattermost/platform/einterfaces"
 	"github.com/mattermost/platform/model"
 	"github.com/mattermost/platform/model/gitlab"
 	"github.com/mattermost/platform/utils"
@@ -59,7 +64,8 @@ func TestCheckUserDomain(t *testing.T) {
 
 func TestCreateOAuthUser(t *testing.T) {
 	th := Setup().InitBasic()
-	glUser := oauthgitlab.GitLabUser{Id: 1000, Username: model.NewId(), Email: model.NewId() + "@simulator.amazonses.com", Name: "Joram Wilander"}
+	r := rand.New(rand.NewSource(time.Now().UnixNano()))
+	glUser := oauthgitlab.GitLabUser{Id: int64(r.Intn(1000)), Username: model.NewId(), Email: model.NewId() + "@simulator.amazonses.com", Name: "Joram Wilander"}
 
 	json := glUser.ToJson()
 	user, err := CreateOAuthUser(model.USER_AUTH_SERVICE_GITLAB, strings.NewReader(json), th.BasicTeam.Id)
@@ -85,3 +91,149 @@ func TestCreateOAuthUser(t *testing.T) {
 	}
 
 }
+
+func TestUpdateOAuthUserAttrs(t *testing.T) {
+	Setup()
+	id := model.NewId()
+	id2 := model.NewId()
+	gitlabProvider := einterfaces.GetOauthProvider("gitlab")
+
+	username := "user" + id
+	username2 := "user" + id2
+
+	email := "user" + id + "@nowhere.com"
+	email2 := "user" + id2 + "@nowhere.com"
+
+	var user, user2 *model.User
+	var gitlabUserObj oauthgitlab.GitLabUser
+	user, gitlabUserObj = createGitlabUser(t, username, email)
+	user2, _ = createGitlabUser(t, username2, email2)
+
+	t.Run("UpdateUsername", func(t *testing.T) {
+		t.Run("NoExistingUserWithSameUsername", func(t *testing.T) {
+			gitlabUserObj.Username = "updateduser" + model.NewId()
+			gitlabUser := getGitlabUserPayload(gitlabUserObj, t)
+			data := bytes.NewReader(gitlabUser)
+
+			user = getUserFromDB(user.Id, t)
+			UpdateOAuthUserAttrs(data, user, gitlabProvider, "gitlab", "http://localhost:8065")
+			user = getUserFromDB(user.Id, t)
+
+			if user.Username != gitlabUserObj.Username {
+				t.Fatal("user's username is not updated")
+			}
+		})
+
+		t.Run("ExistinguserWithSameUsername", func(t *testing.T) {
+			gitlabUserObj.Username = user2.Username
+
+			gitlabUser := getGitlabUserPayload(gitlabUserObj, t)
+			data := bytes.NewReader(gitlabUser)
+
+			user = getUserFromDB(user.Id, t)
+			UpdateOAuthUserAttrs(data, user, gitlabProvider, "gitlab", "http://localhost:8065")
+			user = getUserFromDB(user.Id, t)
+
+			if user.Username == gitlabUserObj.Username {
+				t.Fatal("user's username is updated though there already exists another user with the same username")
+			}
+		})
+	})
+
+	t.Run("UpdateEmail", func(t *testing.T) {
+		t.Run("NoExistingUserWithSameEmail", func(t *testing.T) {
+			gitlabUserObj.Email = "newuser" + model.NewId() + "@nowhere.com"
+			gitlabUser := getGitlabUserPayload(gitlabUserObj, t)
+			data := bytes.NewReader(gitlabUser)
+
+			user = getUserFromDB(user.Id, t)
+			UpdateOAuthUserAttrs(data, user, gitlabProvider, "gitlab", "http://localhost:8065")
+			user = getUserFromDB(user.Id, t)
+
+			if user.Email != gitlabUserObj.Email {
+				t.Fatal("user's email is not updated")
+			}
+
+			if !user.EmailVerified {
+				t.Fatal("user's email should have been verified")
+			}
+		})
+
+		t.Run("ExistingUserWithSameEmail", func(t *testing.T) {
+			gitlabUserObj.Email = user2.Email
+
+			gitlabUser := getGitlabUserPayload(gitlabUserObj, t)
+			data := bytes.NewReader(gitlabUser)
+
+			user = getUserFromDB(user.Id, t)
+			UpdateOAuthUserAttrs(data, user, gitlabProvider, "gitlab", "http://localhost:8065")
+			user = getUserFromDB(user.Id, t)
+
+			if user.Email == gitlabUserObj.Email {
+				t.Fatal("user's email is updated though there already exists another user with the same email")
+			}
+		})
+	})
+
+	t.Run("UpdateFirstName", func(t *testing.T) {
+		gitlabUserObj.Name = "Updated User"
+		gitlabUser := getGitlabUserPayload(gitlabUserObj, t)
+		data := bytes.NewReader(gitlabUser)
+
+		user = getUserFromDB(user.Id, t)
+		UpdateOAuthUserAttrs(data, user, gitlabProvider, "gitlab", "http://localhost:8065")
+		user = getUserFromDB(user.Id, t)
+
+		if user.FirstName != "Updated" {
+			t.Fatal("user's first name is not updated")
+		}
+	})
+
+	t.Run("UpdateLastName", func(t *testing.T) {
+		gitlabUserObj.Name = "Updated Lastname"
+		gitlabUser := getGitlabUserPayload(gitlabUserObj, t)
+		data := bytes.NewReader(gitlabUser)
+
+		user = getUserFromDB(user.Id, t)
+		UpdateOAuthUserAttrs(data, user, gitlabProvider, "gitlab", "http://localhost:8065")
+		user = getUserFromDB(user.Id, t)
+
+		if user.LastName != "Lastname" {
+			t.Fatal("user's last name is not updated")
+		}
+	})
+}
+
+func getUserFromDB(id string, t *testing.T) *model.User {
+	if user, err := GetUser(id); err != nil {
+		t.Fatal("user is not found")
+		return nil
+	} else {
+		return user
+	}
+}
+
+func getGitlabUserPayload(gitlabUser oauthgitlab.GitLabUser, t *testing.T) []byte {
+	var payload []byte
+	var err error
+	if payload, err = json.Marshal(gitlabUser); err != nil {
+		t.Fatal("Serialization of gitlab user to json failed")
+	}
+
+	return payload
+}
+
+func createGitlabUser(t *testing.T, email string, username string) (*model.User, oauthgitlab.GitLabUser) {
+	r := rand.New(rand.NewSource(time.Now().UnixNano()))
+	gitlabUserObj := oauthgitlab.GitLabUser{Id: int64(r.Intn(1000)), Username: username, Login: "user1", Email: email, Name: "Test User"}
+	gitlabUser := getGitlabUserPayload(gitlabUserObj, t)
+
+	var user *model.User
+	var err *model.AppError
+
+	if user, err = CreateOAuthUser("gitlab", bytes.NewReader(gitlabUser), ""); err != nil {
+		t.Fatal("unable to create the user")
+	}
+
+	return user, gitlabUserObj
+}