[MM-17213] Fix domain check when inviting guests and update user checks (#11793)

* Add update user check for guests and tests

* [MM-17213] Fix domain check when inviting guests

* Fix i18n strings

* Fix tests

* Add missing translation

* Adds test case for non guest user

api4/team_test.go

@@ -2305,10 +2305,12 @@ func TestInviteGuestsToTeam(t *testing.T) {
 
 	enableEmailInvitations := *th.App.Config().ServiceSettings.EnableEmailInvitations
 	restrictCreationToDomains := th.App.Config().TeamSettings.RestrictCreationToDomains
+	guestRestrictCreationToDomains := th.App.Config().GuestAccountsSettings.RestrictCreationToDomains
 	enableGuestAccounts := *th.App.Config().GuestAccountsSettings.Enable
 	defer func() {
 		th.App.UpdateConfig(func(cfg *model.Config) { cfg.ServiceSettings.EnableEmailInvitations = &enableEmailInvitations })
 		th.App.UpdateConfig(func(cfg *model.Config) { cfg.TeamSettings.RestrictCreationToDomains = restrictCreationToDomains })
+		th.App.UpdateConfig(func(cfg *model.Config) { cfg.GuestAccountsSettings.RestrictCreationToDomains = guestRestrictCreationToDomains })
 		th.App.UpdateConfig(func(cfg *model.Config) { cfg.GuestAccountsSettings.Enable = &enableGuestAccounts })
 	}()
 
@@ -2378,45 +2380,26 @@ func TestInviteGuestsToTeam(t *testing.T) {
 
 	th.App.UpdateConfig(func(cfg *model.Config) { *cfg.TeamSettings.RestrictCreationToDomains = "@global.com,@common.com" })
 
-	t.Run("restricted domains", func(t *testing.T) {
+	t.Run("team domain restrictions should not affect inviting guests", func(t *testing.T) {
 		err := th.App.InviteGuestsToChannels(th.BasicTeam.Id, &model.GuestsInvite{Emails: emailList, Channels: []string{th.BasicChannel.Id}, Message: "test message"}, th.BasicUser.Id)
-
-		if err == nil {
-			t.Fatal("Adding users with non-restricted domains was allowed")
-		}
-		if err.Where != "InviteGuestsToChannels" || err.Id != "api.team.invite_members.invalid_email.app_error" {
-			t.Log(err)
-			t.Fatal("Got wrong error message!")
-		}
+		require.Nil(t, err, "guest user invites should not be affected by team restrictions")
 	})
 
-	t.Run("override restricted domains", func(t *testing.T) {
-		th.BasicTeam.AllowedDomains = "invalid.com,common.com"
-		if _, err := th.App.UpdateTeam(th.BasicTeam); err == nil {
-			t.Fatal("Should not update the team")
-		}
+	t.Run("guest restrictions should affect guest users", func(t *testing.T) {
+		th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.RestrictCreationToDomains = "@guest.com" })
 
-		th.BasicTeam.AllowedDomains = "common.com"
-		if _, err := th.App.UpdateTeam(th.BasicTeam); err != nil {
-			t.Log(err)
-			t.Fatal("Should update the team")
-		}
+		err := th.App.InviteGuestsToChannels(th.BasicTeam.Id, &model.GuestsInvite{Emails: []string{"guest1@invalid.com"}, Channels: []string{th.BasicChannel.Id}, Message: "test message"}, th.BasicUser.Id)
+		require.NotNil(t, err, "guest user invites should be affected by the guest domain restrictions")
 
-		if err := th.App.InviteGuestsToChannels(th.BasicTeam.Id, &model.GuestsInvite{Emails: []string{"test@global.com"}, Channels: []string{th.BasicChannel.Id}, Message: "test message"}, th.BasicUser.Id); err == nil || err.Where != "InviteGuestsToChannels" {
-			t.Log(err)
-			t.Fatal("Per team restriction should take precedence over the global restriction")
-		}
+		err = th.App.InviteGuestsToChannels(th.BasicTeam.Id, &model.GuestsInvite{Emails: []string{"guest1@guest.com"}, Channels: []string{th.BasicChannel.Id}, Message: "test message"}, th.BasicUser.Id)
+		require.Nil(t, err, "whitelisted guest user email should be allowed by the guest domain restrictions")
+	})
 
-		if err := th.App.InviteGuestsToChannels(th.BasicTeam.Id, &model.GuestsInvite{Emails: []string{"test@common.com"}, Channels: []string{th.BasicChannel.Id}, Message: "test message"}, th.BasicUser.Id); err != nil {
-			t.Log(err)
-			t.Fatal("Failed to invite user which was common between team and global domain restriction")
-		}
-
-		if err := th.App.InviteGuestsToChannels(th.BasicTeam.Id, &model.GuestsInvite{Emails: []string{"test@invalid.com"}, Channels: []string{th.BasicChannel.Id}, Message: "test message"}, th.BasicUser.Id); err == nil {
-			t.Log(err)
-			t.Fatal("Should not invite user")
-		}
+	t.Run("guest restrictions should not affect inviting new team members", func(t *testing.T) {
+		th.App.UpdateConfig(func(cfg *model.Config) { *cfg.GuestAccountsSettings.RestrictCreationToDomains = "@guest.com" })
 
+		err := th.App.InviteNewUsersToTeam([]string{"user@global.com"}, th.BasicTeam.Id, th.BasicUser.Id)
+		require.Nil(t, err, "non guest user invites should not be affected by the guest domain restrictions")
 	})
 }
 

app/team.go

@@ -1103,7 +1103,7 @@ func (a *App) InviteGuestsToChannels(teamId string, guestsInvite *model.GuestsIn
 
 	var invalidEmailList []string
 	for _, email := range guestsInvite.Emails {
-		if !a.isTeamEmailAddressAllowed(email, team.AllowedDomains) {
+		if !CheckEmailDomain(email, *a.Config().GuestAccountsSettings.RestrictCreationToDomains) {
 			invalidEmailList = append(invalidEmailList, email)
 		}
 	}

app/user.go

@@ -404,8 +404,8 @@ func (a *App) CreateOAuthUser(service string, userData io.Reader, teamId string)
 	return ruser, nil
 }
 
-// CheckUserDomain checks that a user's email domain matches a list of space-delimited domains as a string.
-func CheckUserDomain(user *model.User, domains string) bool {
+// CheckEmailDomain checks that an email domain matches a list of space-delimited domains as a string.
+func CheckEmailDomain(email string, domains string) bool {
 	if len(domains) == 0 {
 		return true
 	}
@@ -413,7 +413,7 @@ func CheckUserDomain(user *model.User, domains string) bool {
 	domainArray := strings.Fields(strings.TrimSpace(strings.ToLower(strings.Replace(strings.Replace(domains, "@", " ", -1), ",", " ", -1))))
 
 	for _, d := range domainArray {
-		if strings.HasSuffix(strings.ToLower(user.Email), "@"+d) {
+		if strings.HasSuffix(strings.ToLower(email), "@"+d) {
 			return true
 		}
 	}
@@ -421,6 +421,11 @@ func CheckUserDomain(user *model.User, domains string) bool {
 	return false
 }
 
+// CheckUserDomain checks that a user's email domain matches a list of space-delimited domains as a string.
+func CheckUserDomain(user *model.User, domains string) bool {
+	return CheckEmailDomain(user.Email, domains)
+}
+
 // IsUsernameTaken checks if the username is already used by another user. Return false if the username is invalid.
 func (a *App) IsUsernameTaken(name string) bool {
 	if !model.IsValidUsername(name) {
@@ -1089,8 +1094,14 @@ func (a *App) UpdateUser(user *model.User, sendNotifications bool) (*model.User,
 	}
 
 	if !CheckUserDomain(user, *a.Config().TeamSettings.RestrictCreationToDomains) {
-		if !prev.IsLDAPUser() && !prev.IsSAMLUser() && user.Email != prev.Email {
-			return nil, model.NewAppError("UpdateUser", "api.user.create_user.accepted_domain.app_error", nil, "", http.StatusBadRequest)
+		if !prev.IsGuest() && !prev.IsLDAPUser() && !prev.IsSAMLUser() && user.Email != prev.Email {
+			return nil, model.NewAppError("UpdateUser", "api.user.update_user.accepted_domain.app_error", nil, "", http.StatusBadRequest)
+		}
+	}
+
+	if !CheckUserDomain(user, *a.Config().GuestAccountsSettings.RestrictCreationToDomains) {
+		if prev.IsGuest() && !prev.IsLDAPUser() && !prev.IsSAMLUser() && user.Email != prev.Email {
+			return nil, model.NewAppError("UpdateUser", "api.user.update_user.accepted_guest_domain.app_error", nil, "", http.StatusBadRequest)
 		}
 	}
 

app/user_test.go

@@ -147,11 +147,43 @@ func TestUpdateUserToRestrictedDomain(t *testing.T) {
 	})
 
 	_, err := th.App.UpdateUser(user, false)
-	assert.True(t, err == nil)
+	assert.Nil(t, err)
 
 	user.Email = "asdf@ghjk.l"
 	_, err = th.App.UpdateUser(user, false)
-	assert.False(t, err == nil)
+	assert.NotNil(t, err)
+
+	t.Run("Restricted Domains must be ignored for guest users", func(t *testing.T) {
+		guest := th.CreateGuest()
+		defer th.App.PermanentDeleteUser(guest)
+
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.TeamSettings.RestrictCreationToDomains = "foo.com"
+		})
+
+		guest.Email = "asdf@bar.com"
+		updatedGuest, err := th.App.UpdateUser(guest, false)
+		require.Nil(t, err)
+		require.Equal(t, updatedGuest.Email, guest.Email)
+	})
+
+	t.Run("Guest users should be affected by guest restricted domains", func(t *testing.T) {
+		guest := th.CreateGuest()
+		defer th.App.PermanentDeleteUser(guest)
+
+		th.App.UpdateConfig(func(cfg *model.Config) {
+			*cfg.GuestAccountsSettings.RestrictCreationToDomains = "foo.com"
+		})
+
+		guest.Email = "asdf@bar.com"
+		_, err := th.App.UpdateUser(guest, false)
+		require.NotNil(t, err)
+
+		guest.Email = "asdf@foo.com"
+		updatedGuest, err := th.App.UpdateUser(guest, false)
+		require.Nil(t, err)
+		require.Equal(t, updatedGuest.Email, guest.Email)
+	})
 }
 
 func TestUpdateUserActive(t *testing.T) {

i18n/en.json

@@ -2646,6 +2646,14 @@
     "id": "api.user.update_password.valid_account.app_error",
     "translation": "Update password failed because we couldn't find a valid account"
   },
+  {
+    "id": "api.user.update_user.accepted_domain.app_error",
+    "translation": "The email you provided does not belong to an accepted domain. Please contact your administrator or sign up with a different email."
+  },
+  {
+    "id": "api.user.update_user.accepted_guest_domain.app_error",
+    "translation": "The email you provided does not belong to an accepted domain for guest accounts. Please contact your administrator or sign up with a different email."
+  },
   {
     "id": "api.user.upload_profile_user.array.app_error",
     "translation": "Empty array under 'image' in request"