Skip to content

fix(auth): enforce trusted OAuth identity attributes - #688

Open
XiaoSeS wants to merge 1 commit into
mainfrom
fix/auth-email-assurance-guard-main
Open

fix(auth): enforce trusted OAuth identity attributes#688
XiaoSeS wants to merge 1 commit into
mainfrom
fix/auth-email-assurance-guard-main

Conversation

@XiaoSeS

@XiaoSeS XiaoSeS commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

What

  • Harden external OAuth identity handling so only trusted, verified attributes can drive login and binding.
  • Reject merged and system accounts on external login.
  • Preserve authoritative email handling in identity binding.

Why

How

  • Tighten GitHub claims extraction and identity binding checks.
  • Add explicit exceptions for merged/system account login attempts.
  • Extend auth tests to cover the new guardrails.

Testing

  • cd server && ./mvnw -pl skillhub-auth -am -DskipTests install
  • cd server/skillhub-auth && ../mvnw test -Dtest=IdentityBindingServiceTest,OAuthLoginFlowServiceTest,GitHubClaimsExtractorTest,AccessPolicyTest

Impact

  • No API contract change.
  • This is the first staged implementation PR; 0B and 0C will follow as separate PRs.

Signed-off-by: ylhu16 <ylhu16@iflytek.com>
@CLAassistant

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 sign our Contributor License Agreement before we can accept your contribution.


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 Aug 5, 2026

Copy link
Copy Markdown
Contributor

Review finding (blocking): GitHubClaimsExtractor still uses @Component, but this patch removes the org.springframework.stereotype.Component import. The class will not compile unless the import is restored (or the annotation is intentionally removed and bean registration is handled another way). Please fix this and rerun the server tests. The CLA check is also still pending.

@FenjuFu FenjuFu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rechecked current head 833270b. The Component import is restored, so the earlier compilation blocker is resolved. The identity-binding path now rejects system, merged, pending, and disabled accounts before profile mutation; only verified email is persisted; and the focused tests plus Server, E2E, Web, Docs, Dependency Review, and DCO checks pass. I found no new code blocker. This is not an approval yet because CLA remains pending and the branch is currently unmergeable; recheck the final head after those gates are resolved.

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