* 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]>
# Which Problems Are Solved
- `TestExternalNotFoundOption_ForgedRegistration_IsRejected` flakes
across unrelated PRs: the test reaches PASS, then panics with `Log in
goroutine after Test... has completed` and `rpc error: code = Canceled
desc = context canceled`.
# How the Problems Are Solved
- Replaces `assert.Never` with a synchronous poll helper
(`requireNeverUserByEmail`) so `ListUsers` and `require` run on the test
goroutine and finish before `TestMain` cancels `CTX`.
- Keeps the same “must stay absent for a window” polarity (not
`Eventually`), so a late-projected forged user still fails the test.
# Additional Changes
- None.
# Additional Context
- CI examples: [run
30980062306](https://github.com/zitadel/zitadel/actions/runs/30980062306),
[run
30837302974](https://github.com/zitadel/zitadel/actions/runs/30837302974)
Co-authored-by: Cursor <[email protected]>
* fix(login): prevent external-IDP account pre-hijack in Login V1 (GHSA-738m-7888-jfv8)
The Login V1 endpoint POST /ui/login/externaluser/option
(handleExternalNotFoundOptionCheck) built the new external user's identity and
"verified" flags directly from raw POST fields, without checking the server-side
authReq.LinkingUsers state that a genuine, protocol-verified IDP callback
populates. An unauthenticated attacker could forge that POST to create a real
account bound to an arbitrary (IDPConfigID, ExternalUserID) - enabling account
pre-hijack of a third party whose public external identifier (e.g. a GitHub
numeric user id) was used, so the victim's own future "Sign in with X" logs into
the attacker's pre-created account.
Primary fix:
- Reject the request when authReq.LinkingUsers is empty, mirroring the guarded
sibling autoCreateExternalUser.
- Source IDPConfigID/ExternalUserID and the email/phone "verified" flags from
the trusted linkingUser (the real callback data) instead of the form; the
verified flag only survives when the editable value still matches the IDP's.
Profile fields (name, nickname, language, edited email/phone) still come from
the form, which is the page's legitimate purpose.
Secondary hardening:
- Treat the OAuth error/error_description callback params as untrusted so a
forged GET .../externalidp/callback?error=... can no longer trigger the
local-auth fallback (RequestLocalAuth) and undermine the IDP-first login
priority. Genuine code-exchange failures keep their existing handling.
Tests:
- Unit test for mapExternalNotFoundOptionFormDataToLoginUser proving forged
identity/verified fields are ignored in favor of the trusted callback data.
- A //go:build integration end-to-end reproduction driving the forged
registration over raw HTTP and asserting the account is not created.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
* fix(login): handle IDP error callback for logging, keep local-auth fallback
Addresses review feedback (livio-a) on GHSA-738m-7888-jfv8: the OAuth error
callback is not a security boundary. Blocking only the error= path added no
protection (a garbage code triggers the same fallback), the downgrade only
affects the attacker's own auth request, and a genuine IDP error on the callback
should still fall back to local auth.
- handleExternalLoginCallback: on an IDP error response, log it with context and
route through externalAuthCallbackFailed so local-auth fallback still happens
when the login policy allows it (previous behavior), instead of rendering a
plain login error. The displayed message reuses the existing
Errors.User.ExternalIDP.LoginFailedSwitchLocal via WrapIdPError.
- Drop the now-unused Errors.ExternalIDP.AuthenticationFailed translation (would
otherwise need all-language entries) and regenerate the login statik blob.
- Reword the externalIDPCallbackData Error/ErrorDescription comment: logging-only,
not a security decision.
Co-Authored-By: Claude Opus 4.8 <[email protected]>
---------
Co-authored-by: Claude Opus 4.8 <[email protected]>
Co-authored-by: Livio Spring <[email protected]>
