fix(mailer): reject incoming email tokens of users who cannot sign in (#39833) (#39841)

Backport #39833 by @basavaraj-sm05

Incoming email reply tokens stay valid for a year and are verified with
`user.Rands`, which is not rotated when an account is deactivated or
prohibited from signing in. A suspended user could therefore keep
commenting on public repos by replying to old notifications.

`DecodeToken` now rejects tokens of organizations, inactive users and
users prohibited from signing in, which covers every incoming mail
handler.

Co-authored-by: Basavaraj S m <basavaraj@digiscrypt.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
Giteabot
2026-10-18 12:28:08 +00:00
committed by GitHub
co-authored by Basavaraj S m wxiaoguang
parent dc30660291
commit 1eee73e9bd
3 changed files with 63 additions and 3 deletions
+28 -2
View File
@@ -1021,7 +1021,8 @@ func GetInactiveUsers(ctx context.Context, olderThan time.Duration) ([]*User, er
Find(&users)
}
// GetUserByID returns the user object by given ID if exists.
// GetUserByID returns the user object by given ID if exists. TODO: for most cases, this function should be avoided.
// Use correct GetXxxUser functions instead to handle user types & states correctly.
func GetUserByID(ctx context.Context, id int64) (*User, error) {
u := new(User)
has, err := db.GetEngine(ctx).ID(id).Get(u)
@@ -1033,7 +1034,8 @@ func GetUserByID(ctx context.Context, id int64) (*User, error) {
return u, nil
}
// GetUserByIDs returns the user objects by given IDs if exists.
// GetUserByIDs returns the user objects by given IDs if exists. TODO: for most cases, this function should be avoided.
// Use correct GetXxxUser functions instead to handle user types & states correctly.
func GetUserByIDs(ctx context.Context, ids []int64) ([]*User, error) {
if len(ids) == 0 {
return nil, nil
@@ -1046,6 +1048,13 @@ func GetUserByIDs(ctx context.Context, ids []int64) ([]*User, error) {
return users, err
}
// Glossaries for XxxUser:
// * Possible: The user may or may not exist in the database, but we can still return a valid user object (e.g. Ghost user) for the given ID.
// * Individual: The user is a real person, not an organization or a bot.
// * Doer: The user is the one performing an action
// * Valid: The user is an active user and not prohibited from signing in
// * System: Negative ID (e.g. Ghost, Actions, DeployKey, etc.) and not a real person, Ghost is not included if there is no "Possible" keyword
// GetPossibleUserByID returns the possible user and its ID. If the user doesn't exist, it returns Ghost user
func GetPossibleUserByID(ctx context.Context, id int64) (_ int64, u *User, err error) {
if id < 0 {
@@ -1082,6 +1091,23 @@ func GetPossibleUserByIDs(ctx context.Context, ids []int64) ([]*User, error) {
return users, nil
}
func GetValidDoerUserByID(ctx context.Context, id int64) (*User, error) {
u, err := GetUserByID(ctx, id)
if err != nil {
return nil, err
}
if !u.IsIndividual() && !u.IsTypeBot() {
return nil, util.NewNotExistErrorf("user %d is not a doer", id)
}
if !u.IsActive {
return nil, util.NewPermissionDeniedErrorf("user %d is inactive", id)
}
if u.ProhibitLogin {
return nil, util.NewPermissionDeniedErrorf("user %d is prohibited from login", id)
}
return u, nil
}
func getUserByNameWithTypes(ctx context.Context, name string, types ...UserType) (*User, error) {
u := &User{}
sess := db.GetEngine(ctx).Where(builder.Eq{"lower_name": strings.ToLower(name)})
+1 -1
View File
@@ -98,7 +98,7 @@ func DecodeToken(ctx context.Context, token string) (HandlerType, *user_model.Us
return UnknownHandlerType, nil, nil, err
}
user, err := user_model.GetUserByID(ctx, userID)
user, err := user_model.GetValidDoerUserByID(ctx, userID)
if err != nil {
return UnknownHandlerType, nil, nil, err
}
+34
View File
@@ -0,0 +1,34 @@
// Copyright 2026 The Gitea Authors. All rights reserved.
// SPDX-License-Identifier: MIT
package token
import (
"testing"
"gitea.dev/models/unittest"
user_model "gitea.dev/models/user"
_ "gitea.dev/models"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
func TestMain(m *testing.M) {
unittest.MainTest(m)
}
func TestDecodeTokenRejectsUserWhoCannotSignIn(t *testing.T) {
require.NoError(t, unittest.PrepareTestDatabase())
for id, expectedErr := range map[int64]string{2: "", 3: "not a doer", 9: "inactive", 37: "prohibited"} {
token, err := CreateToken(ReplyHandlerType, unittest.AssertExistsAndLoadBean(t, &user_model.User{ID: id}), []byte{1})
require.NoError(t, err)
_, _, _, err = DecodeToken(t.Context(), token)
if expectedErr == "" {
assert.NoError(t, err)
} else {
assert.ErrorContains(t, err, expectedErr)
}
}
}