Skip to content

docs(auth): define unified identity federation architecture - #630

Open
XiaoSeS wants to merge 3 commits into
mainfrom
docs/unified-identity-federation
Open

docs(auth): define unified identity federation architecture#630
XiaoSeS wants to merge 3 commits into
mainfrom
docs/unified-identity-federation

Conversation

@XiaoSeS

@XiaoSeS XiaoSeS commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

What

  • add the formal unified identity federation architecture
  • define trusted provider resolution, authority locking, typed subjects and aliases, policy boundaries, identity linking, and safe account merging
  • define native-versus-broker protocol support for OAuth/OIDC, LDAP/AD, DingTalk, CAS, SAML, trusted gateways, Kerberos, and SCIM
  • mark the current Direct/Passive principal-returning extension points as compatibility-only
  • define phased PR, migration, rollback, test, big-main, and test-environment gates

Why

SkillHub has multiple protocol requests (#260, #456, #583) and community implementations (#437, #467), but the current OAuth-centered claims and Direct/Passive SPIs do not provide one enforceable identity safety boundary.

This PR provides the architecture and delivery contract for #628 before protocol-specific implementations define incompatible identity models.

How

The design is based on established patterns from Keycloak, authentik, Dex, Authelia, Backstage, Grafana, Kubernetes, Spring Security, and the relevant OIDC, OAuth, LDAP, CAS, SAML, SCIM, and WebAuthn standards. It intentionally uses a small core with capability-specific adapters instead of a universal provider interface.

Testing

  • git diff origin/main...HEAD --check
  • verified all four documentation files are isolated on a clean branch
  • documentation-only change; no runtime behavior changed

Impact

No API, database, deployment, or runtime behavior changes. Implementation remains split into independently reviewable follow-up issues and PRs.

Signed-off-by: XiaoSeS <87064762+XiaoSeS@users.noreply.github.com>
@CLAassistant

CLAassistant commented Jul 30, 2026

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ XiaoSeS
❌ ylhu16


ylhu16 seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@FenjuFu

FenjuFu commented Jul 31, 2026

Copy link
Copy Markdown
Member

Read through the design doc and skimmed the four implementation PRs it anchors (#631, #633, #639, plus #644 which is separate). First: this is an unusually thorough piece of work — surveying Keycloak / authentik / Dex / Authelia / Spring Security, laying out the A/B/C options and landing on the phased hybrid, and naming the concrete gaps (typed subjects, authority pinning to issuer/directory/entityID, cross-issuer impersonation, safe merge) is exactly the right level of rigor for something that touches the auth boundary.

I want to be straight about my role here, though: I'm not going to single-approve this group. #631 (trusted external identity attributes), #633 (global membership on approval), and #639 (isolating the unsafe merge flow) are all authentication-boundary changes where a review mistake is a security hole, and the direction itself — a unified federation model marked as superseding the current Direct/Passive extension points — is a product/architecture decision that a maintainer or security owner should sign off on, not a passing reviewer. So treat my notes as input, not a gate.

Two things I did want to raise from the read:

  1. fix(auth): enforce trusted external identity attributes #631 email assurance: requiring a verified entry from /user/emails before deriving GitHub email assurance, and refusing to let unverified external email populate/overwrite the platform email, is the correct hardening — this is a classic account-takeover vector (unverified provider email → email-domain access). Worth confirming the same "must be verified" rule is enforced for every provider the federation model onboards (OIDC email_verified, DingTalk, SAML assertions), not just GitHub, so the guarantee is uniform rather than per-provider.

  2. fix(auth): isolate unsafe account merge flow #639 fail-closed: returning a single localized 503 on the three legacy merge routes and deleting AccountMergeService is the safe move. The one thing to double-check is that nothing else in the codebase still calls into the removed destructive migration path — a compile-time removal is good, but a reflective/config-driven caller wouldn't show up as a build break.

Sequencing (phased PR + migration + rollback + big-main gates) is laid out well. Deferring to the maintainers on whether to adopt this federation direction and on the security review of the implementation PRs.

Signed-off-by: ylhu16 <ylhu16@iflytek.com>
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.

3 participants