mirror of
https://github.com/andwati/zitadel.git
synced 2026-10-18 05:52:51 +00:00
18a4f883f0
* fix(login): gate username change on the change-username step handleChangeUsername renamed the account identified by authReq.UserID without checking that the login flow had reached the change-username step or that any factor had succeeded. Because authReq.UserID is bound as soon as a login name is submitted (checkLoginName -> SetUserInfo), an unauthenticated caller could start a flow, type a victim's login name and then POST a new username to rename that account, including instance admins. Gate the handler on the current step being *domain.ChangeUsernameStep, mirroring the other login handlers. That step is only produced in nextSteps after the first factor and any required MFA have succeeded, so the gate also proves the caller owns the account. The legitimate forced-username-change flow is unaffected: the UI only ever renders that form when the step is at index 0, and PossibleSteps is recomputed on every request. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01Aup17GXd4tQnXG36xr5UiZ * test(login): add integration reproduction for GHSA-4hgj-wm6c-q7p2 Drive the unauthenticated-rename attack over raw HTTP against a running instance, mirroring the sibling GHSA-738m external-login integration test in the same package: authorize (V1) -> load login page (scrape CSRF) -> POST the victim's login name (binds authReq.UserID, no factor) -> POST a new username. The test first proves the exploit precondition held - step 3 advanced the flow to the victim's password step, so authReq.UserID was bound - then asserts over gRPC that the victim's username is unchanged. The precondition check guards against a false green: without it, a broken upstream hop would make the unchanged-username assertion pass vacuously even against unpatched code. The unchanged-username poll and the sibling test's requireNeverUserByEmail now share a small pollFor helper in the package. Red -> green: fails against the unpatched handler (the rename lands), passes against the fixed handler (the ChangeUsernameStep gate rejects the POST before the command runs). Runs in the existing nx test-integration job via the integration build tag. Co-Authored-By: Claude Opus 4.8 <[email protected]> Claude-Session: https://claude.ai/code/session_01Aup17GXd4tQnXG36xr5UiZ --------- Co-authored-by: Claude Opus 4.8 <[email protected]>
63 lines
2.4 KiB
Go
63 lines
2.4 KiB
Go
package login
|
|
|
|
import (
|
|
"net/http"
|
|
|
|
"github.com/zitadel/zitadel/internal/domain"
|
|
)
|
|
|
|
const (
|
|
tmplChangeUsername = "changeusername"
|
|
tmplChangeUsernameDone = "changeusernamedone"
|
|
)
|
|
|
|
type changeUsernameData struct {
|
|
Username string `schema:"username"`
|
|
}
|
|
|
|
func (l *Login) renderChangeUsername(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest, err error) {
|
|
translator := l.getTranslator(r.Context(), authReq)
|
|
data := l.getUserData(r, authReq, translator, "UsernameChange.Title", "UsernameChange.Description", err)
|
|
l.renderer.RenderTemplate(w, r, translator, l.renderer.Templates[tmplChangeUsername], data, nil)
|
|
}
|
|
|
|
func (l *Login) handleChangeUsername(w http.ResponseWriter, r *http.Request) {
|
|
data := new(changeUsernameData)
|
|
authReq, err := l.ensureAuthRequestAndParseData(r, data)
|
|
if err != nil {
|
|
l.renderError(w, r, authReq, err)
|
|
return
|
|
}
|
|
// The rename may only proceed once the flow has reached the change-username step;
|
|
// isChangeUsernameStep documents why that also proves the caller is authenticated.
|
|
if !isChangeUsernameStep(authReq) {
|
|
l.renderError(w, r, authReq, nil)
|
|
return
|
|
}
|
|
_, err = l.command.ChangeUsername(setContext(r.Context(), authReq.UserOrgID), authReq.UserOrgID, authReq.UserID, data.Username)
|
|
if err != nil {
|
|
l.renderChangeUsername(w, r, authReq, err)
|
|
return
|
|
}
|
|
l.renderChangeUsernameDone(w, r, authReq)
|
|
}
|
|
|
|
func (l *Login) renderChangeUsernameDone(w http.ResponseWriter, r *http.Request, authReq *domain.AuthRequest) {
|
|
translator := l.getTranslator(r.Context(), authReq)
|
|
data := l.getUserData(r, authReq, translator, "UsernameChangeDone.Title", "UsernameChangeDone.Description", nil)
|
|
l.renderer.RenderTemplate(w, r, translator, l.renderer.Templates[tmplChangeUsernameDone], data, nil)
|
|
}
|
|
|
|
// isChangeUsernameStep reports whether the login flow's current step is the change-username
|
|
// step. ChangeUsernameStep is appended in nextSteps only after the first factor and any
|
|
// required MFA have succeeded, so gating on it guarantees the caller is the authenticated
|
|
// owner of authReq.UserID. Without this gate an unauthenticated caller could bind
|
|
// authReq.UserID to any account by submitting its login name and then rename it.
|
|
func isChangeUsernameStep(authReq *domain.AuthRequest) bool {
|
|
if authReq == nil || len(authReq.PossibleSteps) == 0 {
|
|
return false
|
|
}
|
|
_, ok := authReq.PossibleSteps[0].(*domain.ChangeUsernameStep)
|
|
return ok
|
|
}
|