Commit Graph
2 Commits
Author SHA1 Message Date
be5b278069 test(login): avoid assert.Never race in forged registration check (#12548)
# 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]>
2026-08-06 07:50:21 +02:00
a17af0fe93 Merge commit from fork
* 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]>
2026-07-29 05:50:40 +02:00