fix: allow unactivated users to send recovery mails (#9504)

With forgejo/forgejo#9075 the `GetUserByEmail` now actually only used activated emails. This however broke sending recovery mails to unactivated users, as their email are not yet activated.

Use the newly introduced function `GetUserByEmailSimple` to not care about this activated email requirement and also avoid the no-reply address being a valid email address for this functionality.

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/9504
Reviewed-by: Otto <otto@codeberg.org>
Co-authored-by: Gusted <postmaster@gusted.xyz>
Co-committed-by: Gusted <postmaster@gusted.xyz>

models/user/user.go

@@ -1194,7 +1194,9 @@ func ValidateCommitsWithEmails(ctx context.Context, oldCommits []*git.Commit) []
 	return newCommits
 }
 
-// GetUserByEmail returns the user object by given e-mail if exists.
+// GetUserByEmail returns the user associated with the email, if it exists
+// and is activated. If the email is a no-reply address, then the user
+// associated with that no-reply address is returned.
 func GetUserByEmail(ctx context.Context, email string) (*User, error) {
 	if len(email) == 0 {
 		return nil, ErrUserNotExist{Name: email}
@@ -1227,6 +1229,26 @@ func GetUserByEmail(ctx context.Context, email string) (*User, error) {
 	return nil, ErrUserNotExist{Name: email}
 }
 
+// GetUserByEmailSimple returns the user associated with the email, if it exists.
+//
+// NOTE: You likely should use `GetUserByEmail`, which handles the no-reply
+// address and only uses activated emails to get the user.
+func GetUserByEmailSimple(ctx context.Context, email string) (*User, error) {
+	if len(email) == 0 {
+		return nil, ErrUserNotExist{Name: email}
+	}
+
+	emailAddress := &EmailAddress{}
+	has, err := db.GetEngine(ctx).Where("lower_email = ?", strings.ToLower(email)).Get(emailAddress)
+	if err != nil {
+		return nil, err
+	} else if !has {
+		return nil, ErrUserNotExist{Name: email}
+	}
+
+	return GetUserByID(ctx, emailAddress.UID)
+}
+
 // GetUser checks if a user already exists
 func GetUser(ctx context.Context, user *User) (bool, error) {
 	return db.GetEngine(ctx).Get(user)

models/user/user_test.go

@@ -999,6 +999,7 @@ func TestPronounsPrivacy(t *testing.T) {
 
 func TestGetUserByEmail(t *testing.T) {
 	require.NoError(t, unittest.PrepareTestDatabase())
+	defer test.MockVariableValue(&setting.Service.NoReplyAddress, "noreply.example.org")()
 
 	t.Run("Normal", func(t *testing.T) {
 		u, err := user_model.GetUserByEmail(t.Context(), "user2@example.com")
@@ -1017,4 +1018,33 @@ func TestGetUserByEmail(t *testing.T) {
 		require.NoError(t, err)
 		assert.EqualValues(t, 1, u.ID)
 	})
+
+	t.Run("No-reply", func(t *testing.T) {
+		u, err := user_model.GetUserByEmail(t.Context(), "user1@noreply.example.org")
+		require.NoError(t, err)
+		assert.EqualValues(t, 1, u.ID)
+	})
+}
+
+func TestGetUserByEmailSimple(t *testing.T) {
+	require.NoError(t, unittest.PrepareTestDatabase())
+	defer test.MockVariableValue(&setting.Service.NoReplyAddress, "noreply.example.org")()
+
+	t.Run("Normal", func(t *testing.T) {
+		u, err := user_model.GetUserByEmailSimple(t.Context(), "user2@example.com")
+		require.NoError(t, err)
+		assert.EqualValues(t, 2, u.ID)
+	})
+
+	t.Run("Not activated", func(t *testing.T) {
+		u, err := user_model.GetUserByEmailSimple(t.Context(), "user11@example.com")
+		require.NoError(t, err)
+		assert.EqualValues(t, 11, u.ID)
+	})
+
+	t.Run("No-reply", func(t *testing.T) {
+		u, err := user_model.GetUserByEmailSimple(t.Context(), "user1@noreply.example.org")
+		require.ErrorIs(t, err, user_model.ErrUserNotExist{Name: "user1@noreply.example.org"})
+		assert.Nil(t, u)
+	})
 }

routers/web/auth/password.go

@@ -61,7 +61,7 @@ func ForgotPasswdPost(ctx *context.Context) {
 	email := ctx.FormString("email")
 	ctx.Data["Email"] = email
 
-	u, err := user_model.GetUserByEmail(ctx, email)
+	u, err := user_model.GetUserByEmailSimple(ctx, email)
 	if err != nil {
 		if user_model.IsErrUserNotExist(err) {
 			ctx.Data["ResetPwdCodeLives"] = timeutil.MinutesToFriendly(setting.Service.ResetPwdCodeLives, ctx.Locale)

tests/integration/user_recovery_test.go

@@ -0,0 +1,52 @@
+// Copyright 2025 The Forgejo Authors. All rights reserved.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+package integration
+
+import (
+	"net/http"
+	"testing"
+
+	"forgejo.org/models/unittest"
+	user_model "forgejo.org/models/user"
+	"forgejo.org/modules/test"
+	"forgejo.org/modules/translation"
+	"forgejo.org/services/mailer"
+	"forgejo.org/tests"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestForgotPassword(t *testing.T) {
+	defer tests.PrepareTestEnv(t)()
+
+	test := func(t *testing.T, user *user_model.User, email *user_model.EmailAddress) {
+		t.Helper()
+
+		called := false
+		defer test.MockVariableValue(&mailer.SendAsync, func(msgs ...*mailer.Message) {
+			assert.Len(t, msgs, 1)
+			assert.Equal(t, user.EmailTo(), msgs[0].To)
+			assert.EqualValues(t, translation.NewLocale("en-US").Tr("mail.reset_password"), msgs[0].Subject)
+			assert.Contains(t, msgs[0].Body, translation.NewLocale("en-US").Tr("mail.reset_password.text", "3 hours"))
+			called = true
+		})()
+
+		req := NewRequestWithValues(t, "POST", "/user/forgot_password", map[string]string{
+			"_csrf": GetCSRF(t, emptyTestSession(t), "/user/forgot_password"),
+			"email": email.Email,
+		})
+		MakeRequest(t, req, http.StatusOK)
+
+		assert.True(t, called)
+	}
+	t.Run("Unactivated email address", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+		test(t, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 11}), unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{UID: 11}, "is_activated = false"))
+	})
+
+	t.Run("Activated email address", func(t *testing.T) {
+		defer tests.PrintCurrentTest(t)()
+		test(t, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: 12}), unittest.AssertExistsAndLoadBean(t, &user_model.EmailAddress{UID: 12}, "is_activated = true"))
+	})
+}