diff --git a/CHANGELOG.json b/CHANGELOG.json index 162bbf8e..68e4d40e 100644 --- a/CHANGELOG.json +++ b/CHANGELOG.json @@ -8,6 +8,44 @@ "grokify" ], "releases": [ + { + "version": "v0.30.5", + "date": "2026-11-02", + "highlights": [ + { + "description": "A signed-in person can link another provider's sign-in to their account at `/account/sign-in-methods`, so either reaches the same account", + "commit": "18333eb" + } + ], + "added": [ + { + "description": "`/account/sign-in-methods` and `POST /account/sign-in-methods/{provider}`: link a provider's sign-in to the signed-in account (`SignInMethodsPath`)", + "commit": "18333eb" + }, + { + "description": "The security event `systemauth.sign_in_method.linked`, for a completed link and a link the callback refuses, as OCSF User Management, Update", + "commit": "18333eb" + } + ], + "security": [ + { + "description": "Linking needs a session for the account that started it and an allowed origin, never moves an identity from another account, never creates an account and never changes its email", + "commit": "18333eb" + } + ], + "tests": [ + { + "description": "Linking, relinking, signing in with a linked identity, and refusals for another account's identity, a missing session and a cross-origin start", + "commit": "1dc9898" + } + ], + "documentation": [ + { + "description": "The social login guide describes linking sign-in methods; the OCSF guide lists the new event", + "commit": "2159827" + } + ] + }, { "version": "v0.30.4", "date": "2026-11-02", diff --git a/CHANGELOG.md b/CHANGELOG.md index d43ad431..fc5909cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,29 @@ and this changelog is generated by [Structured Changelog](https://github.com/gro ## [Unreleased] +## [v0.30.5] - 2026-11-02 + +### Highlights + +- A signed-in person can link another provider's sign-in to their account at `/account/sign-in-methods`, so either reaches the same account + +### Security + +- Linking needs a session for the account that started it and an allowed origin, never moves an identity from another account, never creates an account and never changes its email ([`18333eb`](https://github.com/plexusone/systemforge/commit/18333eb)) + +### Added + +- `/account/sign-in-methods` and `POST /account/sign-in-methods/{provider}`: link a provider's sign-in to the signed-in account (`SignInMethodsPath`) ([`18333eb`](https://github.com/plexusone/systemforge/commit/18333eb)) +- The security event `systemauth.sign_in_method.linked`, for a completed link and a link the callback refuses, as OCSF User Management, Update ([`18333eb`](https://github.com/plexusone/systemforge/commit/18333eb)) + +### Documentation + +- The social login guide describes linking sign-in methods; the OCSF guide lists the new event ([`2159827`](https://github.com/plexusone/systemforge/commit/2159827)) + +### Tests + +- Linking, relinking, signing in with a linked identity, and refusals for another account's identity, a missing session and a cross-origin start ([`1dc9898`](https://github.com/plexusone/systemforge/commit/1dc9898)) + ## [v0.30.4] - 2026-11-02 ### Highlights @@ -1747,7 +1770,8 @@ and this changelog is generated by [Structured Changelog](https://github.com/gro - GitHub Actions lint workflow with golangci-lint ([`e2f98d5`](https://github.com/plexusone/systemforge/commit/e2f98d5)) - Dependabot configuration for Go modules and GitHub Actions ([`e2f98d5`](https://github.com/plexusone/systemforge/commit/e2f98d5)) -[unreleased]: https://github.com/plexusone/systemforge/compare/v0.30.4...HEAD +[unreleased]: https://github.com/plexusone/systemforge/compare/v0.30.5...HEAD +[v0.30.5]: https://github.com/plexusone/systemforge/compare/v0.30.4...v0.30.5 [v0.30.4]: https://github.com/plexusone/systemforge/compare/v0.30.3...v0.30.4 [v0.30.3]: https://github.com/plexusone/systemforge/compare/v0.30.2...v0.30.3 [v0.30.2]: https://github.com/plexusone/systemforge/compare/v0.30.1...v0.30.2 diff --git a/contract/audit/ocsfaudit/details.go b/contract/audit/ocsfaudit/details.go index 4a74ac92..6eaed4c0 100644 --- a/contract/audit/ocsfaudit/details.go +++ b/contract/audit/ocsfaudit/details.go @@ -42,6 +42,7 @@ var allowed = map[string]map[string]kind{ "systemauth.token_family.revoked": {"reason": kindCode, "subject": kindUUID, "client_id": kindCode}, "systemauth.email_verification.sent": {"purpose": kindCode, "sent": kindBool}, "systemauth.email.verified": {"purpose": kindCode, "method": kindCode}, + "systemauth.sign_in_method.linked": {"method": kindCode, "reason": kindCode}, "systemauth.signing_key.loaded": {"key_id": kindCode, "verification_key_ids": kindCodeList, "ephemeral": kindBool}, "systemauth.production.waiver": {"check": kindCode, "setting": kindCode}, diff --git a/contract/audit/ocsfaudit/doc.go b/contract/audit/ocsfaudit/doc.go index bda60987..1b27824b 100644 --- a/contract/audit/ocsfaudit/doc.go +++ b/contract/audit/ocsfaudit/doc.go @@ -51,6 +51,7 @@ // systemauth.email_change.requested, User Management (3007) Update Informational // systemauth.email_change.cancelled, // systemauth.email.changed +// systemauth.sign_in_method.linked User Management (3007) Update Informational // systemauth.signing_key.loaded Application Lifecycle (6002) Start Informational // systemauth.production.waiver Application Lifecycle (6002) Start Medium // organization.membership Group Management (3006) Add User, Remove User, Update Informational; Medium for ownership_transferred diff --git a/contract/audit/ocsfaudit/mapping.go b/contract/audit/ocsfaudit/mapping.go index c5117e52..fe3008a6 100644 --- a/contract/audit/ocsfaudit/mapping.go +++ b/contract/audit/ocsfaudit/mapping.go @@ -94,6 +94,8 @@ func (m *mapper) toOCSF(ev *audit.Event) any { case "systemauth.email_verification.sent", "systemauth.email.verified", "systemauth.email_change.requested", "systemauth.email_change.cancelled", "systemauth.email.changed": return c.email() + case "systemauth.sign_in_method.linked": + return c.userManagement(events.UserManagementActivityIDUpdate, c.subject()) case "systemauth.signing_key.loaded", "systemauth.production.waiver": return c.startup() case "organization.membership": diff --git a/contract/audit/ocsfaudit/mapping_test.go b/contract/audit/ocsfaudit/mapping_test.go index a9ca8cb7..46e7b52d 100644 --- a/contract/audit/ocsfaudit/mapping_test.go +++ b/contract/audit/ocsfaudit/mapping_test.go @@ -96,6 +96,8 @@ func mappingCases() []mappingCase { {"email change requested", evt("systemauth.email_change.requested", "request_email_change", ok, nil), userMg, events.UserManagementActivityIDUpdate, info}, {"email change cancelled", evt("systemauth.email_change.cancelled", "cancel_email_change", ok, nil), userMg, events.UserManagementActivityIDUpdate, info}, {"email changed", evt("systemauth.email.changed", "change_email", ok, map[string]any{"old_email": leakEmail, "new_email": leakEmail}), userMg, events.UserManagementActivityIDUpdate, info}, + {"sign-in method linked", evt("systemauth.sign_in_method.linked", "link_sign_in_method", ok, map[string]any{"method": "google", "email": leakEmail}), userMg, events.UserManagementActivityIDUpdate, info}, + {"sign-in method link refused", evt("systemauth.sign_in_method.linked", "link_sign_in_method", fail, map[string]any{"method": "github", "reason": "other_account"}), userMg, events.UserManagementActivityIDUpdate, low}, {"signing key loaded", withResource(withActor(evt("systemauth.signing_key.loaded", "load", ok, map[string]any{"key_id": "kid-2026", "verification_key_ids": []string{"kid-2025", "kid-2026"}, "ephemeral": false}), service), audit.Resource{Type: "signing_key", Identifier: "kid-2026"}), appLC, events.ApplicationLifecycleActivityIDStart, info}, {"production waiver", withResource(withActor(evt("systemauth.production.waiver", "waive", ok, map[string]any{"check": "dpop.single_instance", "setting": "dpop.single_instance", "risk": leakRisk}), service), audit.Resource{Type: "production_check", Identifier: "dpop.single_instance"}), appLC, events.ApplicationLifecycleActivityIDStart, medium}, {"membership added", withResource(evt("organization.membership", "added", ok, map[string]any{"organization_id": orgID.String(), "principal_id": resourceID.String(), "role": "member"}), audit.Resource{Type: "membership", ID: resourceID}), groupM, events.GroupManagementActivityIDAddUser, info}, diff --git a/docs/releases/changelog.md b/docs/releases/changelog.md index d43ad431..fc5909cd 100644 --- a/docs/releases/changelog.md +++ b/docs/releases/changelog.md @@ -9,6 +9,29 @@ and this changelog is generated by [Structured Changelog](https://github.com/gro ## [Unreleased] +## [v0.30.5] - 2026-11-02 + +### Highlights + +- A signed-in person can link another provider's sign-in to their account at `/account/sign-in-methods`, so either reaches the same account + +### Security + +- Linking needs a session for the account that started it and an allowed origin, never moves an identity from another account, never creates an account and never changes its email ([`18333eb`](https://github.com/plexusone/systemforge/commit/18333eb)) + +### Added + +- `/account/sign-in-methods` and `POST /account/sign-in-methods/{provider}`: link a provider's sign-in to the signed-in account (`SignInMethodsPath`) ([`18333eb`](https://github.com/plexusone/systemforge/commit/18333eb)) +- The security event `systemauth.sign_in_method.linked`, for a completed link and a link the callback refuses, as OCSF User Management, Update ([`18333eb`](https://github.com/plexusone/systemforge/commit/18333eb)) + +### Documentation + +- The social login guide describes linking sign-in methods; the OCSF guide lists the new event ([`2159827`](https://github.com/plexusone/systemforge/commit/2159827)) + +### Tests + +- Linking, relinking, signing in with a linked identity, and refusals for another account's identity, a missing session and a cross-origin start ([`1dc9898`](https://github.com/plexusone/systemforge/commit/1dc9898)) + ## [v0.30.4] - 2026-11-02 ### Highlights @@ -1747,7 +1770,8 @@ and this changelog is generated by [Structured Changelog](https://github.com/gro - GitHub Actions lint workflow with golangci-lint ([`e2f98d5`](https://github.com/plexusone/systemforge/commit/e2f98d5)) - Dependabot configuration for Go modules and GitHub Actions ([`e2f98d5`](https://github.com/plexusone/systemforge/commit/e2f98d5)) -[unreleased]: https://github.com/plexusone/systemforge/compare/v0.30.4...HEAD +[unreleased]: https://github.com/plexusone/systemforge/compare/v0.30.5...HEAD +[v0.30.5]: https://github.com/plexusone/systemforge/compare/v0.30.4...v0.30.5 [v0.30.4]: https://github.com/plexusone/systemforge/compare/v0.30.3...v0.30.4 [v0.30.3]: https://github.com/plexusone/systemforge/compare/v0.30.2...v0.30.3 [v0.30.2]: https://github.com/plexusone/systemforge/compare/v0.30.1...v0.30.2 diff --git a/docs/releases/index.md b/docs/releases/index.md index 9cbeb77d..9e179276 100644 --- a/docs/releases/index.md +++ b/docs/releases/index.md @@ -4,7 +4,10 @@ Release notes, one file per version, newest first. The [changelog](changelog.md) is generated from `CHANGELOG.json` and lists every change with its commit. -The latest release, [v0.30.4](v0.30.4.md), adds +The latest release, [v0.30.5](v0.30.5.md), lets a signed-in person link +another provider's sign-in to their account. It adds no breaking change. + +Before it, [v0.30.4](v0.30.4.md) added `social_login.signed_out_url`, so every sign-out can land on the product's marketing site, and links the waitlist page back to it by the product's name. It adds no breaking change. diff --git a/docs/releases/v0.30.5.md b/docs/releases/v0.30.5.md new file mode 100644 index 00000000..9a891bfe --- /dev/null +++ b/docs/releases/v0.30.5.md @@ -0,0 +1,24 @@ +# v0.30.5 + +**Release Date:** 2026-11-02 + +One account, several sign-ins: + +- **Linking sign-in methods.** A person who signs in with GitHub under one + email would get a second account by signing in with Google under another, + since a sign-in joins an existing account only by an email its provider is + authoritative for. Signed in, they can now open `/account/sign-in-methods` + on the auth host and link the other provider first. Either sign-in then + reaches the same account and its work. A sign-in that already has an + account of its own is not linked: merging accounts, and removing a sign-in + method, are not available yet. +- **Safe by default.** Linking needs a session for the account that started + it, is origin-checked like sign-out, never moves an identity from another + account, never creates an account and never changes the account's email. + A completed link, and a link the callback refuses, is the security event + `systemauth.sign_in_method.linked`, mapped to OCSF User Management. + +It adds no breaking change and no setting: the page is served wherever +social login is. + +See [Linking sign-in methods](../systemauth/social-login.md#linking-sign-in-methods). diff --git a/docs/security/ocsf-events.md b/docs/security/ocsf-events.md index a0e87105..90435ea0 100644 --- a/docs/security/ocsf-events.md +++ b/docs/security/ocsf-events.md @@ -100,6 +100,7 @@ selected by it as well as by OCSF class. | `systemauth.token_family.revoked` | Authentication (3002) | Logoff, protocol OAuth 2.0 | Informational | | `systemauth.email_verification.sent`, `systemauth.email.verified` | Authentication (3002) for purpose `verify_email`; User Management (3007) for `email_change` | Preauth; Update | Informational | | `systemauth.email_change.requested`, `.cancelled`, `systemauth.email.changed` | User Management (3007) | Update | Informational | +| `systemauth.sign_in_method.linked` | User Management (3007) | Update | Informational; Low when refused | | `systemauth.signing_key.loaded` | Application Lifecycle (6002) | Start | Informational | | `systemauth.production.waiver` | Application Lifecycle (6002) | Start | Medium | | `organization.membership` | Group Management (3006) | Add User (`added`), Remove User (`removed`), Update (`updated`, `ownership_transferred`) | Informational; Medium for `ownership_transferred` | diff --git a/docs/systemauth/social-login.md b/docs/systemauth/social-login.md index cff9737e..d19d489d 100644 --- a/docs/systemauth/social-login.md +++ b/docs/systemauth/social-login.md @@ -18,6 +18,7 @@ endpoints: |-------|---------| | `GET /login?return_to=...` | The sign-in page: a link per provider, waiting for the person to choose, even with one provider. With `auto_select_single_provider`, redirects straight to the only provider. | | `GET /login/{provider}?return_to=...` | Starts the upstream authorization-code flow (`github` or `google`). | +| `GET /account/sign-in-methods` | A signed-in person's page to link another provider's sign-in to their account (see [Linking sign-in methods](#linking-sign-in-methods)). | | `GET /login/{provider}/callback` | Upstream callback. Resolves the principal, sets the `__Host-sf_login` session, and redirects to `return_to`. | | `GET /logout?return_to=...` | Renders a sign-out confirmation form (never signs out by itself). | | `POST /logout` | Deletes the login session, clears the cookie, revokes all of the principal's access and refresh tokens, and redirects (`303`) to the validated `return_to` (default `default_redirect`). | @@ -192,6 +193,32 @@ email already belongs to an account, the sign-in is refused as a conflict (`ErrEmailConflict`), and the person signs in the way they signed in before. GitHub sign-in uses the account's primary email only. +### Linking sign-in methods + +A signed-in person can link another provider's sign-in to their account at +`/account/sign-in-methods`, so either one signs them in to the same account, +whatever email each provider reports. The page lists a **Link** button per +configured provider. The button's form posts to +`/account/sign-in-methods/{provider}`, which is origin-checked like sign-out +and needs a login session. It sends the browser through the provider's +sign-in. The callback then attaches that provider identity to the account: + +- The browser must still be signed in as the account that started the link. +- An identity already on another account is never moved, and the page says + so. Removing a sign-in method from an account, or merging two accounts, is + not available yet: link a provider before its first sign-in makes an + account of its own. +- An identity already on this account is a success that changes nothing. +- No account is created, no sign-up gate or age question applies, and the + account's own email is unchanged. + +A completed link, and a link the callback refuses (the identity is on +another account, or the browser is no longer signed in as the account that +started it), is the security event `systemauth.sign_in_method.linked`. A +start refused before the provider, such as a cross-origin request, is not; +nor is a denial at the provider, which is a refused sign-in +(`systemauth.sign_in`). + ## Security properties - `state` is stored server-side (single use, 10 minute TTL) **and** bound to the diff --git a/identity/systemauth/social_link.go b/identity/systemauth/social_link.go new file mode 100644 index 00000000..a1992b8d --- /dev/null +++ b/identity/systemauth/social_link.go @@ -0,0 +1,218 @@ +package systemauth + +import ( + "context" + "errors" + "html/template" + "net/http" + "net/url" + "strings" + + "github.com/go-chi/chi/v5" + "github.com/google/uuid" + "golang.org/x/oauth2" + + "github.com/plexusone/systemforge/contract/audit" + "github.com/plexusone/systemforge/identity/oauthclient" +) + +const ( + // SignInMethodsPath shows a signed-in person their account's sign-in + // methods and lets them link another provider's sign-in to the same + // account, so either signs them in. + SignInMethodsPath = "/account/sign-in-methods" + // linkStartPattern starts linking a provider: a POST from the page. + linkStartPattern = SignInMethodsPath + "/{provider}" + + // EventSignInMethodLinked records linking a provider's sign-in to an + // account, or a refusal to (Details: method, and reason on failure). + EventSignInMethodLinked = "systemauth.sign_in_method.linked" + + // linkNoncePrefix marks a login state that links to the account named + // after it rather than signing in. + linkNoncePrefix = "link:" +) + +var signInMethodsTemplate = template.Must(template.New("sign-in-methods").Parse(` +` + pageStyleLink + ` +Sign-in methods +

Sign-in methods

+

Link another sign-in to this account, so either one signs you in to the same account and its work.

+{{with .Linked}}

{{.}} is linked to your account. You can now sign in with it.

+{{end}}{{with .Problem}}

{{.}}

+{{end}}
+`)) + +// linkProblems are the page's messages for a refused link, by reason. +var linkProblems = map[string]string{ + "other_account": "That sign-in already belongs to another account, so it was not linked. Signing in with it opens that other account; removing a sign-in method or merging accounts is not available yet.", + "session": "You were signed out while linking. Sign in and try again.", + "failed": "Linking did not complete. Try again.", +} + +// handleSignInMethods serves GET SignInMethodsPath. Without a login session +// it sends the browser to sign in first, and back here. +func (sl *socialLogin) handleSignInMethods(w http.ResponseWriter, r *http.Request) { + noStore(w) + if _, err := sl.currentSession(r); err != nil { + http.Redirect(w, r, LoginPath+"?return_to="+url.QueryEscape(SignInMethodsPath), http.StatusFound) + return + } + type provider struct{ Name, Action string } + var providers []provider + var authorizeURLs []string + for _, p := range sl.providers { + providers = append(providers, provider{Name: displayName(p), Action: SignInMethodsPath + "/" + url.PathEscape(p)}) + // Each form's POST is answered with a redirect to the provider, + // which form-action must allow. + authorizeURLs = append(authorizeURLs, sl.connectors[p].AuthCodeURL("state")) + } + var linked, problem string + q := r.URL.Query() + if p := q.Get("linked"); p != "" && sl.connectors[p] != nil { + linked = displayName(p) + } + problem = linkProblems[q.Get("error")] + allowFormTargets(w, formSources(authorizeURLs...)) + w.Header().Set("Content-Type", "text/html; charset=utf-8") + if err := signInMethodsTemplate.Execute(w, struct { + Providers []provider + Linked, Problem string + }{providers, linked, problem}); err != nil { + LoggerFromContext(r.Context()).Error("rendering sign-in methods page", "error", err) + } +} + +// handleLinkStart serves POST linkStartPattern: it starts the provider's +// sign-in with a login state that links the result to the signed-in +// account instead of signing in. +// +// CSRF: the request must come from the issuer's origin or an allowed +// relying-party origin, as sign-out's does, and the browser must hold a +// login session; the callback checks the session again. +func (sl *socialLogin) handleLinkStart(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + logger := LoggerFromContext(ctx) + noStore(w) + if !sl.redirects.fromTrustedOrigin(r) { + http.Error(w, "cross-origin request rejected", http.StatusForbidden) + return + } + if !sl.limit(w, r, "social_login_start", DefaultSocialLoginLimit) { + return + } + provider := chi.URLParam(r, "provider") + conn, ok := sl.connectors[provider] + if !ok { + http.NotFound(w, r) + return + } + sess, err := sl.currentSession(r) + if err != nil { + http.Redirect(w, r, LoginPath+"?return_to="+url.QueryEscape(SignInMethodsPath), http.StatusSeeOther) + return + } + state, err := oauthclient.GenerateState() + if err != nil { + logger.Error("generating link state", "error", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + verifier := oauth2.GenerateVerifier() + if err := sl.states.Put(ctx, state, oauthclient.StateData{ + Provider: provider, + RedirectURL: SignInMethodsPath, + Nonce: linkNoncePrefix + sess.PrincipalID.String(), + PKCEVerifier: verifier, + }, loginStateTTL); err != nil { + logger.Error("storing link state", "error", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + sl.cookies.setState(w, state, loginStateTTL) + //nolint:gosec // G710: redirect target is the configured provider's authorization endpoint + http.Redirect(w, r, conn.AuthCodeURL(state, oauth2.S256ChallengeOption(verifier)), http.StatusSeeOther) +} + +// linkingPrincipal returns the account a login state links to, when it is +// a link and not a sign-in. +func linkingPrincipal(nonce string) (uuid.UUID, bool) { + rest, ok := strings.CutPrefix(nonce, linkNoncePrefix) + if !ok { + return uuid.Nil, false + } + id, err := uuid.Parse(rest) + return id, err == nil +} + +// completeLink links login's identity to principalID, the account that +// started the link, and returns the browser to the sign-in methods page. +// The browser must still be signed in as that account. An identity already +// on another account is never moved; one already on this account is a +// success. No account is created, and the account's email is unchanged. +func (sl *socialLogin) completeLink(ctx context.Context, w http.ResponseWriter, r *http.Request, principalID uuid.UUID, login ExternalLogin) { + back := func(param, value string) { + http.Redirect(w, r, SignInMethodsPath+"?"+param+"="+url.QueryEscape(value), http.StatusFound) + } + sess, err := sl.currentSession(r) + if err != nil || sess.PrincipalID != principalID { + sl.linkRefused(ctx, principalID, login.Provider, "session") + back("error", "session") + return + } + existing, err := sl.directory.FindByExternalIdentity(ctx, login.Provider, login.Subject) + switch { + case err == nil && existing.ID == principalID: + back("linked", login.Provider) + return + case err == nil: + sl.linkRefused(ctx, principalID, login.Provider, "other_account") + back("error", "other_account") + return + case !errors.Is(err, ErrPrincipalNotFound): + LoggerFromContext(ctx).Error("looking up the identity to link", "error", err) + back("error", "failed") + return + } + if err := sl.directory.LinkExternalIdentity(ctx, principalID, login); err != nil { + if errors.Is(err, ErrExternalIdentityExists) { + sl.linkRefused(ctx, principalID, login.Provider, "other_account") + back("error", "other_account") + return + } + LoggerFromContext(ctx).Error("linking an identity", "error", err) + back("error", "failed") + return + } + sl.emit(ctx, &audit.Event{ + EventType: EventSignInMethodLinked, + Action: "link_sign_in_method", + Actor: audit.Actor{ID: principalID, Type: actorPrincipal}, + 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) + back("linked", login.Provider) +} + +// linkRefused records a refused link. +func (sl *socialLogin) linkRefused(ctx context.Context, principalID uuid.UUID, provider, reason string) { + sl.emit(ctx, &audit.Event{ + EventType: EventSignInMethodLinked, + Action: "link_sign_in_method", + Actor: audit.Actor{ID: principalID, Type: actorPrincipal}, + Resource: audit.Resource{Type: "principal", ID: principalID}, + Outcome: audit.OutcomeFailure, + Details: details("method", provider, "reason", reason), + }) +} + +// displayName is a provider's name for people. +func displayName(provider string) string { + if name := providerDisplayNames[provider]; name != "" { + return name + } + return provider +} diff --git a/identity/systemauth/social_link_test.go b/identity/systemauth/social_link_test.go new file mode 100644 index 00000000..16590008 --- /dev/null +++ b/identity/systemauth/social_link_test.go @@ -0,0 +1,144 @@ +package systemauth + +import ( + "context" + "net/http" + "net/http/httptest" + "net/url" + "strings" + "testing" +) + +// linkStart posts the page's Link button for GitHub as the signed-in +// browser and returns the provider redirect's state and its cookie. +func linkStart(t *testing.T, s *Server, sess *http.Cookie) (string, *http.Cookie) { + t.Helper() + w := postForm(s, SignInMethodsPath+"/github", testIssuer, url.Values{}, sess) + if w.Code != http.StatusSeeOther { + t.Fatalf("link start: %d %s", w.Code, w.Body.String()) + } + loc, err := url.Parse(w.Header().Get("Location")) + if err != nil { + t.Fatal(err) + } + state := loc.Query().Get("state") + cookie := findCookie(t, w.Result(), loginStateCookieName) + if state == "" || cookie == nil { + t.Fatalf("link start: no state in %q", w.Header().Get("Location")) + } + return state, cookie +} + +func TestLinkSignInMethod(t *testing.T) { + f := newFakeGitHub(t) + dir := NewMemoryPrincipalDirectory() + s := newSocialTestServer(t, f, dir) + ctx := context.Background() + + // Signed out, the page sends the browser to sign in and back. + w := httptest.NewRecorder() + s.ServeHTTP(w, httptest.NewRequest(http.MethodGet, SignInMethodsPath, nil)) + if w.Code != http.StatusFound || w.Header().Get("Location") != LoginPath+"?return_to="+url.QueryEscape(SignInMethodsPath) { + t.Fatalf("signed out: %d %q", w.Code, w.Header().Get("Location")) + } + + // Sign in as GitHub user 4242: the account. + sess := socialSession(t, s) + account, err := dir.FindByExternalIdentity(ctx, "github", "4242") + if err != nil { + t.Fatal(err) + } + + // The page offers each provider, styled, and lets its form reach the + // provider through the POST's redirect. + req := httptest.NewRequest(http.MethodGet, SignInMethodsPath, nil) + req.AddCookie(sess) + w = httptest.NewRecorder() + s.ServeHTTP(w, req) + if w.Code != http.StatusOK || !strings.Contains(w.Body.String(), ">Link GitHub") || !strings.Contains(w.Body.String(), pageStyleLink) { + t.Fatalf("page: %d %s", w.Code, w.Body.String()) + } + // The fake provider is on loopback, which form-action names by host. + if csp := w.Header().Get("Content-Security-Policy"); !strings.Contains(csp, "form-action 'self' http://127.0.0.1") { + t.Errorf("form-action does not allow the provider: %s", csp) + } + + // A cross-origin POST is refused. + if w := postForm(s, SignInMethodsPath+"/github", "https://evil.example.com", url.Values{}, sess); w.Code != http.StatusForbidden { + t.Errorf("cross-origin link: %d", w.Code) + } + + // Link a second GitHub identity, 5555, with another email: it joins the + // account; no account is created and the account's email stays. + f.userID, f.email = 5555, "other@example.com" + state, stateCookie := linkStart(t, s, sess) + w = callback(s, state, stateCookie, sess) + if w.Code != http.StatusFound || w.Header().Get("Location") != SignInMethodsPath+"?linked=github" { + t.Fatalf("link: %d %q %s", w.Code, w.Header().Get("Location"), w.Body.String()) + } + linked, err := dir.FindByExternalIdentity(ctx, "github", "5555") + if err != nil || linked.ID != account.ID || linked.Email != account.Email { + t.Fatalf("linked identity: %+v %v; account %+v", linked, err, account) + } + + // Linking it again is a success that changes nothing. + state, stateCookie = linkStart(t, s, sess) + if w := callback(s, state, stateCookie, sess); w.Header().Get("Location") != SignInMethodsPath+"?linked=github" { + t.Errorf("relink: %q", w.Header().Get("Location")) + } + + // Signing in with the linked identity signs in to the same account. + sess2 := socialSession(t, s) + r2 := httptest.NewRequest(http.MethodGet, "/", nil) + r2.AddCookie(sess2) + got, err := s.CurrentLoginSession(r2) + if err != nil || got.PrincipalID != account.ID { + t.Errorf("signing in with the linked identity: %+v %v", got, err) + } +} + +func TestLinkSignInMethodRefusals(t *testing.T) { + f := newFakeGitHub(t) + dir := NewMemoryPrincipalDirectory() + s := newSocialTestServer(t, f, dir) + ctx := context.Background() + + // Two accounts: 4242 and 7777. + sessA := socialSession(t, s) + f.userID, f.email = 7777, "b@example.com" + _ = socialSession(t, s) //nolint:errcheck // the second account only has to exist; its session is not used + other, err := dir.FindByExternalIdentity(ctx, "github", "7777") + if err != nil { + t.Fatal(err) + } + + // An identity on another account is never moved. + state, stateCookie := linkStart(t, s, sessA) + w := callback(s, state, stateCookie, sessA) + if w.Header().Get("Location") != SignInMethodsPath+"?error=other_account" { + t.Errorf("another account's identity: %q", w.Header().Get("Location")) + } + if still, err := dir.FindByExternalIdentity(ctx, "github", "7777"); err != nil || still.ID != other.ID { + t.Errorf("the identity moved: %+v %v", still, err) + } + + // A link finished without the session that started it is refused. + f.userID, f.email = 8888, "c@example.com" + state, stateCookie = linkStart(t, s, sessA) + w = callback(s, state, stateCookie) + if w.Header().Get("Location") != SignInMethodsPath+"?error=session" { + t.Errorf("no session at the callback: %q", w.Header().Get("Location")) + } + if _, err := dir.FindByExternalIdentity(ctx, "github", "8888"); err == nil { + t.Error("an identity was linked or created without the session") + } + + // The page names the problem, and only known ones. + req := httptest.NewRequest(http.MethodGet, SignInMethodsPath+"?error=other_account&linked=x", nil) + req.AddCookie(sessA) + rec := httptest.NewRecorder() + s.ServeHTTP(rec, req) + if body := rec.Body.String(); !strings.Contains(body, "already belongs to another account") || strings.Contains(body, "x") { + t.Errorf("page messages: %s", body) + } +} diff --git a/identity/systemauth/social_login.go b/identity/systemauth/social_login.go index 57f43367..b68cc3f1 100644 --- a/identity/systemauth/social_login.go +++ b/identity/systemauth/social_login.go @@ -335,6 +335,8 @@ func (sl *socialLogin) registerRoutes(r chi.Router) { r.Get(LoginPath, sl.handleChooser) r.Get(loginStartPattern, sl.handleStart) r.Get(loginCallbackPattern, sl.handleCallback) + r.Get(SignInMethodsPath, sl.handleSignInMethods) + r.Post(linkStartPattern, sl.handleLinkStart) if sl.cfg.SignupMinimumAge > 0 { r.Get(SignupAgePath, sl.handleAgePage) r.Post(SignupAgePath, sl.handleAgeAnswer) @@ -600,6 +602,22 @@ func (sl *socialLogin) handleCallback(w http.ResponseWriter, r *http.Request) { return } + // A state that links a sign-in method to the signed-in account: no + // sign-in, age question or sign-up gate applies. + if principalID, ok := linkingPrincipal(data.Nonce); ok { + sl.completeLink(ctx, w, r, principalID, ExternalLogin{ + Provider: provider, + Subject: user.ProviderID, + Email: user.Email, + EmailVerified: user.EmailVerified, + EmailAuthoritative: AuthoritativeEmail(user), + Name: user.Name, + Username: user.Username, + AvatarURL: user.AvatarURL, + }) + return + } + if sl.cfg.SignupMinimumAge > 0 { ctx = sl.takeAgeConfirmation(ctx, w, r, provider) } diff --git a/mkdocs.yml b/mkdocs.yml index 4131dcc6..4c59eab2 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -169,6 +169,7 @@ nav: - Releases: - Overview: releases/index.md - Changelog: releases/changelog.md + - v0.30.5: releases/v0.30.5.md - v0.30.4: releases/v0.30.4.md - v0.30.3: releases/v0.30.3.md - v0.30.2: releases/v0.30.2.md