feat: Support multi-user and role management#12588
Conversation
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
![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: 707fc24fb5
ℹ️ 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".
| return BeginMFALogin(c, nameSetting.Value, entrance, mfaSetting.Value), "", nil | ||
| } | ||
|
|
||
| sessionUser := psession.SessionUser{ID: psession.SuperAdminSessionUserID, Name: info.Name, Role: "ADMIN"} |
There was a problem hiding this comment.
Login validates only the password and never verifies that info.Name matches the configured account, then writes info.Name into the session. With MFA disabled, anyone with the correct password can authenticate as an arbitrary displayed username, which breaks identity/audit semantics and enables user impersonation in logs/UI. Either reject mismatched usernames (as before) or always bind the session name to the stored user record.
Useful? React with 👍 / 👎.
| request.oldPassword = Base64.encode(request.oldPassword); | ||
| } | ||
| if (request.password) { | ||
| request.password = Base64.encode(request.password); |
There was a problem hiding this comment.
This client now Base64-encodes oldPassword/password before calling /core/auth/current/update, but the server path (auth.UpdateCurrentUserInfo -> auth.HandlePasswordExpired) compares the old password as plaintext against the decrypted stored password and does not decode Base64 first. As a result, changing password from the user profile will consistently fail with an initial-password mismatch. Keep the payload plaintext (or add server-side decoding) so password updates work.
Useful? React with 👍 / 👎.
| router.POST("/search", baseApi.GetSettingInfo) | ||
| router.POST("/expired/handle", baseApi.HandlePasswordExpired) | ||
| router.POST("/search/base", baseApi.GetSettingBaseInfo) | ||
|
|
There was a problem hiding this comment.
The settings router no longer registers /core/settings/expired/handle, but the expired-password flow still posts to that endpoint from the frontend (setting/expired.vue via handleExpired), and middleware still treats that path as a special case. When users are redirected after HTTP 313, submitting a new password now hits a missing route instead of resetting credentials. Reintroduce the route or migrate the frontend flow to the new auth endpoint.
Useful? React with 👍 / 👎.
2 participants
No description provided.