Skip to content

Fix GitHub IdP mapping for names and private email#8

Merged
JoseSzycho merged 1 commit into
mainfrom
fix/github-idp-registration
Sep 12, 2025
Merged

Fix GitHub IdP mapping for names and private email#8
JoseSzycho merged 1 commit into
mainfrom
fix/github-idp-registration

Conversation

@JoseSzycho

@JoseSzycho JoseSzycho commented Sep 12, 2025

Copy link
Copy Markdown
Collaborator

When users try to log in or register using the GitHub IdP, the following error appears on the front page:

[invalid_argument] invalid AddHumanUserRequest.Profile: embedded message failed validation | caused by: invalid SetHumanProfile.GivenName: value length must be between 1 and 200 runes, inclusive

The root of the problem is related to the auth-ui not being able to map the givenName, familyName, and email address provided by GitHub.

This PR introduces a new way of populating the givenName and familyName fields from GitHub and also adds support for fetching the user’s email address when it is private on GitHub.

Fetching the primary email address is important, as it is used by the notification system.

@JoseSzycho JoseSzycho marked this pull request as draft September 12, 2025 14:26
@JoseSzycho JoseSzycho force-pushed the fix/github-idp-registration branch from 98bf7e2 to aa296a7 Compare September 12, 2025 20:02
@JoseSzycho JoseSzycho changed the title fix: ensure mandatory profile fields are populated for ZITADEL validation Fix GitHub IdP mapping for names and private email Sep 12, 2025
@JoseSzycho JoseSzycho marked this pull request as ready for review September 12, 2025 20:12
@JoseSzycho JoseSzycho merged commit 359f407 into main Sep 12, 2025
2 of 3 checks passed
@ecv ecv deleted the fix/github-idp-registration branch May 13, 2026 02:41
yahyafakhroji added a commit that referenced this pull request Jul 5, 2026
Fixes 31 of 32 confirmed findings from the adversarially-verified ultracode audit
of the auth-ui rebuild; #16 is partially fixed with a documented residual. All
changes typecheck clean and pass the mirrored cypress component specs. See
docs/AUDIT-FINDINGS.md for the full per-finding detail and status.

Highlights by subsystem:
- session/device: device-authorization grants no longer auto-complete from the
  forgeable GET /signed-in; they route to the CSRF-protected /device/authorize
  consent screen (explicit Approve showing app + scope). (#10)
- rate-limit: limiters mount on '*' and self-guard on a normalized (lowercased,
  .data-stripped) path, closing the case-variant and single-fetch bypasses. (#1, #11)
- verify: email-code resend is gated on server-verified session ownership and never
  500s / enumerates; requestId validated + encoded into the /authorize hand-back.
  (#9, #25, #30)
- signup: registration policy (allowRegister/allowPassword/passkeysType) enforced in
  the actions; duplicate-email responses emit a Set-Cookie (presence oracle closed,
  content/size residual documented — #16); fingerprintId used instead of the MaxMind
  token; requestId resumed on email-link complete. (#3, #4, #14, #16, #17)
- sso: guarded getSession/startIdpIntent (graceful branded errors, not 500s); LDAP
  reauth cookie cleared; ProviderError on sign-in mapped to the branded page;
  deviceTrackingToken threaded on auto-link. (#2, #5, #6, #13, #19, #20, #21)
- providers/mappers: U2F factor derived from the webAuthN proto factor (fixes the
  U2F login loop); failedAttempts extracted with the proper schema; forceMfaLocalOnly
  kept distinct from forceMfa. (#0, #7, #22)
- session cookie: dual-format (ISO/epoch) timestamp parsing in the eviction pre-pass
  so the 2KB budget is respected. (#8)
- webauthn: real browsers without WebAuthn see the unsupported message instead of
  submitting the Cypress fake credential. (#27)
- fraud: MaxMind token falls back to the cookie when it lands after the poll budget;
  default-org cache keyed per service URL. (#28, #29)
- audit logging: raw loginName replaced with hashActor across OTP/webauthn/password
  reset events. (#23, #24, #26)
- setup: safeParse on tampered skip params; TOTP register guarded. (#12, #31)

Verification (2026-07-04): the 12 findings applied in the prior batch were
independently re-verified (verifier + adversary per finding). 9 closed cleanly;
#10 (device-auth) was incomplete and #30 (resend test) was a regression — both
re-fixed here; #16 (signup enumeration) is partial, with the residual documented
and full closure (sessions-cookie encryption) deferred by owner decision.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017UhyUkaSmoNFax5wu2aHfi
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants