Conversation
Signed-off-by: Zzde <zhangxh1997@gmail.com>
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9228951afc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| errMsg := fmt.Sprintf("%s login failed for %s: %v", strings.ToUpper(provider), username, err) | ||
| klog.Warning(errMsg) | ||
| if isCredentialFailure(err) { | ||
| c.JSON(http.StatusUnauthorized, gin.H{"error": errMsg}) |
There was a problem hiding this comment.
The response now echoes provider, username, and the raw backend error string to the client on both auth failures and server failures. This leaks internal details (for example DB/LDAP error text) and exposes attempted usernames, and it also breaks the frontend's login.errors.invalidCredentials mapping because the error key is no longer stable. Keep the detailed string in logs, but return a fixed client-safe message.
Useful? React with 👍 / 👎.
| if err := DB.Create(&setting).Error; err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
When the LDAP settings row does not exist, concurrent first-time requests can both pass First(&setting, 1) and race on Create(&setting). The loser gets a duplicate-key error and this path returns it directly, causing transient failures (e.g., /admin/ldap-setting returning 500). After a duplicate-key on create, this should re-read the row instead of failing.
Useful? React with 👍 / 👎.
1 participant
fix: #431, #417