From ea17d7da0d2c332cdd0ecda632d7dd1c898fccbb Mon Sep 17 00:00:00 2001 From: Astra Date: Tue, 15 Sep 2026 16:22:00 +0100 Subject: [PATCH] admin: bypass the reserved-username blocklist at write time too UpdateUsernameAdmin already skipped the reserved-word check in the availability lookup, but UserStore.UpdateUsername's own write path (replacePeerUsernameTx / CollectibleUsernameStore.SetEditableUsername) enforces the same operator blocklist a second time, independently and unconditionally. That second check is what was still rejecting an admin handing out a word they'd deliberately reserved, with "username occupied". Add UpdateUsernameAdmin/SetEditableUsernameAdmin bypass variants down the write path (postgres and memory) and route users.Service's actual write through them when the availability check was already bypassed. --- internal/app/users/service.go | 48 ++++++++++++++++++- internal/app/users/service_test.go | 33 +++++++++++++ internal/store/memory/collectible_username.go | 15 +++++- internal/store/memory/users.go | 36 +++++++++++++- internal/store/postgres/peer_username.go | 39 +++++++++++---- internal/store/postgres/user.go | 24 +++++++++- 6 files changed, 180 insertions(+), 15 deletions(-) diff --git a/internal/app/users/service.go b/internal/app/users/service.go index bbb1c2f1..2bfdd679 100644 --- a/internal/app/users/service.go +++ b/internal/app/users/service.go @@ -48,6 +48,24 @@ type usernameAvailabilityStore interface { CheckUsername(ctx context.Context, userID int64, username string) (bool, error) } +// adminUsernameAvailabilityStore is usernameAvailabilityStore's admin-bypass +// counterpart: CheckUsernameAdmin skips the operator reserved-username +// blocklist, so UpdateUsernameAdmin can hand a deliberately reserved word to +// a specific account instead of that same reservation blocking the operator's +// own assignment. +type adminUsernameAvailabilityStore interface { + CheckUsernameAdmin(ctx context.Context, userID int64, username string) (bool, error) +} + +// adminUsernameStore is store.UserStore's admin-bypass counterpart for the +// actual write: UpdateUsername's own write path enforces the reserved-word +// blocklist a second time (independent of the availability check), so +// bypassing only checkUsernameAvailable isn't enough -- the write itself +// needs UpdateUsernameAdmin too. +type adminUsernameStore interface { + UpdateUsernameAdmin(ctx context.Context, userID int64, username string) (domain.User, error) +} + type moderationFlagAudienceStore interface { ModerationFlagAudience(ctx context.Context, userID int64, limit int) ([]int64, error) } @@ -318,6 +336,17 @@ func (s *Service) checkUsernameAvailable(ctx context.Context, selfID int64, user return !found || u.ID == selfID, nil } +// checkUsernameAvailableAdmin is checkUsernameAvailable without the operator +// reserved-username blocklist. Falls back to checkUsernameAvailable (which +// does enforce it) if the store doesn't implement the admin-bypass method -- +// availability checking still works, just without the bypass. +func (s *Service) checkUsernameAvailableAdmin(ctx context.Context, selfID int64, username string) (bool, error) { + if checker, ok := s.users.(adminUsernameAvailabilityStore); ok { + return checker.CheckUsernameAdmin(ctx, selfID, username) + } + return s.checkUsernameAvailable(ctx, selfID, username) +} + // UpdateUsername 修改当前用户的主 username（self-service）。空字符串表示删除 username。 func (s *Service) UpdateUsername(ctx context.Context, userID int64, username string) (domain.User, error) { return s.updateUsername(ctx, userID, username, true) @@ -350,7 +379,15 @@ func (s *Service) updateUsername(ctx context.Context, userID int64, username str if !validUsername(username) || (enforceReserved && s.reserved.Contains(username)) { return domain.User{}, domain.ErrUsernameInvalid } - ok, err := s.checkUsernameAvailable(ctx, self.ID, username) + var ( + ok bool + err error + ) + if enforceReserved { + ok, err = s.checkUsernameAvailable(ctx, self.ID, username) + } else { + ok, err = s.checkUsernameAvailableAdmin(ctx, self.ID, username) + } if err != nil { return domain.User{}, err } @@ -358,7 +395,14 @@ func (s *Service) updateUsername(ctx context.Context, userID int64, username str return domain.User{}, domain.ErrUsernameOccupied } } - u, err := s.users.UpdateUsername(ctx, self.ID, username) + var u domain.User + if enforceReserved { + u, err = s.users.UpdateUsername(ctx, self.ID, username) + } else if admin, ok := s.users.(adminUsernameStore); ok { + u, err = admin.UpdateUsernameAdmin(ctx, self.ID, username) + } else { + u, err = s.users.UpdateUsername(ctx, self.ID, username) + } if err != nil { return domain.User{}, err } diff --git a/internal/app/users/service_test.go b/internal/app/users/service_test.go index 9321367e..3c505df9 100644 --- a/internal/app/users/service_test.go +++ b/internal/app/users/service_test.go @@ -151,6 +151,39 @@ func TestServiceUpdateUsernameAdminBypassesReserved(t *testing.T) { } } +// TestServiceUpdateUsernameAdminBypassesOperatorReservedTable locks in that +// UpdateUsernameAdmin also bypasses the *database-backed* reserved-usernames +// blocklist (the admin console's own Reserved Usernames feature), not just +// config.ReservedUsernames -- an operator who deliberately reserves a word +// via that feature must still be able to hand it to a specific account, +// instead of their own reservation blocking them with "username occupied". +func TestServiceUpdateUsernameAdminBypassesOperatorReservedTable(t *testing.T) { + ctx := context.Background() + userStore := memory.NewUserStore() + reserved := memory.NewReservedUsernameStore() + registry := memory.NewCollectibleUsernameStore().WithReservedUsernames(reserved) + userStore.AttachUsernameRegistry(registry) + if _, err := reserved.ReserveUsername(ctx, "durov", "brand protection", "operator"); err != nil { + t.Fatalf("reserve username: %v", err) + } + target, err := userStore.Create(ctx, domain.User{AccessHash: 1, Phone: "15550000006", FirstName: "Target"}) + if err != nil { + t.Fatalf("create target: %v", err) + } + svc := NewService(userStore) + + if _, err := svc.UpdateUsername(ctx, target.ID, "durov"); !errors.Is(err, domain.ErrUsernameOccupied) { + t.Fatalf("self-service claim of operator-reserved username err = %v, want username occupied", err) + } + u, err := svc.UpdateUsernameAdmin(ctx, target.ID, "durov") + if err != nil { + t.Fatalf("admin claim of operator-reserved username: %v", err) + } + if u.Username != "durov" { + t.Fatalf("admin claim of operator-reserved username: got username %q, want %q", u.Username, "durov") + } +} + // marksbotOverrideStore wraps memory.UserStore to serve domain.VerifierBotUser() // for a fixed username lookup, since memory.UserStore.Create always assigns an // id from its own auto-increment sequence and can never produce the fixed diff --git a/internal/store/memory/collectible_username.go b/internal/store/memory/collectible_username.go index f4c79f8a..ed751ce0 100644 --- a/internal/store/memory/collectible_username.go +++ b/internal/store/memory/collectible_username.go @@ -105,7 +105,18 @@ func NewCollectibleUsernameStore() *CollectibleUsernameStore { // usernames on the user and channel rows, so tests need this hook to give a peer // the editable registry row the projection expects. An empty username clears the // slot. -func (s *CollectibleUsernameStore) SetEditableUsername(_ context.Context, peer domain.Peer, username string) (bool, error) { +func (s *CollectibleUsernameStore) SetEditableUsername(ctx context.Context, peer domain.Peer, username string) (bool, error) { + return s.setEditableUsernameChecked(ctx, peer, username, true) +} + +// SetEditableUsernameAdmin is SetEditableUsername without the operator +// reserved-username blocklist check, for the admin console deliberately +// assigning a reserved word to a specific account. +func (s *CollectibleUsernameStore) SetEditableUsernameAdmin(ctx context.Context, peer domain.Peer, username string) (bool, error) { + return s.setEditableUsernameChecked(ctx, peer, username, false) +} + +func (s *CollectibleUsernameStore) setEditableUsernameChecked(_ context.Context, peer domain.Peer, username string, checkReserved bool) (bool, error) { if !validCollectibleUsernamePeer(peer) { return false, domain.ErrUsernameInvalid } @@ -121,7 +132,7 @@ func (s *CollectibleUsernameStore) SetEditableUsername(_ context.Context, peer d return false, domain.ErrUsernameInvalid } key := strings.ToLower(username) - if s.nameReserved(key) { + if checkReserved && s.nameReserved(key) { return false, domain.ErrUsernameOccupied } if existing, ok := s.registry[key]; ok { diff --git a/internal/store/memory/users.go b/internal/store/memory/users.go index 4ed34206..5f3c34e5 100644 --- a/internal/store/memory/users.go +++ b/internal/store/memory/users.go @@ -163,10 +163,25 @@ func (s *UserStore) CheckUsername(_ context.Context, userID int64, username stri if s.usernameRegistry != nil && s.usernameRegistry.nameReserved(username) { return false, nil } + return s.usernameAvailableIgnoringReserved(userID, username) +} + +// CheckUsernameAdmin is CheckUsername without the operator reserved-username +// blocklist check, for the admin console deliberately assigning a reserved +// word to a specific account. +func (s *UserStore) CheckUsernameAdmin(_ context.Context, userID int64, username string) (bool, error) { + username = strings.ToLower(strings.TrimSpace(strings.TrimPrefix(username, "@"))) + if username == "" { + return true, nil + } + return s.usernameAvailableIgnoringReserved(userID, username) +} + +func (s *UserStore) usernameAvailableIgnoringReserved(userID int64, usernameLower string) (bool, error) { s.mu.RLock() defer s.mu.RUnlock() for id, u := range s.byID { - if !u.Deleted && strings.ToLower(u.Username) == username && id != userID { + if !u.Deleted && strings.ToLower(u.Username) == usernameLower && id != userID { return false, nil } } @@ -211,6 +226,17 @@ func (s *UserStore) Search(_ context.Context, currentUserID int64, query, phoneQ } func (s *UserStore) UpdateUsername(ctx context.Context, userID int64, username string) (domain.User, error) { + return s.updateUsernameChecked(ctx, userID, username, true) +} + +// UpdateUsernameAdmin is UpdateUsername without the operator reserved-username +// blocklist check, for the admin console deliberately assigning a reserved +// word to a specific account. +func (s *UserStore) UpdateUsernameAdmin(ctx context.Context, userID int64, username string) (domain.User, error) { + return s.updateUsernameChecked(ctx, userID, username, false) +} + +func (s *UserStore) updateUsernameChecked(ctx context.Context, userID int64, username string, checkReserved bool) (domain.User, error) { username = strings.TrimSpace(strings.TrimPrefix(username, "@")) usernameLower := strings.ToLower(username) s.mu.Lock() @@ -227,7 +253,13 @@ func (s *UserStore) UpdateUsername(ctx context.Context, userID int64, username s } } if s.usernameRegistry != nil { - if _, err := s.usernameRegistry.SetEditableUsername(ctx, domain.Peer{Type: domain.PeerTypeUser, ID: userID}, username); err != nil { + var err error + if checkReserved { + _, err = s.usernameRegistry.SetEditableUsername(ctx, domain.Peer{Type: domain.PeerTypeUser, ID: userID}, username) + } else { + _, err = s.usernameRegistry.SetEditableUsernameAdmin(ctx, domain.Peer{Type: domain.PeerTypeUser, ID: userID}, username) + } + if err != nil { return domain.User{}, err } } diff --git a/internal/store/postgres/peer_username.go b/internal/store/postgres/peer_username.go index 7b099eee..77dc7dbe 100644 --- a/internal/store/postgres/peer_username.go +++ b/internal/store/postgres/peer_username.go @@ -81,10 +81,21 @@ func usernameReservedTx(ctx context.Context, db sqlcgen.DBTX, usernameLower stri } func peerUsernameAvailable(ctx context.Context, db sqlcgen.DBTX, usernameLower, peerType string, peerID int64) (bool, error) { - if reserved, err := usernameReservedTx(ctx, db, usernameLower); err != nil { - return false, err - } else if reserved { - return false, nil + return peerUsernameAvailableChecked(ctx, db, usernameLower, peerType, peerID, true) +} + +// peerUsernameAvailableChecked is peerUsernameAvailable with the operator +// blocklist check optional: an operator deliberately reserving a word still +// needs to be able to hand it to a specific account via the admin console, +// so the admin-initiated username-set path skips it (checkReserved=false) +// while self-service username changes always enforce it. +func peerUsernameAvailableChecked(ctx context.Context, db sqlcgen.DBTX, usernameLower, peerType string, peerID int64, checkReserved bool) (bool, error) { + if checkReserved { + if reserved, err := usernameReservedTx(ctx, db, usernameLower); err != nil { + return false, err + } else if reserved { + return false, nil + } } owner, found, err := getPeerUsernameOwner(ctx, db, usernameLower, false) if err != nil || !found { @@ -134,11 +145,23 @@ WHERE peer_type = $1 // collectible_usernames and must survive every client-driven username edit, // otherwise account.updateUsername would silently release a minted asset. func replacePeerUsernameTx(ctx context.Context, tx pgx.Tx, peerType string, peerID int64, username, usernameLower string) error { + return replacePeerUsernameTxChecked(ctx, tx, peerType, peerID, username, usernameLower, true) +} + +// replacePeerUsernameTxChecked is replacePeerUsernameTx with the operator +// blocklist check optional: the admin-initiated username-set path +// (UserStore.UpdateUsernameAdmin) skips it so an operator can deliberately +// hand a reserved word to a specific account, while every other caller +// (self-service, bots, channel settings, account deletion) always enforces +// it via replacePeerUsernameTx. +func replacePeerUsernameTxChecked(ctx context.Context, tx pgx.Tx, peerType string, peerID int64, username, usernameLower string, checkReserved bool) error { if usernameLower != "" { - if reserved, err := usernameReservedTx(ctx, tx, usernameLower); err != nil { - return err - } else if reserved { - return domain.ErrUsernameOccupied + if checkReserved { + if reserved, err := usernameReservedTx(ctx, tx, usernameLower); err != nil { + return err + } else if reserved { + return domain.ErrUsernameOccupied + } } owner, found, err := getPeerUsernameOwner(ctx, tx, usernameLower, true) if err != nil { diff --git a/internal/store/postgres/user.go b/internal/store/postgres/user.go index 8a7e97bb..534bc96c 100644 --- a/internal/store/postgres/user.go +++ b/internal/store/postgres/user.go @@ -163,6 +163,17 @@ func (s *UserStore) CheckUsername(ctx context.Context, userID int64, username st return peerUsernameAvailable(ctx, s.db, usernameLower, peerUsernameTypeUser, userID) } +// CheckUsernameAdmin is CheckUsername without the operator reserved-username +// blocklist check, for the admin console deliberately assigning a reserved +// word to a specific account. +func (s *UserStore) CheckUsernameAdmin(ctx context.Context, userID int64, username string) (bool, error) { + usernameLower := strings.ToLower(strings.TrimSpace(strings.TrimPrefix(username, "@"))) + if usernameLower == "" { + return true, nil + } + return peerUsernameAvailableChecked(ctx, s.db, usernameLower, peerUsernameTypeUser, userID, false) +} + func (s *UserStore) Search(ctx context.Context, currentUserID int64, query, phoneQuery string, limit int) (domain.UserSearchResult, error) { query = strings.ToLower(strings.TrimSpace(query)) if currentUserID == 0 || query == "" { @@ -257,6 +268,17 @@ func (s *UserStore) UpdatePhone(ctx context.Context, userID int64, phone string) } func (s *UserStore) UpdateUsername(ctx context.Context, userID int64, username string) (domain.User, error) { + return s.updateUsernameChecked(ctx, userID, username, true) +} + +// UpdateUsernameAdmin is UpdateUsername without the operator reserved-username +// blocklist check, for the admin console deliberately assigning a reserved +// word to a specific account. +func (s *UserStore) UpdateUsernameAdmin(ctx context.Context, userID int64, username string) (domain.User, error) { + return s.updateUsernameChecked(ctx, userID, username, false) +} + +func (s *UserStore) updateUsernameChecked(ctx context.Context, userID int64, username string, checkReserved bool) (domain.User, error) { username = strings.TrimSpace(strings.TrimPrefix(username, "@")) usernameLower := strings.ToLower(username) beginner, ok := s.db.(txBeginner) @@ -281,7 +303,7 @@ func (s *UserStore) UpdateUsername(ctx context.Context, userID int64, username s } return domain.User{}, fmt.Errorf("lock user for username update: %w", err) } - if err := replacePeerUsernameTx(ctx, tx, peerUsernameTypeUser, userID, username, usernameLower); err != nil { + if err := replacePeerUsernameTxChecked(ctx, tx, peerUsernameTypeUser, userID, username, usernameLower, checkReserved); err != nil { return domain.User{}, err } row, err := qtx.UpdateUserUsername(ctx, sqlcgen.UpdateUserUsernameParams{