Auth: Fix email verification bypass when using basic authentication (#83492)

2024-02-27 16:35:42 +01:00
parent 0c72a4e47e
commit efec060256
22 changed files with 1347 additions and 16 deletions
@@ -184,6 +184,11 @@ func (hs *HTTPServer) registerRoutes() {
 	r.Post("/api/user/signup", quota(user.QuotaTargetSrv), quota(org.QuotaTargetSrv), routing.Wrap(hs.SignUp))
 	r.Post("/api/user/signup/step2", routing.Wrap(hs.SignUpStep2))

+	// update user email
+	if hs.Cfg.Smtp.Enabled && setting.VerifyEmailEnabled {
+		r.Get("/user/email/update", reqSignedInNoAnonymous, routing.Wrap(hs.UpdateUserEmail))
+	}
+
 	// invited
 	r.Get("/api/user/invite/:code", routing.Wrap(hs.GetInviteInfoByCode))
 	r.Post("/api/user/invite/complete", routing.Wrap(hs.CompleteInvite))
@@ -233,6 +233,10 @@ func setupScenarioContext(t *testing.T, url string) *scenarioContext {
 	return sc
 }

+func authedUserWithPermissions(userID, orgID int64, permissions []accesscontrol.Permission) *user.SignedInUser {
+	return &user.SignedInUser{UserID: userID, OrgID: orgID, OrgRole: org.RoleViewer, Permissions: map[int64]map[string][]string{orgID: accesscontrol.GroupScopesByAction(permissions)}}
+}
+
 type fakeRenderService struct {
 	rendering.Service
 }
@@ -179,7 +179,7 @@ type HTTPServer struct {
 	authInfoService              login.AuthInfoService
 	authenticator                loginpkg.Authenticator
 	teamPermissionsService       accesscontrol.TeamPermissionsService
-	NotificationService          *notifications.NotificationService
+	NotificationService          notifications.Service
 	DashboardService             dashboards.DashboardService
 	dashboardProvisioningService dashboards.DashboardProvisioningService
 	folderService                folder.Service
@@ -238,7 +238,7 @@ func ProvideHTTPServer(opts ServerOptions, cfg *setting.Cfg, routeRegister routi
 	dataSourcesService datasources.DataSourceService, queryDataService query.Service, pluginFileStore plugins.FileStore,
 	serviceaccountsService serviceaccounts.Service,
 	authInfoService login.AuthInfoService, storageService store.StorageService, httpEntityStore httpentitystore.HTTPEntityStore,
-	notificationService *notifications.NotificationService, dashboardService dashboards.DashboardService,
+	notificationService notifications.Service, dashboardService dashboards.DashboardService,
 	dashboardProvisioningService dashboards.DashboardProvisioningService, folderService folder.Service,
 	datasourcePermissionsService permissions.DatasourcePermissionsService, alertNotificationService *alerting.AlertNotificationService,
 	dashboardsnapshotsService dashboardsnapshots.Service, pluginSettings pluginSettings.Service,
@@ -4,16 +4,22 @@ import (
 	"context"
 	"errors"
 	"net/http"
+	"net/mail"
+	"net/url"
 	"strconv"
 	"strings"
+	"time"

 	"github.com/grafana/grafana/pkg/api/dtos"
 	"github.com/grafana/grafana/pkg/api/response"
 	contextmodel "github.com/grafana/grafana/pkg/services/contexthandler/model"
 	"github.com/grafana/grafana/pkg/services/login"
+	"github.com/grafana/grafana/pkg/services/notifications"
 	"github.com/grafana/grafana/pkg/services/org"
 	"github.com/grafana/grafana/pkg/services/team"
+	tempuser "github.com/grafana/grafana/pkg/services/temp_user"
 	"github.com/grafana/grafana/pkg/services/user"
+	"github.com/grafana/grafana/pkg/setting"
 	"github.com/grafana/grafana/pkg/util"
 	"github.com/grafana/grafana/pkg/web"
 )
@@ -122,6 +128,7 @@ func (hs *HTTPServer) GetUserByLoginOrEmail(c *contextmodel.ReqContext) response
 // 200: okResponse
 // 401: unauthorisedError
 // 403: forbiddenError
+// 409: conflictError
 // 500: internalServerError
 func (hs *HTTPServer) UpdateSignedInUser(c *contextmodel.ReqContext) response.Response {
 	cmd := user.UpdateUserCommand{}
@@ -156,6 +163,7 @@ func (hs *HTTPServer) UpdateSignedInUser(c *contextmodel.ReqContext) response.Re
 // 401: unauthorisedError
 // 403: forbiddenError
 // 404: notFoundError
+// 409: conflictError
 // 500: internalServerError
 func (hs *HTTPServer) UpdateUser(c *contextmodel.ReqContext) response.Response {
 	cmd := user.UpdateUserCommand{}
@@ -217,6 +225,39 @@ func (hs *HTTPServer) handleUpdateUser(ctx context.Context, cmd user.UpdateUserC
 		}
 	}

+	// If email is being updated, we need to verify it. Likewise, if username is being updated and the new username
+	// is an email, we also need to verify it.
+	// To avoid breaking changes, email verification is implemented in a way that if the email field is being updated,
+	// all the other fields being updated in the same request are disregarded. We do this because email might need to
+	// be verified and if so, it goes through a different code flow.
+	if hs.Cfg.Smtp.Enabled && setting.VerifyEmailEnabled {
+		query := user.GetUserByIDQuery{ID: cmd.UserID}
+		usr, err := hs.userService.GetByID(ctx, &query)
+		if err != nil {
+			if errors.Is(err, user.ErrUserNotFound) {
+				return response.Error(http.StatusNotFound, user.ErrUserNotFound.Error(), nil)
+			}
+			return response.Error(http.StatusInternalServerError, "Failed to get user", err)
+		}
+
+		if len(cmd.Email) != 0 && usr.Email != cmd.Email {
+			// Email is being updated
+			newEmail, err := ValidateAndNormalizeEmail(cmd.Email)
+			if err != nil {
+				return response.Error(http.StatusBadRequest, "Invalid email address", err)
+			}
+
+			return hs.verifyEmailUpdate(ctx, newEmail, user.EmailUpdateAction, usr)
+		}
+		if len(cmd.Login) != 0 && usr.Login != cmd.Login {
+			// Username is being updated. If it's an email, go through the email verification flow
+			newEmailLogin, err := ValidateAndNormalizeEmail(cmd.Login)
+			if err == nil && newEmailLogin != usr.Email {
+				return hs.verifyEmailUpdate(ctx, newEmailLogin, user.LoginUpdateAction, usr)
+			}
+		}
+	}
+
 	if err := hs.userService.Update(ctx, &cmd); err != nil {
 		if errors.Is(err, user.ErrCaseInsensitive) {
 			return response.Error(http.StatusConflict, "Update would result in user login conflict", err)
@@ -227,6 +268,104 @@ func (hs *HTTPServer) handleUpdateUser(ctx context.Context, cmd user.UpdateUserC
 	return response.Success("User updated")
 }

+func (hs *HTTPServer) verifyEmailUpdate(ctx context.Context, email string, field user.UpdateEmailActionType, usr *user.User) response.Response {
+	// Verify that email is not already being used
+	query := user.GetUserByLoginQuery{LoginOrEmail: email}
+	existingUsr, err := hs.userService.GetByLogin(ctx, &query)
+	if err != nil && !errors.Is(err, user.ErrUserNotFound) {
+		return response.Error(http.StatusInternalServerError, "Failed to validate if email is already in use", err)
+	}
+	if existingUsr != nil {
+		return response.Error(http.StatusConflict, "Email is already being used", nil)
+	}
+
+	// Invalidate any pending verifications for this user
+	expireCmd := tempuser.ExpirePreviousVerificationsCommand{InvitedByUserID: usr.ID}
+	err = hs.tempUserService.ExpirePreviousVerifications(ctx, &expireCmd)
+	if err != nil {
+		return response.Error(http.StatusInternalServerError, "Could not invalidate pending email verifications", err)
+	}
+
+	code, err := util.GetRandomString(20)
+	if err != nil {
+		return response.Error(http.StatusInternalServerError, "Failed to generate random string", err)
+	}
+
+	tempCmd := tempuser.CreateTempUserCommand{
+		OrgID:  -1,
+		Email:  email,
+		Code:   code,
+		Status: tempuser.TmpUserEmailUpdateStarted,
+		// used to fetch the User in the second step of the verification flow
+		InvitedByUserID: usr.ID,
+		// used to determine if the user was updating their email or username in the second step of the verification flow
+		Name: string(field),
+	}
+
+	tempUser, err := hs.tempUserService.CreateTempUser(ctx, &tempCmd)
+	if err != nil {
+		return response.Error(http.StatusInternalServerError, "Failed to create email change", err)
+	}
+
+	emailCmd := notifications.SendVerifyEmailCommand{Email: tempUser.Email, Code: tempUser.Code, User: usr}
+	err = hs.NotificationService.SendVerificationEmail(ctx, &emailCmd)
+	if err != nil {
+		return response.Error(http.StatusInternalServerError, "Failed to send verification email", err)
+	}
+
+	// Record email as sent
+	emailSentCmd := tempuser.UpdateTempUserWithEmailSentCommand{Code: tempUser.Code}
+	err = hs.tempUserService.UpdateTempUserWithEmailSent(ctx, &emailSentCmd)
+	if err != nil {
+		return response.Error(http.StatusInternalServerError, "Failed to record verification email", err)
+	}
+
+	return response.Success("Email sent for verification")
+}
+
+// swagger:route GET /user/email/update user updateUserEmail
+//
+// Update user email.
+//
+// Update the email of user given a verification code.
+//
+// Responses:
+// 302: okResponse
+func (hs *HTTPServer) UpdateUserEmail(c *contextmodel.ReqContext) response.Response {
+	var err error
+
+	q := c.Req.URL.Query()
+	code, err := url.QueryUnescape(q.Get("code"))
+	if err != nil || code == "" {
+		return hs.RedirectResponseWithError(c, errors.New("bad request data"))
+	}
+
+	tempUser, err := hs.validateEmailCode(c.Req.Context(), code)
+	if err != nil {
+		return hs.RedirectResponseWithError(c, err)
+	}
+
+	cmd, err := hs.updateCmdFromEmailVerification(c.Req.Context(), tempUser)
+	if err != nil {
+		return hs.RedirectResponseWithError(c, err)
+	}
+
+	if err := hs.userService.Update(c.Req.Context(), cmd); err != nil {
+		if errors.Is(err, user.ErrCaseInsensitive) {
+			return hs.RedirectResponseWithError(c, errors.New("update would result in user login conflict"))
+		}
+		return hs.RedirectResponseWithError(c, errors.New("failed to update user"))
+	}
+
+	// Mark temp user as completed
+	updateTmpUserCmd := tempuser.UpdateTempUserStatusCommand{Code: code, Status: tempuser.TmpUserEmailUpdateCompleted}
+	if err := hs.tempUserService.UpdateTempUserStatus(c.Req.Context(), &updateTmpUserCmd); err != nil {
+		return hs.RedirectResponseWithError(c, errors.New("failed to update verification status"))
+	}
+
+	return response.Redirect(hs.Cfg.AppSubURL + "/profile")
+}
+
 func (hs *HTTPServer) isExternalUser(ctx context.Context, userID int64) (bool, error) {
 	getAuthQuery := login.GetAuthInfoQuery{UserId: userID}
 	var err error
@@ -529,6 +668,57 @@ func (hs *HTTPServer) ClearHelpFlags(c *contextmodel.ReqContext) response.Respon
 	return response.JSON(http.StatusOK, &util.DynMap{"message": "Help flag set", "helpFlags1": cmd.HelpFlags1})
 }

+func (hs *HTTPServer) updateCmdFromEmailVerification(ctx context.Context, tempUser *tempuser.TempUserDTO) (*user.UpdateUserCommand, error) {
+	userQuery := user.GetUserByLoginQuery{LoginOrEmail: tempUser.InvitedByLogin}
+	usr, err := hs.userService.GetByLogin(ctx, &userQuery)
+	if err != nil {
+		if errors.Is(err, user.ErrUserNotFound) {
+			return nil, user.ErrUserNotFound
+		}
+		return nil, errors.New("failed to get user")
+	}
+
+	cmd := &user.UpdateUserCommand{UserID: usr.ID, Email: tempUser.Email}
+
+	switch tempUser.Name {
+	case string(user.EmailUpdateAction):
+		// User updated the email field
+		if _, err := mail.ParseAddress(usr.Login); err == nil {
+			// If username was also an email, we update it to keep it in sync with the email field
+			cmd.Login = tempUser.Email
+		}
+	case string(user.LoginUpdateAction):
+		// User updated the username field with a new email
+		cmd.Login = tempUser.Email
+	default:
+		return nil, errors.New("trying to update email on unknown field")
+	}
+	return cmd, nil
+}
+
+func (hs *HTTPServer) validateEmailCode(ctx context.Context, code string) (*tempuser.TempUserDTO, error) {
+	tempUserQuery := tempuser.GetTempUserByCodeQuery{Code: code}
+	tempUser, err := hs.tempUserService.GetTempUserByCode(ctx, &tempUserQuery)
+	if err != nil {
+		if errors.Is(err, tempuser.ErrTempUserNotFound) {
+			return nil, errors.New("invalid email verification code")
+		}
+		return nil, errors.New("failed to read temp user")
+	}
+
+	if tempUser.Status != tempuser.TmpUserEmailUpdateStarted {
+		return nil, errors.New("invalid email verification code")
+	}
+	if !tempUser.EmailSent {
+		return nil, errors.New("verification email was not recorded as sent")
+	}
+	if tempUser.EmailSentOn.Add(hs.Cfg.VerificationEmailMaxLifetime).Before(time.Now()) {
+		return nil, errors.New("invalid email verification code")
+	}
+
+	return tempUser, nil
+}
+
 // swagger:parameters searchUsers
 type SearchUsersParams struct {
 	// Limit the maximum number of users to return per page
@@ -5,9 +5,18 @@ import (
 	"encoding/json"
 	"fmt"
 	"net/http"
+	"net/url"
+	"strings"
 	"testing"
 	"time"

+	"github.com/grafana/grafana/pkg/services/accesscontrol"
+	"github.com/grafana/grafana/pkg/services/notifications"
+	"github.com/grafana/grafana/pkg/services/secrets/fakes"
+	tempuser "github.com/grafana/grafana/pkg/services/temp_user"
+	"github.com/grafana/grafana/pkg/services/temp_user/tempuserimpl"
+	"github.com/grafana/grafana/pkg/web/webtest"
+
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/oauth2"
@@ -41,6 +50,8 @@ import (
 	"github.com/grafana/grafana/pkg/setting"
 )

+const newEmail = "newEmail@localhost"
+
 func TestUserAPIEndpoint_userLoggedIn(t *testing.T) {
 	settings := setting.NewCfg()
 	sqlStore := db.InitTestDB(t)
@@ -73,7 +84,6 @@ func TestUserAPIEndpoint_userLoggedIn(t *testing.T) {
 		hs.authInfoService = srv
 		orgSvc, err := orgimpl.ProvideService(sqlStore, sqlStore.Cfg, quotatest.New(false, nil))
 		require.NoError(t, err)
-		require.NoError(t, err)
 		userSvc, err := userimpl.ProvideService(sqlStore, orgSvc, sc.cfg, nil, nil, quotatest.New(false, nil), supportbundlestest.NewFakeBundleService())
 		require.NoError(t, err)
 		hs.userService = userSvc
@@ -367,6 +377,627 @@ func TestHTTPServer_UpdateUser(t *testing.T) {
 	}, hs)
 }

+func TestUser_UpdateEmail(t *testing.T) {
+	doReq := func(req *http.Request, usr *user.User) (*http.Response, error) {
+		r := webtest.RequestWithSignedInUser(
+			req,
+			authedUserWithPermissions(
+				usr.ID,
+				usr.OrgID,
+				[]accesscontrol.Permission{
+					{
+						Action: accesscontrol.ActionUsersWrite,
+						Scope:  accesscontrol.ScopeGlobalUsersAll,
+					},
+				},
+			),
+		)
+		client := &http.Client{
+			CheckRedirect: func(req *http.Request, via []*http.Request) error {
+				return http.ErrUseLastResponse
+			}}
+		return client.Do(r)
+	}
+
+	sendUpdateReq := func(server *webtest.Server, usr *user.User, body string) {
+		req := server.NewRequest(
+			http.MethodPut,
+			"/api/user",
+			strings.NewReader(body),
+		)
+		req.Header.Add("Content-Type", "application/json")
+		res, err := doReq(req, usr)
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusOK, res.StatusCode)
+		require.NoError(t, res.Body.Close())
+	}
+
+	sendVerificationReq := func(server *webtest.Server, usr *user.User, code string) {
+		url := fmt.Sprintf("/user/email/update?code=%s", url.QueryEscape(code))
+		req := server.NewGetRequest(url)
+		res, err := doReq(req, usr)
+		require.NoError(t, err)
+		assert.Equal(t, http.StatusFound, res.StatusCode)
+		require.NoError(t, res.Body.Close())
+	}
+
+	getVerificationTempUser := func(tempUserSvc tempuser.Service, code string) *tempuser.TempUserDTO {
+		tmpUserQuery := tempuser.GetTempUserByCodeQuery{Code: code}
+		tmpUser, err := tempUserSvc.GetTempUserByCode(context.Background(), &tmpUserQuery)
+		require.NoError(t, err)
+		return tmpUser
+	}
+
+	verifyEmailData := func(tempUserSvc tempuser.Service, nsMock *notifications.NotificationServiceMock, originalUsr *user.User, newEmail string) {
+		verification := nsMock.EmailVerification
+		tmpUsr := getVerificationTempUser(tempUserSvc, verification.Code)
+
+		require.True(t, nsMock.EmailVerified)
+		require.Equal(t, newEmail, verification.Email)
+		require.Equal(t, originalUsr.ID, verification.User.ID)
+		require.Equal(t, tmpUsr.Code, verification.Code)
+	}
+
+	verifyUserNotUpdated := func(userSvc user.Service, usr *user.User) {
+		userQuery := user.GetUserByIDQuery{ID: usr.ID}
+		checkUsr, err := userSvc.GetByID(context.Background(), &userQuery)
+		require.NoError(t, err)
+		require.Equal(t, usr.Email, checkUsr.Email)
+		require.Equal(t, usr.Login, checkUsr.Login)
+		require.Equal(t, usr.Name, checkUsr.Name)
+	}
+
+	setupScenario := func(cfg *setting.Cfg) (*webtest.Server, user.Service, tempuser.Service, *notifications.NotificationServiceMock) {
+		setting.VerifyEmailEnabled = true
+		settings := setting.NewCfg()
+		settings.Smtp.Enabled = true
+		settings.VerificationEmailMaxLifetime = 1 * time.Hour
+
+		if cfg != nil {
+			settings = cfg
+		}
+
+		nsMock := notifications.MockNotificationService()
+		sqlStore := db.InitTestDB(t)
+		sqlStore.Cfg = settings
+
+		tempUserSvc := tempuserimpl.ProvideService(sqlStore, settings)
+		orgSvc, err := orgimpl.ProvideService(sqlStore, settings, quotatest.New(false, nil))
+		require.NoError(t, err)
+		userSvc, err := userimpl.ProvideService(sqlStore, orgSvc, settings, nil, nil, quotatest.New(false, nil), supportbundlestest.NewFakeBundleService())
+		require.NoError(t, err)
+
+		server := SetupAPITestServer(t, func(hs *HTTPServer) {
+			hs.Cfg = settings
+
+			hs.SQLStore = sqlStore
+			hs.userService = userSvc
+			hs.tempUserService = tempUserSvc
+			hs.NotificationService = nsMock
+			hs.SecretsService = fakes.NewFakeSecretsService()
+			// User is internal
+			hs.authInfoService = &logintest.AuthInfoServiceFake{ExpectedError: user.ErrUserNotFound}
+		})
+
+		return server, userSvc, tempUserSvc, nsMock
+	}
+
+	createUser := func(userSvc user.Service, name string, email string, login string) *user.User {
+		createUserCmd := user.CreateUserCommand{
+			Email:   email,
+			Name:    name,
+			Login:   login,
+			Company: "testCompany",
+			IsAdmin: true,
+		}
+		usr, err := userSvc.Create(context.Background(), &createUserCmd)
+		require.NoError(t, err)
+		return usr
+	}
+
+	disabledCases := []struct {
+		Name  string
+		Field user.UpdateEmailActionType
+	}{
+		{
+			Name:  "Updating Email field",
+			Field: user.EmailUpdateAction,
+		},
+		{
+			Name:  "Updating Login (username) field",
+			Field: user.LoginUpdateAction,
+		},
+	}
+
+	for _, tt := range disabledCases {
+		t.Run(tt.Name, func(t *testing.T) {
+			t.Run("With verification disabled should update without verifying", func(t *testing.T) {
+				tests := []struct {
+					name               string
+					smtpConfigured     bool
+					verifyEmailEnabled bool
+				}{
+					{
+						name:               "SMTP not configured",
+						smtpConfigured:     false,
+						verifyEmailEnabled: true,
+					},
+					{
+						name:               "config verify_email_enabled = false",
+						smtpConfigured:     true,
+						verifyEmailEnabled: false,
+					},
+					{
+						name:               "config verify_email_enabled = false and SMTP not configured",
+						smtpConfigured:     false,
+						verifyEmailEnabled: false,
+					},
+				}
+				for _, ttt := range tests {
+					var body string
+					newName := "newName"
+
+					settings := setting.NewCfg()
+					settings.Smtp.Enabled = ttt.smtpConfigured
+					server, userSvc, _, nsMock := setupScenario(settings)
+
+					// Override after calling setupScenario()
+					setting.VerifyEmailEnabled = ttt.verifyEmailEnabled
+
+					originalUsr := createUser(userSvc, "name", "email@localhost", "login")
+
+					// Verify that no email has been sent yet
+					require.False(t, nsMock.EmailVerified)
+
+					// Start email update
+					switch tt.Field {
+					case user.LoginUpdateAction:
+						body = fmt.Sprintf(`{"login": "%s", "name": "%s"}`, newEmail, newName)
+					case user.EmailUpdateAction:
+						body = fmt.Sprintf(`{"email": "%s", "login": "%s", "name": "%s"}`, newEmail, originalUsr.Login, newName)
+					}
+					sendUpdateReq(server, originalUsr, body)
+
+					// Verify that email has not been sent
+					require.False(t, nsMock.EmailVerified)
+
+					// Verify Email has been updated
+					userQuery := user.GetUserByIDQuery{ID: originalUsr.ID}
+					updatedUsr, err := userSvc.GetByID(context.Background(), &userQuery)
+					require.NoError(t, err)
+
+					switch tt.Field {
+					case user.LoginUpdateAction:
+						require.Equal(t, originalUsr.Email, updatedUsr.Email)
+						require.NotEqual(t, originalUsr.Login, updatedUsr.Login)
+						require.Equal(t, newEmail, updatedUsr.Login)
+					case user.EmailUpdateAction:
+						require.Equal(t, originalUsr.Login, updatedUsr.Login)
+						require.NotEqual(t, originalUsr.Email, updatedUsr.Email)
+						require.Equal(t, newEmail, updatedUsr.Email)
+					}
+
+					// Verify name has been updated
+					require.NotEqual(t, originalUsr.Name, updatedUsr.Name)
+					require.Equal(t, newName, updatedUsr.Name)
+
+					// Fields unchanged
+					require.Equal(t, originalUsr.Company, updatedUsr.Company)
+				}
+			})
+		})
+	}
+
+	t.Run("Update Email and disregard other fields", func(t *testing.T) {
+		server, userSvc, tempUserSvc, nsMock := setupScenario(nil)
+
+		originalUsr := createUser(userSvc, "name", "email@localhost", "login")
+
+		// Verify that no email has been sent yet
+		require.False(t, nsMock.EmailVerified)
+
+		// Start email update
+		newName := "newName"
+		body := fmt.Sprintf(`{"email": "%s", "name": "%s"}`, newEmail, newName)
+		sendUpdateReq(server, originalUsr, body)
+
+		// Verify email data
+		verifyEmailData(tempUserSvc, nsMock, originalUsr, newEmail)
+
+		// Verify user has not been updated yet
+		verifyUserNotUpdated(userSvc, originalUsr)
+
+		// Second part of the verification flow, when user clicks email button
+		code := nsMock.EmailVerification.Code
+		sendVerificationReq(server, originalUsr, code)
+
+		// Verify Email has been updated
+		userQuery := user.GetUserByIDQuery{ID: originalUsr.ID}
+		updatedUsr, err := userSvc.GetByID(context.Background(), &userQuery)
+		require.NoError(t, err)
+		require.NotEqual(t, originalUsr.Email, updatedUsr.Email)
+		require.Equal(t, newEmail, updatedUsr.Email)
+		// Fields unchanged
+		require.Equal(t, originalUsr.Login, updatedUsr.Login)
+		require.Equal(t, originalUsr.Name, updatedUsr.Name)
+		require.NotEqual(t, newName, updatedUsr.Name)
+	})
+
+	t.Run("Update Email when Login was also an email should update both", func(t *testing.T) {
+		server, userSvc, tempUserSvc, nsMock := setupScenario(nil)
+
+		originalUsr := createUser(userSvc, "name", "email@localhost", "email@localhost")
+
+		// Verify that no email has been sent yet
+		require.False(t, nsMock.EmailVerified)
+
+		// Start email update
+		body := fmt.Sprintf(`{"email": "%s"}`, newEmail)
+		sendUpdateReq(server, originalUsr, body)
+
+		// Verify email data
+		verifyEmailData(tempUserSvc, nsMock, originalUsr, newEmail)
+
+		// Verify user has not been updated yet
+		verifyUserNotUpdated(userSvc, originalUsr)
+
+		// Second part of the verification flow, when user clicks email button
+		code := nsMock.EmailVerification.Code
+		sendVerificationReq(server, originalUsr, code)
+
+		// Verify Email and Login have been updated
+		userQuery := user.GetUserByIDQuery{ID: originalUsr.ID}
+		updatedUsr, err := userSvc.GetByID(context.Background(), &userQuery)
+		require.NoError(t, err)
+		require.NotEqual(t, originalUsr.Email, updatedUsr.Email)
+		require.Equal(t, newEmail, updatedUsr.Email)
+		require.Equal(t, newEmail, updatedUsr.Login)
+		// Fields unchanged
+		require.Equal(t, originalUsr.Name, updatedUsr.Name)
+	})
+
+	t.Run("Update Login with an email should update Email too", func(t *testing.T) {
+		server, userSvc, tempUserSvc, nsMock := setupScenario(nil)
+
+		originalUsr := createUser(userSvc, "name", "email@localhost", "login")
+
+		// Verify that no email has been sent yet
+		require.False(t, nsMock.EmailVerified)
+
+		// Start email update
+		body := fmt.Sprintf(`{"login": "%s"}`, newEmail)
+		sendUpdateReq(server, originalUsr, body)
+
+		// Verify email data
+		verifyEmailData(tempUserSvc, nsMock, originalUsr, newEmail)
+
+		// Verify user has not been updated yet
+		verifyUserNotUpdated(userSvc, originalUsr)
+
+		// Second part of the verification flow, when user clicks email button
+		code := nsMock.EmailVerification.Code
+		sendVerificationReq(server, originalUsr, code)
+
+		// Verify Email and Login have been updated
+		userQuery := user.GetUserByIDQuery{ID: originalUsr.ID}
+		updatedUsr, err := userSvc.GetByID(context.Background(), &userQuery)
+		require.NoError(t, err)
+		require.NotEqual(t, originalUsr.Email, updatedUsr.Email)
+		require.NotEqual(t, originalUsr.Login, updatedUsr.Login)
+		require.Equal(t, newEmail, updatedUsr.Email)
+		require.Equal(t, newEmail, updatedUsr.Login)
+		// Fields unchanged
+		require.Equal(t, originalUsr.Name, updatedUsr.Name)
+	})
+
+	t.Run("Update Login should not need verification if it is not an email", func(t *testing.T) {
+		server, userSvc, _, nsMock := setupScenario(nil)
+
+		originalUsr := createUser(userSvc, "name", "email@localhost", "login")
+
+		// Verify that no email has been sent yet
+		require.False(t, nsMock.EmailVerified)
+
+		// Start email update
+		newLogin := "newLogin"
+		newName := "newName"
+		body := fmt.Sprintf(`{"login": "%s", "name": "%s"}`, newLogin, newName)
+		sendUpdateReq(server, originalUsr, body)
+
+		// Verify that email has not been sent
+		require.False(t, nsMock.EmailVerified)
+
+		// Verify Login has been updated
+		userQuery := user.GetUserByIDQuery{ID: originalUsr.ID}
+		updatedUsr, err := userSvc.GetByID(context.Background(), &userQuery)
+		require.NoError(t, err)
+		require.NotEqual(t, originalUsr.Login, updatedUsr.Login)
+		require.NotEqual(t, originalUsr.Name, updatedUsr.Name)
+		require.Equal(t, newLogin, updatedUsr.Login)
+		require.Equal(t, newName, updatedUsr.Name)
+		// Fields unchanged
+		require.Equal(t, originalUsr.Email, updatedUsr.Email)
+	})
+
+	t.Run("Update Login should not need verification if it is being updated to the already configured email", func(t *testing.T) {
+		server, userSvc, _, nsMock := setupScenario(nil)
+
+		originalUsr := createUser(userSvc, "name", "email@localhost", "login")
+
+		// Verify that no email has been sent yet
+		require.False(t, nsMock.EmailVerified)
+
+		// Start email update
+		body := fmt.Sprintf(`{"login": "%s"}`, originalUsr.Email)
+		sendUpdateReq(server, originalUsr, body)
+
+		// Verify that email has not been sent
+		require.False(t, nsMock.EmailVerified)
+
+		// Verify Login has been updated
+		userQuery := user.GetUserByIDQuery{ID: originalUsr.ID}
+		updatedUsr, err := userSvc.GetByID(context.Background(), &userQuery)
+		require.NoError(t, err)
+		require.NotEqual(t, originalUsr.Login, updatedUsr.Login)
+		require.Equal(t, originalUsr.Email, updatedUsr.Login)
+		require.Equal(t, originalUsr.Email, updatedUsr.Email)
+	})
+
+	t.Run("Update Login and Email with different email values at once should disregard the Login update", func(t *testing.T) {
+		server, userSvc, tempUserSvc, nsMock := setupScenario(nil)
+
+		originalUsr := createUser(userSvc, "name", "email@localhost", "login")
+
+		// Verify that no email has been sent yet
+		require.False(t, nsMock.EmailVerified)
+
+		// Start email update
+		newLogin := "newEmail2@localhost"
+		body := fmt.Sprintf(`{"email": "%s", "login": "%s"}`, newEmail, newLogin)
+		sendUpdateReq(server, originalUsr, body)
+
+		// Verify email data
+		verifyEmailData(tempUserSvc, nsMock, originalUsr, newEmail)
+
+		// Verify user has not been updated yet
+		verifyUserNotUpdated(userSvc, originalUsr)
+
+		// Second part of the verification flow, when user clicks email button
+		code := nsMock.EmailVerification.Code
+		sendVerificationReq(server, originalUsr, code)
+
+		// Verify only Email has been updated
+		userQuery := user.GetUserByIDQuery{ID: originalUsr.ID}
+		updatedUsr, err := userSvc.GetByID(context.Background(), &userQuery)
+		require.NoError(t, err)
+		require.NotEqual(t, originalUsr.Email, updatedUsr.Email)
+		require.Equal(t, newEmail, updatedUsr.Email)
+		// Fields unchanged
+		require.NotEqual(t, newLogin, updatedUsr.Login)
+		require.Equal(t, originalUsr.Login, updatedUsr.Login)
+		require.Equal(t, originalUsr.Name, updatedUsr.Name)
+	})
+
+	t.Run("Update Login and Email with different email values at once when Login was already an email should update both with Email", func(t *testing.T) {
+		server, userSvc, tempUserSvc, nsMock := setupScenario(nil)
+
+		originalUsr := createUser(userSvc, "name", "email@localhost", "email@localhost")
+
+		// Verify that no email has been sent yet
+		require.False(t, nsMock.EmailVerified)
+
+		// Start email update
+		newLogin := "newEmail2@localhost"
+		body := fmt.Sprintf(`{"email": "%s", "login": "%s"}`, newEmail, newLogin)
+		sendUpdateReq(server, originalUsr, body)
+
+		// Verify email data
+		verifyEmailData(tempUserSvc, nsMock, originalUsr, newEmail)
+
+		// Verify user has not been updated yet
+		verifyUserNotUpdated(userSvc, originalUsr)
+
+		// Second part of the verification flow, when user clicks email button
+		code := nsMock.EmailVerification.Code
+		sendVerificationReq(server, originalUsr, code)
+
+		// Verify only Email has been updated
+		userQuery := user.GetUserByIDQuery{ID: originalUsr.ID}
+		updatedUsr, err := userSvc.GetByID(context.Background(), &userQuery)
+		require.NoError(t, err)
+		require.NotEqual(t, originalUsr.Email, updatedUsr.Email)
+		require.NotEqual(t, originalUsr.Login, updatedUsr.Login)
+		require.NotEqual(t, newLogin, updatedUsr.Login)
+		require.Equal(t, newEmail, updatedUsr.Email)
+		require.Equal(t, newEmail, updatedUsr.Login)
+		// Fields unchanged
+		require.Equal(t, originalUsr.Name, updatedUsr.Name)
+	})
+
+	t.Run("Email verification should expire", func(t *testing.T) {
+		cfg := setting.NewCfg()
+		cfg.Smtp.Enabled = true
+		cfg.VerificationEmailMaxLifetime = 0 // Expire instantly
+
+		server, userSvc, tempUserSvc, nsMock := setupScenario(cfg)
+
+		originalUsr := createUser(userSvc, "name", "email@localhost", "login")
+
+		// Verify that no email has been sent yet
+		require.False(t, nsMock.EmailVerified)
+
+		// Start email update
+		body := fmt.Sprintf(`{"email": "%s"}`, newEmail)
+		sendUpdateReq(server, originalUsr, body)
+
+		// Verify email data
+		verifyEmailData(tempUserSvc, nsMock, originalUsr, newEmail)
+
+		// Verify user has not been updated yet
+		verifyUserNotUpdated(userSvc, originalUsr)
+
+		// Second part of the verification flow, when user clicks email button
+		code := nsMock.EmailVerification.Code
+		sendVerificationReq(server, originalUsr, code)
+
+		// Verify user has not been updated
+		userQuery := user.GetUserByIDQuery{ID: originalUsr.ID}
+		updatedUsr, err := userSvc.GetByID(context.Background(), &userQuery)
+		require.NoError(t, err)
+		require.NotEqual(t, newEmail, updatedUsr.Email)
+		require.Equal(t, originalUsr.Email, updatedUsr.Email)
+		require.Equal(t, originalUsr.Login, updatedUsr.Login)
+	})
+
+	t.Run("A new verification should revoke other pending verifications", func(t *testing.T) {
+		server, userSvc, tempUserSvc, nsMock := setupScenario(nil)
+
+		originalUsr := createUser(userSvc, "name", "email@localhost", "login")
+
+		// Verify that no email has been sent yet
+		require.False(t, nsMock.EmailVerified)
+
+		// First email verification
+		firstNewEmail := "newEmail1@localhost"
+		body := fmt.Sprintf(`{"email": "%s"}`, firstNewEmail)
+		sendUpdateReq(server, originalUsr, body)
+		verifyEmailData(tempUserSvc, nsMock, originalUsr, firstNewEmail)
+		firstCode := nsMock.EmailVerification.Code
+
+		// Second email verification
+		secondNewEmail := "newEmail2@localhost"
+		body = fmt.Sprintf(`{"email": "%s"}`, secondNewEmail)
+		sendUpdateReq(server, originalUsr, body)
+		verifyEmailData(tempUserSvc, nsMock, originalUsr, secondNewEmail)
+		secondCode := nsMock.EmailVerification.Code
+
+		// Verify user has not been updated yet
+		verifyUserNotUpdated(userSvc, originalUsr)
+
+		// Try to follow through with the first verification unsuccessfully
+		sendVerificationReq(server, originalUsr, firstCode)
+		verifyUserNotUpdated(userSvc, originalUsr)
+
+		// Follow through with second verification successfully
+		sendVerificationReq(server, originalUsr, secondCode)
+
+		userQuery := user.GetUserByIDQuery{ID: originalUsr.ID}
+		updatedUsr, err := userSvc.GetByID(context.Background(), &userQuery)
+		require.NoError(t, err)
+		require.NotEqual(t, originalUsr.Email, updatedUsr.Email)
+		require.Equal(t, secondNewEmail, updatedUsr.Email)
+		// Fields unchanged
+		require.Equal(t, originalUsr.Login, updatedUsr.Login)
+	})
+
+	t.Run("Email verification should fail if code is not valid", func(t *testing.T) {
+		server, userSvc, tempUserSvc, nsMock := setupScenario(nil)
+
+		originalUsr := createUser(userSvc, "name", "email@localhost", "login")
+
+		// Verify that no email has been sent yet
+		require.False(t, nsMock.EmailVerified)
+
+		// Start email update
+		body := fmt.Sprintf(`{"email": "%s"}`, newEmail)
+		sendUpdateReq(server, originalUsr, body)
+
+		// Verify email data
+		verifyEmailData(tempUserSvc, nsMock, originalUsr, newEmail)
+
+		// Verify user has not been updated yet
+		verifyUserNotUpdated(userSvc, originalUsr)
+
+		// Second part of the verification flow should fail if using the wrong code
+		sendVerificationReq(server, originalUsr, "notTheRightCode")
+		verifyUserNotUpdated(userSvc, originalUsr)
+	})
+
+	t.Run("Email verification code can only be used once", func(t *testing.T) {
+		server, userSvc, _, nsMock := setupScenario(nil)
+
+		originalUsr := createUser(userSvc, "name", "email@localhost", "login")
+
+		// Start email update
+		require.NotEqual(t, originalUsr.Email, newEmail)
+
+		body := fmt.Sprintf(`{"email": "%s"}`, newEmail)
+		sendUpdateReq(server, originalUsr, body)
+
+		// Verify user has not been updated yet
+		verifyUserNotUpdated(userSvc, originalUsr)
+
+		// Use code to verify successfully
+		codeToReuse := nsMock.EmailVerification.Code
+		sendVerificationReq(server, originalUsr, codeToReuse)
+
+		// User should have an updated Email
+		userQuery := user.GetUserByIDQuery{ID: originalUsr.ID}
+		updatedUsr, err := userSvc.GetByID(context.Background(), &userQuery)
+		require.NoError(t, err)
+		require.Equal(t, newEmail, updatedUsr.Email)
+
+		// Change email back to what it was
+		body = fmt.Sprintf(`{"email": "%s"}`, originalUsr.Email)
+		sendUpdateReq(server, originalUsr, body)
+		sendVerificationReq(server, originalUsr, nsMock.EmailVerification.Code)
+		verifyUserNotUpdated(userSvc, originalUsr)
+
+		// Re-use code to verify new email again, unsuccessfully
+		sendVerificationReq(server, originalUsr, codeToReuse)
+		verifyUserNotUpdated(userSvc, originalUsr)
+	})
+
+	t.Run("Update Email with an email that is already being used should fail", func(t *testing.T) {
+		testCases := []struct {
+			description string
+			clashLogin  bool
+		}{
+			{
+				description: "when Email clashes",
+				clashLogin:  false,
+			},
+			{
+				description: "when Login clashes",
+				clashLogin:  true,
+			},
+		}
+		for _, tt := range testCases {
+			t.Run(tt.description, func(t *testing.T) {
+				server, userSvc, _, nsMock := setupScenario(nil)
+
+				originalUsr := createUser(userSvc, "name1", "email1@localhost", "login1@localhost")
+				badUsr := createUser(userSvc, "name2", "email2@localhost", "login2")
+
+				// Verify that no email has been sent yet
+				require.False(t, nsMock.EmailVerified)
+
+				// Update `badUsr` to use the same email as `originalUsr`
+				body := fmt.Sprintf(`{"email": "%s"}`, originalUsr.Email)
+				if tt.clashLogin {
+					body = fmt.Sprintf(`{"login": "%s"}`, originalUsr.Login)
+				}
+				req := server.NewRequest(
+					http.MethodPut,
+					"/api/user",
+					strings.NewReader(body),
+				)
+				req.Header.Add("Content-Type", "application/json")
+				res, err := doReq(req, badUsr)
+				require.NoError(t, err)
+				assert.Equal(t, http.StatusConflict, res.StatusCode)
+				require.NoError(t, res.Body.Close())
+
+				// Verify that no email has been sent
+				require.False(t, nsMock.EmailVerified)
+
+				// Verify user has not been updated
+				verifyUserNotUpdated(userSvc, badUsr)
+			})
+		}
+	})
+}
+
 type updateUserContext struct {
 	desc         string
 	url          string