diff --git a/api4/user.go b/api4/user.go index 4a45de64c6..69edb431ab 100644 --- a/api4/user.go +++ b/api4/user.go @@ -170,7 +170,13 @@ func getUserByEmail(c *Context, w http.ResponseWriter, r *http.Request) { return } - // No permission check required + // No permission check required, but still prevent users who can't see another user's email address from using this + + sanitizeOptions := c.App.GetSanitizeOptions(c.IsSystemAdmin()) + if !sanitizeOptions["email"] { + c.Err = model.NewAppError("getUserByEmail", "api.user.get_user_by_email.permissions.app_error", nil, "userId="+c.App.Session.UserId, http.StatusForbidden) + return + } user, err := c.App.GetUserByEmail(c.Params.Email) if err != nil { diff --git a/api4/user_test.go b/api4/user_test.go index f20ffd8ed3..246ec8ce26 100644 --- a/api4/user_test.go +++ b/api4/user_test.go @@ -495,66 +495,118 @@ func TestGetUserByUsername(t *testing.T) { func TestGetUserByEmail(t *testing.T) { th := Setup().InitBasic() defer th.TearDown() - Client := th.Client - - showEmailAddress := th.App.Config().PrivacySettings.ShowEmailAddress - showFullName := th.App.Config().PrivacySettings.ShowFullName - defer func() { - th.App.UpdateConfig(func(cfg *model.Config) { cfg.PrivacySettings.ShowEmailAddress = showEmailAddress }) - th.App.UpdateConfig(func(cfg *model.Config) { cfg.PrivacySettings.ShowFullName = showFullName }) - }() user := th.CreateUser() - ruser, resp := Client.GetUserByEmail(user.Email, "") - CheckNoError(t, resp) - CheckUserSanitization(t, ruser) + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.PrivacySettings.ShowEmailAddress = true + cfg.PrivacySettings.ShowFullName = true + }) - if ruser.Email != user.Email { - t.Fatal("emails did not match") - } + t.Run("should be able to get another user by email", func(t *testing.T) { + ruser, resp := th.Client.GetUserByEmail(user.Email, "") + CheckNoError(t, resp) + CheckUserSanitization(t, ruser) - ruser, resp = Client.GetUserByEmail(user.Email, resp.Etag) - CheckEtag(t, ruser, resp) + if ruser.Email != user.Email { + t.Fatal("emails did not match") + } + }) - _, resp = Client.GetUserByEmail(GenerateTestUsername(), "") - CheckBadRequestStatus(t, resp) + t.Run("should return not modified when provided with a matching etag", func(t *testing.T) { + _, resp := th.Client.GetUserByEmail(user.Email, "") + CheckNoError(t, resp) - _, resp = Client.GetUserByEmail(th.GenerateTestEmail(), "") - CheckNotFoundStatus(t, resp) + ruser, resp := th.Client.GetUserByEmail(user.Email, resp.Etag) + CheckEtag(t, ruser, resp) + }) - // Check against privacy config settings - th.App.UpdateConfig(func(cfg *model.Config) { cfg.PrivacySettings.ShowEmailAddress = false }) - th.App.UpdateConfig(func(cfg *model.Config) { cfg.PrivacySettings.ShowFullName = false }) + t.Run("should return bad request when given an invalid email", func(t *testing.T) { + _, resp := th.Client.GetUserByEmail(GenerateTestUsername(), "") + CheckBadRequestStatus(t, resp) + }) - ruser, resp = Client.GetUserByEmail(user.Email, "") - CheckNoError(t, resp) + t.Run("should return 404 when given a non-existent email", func(t *testing.T) { + _, resp := th.Client.GetUserByEmail(th.GenerateTestEmail(), "") + CheckNotFoundStatus(t, resp) + }) - if ruser.Email != "" { - t.Fatal("email should be blank") - } - if ruser.FirstName != "" { - t.Fatal("first name should be blank") - } - if ruser.LastName != "" { - t.Fatal("last name should be blank") - } + t.Run("should sanitize full name for non-admin based on privacy settings", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.PrivacySettings.ShowEmailAddress = true + cfg.PrivacySettings.ShowFullName = false + }) - Client.Logout() - _, resp = Client.GetUserByEmail(user.Email, "") - CheckUnauthorizedStatus(t, resp) + ruser, resp := th.Client.GetUserByEmail(user.Email, "") + CheckNoError(t, resp) + assert.Equal(t, "", ruser.FirstName, "first name should be blank") + assert.Equal(t, "", ruser.LastName, "last name should be blank") - // System admins should ignore privacy settings - ruser, _ = th.SystemAdminClient.GetUserByEmail(user.Email, resp.Etag) - if ruser.Email == "" { - t.Fatal("email should not be blank") - } - if ruser.FirstName == "" { - t.Fatal("first name should not be blank") - } - if ruser.LastName == "" { - t.Fatal("last name should not be blank") - } + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.PrivacySettings.ShowFullName = true + }) + + ruser, resp = th.Client.GetUserByEmail(user.Email, "") + CheckNoError(t, resp) + assert.NotEqual(t, "", ruser.FirstName, "first name should be set") + assert.NotEqual(t, "", ruser.LastName, "last name should be set") + }) + + t.Run("should not sanitize full name for admin, regardless of privacy settings", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.PrivacySettings.ShowEmailAddress = true + cfg.PrivacySettings.ShowFullName = false + }) + + ruser, resp := th.SystemAdminClient.GetUserByEmail(user.Email, "") + CheckNoError(t, resp) + assert.NotEqual(t, "", ruser.FirstName, "first name should be set") + assert.NotEqual(t, "", ruser.LastName, "last name should be set") + + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.PrivacySettings.ShowFullName = true + }) + + ruser, resp = th.SystemAdminClient.GetUserByEmail(user.Email, "") + CheckNoError(t, resp) + assert.NotEqual(t, "", ruser.FirstName, "first name should be set") + assert.NotEqual(t, "", ruser.LastName, "last name should be set") + }) + + t.Run("should return forbidden for non-admin when privacy settings hide email", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.PrivacySettings.ShowEmailAddress = false + }) + + _, resp := th.Client.GetUserByEmail(user.Email, "") + CheckForbiddenStatus(t, resp) + + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.PrivacySettings.ShowEmailAddress = true + }) + + ruser, resp := th.Client.GetUserByEmail(user.Email, "") + CheckNoError(t, resp) + assert.Equal(t, user.Email, ruser.Email, "email should be set") + }) + + t.Run("should always return email for admin, regardless of privacy settings", func(t *testing.T) { + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.PrivacySettings.ShowEmailAddress = false + }) + + ruser, resp := th.SystemAdminClient.GetUserByEmail(user.Email, "") + CheckNoError(t, resp) + assert.Equal(t, user.Email, ruser.Email, "email should be set") + + th.App.UpdateConfig(func(cfg *model.Config) { + cfg.PrivacySettings.ShowEmailAddress = true + }) + + ruser, resp = th.SystemAdminClient.GetUserByEmail(user.Email, "") + CheckNoError(t, resp) + assert.Equal(t, user.Email, ruser.Email, "email should be set") + }) } func TestSearchUsers(t *testing.T) { diff --git a/app/user.go b/app/user.go index 2fd399bea2..9dee5df14c 100644 --- a/app/user.go +++ b/app/user.go @@ -930,13 +930,19 @@ func (a *App) UpdateActive(user *model.User, active bool) (*model.User, *model.A return ruser, nil } -func (a *App) SanitizeProfile(user *model.User, asAdmin bool) { +func (a *App) GetSanitizeOptions(asAdmin bool) map[string]bool { options := a.Config().GetSanitizeOptions() if asAdmin { options["email"] = true options["fullname"] = true options["authservice"] = true } + return options +} + +func (a *App) SanitizeProfile(user *model.User, asAdmin bool) { + options := a.GetSanitizeOptions(asAdmin) + user.SanitizeProfile(options) } diff --git a/i18n/en.json b/i18n/en.json index 6880266018..2b27497893 100644 --- a/i18n/en.json +++ b/i18n/en.json @@ -2234,6 +2234,10 @@ "id": "api.user.get_profile_image.not_found.app_error", "translation": "Unable to get profile image, user not found." }, + { + "id": "api.user.get_user_by_email.permissions.app_error", + "translation": "Unable to get user by email." + }, { "id": "api.user.ldap_to_email.not_available.app_error", "translation": "AD/LDAP not available on this server"