From 1ccd539b6c98b18080ee7189641a1d701ded78ec Mon Sep 17 00:00:00 2001 From: forgejo-backport-action Date: Wed, 2 Jul 2025 13:54:45 +0200 Subject: [PATCH] [v12.0/forgejo] fix: user activation with uppercase email address (#8386) **Backport:** https://codeberg.org/forgejo/forgejo/pulls/8367 - Right before the call to user email activation, the user is updated [^1]. This causes the email to be lowered, which in turn makes the call to activate the user activation fail (on database where collation is case sensitive, which is the recommend collation by Forgejo). - The code in `BeforeUpdate` is quite confusing, the comment has become slightly out of date and was reworded to reflect reality and its purpose. The code is also slightly reworked to only lower the email for the `AvatarEmail` field to avoid causing side-effect. - Added unit test. - Resolves forgejo/forgejo#8354 [^1]: https://codeberg.org/forgejo/forgejo/src/commit/4927d4ee3d53ad79fe88dfe862c33339c0a9cdc6/routers/web/auth/auth.go#L785 Co-authored-by: Gusted Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/8386 Reviewed-by: Earl Warren Co-authored-by: forgejo-backport-action Co-committed-by: forgejo-backport-action --- .../TestActivateUserEmail/email_address.yml | 7 +++++++ models/fixtures/TestActivateUserEmail/user.yml | 12 ++++++++++++ models/user/email_address_test.go | 17 +++++++++++++++++ models/user/user.go | 6 +++--- 4 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 models/fixtures/TestActivateUserEmail/email_address.yml create mode 100644 models/fixtures/TestActivateUserEmail/user.yml diff --git a/models/fixtures/TestActivateUserEmail/email_address.yml b/models/fixtures/TestActivateUserEmail/email_address.yml new file mode 100644 index 0000000000..cf41ff8241 --- /dev/null +++ b/models/fixtures/TestActivateUserEmail/email_address.yml @@ -0,0 +1,7 @@ +- + id: 1001 + uid: 1001 + email: AnotherTestUserWithUpperCaseEmail@otto.splvs.net + lower_email: anothertestuserwithuppercaseemail@otto.splvs.net + is_activated: false + is_primary: true diff --git a/models/fixtures/TestActivateUserEmail/user.yml b/models/fixtures/TestActivateUserEmail/user.yml new file mode 100644 index 0000000000..0a68e70a4a --- /dev/null +++ b/models/fixtures/TestActivateUserEmail/user.yml @@ -0,0 +1,12 @@ +- + id: 1001 + lower_name: user1001 + name: user1001 + full_name: User That loves Upper Cases + email: AnotherTestUserWithUpperCaseEmail@otto.splvs.net + passwd: ZogKvWdyEx:password + passwd_hash_algo: dummy + avatar: '' + avatar_email: anothertestuserwithuppercaseemail@otto.splvs.net + login_name: user1 + created_unix: 1672578000 diff --git a/models/user/email_address_test.go b/models/user/email_address_test.go index 1801f57a23..85f5b16c65 100644 --- a/models/user/email_address_test.go +++ b/models/user/email_address_test.go @@ -181,3 +181,20 @@ func TestDeletePrimaryEmailAddressOfUser(t *testing.T) { assert.True(t, user_model.IsErrEmailAddressNotExist(err)) assert.Nil(t, email) } + +func TestActivateUserEmail(t *testing.T) { + defer unittest.OverrideFixtures("models/fixtures/TestActivateUserEmail")() + require.NoError(t, unittest.PrepareTestDatabase()) + + t.Run("Activate email", func(t *testing.T) { + require.NoError(t, user_model.ActivateUserEmail(t.Context(), 1001, "AnotherTestUserWithUpperCaseEmail@otto.splvs.net", true)) + + unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{UID: 1001}, "is_activated = true") + }) + + t.Run("Deactivate email", func(t *testing.T) { + require.NoError(t, user_model.ActivateUserEmail(t.Context(), 1001, "AnotherTestUserWithUpperCaseEmail@otto.splvs.net", false)) + + unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{UID: 1001}, "is_activated = false") + }) +} diff --git a/models/user/user.go b/models/user/user.go index eedd1db80e..b124572bb6 100644 --- a/models/user/user.go +++ b/models/user/user.go @@ -182,11 +182,11 @@ func (u *User) BeforeUpdate() { u.MaxRepoCreation = -1 } - // Organization does not need email - u.Email = strings.ToLower(u.Email) + // Ensure AvatarEmail is set for non-organization users, because organization + // are not required to have a email set. if !u.IsOrganization() { if len(u.AvatarEmail) == 0 { - u.AvatarEmail = u.Email + u.AvatarEmail = strings.ToLower(u.Email) } }