mirror of
https://github.com/go-gitea/gitea.git
synced 2026-10-19 04:17:43 +02:00
fix(mailer): reject incoming email tokens of users who cannot sign in (#39833)
Incoming email reply tokens are embedded in every notification email and stay valid for a year. `DecodeToken` verifies them with `user.Rands`, which is not rotated when an admin disables sign-in or the account is deactivated, and `ReplyHandler` only checks repo read permission. A suspended user could therefore keep posting comments, review comments and attachments on any public repo by replying to old notifications, while the web, API, git and package paths all refuse them. `DecodeToken` now rejects tokens of users who are inactive or prohibited from signing in, which covers every incoming mail handler. --------- Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
This commit is contained in:
1 parent
52b18b4903
commit
18d0ee174d
3 files changed
+63
-3
No files matched your search
+28
-2
@@ -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)})
|
||||
|
||||
@@ -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
|
||||
}
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user