Compare commits

..

No commits in common. "d1af032fbf39dfa334e804bb7822384fbe881197" and "53872f8fc911d78b52260f6935a057b1efa9f60d" have entirely different histories.

6 changed files with 15 additions and 180 deletions

View file

@ -48,24 +48,6 @@ type usernameAvailabilityStore interface {
CheckUsername(ctx context.Context, userID int64, username string) (bool, error) 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 { type moderationFlagAudienceStore interface {
ModerationFlagAudience(ctx context.Context, userID int64, limit int) ([]int64, error) ModerationFlagAudience(ctx context.Context, userID int64, limit int) ([]int64, error)
} }
@ -336,17 +318,6 @@ func (s *Service) checkUsernameAvailable(ctx context.Context, selfID int64, user
return !found || u.ID == selfID, nil 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 修改当前用户的主 usernameself-service。空字符串表示删除 username。 // UpdateUsername 修改当前用户的主 usernameself-service。空字符串表示删除 username。
func (s *Service) UpdateUsername(ctx context.Context, userID int64, username string) (domain.User, error) { func (s *Service) UpdateUsername(ctx context.Context, userID int64, username string) (domain.User, error) {
return s.updateUsername(ctx, userID, username, true) return s.updateUsername(ctx, userID, username, true)
@ -379,15 +350,7 @@ func (s *Service) updateUsername(ctx context.Context, userID int64, username str
if !validUsername(username) || (enforceReserved && s.reserved.Contains(username)) { if !validUsername(username) || (enforceReserved && s.reserved.Contains(username)) {
return domain.User{}, domain.ErrUsernameInvalid return domain.User{}, domain.ErrUsernameInvalid
} }
var ( ok, err := s.checkUsernameAvailable(ctx, self.ID, username)
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 { if err != nil {
return domain.User{}, err return domain.User{}, err
} }
@ -395,14 +358,7 @@ func (s *Service) updateUsername(ctx context.Context, userID int64, username str
return domain.User{}, domain.ErrUsernameOccupied return domain.User{}, domain.ErrUsernameOccupied
} }
} }
var u domain.User u, err := s.users.UpdateUsername(ctx, self.ID, username)
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 { if err != nil {
return domain.User{}, err return domain.User{}, err
} }

View file

@ -151,39 +151,6 @@ 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() // marksbotOverrideStore wraps memory.UserStore to serve domain.VerifierBotUser()
// for a fixed username lookup, since memory.UserStore.Create always assigns an // 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 // id from its own auto-increment sequence and can never produce the fixed

View file

@ -105,18 +105,7 @@ func NewCollectibleUsernameStore() *CollectibleUsernameStore {
// usernames on the user and channel rows, so tests need this hook to give a peer // 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 // the editable registry row the projection expects. An empty username clears the
// slot. // slot.
func (s *CollectibleUsernameStore) SetEditableUsername(ctx context.Context, peer domain.Peer, username string) (bool, error) { func (s *CollectibleUsernameStore) SetEditableUsername(_ 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) { if !validCollectibleUsernamePeer(peer) {
return false, domain.ErrUsernameInvalid return false, domain.ErrUsernameInvalid
} }
@ -132,7 +121,7 @@ func (s *CollectibleUsernameStore) setEditableUsernameChecked(_ context.Context,
return false, domain.ErrUsernameInvalid return false, domain.ErrUsernameInvalid
} }
key := strings.ToLower(username) key := strings.ToLower(username)
if checkReserved && s.nameReserved(key) { if s.nameReserved(key) {
return false, domain.ErrUsernameOccupied return false, domain.ErrUsernameOccupied
} }
if existing, ok := s.registry[key]; ok { if existing, ok := s.registry[key]; ok {

View file

@ -163,25 +163,10 @@ func (s *UserStore) CheckUsername(_ context.Context, userID int64, username stri
if s.usernameRegistry != nil && s.usernameRegistry.nameReserved(username) { if s.usernameRegistry != nil && s.usernameRegistry.nameReserved(username) {
return false, nil 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() s.mu.RLock()
defer s.mu.RUnlock() defer s.mu.RUnlock()
for id, u := range s.byID { for id, u := range s.byID {
if !u.Deleted && strings.ToLower(u.Username) == usernameLower && id != userID { if !u.Deleted && strings.ToLower(u.Username) == username && id != userID {
return false, nil return false, nil
} }
} }
@ -226,17 +211,6 @@ func (s *UserStore) Search(_ context.Context, currentUserID int64, query, phoneQ
} }
func (s *UserStore) UpdateUsername(ctx context.Context, userID int64, username string) (domain.User, error) { 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, "@")) username = strings.TrimSpace(strings.TrimPrefix(username, "@"))
usernameLower := strings.ToLower(username) usernameLower := strings.ToLower(username)
s.mu.Lock() s.mu.Lock()
@ -253,13 +227,7 @@ func (s *UserStore) updateUsernameChecked(ctx context.Context, userID int64, use
} }
} }
if s.usernameRegistry != nil { if s.usernameRegistry != nil {
var err error if _, err := s.usernameRegistry.SetEditableUsername(ctx, domain.Peer{Type: domain.PeerTypeUser, ID: userID}, username); err != nil {
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 return domain.User{}, err
} }
} }

View file

@ -81,22 +81,11 @@ 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) { func peerUsernameAvailable(ctx context.Context, db sqlcgen.DBTX, usernameLower, peerType string, peerID int64) (bool, error) {
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 { if reserved, err := usernameReservedTx(ctx, db, usernameLower); err != nil {
return false, err return false, err
} else if reserved { } else if reserved {
return false, nil return false, nil
} }
}
owner, found, err := getPeerUsernameOwner(ctx, db, usernameLower, false) owner, found, err := getPeerUsernameOwner(ctx, db, usernameLower, false)
if err != nil || !found { if err != nil || !found {
return !found, err return !found, err
@ -145,24 +134,12 @@ WHERE peer_type = $1
// collectible_usernames and must survive every client-driven username edit, // collectible_usernames and must survive every client-driven username edit,
// otherwise account.updateUsername would silently release a minted asset. // 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 { 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 usernameLower != "" {
if checkReserved {
if reserved, err := usernameReservedTx(ctx, tx, usernameLower); err != nil { if reserved, err := usernameReservedTx(ctx, tx, usernameLower); err != nil {
return err return err
} else if reserved { } else if reserved {
return domain.ErrUsernameOccupied return domain.ErrUsernameOccupied
} }
}
owner, found, err := getPeerUsernameOwner(ctx, tx, usernameLower, true) owner, found, err := getPeerUsernameOwner(ctx, tx, usernameLower, true)
if err != nil { if err != nil {
return err return err

View file

@ -163,17 +163,6 @@ func (s *UserStore) CheckUsername(ctx context.Context, userID int64, username st
return peerUsernameAvailable(ctx, s.db, usernameLower, peerUsernameTypeUser, userID) 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) { func (s *UserStore) Search(ctx context.Context, currentUserID int64, query, phoneQuery string, limit int) (domain.UserSearchResult, error) {
query = strings.ToLower(strings.TrimSpace(query)) query = strings.ToLower(strings.TrimSpace(query))
if currentUserID == 0 || query == "" { if currentUserID == 0 || query == "" {
@ -268,17 +257,6 @@ 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) { 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, "@")) username = strings.TrimSpace(strings.TrimPrefix(username, "@"))
usernameLower := strings.ToLower(username) usernameLower := strings.ToLower(username)
beginner, ok := s.db.(txBeginner) beginner, ok := s.db.(txBeginner)
@ -303,7 +281,7 @@ func (s *UserStore) updateUsernameChecked(ctx context.Context, userID int64, use
} }
return domain.User{}, fmt.Errorf("lock user for username update: %w", err) return domain.User{}, fmt.Errorf("lock user for username update: %w", err)
} }
if err := replacePeerUsernameTxChecked(ctx, tx, peerUsernameTypeUser, userID, username, usernameLower, checkReserved); err != nil { if err := replacePeerUsernameTx(ctx, tx, peerUsernameTypeUser, userID, username, usernameLower); err != nil {
return domain.User{}, err return domain.User{}, err
} }
row, err := qtx.UpdateUserUsername(ctx, sqlcgen.UpdateUserUsernameParams{ row, err := qtx.UpdateUserUsername(ctx, sqlcgen.UpdateUserUsernameParams{