Repository navigation
Feat/link sign in methods - #81
Conversation
…count A person with accounts at two providers, or two emails, ended up with two accounts, since a sign-in joins an existing account only by an authoritative email. /account/sign-in-methods lets a signed-in person link another provider's sign-in: its form posts, origin-checked, to start that provider's sign-in with a login state naming the account, and the callback attaches the identity to it. The browser must still be signed in as that account; an identity on another account is never moved; no account is created and the account's email is unchanged. Each link and refusal is systemauth.sign_in_method.linked, as OCSF User Management. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 34 minutes. View limit details
 |
| Layer / File(s) | Summary |
|---|---|
Link page and flow initiation identity/systemauth/social_link.go, identity/systemauth/social_login.go |
Adds the sign-in-methods page and link-start flow. The flow checks origin, rate limits requests, and stores state that identifies the initiating account. |
Callback completion and audit events identity/systemauth/social_login.go, identity/systemauth/social_link.go, identity/systemauth/social_link_test.go, contract/audit/ocsfaudit/*, docs/security/ocsf-events.md |
The callback directs linking flows to account-checked completion. Existing identities on the same account succeed; identities on another account are refused. Link outcomes emit audit events, and tests and OCSF mappings cover the behavior. |
User and release documentation docs/systemauth/social-login.md, docs/releases/*, CHANGELOG.json, CHANGELOG.md, mkdocs.yml |
Documents the linking flow and its restrictions. Adds v0.30.5 release notes and updates the release index, changelog comparisons, and navigation. |
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant Browser
participant SocialLoginCallback
participant Provider
participant IdentityLookup
participant AuditEvent
Browser->>Provider: authorize link request
Provider->>SocialLoginCallback: return provider profile
SocialLoginCallback->>IdentityLookup: check and link identity for initiating account
SocialLoginCallback->>AuditEvent: record link outcome
SocialLoginCallback->>Browser: redirect to sign-in-methods page
Merge Risk: 
ffcc2
-
The linking guidance promises an unavailable recovery and overstates some guarantees. Correct or accept these documentation gaps before merging.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "Feat/link sign in methods" matches the main change: a signed-in user can link another provider's sign-in (GitHub or Google) to an existing account via /account/sign-in-methods. It names the… Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request. Full details: Title check
-
Explanation
The title "Feat/link sign in methods" matches the main change: a signed-in user can link another provider's sign-in (GitHub or Google) to an existing account via /account/sign-in-methods. It names the feature area, though it does not state the provider scope, the refusal rules, or that the change is a feature for account linking. The branch-style prefix "Feat/" is terse but the subject is clear.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 functions across 7 files. (8 skipped: 8 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches
-
Generate docstrings
-
- Commit to this branch
- Create a new PR
Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
CodeRabbit Verify
- Run CodeRabbit Verify to test the changes in this pull request.
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| Resource: audit.Resource{Type: "principal", ID: principalID}, | ||
| Details: details("method", login.Provider), | ||
| }) | ||
| LoggerFromContext(ctx).Info("sign-in method linked", "principal_id", principalID, "provider", login.Provider) |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
… is refused The sign-in methods page told a person whose identity is on another account to sign in with it and remove it there, which no page offers. It now says signing in with it opens that account, and that removing a sign-in method or merging accounts is not available yet. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Removing a sign-in method or merging accounts is not available, so a provider is linked before its first sign-in makes an account of its own. The security event covers a completed link and a link the callback refuses; a refused start or a denial at the provider is not that event. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Linking needs a session for the account that started it, not that same session; an identity already on another account is not linked, since merging is not available; and the security event covers a completed link and a link the callback refuses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Summary by CodeRabbit