Skip to content

feat: 支持优先使用第三方用户名 #2900

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
feitianbubu wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat: 支持优先使用第三方用户名 #2900

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions controller/oauth.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,13 @@ func findOrCreateOAuthUser(c *gin.Context, provider oauth.Provider, oauthUser *o

// Set up new user
user.Username = provider.GetProviderPrefix() + strconv.Itoa(model.GetMaxUserId()+1)

if oauthUser.Username != "" {
if exists, err := model.CheckUserExistOrDeleted(oauthUser.Username, ""); err == nil && !exists {
user.Username = oauthUser.Username
}
}
Comment on lines 239 to +245
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, find the controller/oauth.go file and verify it exists
find . -name "oauth.go" -path "*/controller/*" | head -5

Repository: QuantumNous/new-api

Length of output: 84

🏁 Script executed:

#!/bin/bash
# Check the code at lines 239-245 in controller/oauth.go
cat -n controller/oauth.go | sed -n '235,250p'

Repository: QuantumNous/new-api

Length of output: 663

🏁 Script executed:

#!/bin/bash
# Search for unique constraints on username column in Go code
rg -n "username.*unique|UniqueIndex.*username|unique.*username" --type=go -i

Repository: QuantumNous/new-api

Length of output: 168

🏁 Script executed:

#!/bin/bash
# Search for gorm tags with unique on username field
ast-grep --pattern 'Username $_ `gorm:"$$$unique$$$"`'

Repository: QuantumNous/new-api

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Look for database migrations that might set up username uniqueness
find . -type f -name "*.go" -path "*migrat*" | xargs rg -l "username" | head -5

Repository: QuantumNous/new-api

Length of output: 45

🏁 Script executed:

#!/bin/bash
# Search for CheckUserExistOrDeleted and InsertWithTx implementations
rg -n "func.*CheckUserExistOrDeleted|func.*InsertWithTx" --type=go -A 5

Repository: QuantumNous/new-api

Length of output: 702

🏁 Script executed:

#!/bin/bash
# Get full InsertWithTx implementation
cat -n model/user.go | sed -n '435,470p'

Repository: QuantumNous/new-api

Length of output: 1406

🏁 Script executed:

#!/bin/bash
# Get full CheckUserExistOrDeleted implementation
cat -n model/user.go | sed -n '159,180p'

Repository: QuantumNous/new-api

Length of output: 853

🏁 Script executed:

#!/bin/bash
# Search for where InsertWithTx is called in oauth.go and how errors are handled
rg -n "InsertWithTx" controller/oauth.go -B 2 -A 5

Repository: QuantumNous/new-api

Length of output: 627

🏁 Script executed:

#!/bin/bash
# Check if there's any retry or special handling for unique constraint errors anywhere in oauth.go
rg -n "unique|duplicate|constraint|Conflict|retry" controller/oauth.go -i

Repository: QuantumNous/new-api

Length of output: 145

Add graceful handling for unique constraint violation on username.

The TOCTOU race condition exists between CheckUserExistOrDeleted (line 242) and InsertWithTx: a concurrent request could claim the same username before the insert executes. The same race applies to the generated fallback username on line 239 (GetMaxUserId()+1).

The username column has a unique constraint (gorm:"unique;index" in model/user.go), so the database will reject duplicate insertions. However, there's currently no graceful error handling—the raw constraint violation error bubbles up to the user. Consider catching the unique constraint violation from InsertWithTx and retrying with an alternative (e.g., the generated fallback username) to improve the user experience.

🤖 Prompt for AI Agents 
In `@controller/oauth.go` around lines 239 - 245, There is a TOCTOU race between
CheckUserExistOrDeleted / GetMaxUserId and InsertWithTx that can cause
unique-constraint DB errors to bubble up; update the oauth user creation flow
(the block that sets user.Username using
provider.GetProviderPrefix()+strconv.Itoa(model.GetMaxUserId()+1) and the branch
that uses oauthUser.Username) to catch unique constraint violations returned by
model.InsertWithTx, and implement a small retry strategy: on unique-constraint
error detect it (by SQL state / error message or DB-specific error type),
regenerate a new username (e.g., append/increment a numeric suffix or call
GetMaxUserId()+n again or use a random disambiguator) and retry InsertWithTx a
few times before failing; ensure retries cover both the oauthUser.Username
branch and the generated fallback so concurrent inserts won’t surface raw DB
constraint errors to users.

Comment on lines +241 to +245
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

rg -n -C3 'func.*Insert' --type=go -g '*user*' | head -50

Repository: QuantumNous/new-api

Length of output: 924

🏁 Script executed:

rg -nP 'Username.*valid|validateUsername|isValidUsername|usernameRegex|username.*regexp' --type=go | head -50

Repository: QuantumNous/new-api

Length of output: 168

🏁 Script executed:

rg -nP 'func.*Register' --type=go -g '*/controller/*' | head -30

Repository: QuantumNous/new-api

Length of output: 211

🏁 Script executed:

cat controller/oauth.go | head -250 | tail -30

Repository: QuantumNous/new-api

Length of output: 1104

🏁 Script executed:

rg -n 'CheckUserExistOrDeleted' --type=go -B2 -A2 | head -40

Repository: QuantumNous/new-api

Length of output: 1105

🏁 Script executed:

cat -n model/user.go | sed -n '435,480p'

Repository: QuantumNous/new-api

Length of output: 1806

🏁 Script executed:

rg -n 'Validate|validator' --type=go -B2 -A2 | grep -A5 -B5 'InsertWithTx\|oauth'

Repository: QuantumNous/new-api

Length of output: 876

🏁 Script executed:

rg -n 'oauthUser\.' --type=go controller/oauth.go | head -20

Repository: QuantumNous/new-api

Length of output: 1269

🏁 Script executed:

rg -n 'struct.*OAuthUser|type.*OAuthUser' --type=go -A10 | head -30

Repository: QuantumNous/new-api

Length of output: 1215

🏁 Script executed:

rg -n 'Username.*sanitize|sanitize.*Username|clean.*Username' --type=go

Repository: QuantumNous/new-api

Length of output: 264

🏁 Script executed:

cat -n controller/user.go | sed -n '140,200p'

Repository: QuantumNous/new-api

Length of output: 2384

🏁 Script executed:

rg -n 'cleanUser.*:=' --type=go -B3 -A1 controller/user.go

Repository: QuantumNous/new-api

Length of output: 515

🏁 Script executed:

rg -n 'func.*Clean|func.*Sanitize' --type=go -A5 | head -40

Repository: QuantumNous/new-api

Length of output: 2118

🏁 Script executed:

rg -n 'gorm.*validate|Create.*validate' --type=go | head -20

Repository: QuantumNous/new-api

Length of output: 703

Add validation for OAuth-provided username to match regular registration constraints.

oauthUser.Username is assigned directly without validation, unlike the regular registration flow (controller/user.go:146) which explicitly calls common.Validate.Struct(&user) to enforce the validate:"max=20" constraint and other rules. This inconsistency allows OAuth users to be created with usernames exceeding 20 characters or containing invalid character sets, violating application-level constraints and creating potential data integrity issues.

Apply the same validation to the User object before calling InsertWithTx:

if err := common.Validate.Struct(&user); err != nil {
    return nil, fmt.Errorf("invalid user data: %w", err)
}
🤖 Prompt for AI Agents 
In `@controller/oauth.go` around lines 241 - 245, The oauth flow assigns
oauthUser.Username directly to user if
model.CheckUserExistOrDeleted(oauthUser.Username, "") passes; add the same
validation used in regular registration by running common.Validate.Struct(&user)
(and handling the error) after setting user.Username and before calling
InsertWithTx so usernames respect constraints like validate:"max=20" and
character rules; ensure you return an error (e.g., fmt.Errorf("invalid user
data: %w", err)) when validation fails.


if oauthUser.DisplayName != "" {
user.DisplayName = oauthUser.DisplayName
} else if oauthUser.Username != "" {
Expand Down