diff --git a/CHANGELOG.json b/CHANGELOG.json index d5243db0..65cc893c 100644 --- a/CHANGELOG.json +++ b/CHANGELOG.json @@ -8,6 +8,76 @@ "grokify" ], "releases": [ + { + "version": "v0.32.0", + "date": "2026-11-05", + "highlights": [ + { + "description": "A manager replaces a managed principal's login tokens in one call: every live token is revoked and one issued, atomically, so a lost card is retired and reprinted in one step", + "commit": "289b658" + }, + { + "description": "The sign-in methods page shows the account's linked methods, offers only the others, and links back to the application through a validated `return_to`", + "commit": "19553ad" + } + ], + "added": [ + { + "description": "`managedprincipal.Service.ReplaceLoginTokens` revokes every live login token and issues one in one transaction; a failure while issuing revokes nothing", + "commit": "289b658" + }, + { + "description": "`POST /{principalID}/login-tokens/replace` answers 201 with the new token and the number revoked; `mpclient.Client.ReplaceLoginTokens` calls it", + "commit": "289b658" + }, + { + "description": "Audit event `managed_principal.login_tokens_replaced`, with the number of tokens revoked", + "commit": "289b658" + }, + { + "description": "The sign-in methods page shows the account's email and its linked providers, with a Link button only for the others", + "commit": "19553ad" + }, + { + "description": "`/account/sign-in-methods?return_to=...` shows a back link to an allowed `return_to`, kept through signing in, linking and the callback and validated again at each step", + "commit": "19553ad" + }, + { + "description": "Optional `LinkedProviderLister` directory interface, implemented by the Ent and memory directories", + "commit": "19553ad" + } + ], + "security": [ + { + "description": "Issuing and replacing a login token check the issuer's authority under the managed principal's row lock, in the transaction that issues, so a manager removed at the same moment can no longer be issued a token", + "commit": "0d8d277" + } + ], + "tests": [ + { + "description": "Replacing login tokens: every live token revoked and one issued, a failed issue keeps the old token, refusals revoke nothing, single-use and expiry honored, and the route and client", + "commit": "f263e91" + }, + { + "description": "Sign-in methods page: linked providers listed and not offered, valid and invalid `return_to`, and `return_to` kept through the link and the signed-out redirect", + "commit": "53b650d" + }, + { + "description": "A manager removed between the client check and the transaction is refused for issuing and replacing; two concurrent replaces on PostgreSQL leave one live token; a failed replace had revoked the old token before rolling back", + "commit": "8630cb2" + } + ], + "documentation": [ + { + "description": "Replacing a card in the managed principals guide and the architecture's route list", + "commit": "6a73560" + }, + { + "description": "Linked methods and `return_to` on the sign-in methods page in the social login guide", + "commit": "c6be3bb" + } + ] + }, { "version": "v0.31.0", "date": "2026-11-04", diff --git a/CHANGELOG.md b/CHANGELOG.md index c9c207df..e59d5ca7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,37 @@ and this changelog is generated by [Structured Changelog](https://github.com/gro ## [Unreleased] +## [v0.32.0] - 2026-11-05 + +### Highlights + +- A manager replaces a managed principal's login tokens in one call: every live token is revoked and one issued, atomically, so a lost card is retired and reprinted in one step +- The sign-in methods page shows the account's linked methods, offers only the others, and links back to the application through a validated `return_to` + +### Security + +- Issuing and replacing a login token check the issuer's authority under the managed principal's row lock, in the transaction that issues, so a manager removed at the same moment can no longer be issued a token ([`0d8d277`](https://github.com/plexusone/systemforge/commit/0d8d277)) + +### Added + +- `managedprincipal.Service.ReplaceLoginTokens` revokes every live login token and issues one in one transaction; a failure while issuing revokes nothing ([`289b658`](https://github.com/plexusone/systemforge/commit/289b658)) +- `POST /{principalID}/login-tokens/replace` answers 201 with the new token and the number revoked; `mpclient.Client.ReplaceLoginTokens` calls it ([`289b658`](https://github.com/plexusone/systemforge/commit/289b658)) +- Audit event `managed_principal.login_tokens_replaced`, with the number of tokens revoked ([`289b658`](https://github.com/plexusone/systemforge/commit/289b658)) +- The sign-in methods page shows the account's email and its linked providers, with a Link button only for the others ([`19553ad`](https://github.com/plexusone/systemforge/commit/19553ad)) +- `/account/sign-in-methods?return_to=...` shows a back link to an allowed `return_to`, kept through signing in, linking and the callback and validated again at each step ([`19553ad`](https://github.com/plexusone/systemforge/commit/19553ad)) +- Optional `LinkedProviderLister` directory interface, implemented by the Ent and memory directories ([`19553ad`](https://github.com/plexusone/systemforge/commit/19553ad)) + +### Documentation + +- Replacing a card in the managed principals guide and the architecture's route list ([`6a73560`](https://github.com/plexusone/systemforge/commit/6a73560)) +- Linked methods and `return_to` on the sign-in methods page in the social login guide ([`c6be3bb`](https://github.com/plexusone/systemforge/commit/c6be3bb)) + +### Tests + +- Replacing login tokens: every live token revoked and one issued, a failed issue keeps the old token, refusals revoke nothing, single-use and expiry honored, and the route and client ([`f263e91`](https://github.com/plexusone/systemforge/commit/f263e91)) +- Sign-in methods page: linked providers listed and not offered, valid and invalid `return_to`, and `return_to` kept through the link and the signed-out redirect ([`53b650d`](https://github.com/plexusone/systemforge/commit/53b650d)) +- A manager removed between the client check and the transaction is refused for issuing and replacing; two concurrent replaces on PostgreSQL leave one live token; a failed replace had revoked the old token before rolling back ([`8630cb2`](https://github.com/plexusone/systemforge/commit/8630cb2)) + ## [v0.31.0] - 2026-11-04 ### Highlights @@ -1824,7 +1855,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.31.0...HEAD +[unreleased]: https://github.com/plexusone/systemforge/compare/v0.32.0...HEAD +[v0.32.0]: https://github.com/plexusone/systemforge/compare/v0.31.0...v0.32.0 [v0.31.0]: https://github.com/plexusone/systemforge/compare/v0.30.5...v0.31.0 [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 diff --git a/docs/design/architecture/managed-principals.md b/docs/design/architecture/managed-principals.md index 6eae7be8..3172717e 100644 --- a/docs/design/architecture/managed-principals.md +++ b/docs/design/architecture/managed-principals.md @@ -258,10 +258,12 @@ ClientID, CreatedAt, ExpiresAt, LastUsedAt, ClientIP}`: | `principal_token` | `sf_principal_tokens`, one entry per family | Family marked revoked | `ListLoginTokens` returns live login tokens without their values; -`RevokeLoginToken` revokes one; `RevokeSession(kind, id)` ends one session; +`RevokeLoginToken` revokes one; `ReplaceLoginTokens` revokes every live +login token and issues one in one transaction, so a failure leaves the old +tokens working; `RevokeSession(kind, id)` ends one session; `RevokeAll` revokes every login token and ends every session in one transaction. Each emits an audit event (`login_token_revoked`, -`session_revoked`, `all_revoked`). `managedprincipal.NewHandler(svc)` +`login_tokens_replaced`, `session_revoked`, `all_revoked`). `managedprincipal.NewHandler(svc)` exposes these and the managing operations as a chi router to mount behind `requestprincipal.Middleware`, which identifies the calling principal. The caller is always the actor; every route runs the service operation and so @@ -283,6 +285,7 @@ POST /manager-invites/decline DeclineManagerInvite (JSON: to DELETE /manager-invites/{inviteID} RevokeManagerInvite GET /{principalID}/login-tokens POST /{principalID}/login-tokens IssueLoginToken (JSON: client_id, name?, single_use?, expires_at?) +POST /{principalID}/login-tokens/replace ReplaceLoginTokens (same JSON); 201 with the token and `revoked` DELETE /{principalID}/login-tokens/{credentialID} GET /{principalID}/sessions DELETE /{principalID}/sessions/{kind}/{sessionID} diff --git a/docs/identity/managed-principals.md b/docs/identity/managed-principals.md index 74236294..2e9d429e 100644 --- a/docs/identity/managed-principals.md +++ b/docs/identity/managed-principals.md @@ -334,9 +334,33 @@ principal is signed in: | Method | Covers | |--------|--------| | `ListLoginTokens`, `RevokeLoginToken` | Live login tokens, never their values | +| `ReplaceLoginTokens` | Revokes every live login token and issues one, in one transaction | | `ListSessions`, `RevokeSession` | Browser login sessions, OAuth token families, principal token families | | `RevokeAll` | All of the above, in one transaction | +### Replacing a Card + +A printed card is a reusable token, and tokens are stored only as hashes, so +a lost or shared card cannot be printed again from what is stored. +`ReplaceLoginTokens` takes the same input as `IssueLoginToken`, revokes every +live login token and issues one new token, in one transaction: + +```go +out, err := svc.ReplaceLoginTokens(ctx, managedprincipal.IssueLoginTokenInput{ + ManagedID: learnerID, + ClientID: "learning-app", + IssuedBy: parentID, + Name: "Robin's card", +}) +// out.Token.PlainToken is the new card; out.Revoked counts the old ones. +``` + +If issuing fails, nothing is revoked, so the old card keeps working rather +than none. Sessions already signed in with an old token keep running; +`RevokeAll` ends them too. It is audited once, as `login_tokens_replaced`, +with the number revoked. `mpclient.Client.ReplaceLoginTokens` calls it over +HTTP. + Revoking a token family revokes all its tokens, so a refresh token cannot renew it. `managedprincipal.NewHandler` serves these as JSON for the manager identified by `requestprincipal`: @@ -349,6 +373,7 @@ router.With(requestprincipal.Middleware(opts, authenticators...)). ```text GET /managed-principals/{id}/login-tokens POST /managed-principals/{id}/login-tokens +POST /managed-principals/{id}/login-tokens/replace DELETE /managed-principals/{id}/login-tokens/{credentialID} GET /managed-principals/{id}/sessions DELETE /managed-principals/{id}/sessions/{kind}/{sessionID} diff --git a/docs/releases/changelog.md b/docs/releases/changelog.md index c9c207df..e59d5ca7 100644 --- a/docs/releases/changelog.md +++ b/docs/releases/changelog.md @@ -9,6 +9,37 @@ and this changelog is generated by [Structured Changelog](https://github.com/gro ## [Unreleased] +## [v0.32.0] - 2026-11-05 + +### Highlights + +- A manager replaces a managed principal's login tokens in one call: every live token is revoked and one issued, atomically, so a lost card is retired and reprinted in one step +- The sign-in methods page shows the account's linked methods, offers only the others, and links back to the application through a validated `return_to` + +### Security + +- Issuing and replacing a login token check the issuer's authority under the managed principal's row lock, in the transaction that issues, so a manager removed at the same moment can no longer be issued a token ([`0d8d277`](https://github.com/plexusone/systemforge/commit/0d8d277)) + +### Added + +- `managedprincipal.Service.ReplaceLoginTokens` revokes every live login token and issues one in one transaction; a failure while issuing revokes nothing ([`289b658`](https://github.com/plexusone/systemforge/commit/289b658)) +- `POST /{principalID}/login-tokens/replace` answers 201 with the new token and the number revoked; `mpclient.Client.ReplaceLoginTokens` calls it ([`289b658`](https://github.com/plexusone/systemforge/commit/289b658)) +- Audit event `managed_principal.login_tokens_replaced`, with the number of tokens revoked ([`289b658`](https://github.com/plexusone/systemforge/commit/289b658)) +- The sign-in methods page shows the account's email and its linked providers, with a Link button only for the others ([`19553ad`](https://github.com/plexusone/systemforge/commit/19553ad)) +- `/account/sign-in-methods?return_to=...` shows a back link to an allowed `return_to`, kept through signing in, linking and the callback and validated again at each step ([`19553ad`](https://github.com/plexusone/systemforge/commit/19553ad)) +- Optional `LinkedProviderLister` directory interface, implemented by the Ent and memory directories ([`19553ad`](https://github.com/plexusone/systemforge/commit/19553ad)) + +### Documentation + +- Replacing a card in the managed principals guide and the architecture's route list ([`6a73560`](https://github.com/plexusone/systemforge/commit/6a73560)) +- Linked methods and `return_to` on the sign-in methods page in the social login guide ([`c6be3bb`](https://github.com/plexusone/systemforge/commit/c6be3bb)) + +### Tests + +- Replacing login tokens: every live token revoked and one issued, a failed issue keeps the old token, refusals revoke nothing, single-use and expiry honored, and the route and client ([`f263e91`](https://github.com/plexusone/systemforge/commit/f263e91)) +- Sign-in methods page: linked providers listed and not offered, valid and invalid `return_to`, and `return_to` kept through the link and the signed-out redirect ([`53b650d`](https://github.com/plexusone/systemforge/commit/53b650d)) +- A manager removed between the client check and the transaction is refused for issuing and replacing; two concurrent replaces on PostgreSQL leave one live token; a failed replace had revoked the old token before rolling back ([`8630cb2`](https://github.com/plexusone/systemforge/commit/8630cb2)) + ## [v0.31.0] - 2026-11-04 ### Highlights @@ -1824,7 +1855,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.31.0...HEAD +[unreleased]: https://github.com/plexusone/systemforge/compare/v0.32.0...HEAD +[v0.32.0]: https://github.com/plexusone/systemforge/compare/v0.31.0...v0.32.0 [v0.31.0]: https://github.com/plexusone/systemforge/compare/v0.30.5...v0.31.0 [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 diff --git a/docs/releases/index.md b/docs/releases/index.md index c9ee012b..d9085df4 100644 --- a/docs/releases/index.md +++ b/docs/releases/index.md @@ -4,7 +4,12 @@ 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.31.0](v0.31.0.md), is a security release: only +The latest release, [v0.32.0](v0.32.0.md), lets a manager replace a +managed principal's login tokens in one call, so a lost printed card is +retired and reprinted in one step. Its sign-in methods page shows what is +linked and links back to the application. It adds no breaking change. + +Before it, [v0.31.0](v0.31.0.md) was a security release: only owners manage admins, return addresses with dot segments are refused, and logo, website and avatar URLs must be https when written; URLs already stored are not changed. It is breaking; read its upgrade notes. diff --git a/docs/releases/v0.32.0.md b/docs/releases/v0.32.0.md new file mode 100644 index 00000000..2cf716eb --- /dev/null +++ b/docs/releases/v0.32.0.md @@ -0,0 +1,60 @@ +# v0.32.0 + +**Release Date:** 2026-11-05 + +A manager can now replace a managed principal's printed card in one step, +and the sign-in methods page becomes a page an application can send people +to from its own settings. + +- **Replace login tokens.** `managedprincipal.Service.ReplaceLoginTokens` + revokes every live login token of a managed principal and issues one new + token, in one transaction. It takes the same input as `IssueLoginToken` + (name, single use, expiry) and is authorized the same way, by + `manage_credentials`. If issuing fails, nothing is revoked, so a failure + leaves the old card working rather than none. Two replaces at once on + PostgreSQL leave one token. Sessions already signed in with an old token + keep running; `RevokeAll` ends them too. +- **Over HTTP.** `managedprincipal.NewHandler` serves it as + `POST /{principalID}/login-tokens/replace`, with the issue body. It answers + 201 with the new token and `revoked`, the number of tokens it revoked. + `mpclient.Client.ReplaceLoginTokens` calls it. +- **Audited once.** A replace emits `managed_principal.login_tokens_replaced` + with the new token's identifiers and the number revoked. +- **Checked under the lock.** Issuing and replacing now check the issuer's + authority under the managed principal's row lock, in the transaction that + issues, as removing a manager does. A manager removed at the same moment + can no longer be issued a token, which `IssueLoginToken` previously + allowed. + +Tokens are stored only as hashes, so a lost or shared card cannot be printed +again from what is stored; replacing it is how a card is reprinted. + +## Sign-in methods + +- **What is linked.** `/account/sign-in-methods` shows the account's email + and the providers already linked to it, and offers a **Link** button only + for the others. Directories list linked providers through the optional + `LinkedProviderLister` interface, which the Ent and memory directories + implement; with another directory the page offers every provider, as + before. +- **Back to the application.** An application links to the page with + `return_to`, such as + `https://auth.example.com/account/sign-in-methods?return_to=https://app.example.com/settings`. + It passes the same allowlist as sign-in's `return_to` (a path on the + issuer, or an origin in `allowed_redirect_origins`) and shows a + **Back to** link, named by `signed_out_name` or else its host. It is kept + through signing in first, the link button, the provider and the return to + the page, and validated again at each step. An invalid `return_to` is + ignored. + +The release adds no breaking change. + +## Upgrading + +Nothing is required. Applications that print login cards can offer "Replace +card" with the new call instead of revoking each token and issuing a new +one. Applications can link their account settings to +`/account/sign-in-methods?return_to=`. + +See [Replacing a Card](../identity/managed-principals.md#replacing-a-card) and +[Linking sign-in methods](../systemauth/social-login.md#linking-sign-in-methods). diff --git a/docs/specs/initiatives/INIT-SYSTEMFORGE-007/ROADMAP.md b/docs/specs/initiatives/INIT-SYSTEMFORGE-007/ROADMAP.md index 6d435c23..f4ede675 100644 --- a/docs/specs/initiatives/INIT-SYSTEMFORGE-007/ROADMAP.md +++ b/docs/specs/initiatives/INIT-SYSTEMFORGE-007/ROADMAP.md @@ -51,5 +51,5 @@ implementing an item carry `Refs: RMI-SYSTEMFORGE-`. **Theme:** A lost card is retired and replaced in one step -- [ ] `RMI-SYSTEMFORGE-222` Replace a managed principal's login tokens in one call: revoke every live token and issue one, atomically, in the service, the handler and `mpclient` +- [x] `RMI-SYSTEMFORGE-222` Replace a managed principal's login tokens in one call: revoke every live token and issue one, atomically, in the service, the handler and `mpclient` - Depends on: `RMI-SYSTEMFORGE-108` diff --git a/docs/systemauth/social-login.md b/docs/systemauth/social-login.md index d19d489d..d308adae 100644 --- a/docs/systemauth/social-login.md +++ b/docs/systemauth/social-login.md @@ -18,7 +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 /account/sign-in-methods?return_to=...` | A signed-in person's page: their account's email, the sign-in methods linked to it, a button to link each other provider, and a link back to the validated `return_to` (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`). | @@ -197,8 +197,9 @@ GitHub sign-in uses the account's primary email only. 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 +whatever email each provider reports. The page shows the account's email +and the providers already linked to it, and a **Link** button for each +configured provider that is not. 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: @@ -212,6 +213,21 @@ sign-in. The callback then attaches that provider identity to the account: - No account is created, no sign-up gate or age question applies, and the account's own email is unchanged. +An application links to the page from its own account settings with +`return_to`, such as +`https://auth.example.com/account/sign-in-methods?return_to=https://app.example.com/settings`. +`return_to` passes the same allowlist as sign-in's: a path on the issuer, or +an origin in `allowed_redirect_origins`. A valid one shows a **Back to** +link named by `signed_out_name`, or by its host when that is not set, and is +kept through signing in first, the link button, the provider and the +callback's return to the page; it is validated again at each step. An +invalid one is ignored: the page shows no back link and never redirects to +it. + +A directory lists the linked providers through the optional +`LinkedProviderLister` interface, which the Ent and memory directories +implement. With a directory that does not, the page offers every provider. + 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 diff --git a/identity/managedprincipal/handler.go b/identity/managedprincipal/handler.go index 481d0240..310f80da 100644 --- a/identity/managedprincipal/handler.go +++ b/identity/managedprincipal/handler.go @@ -59,6 +59,7 @@ type LoginTokenView struct { // // GET /{principalID}/login-tokens // POST /{principalID}/login-tokens issue; the caller is an active manager +// POST /{principalID}/login-tokens/replace revoke every live token and issue one, atomically // DELETE /{principalID}/login-tokens/{credentialID} // GET /{principalID}/sessions // DELETE /{principalID}/sessions/{kind}/{sessionID} @@ -87,6 +88,7 @@ func NewHandler(svc *Service, opts ...HandlerOption) http.Handler { r.Post("/{principalID}/claims", h.issueClaim) r.Get("/{principalID}/login-tokens", h.listLoginTokens) r.Post("/{principalID}/login-tokens", h.issueLoginToken) + r.Post("/{principalID}/login-tokens/replace", h.replaceLoginTokens) r.Delete("/{principalID}/login-tokens/{credentialID}", h.revokeLoginToken) r.Get("/{principalID}/sessions", h.listSessions) r.Delete("/{principalID}/sessions/{kind}/{sessionID}", h.revokeSession) diff --git a/identity/managedprincipal/handler_remote.go b/identity/managedprincipal/handler_remote.go index b101e594..0cde4d6e 100644 --- a/identity/managedprincipal/handler_remote.go +++ b/identity/managedprincipal/handler_remote.go @@ -156,29 +156,64 @@ type IssuedLoginToken struct { ExpiresAt *time.Time `json:"expires_at,omitempty"` } +// ReplacedLoginTokens is the response to POST +// /{principalID}/login-tokens/replace: the new token, whose value is +// returned only here, and how many live tokens were revoked. +type ReplacedLoginTokens struct { + IssuedLoginToken + Revoked int `json:"revoked"` +} + func (h *handler) issueLoginToken(w http.ResponseWriter, r *http.Request) { - actor, managed, ok := h.ids(w, r) + in, ok := h.loginTokenInput(w, r) if !ok { return } - var req IssueLoginTokenRequest - if !h.decode(w, r, &req) { + tok, err := h.svc.IssueLoginToken(r.Context(), in) + if err != nil { + h.fail(w, r, err) return } - clientID, ok := h.client(w, r, req.ClientID) + h.writeCreated(w, r, issuedLoginToken(tok)) +} + +func (h *handler) replaceLoginTokens(w http.ResponseWriter, r *http.Request) { + in, ok := h.loginTokenInput(w, r) if !ok { return } - tok, err := h.svc.IssueLoginToken(r.Context(), IssueLoginTokenInput{ - ManagedID: managed, ClientID: clientID, IssuedBy: actor, - ExpiresAt: req.ExpiresAt, SingleUse: req.SingleUse, Name: req.Name, - }) + out, err := h.svc.ReplaceLoginTokens(r.Context(), in) if err != nil { h.fail(w, r, err) return } - h.writeCreated(w, r, IssuedLoginToken{ + h.writeCreated(w, r, ReplacedLoginTokens{IssuedLoginToken: issuedLoginToken(out.Token), Revoked: out.Revoked}) +} + +// loginTokenInput reads an IssueLoginTokenRequest, which issuing and +// replacing share. On failure it has written the response. +func (h *handler) loginTokenInput(w http.ResponseWriter, r *http.Request) (IssueLoginTokenInput, bool) { + actor, managed, ok := h.ids(w, r) + if !ok { + return IssueLoginTokenInput{}, false + } + var req IssueLoginTokenRequest + if !h.decode(w, r, &req) { + return IssueLoginTokenInput{}, false + } + clientID, ok := h.client(w, r, req.ClientID) + if !ok { + return IssueLoginTokenInput{}, false + } + return IssueLoginTokenInput{ + ManagedID: managed, ClientID: clientID, IssuedBy: actor, + ExpiresAt: req.ExpiresAt, SingleUse: req.SingleUse, Name: req.Name, + }, true +} + +func issuedLoginToken(tok *credential.GeneratedLoginToken) IssuedLoginToken { + return IssuedLoginToken{ ID: tok.Credential.ID, Token: tok.PlainToken, Prefix: tok.Prefix, SingleUse: credential.LoginTokenSingleUse(tok.Credential), ExpiresAt: tok.Credential.ExpiresAt, - }) + } } diff --git a/identity/managedprincipal/managedprincipal.go b/identity/managedprincipal/managedprincipal.go index 73e8dea4..70727844 100644 --- a/identity/managedprincipal/managedprincipal.go +++ b/identity/managedprincipal/managedprincipal.go @@ -128,3 +128,11 @@ type IssueLoginTokenInput struct { // Name optionally labels the token, such as the card or device. Name string } + +// LoginTokenReplacement is the result of ReplaceLoginTokens: the new token, +// whose plain value is available only here, and how many live tokens were +// revoked. +type LoginTokenReplacement struct { + Token *credential.GeneratedLoginToken + Revoked int +} diff --git a/identity/managedprincipal/mpclient/mpclient.go b/identity/managedprincipal/mpclient/mpclient.go index 086d0240..a37ba8dd 100644 --- a/identity/managedprincipal/mpclient/mpclient.go +++ b/identity/managedprincipal/mpclient/mpclient.go @@ -124,6 +124,13 @@ type IssuedLoginToken struct { ExpiresAt *time.Time `json:"expires_at,omitempty"` } +// ReplacedLoginTokens is the result of ReplaceLoginTokens: the new token, +// returned only once, and how many live tokens were revoked. +type ReplacedLoginTokens struct { + IssuedLoginToken + Revoked int `json:"revoked"` +} + // LoginToken is a live login token, without its value. type LoginToken struct { ID uuid.UUID `json:"id"` @@ -235,6 +242,17 @@ func (c *Client) IssueLoginToken(ctx context.Context, token string, id uuid.UUID return &out, nil } +// ReplaceLoginTokens revokes every live login token of a managed principal +// and issues one, atomically: a lost or shared card is retired and a new +// one printed in one call. If the call fails, the old tokens still work. +func (c *Client) ReplaceLoginTokens(ctx context.Context, token string, id uuid.UUID, in IssueLoginTokenInput) (*ReplacedLoginTokens, error) { + var out ReplacedLoginTokens + if err := c.do(ctx, token, http.MethodPost, "/"+id.String()+"/login-tokens/replace", in, &out); err != nil { + return nil, err + } + return &out, nil +} + // ListLoginTokens returns a managed principal's live login tokens. func (c *Client) ListLoginTokens(ctx context.Context, token string, id uuid.UUID) ([]LoginToken, error) { var out struct { diff --git a/identity/managedprincipal/mpclient/mpclient_test.go b/identity/managedprincipal/mpclient/mpclient_test.go index a1eb32ba..247202a8 100644 --- a/identity/managedprincipal/mpclient/mpclient_test.go +++ b/identity/managedprincipal/mpclient/mpclient_test.go @@ -115,7 +115,14 @@ func TestClientManagesAPrincipal(t *testing.T) { if err != nil || len(tokens) != 1 || tokens[0].ID != issued.ID || tokens[0].Name != "card" { t.Fatalf("tokens %+v %v", tokens, err) } - if err := f.api.RevokeLoginToken(ctx, teacherTok, id, issued.ID); err != nil { + replaced, err := f.api.ReplaceLoginTokens(ctx, teacherTok, id, mpclient.IssueLoginTokenInput{Name: "new card"}) + if err != nil || replaced.Revoked != 1 || replaced.Token == "" || replaced.ID == issued.ID { + t.Fatalf("replace %+v %v", replaced, err) + } + if err := f.api.RevokeLoginToken(ctx, teacherTok, id, issued.ID); err == nil { + t.Fatal("revoked a token the replace had already revoked") + } + if err := f.api.RevokeLoginToken(ctx, teacherTok, id, replaced.ID); err != nil { t.Fatal(err) } if sessions, err := f.api.ListSessions(ctx, teacherTok, id); err != nil || len(sessions) != 0 { diff --git a/identity/managedprincipal/replace_test.go b/identity/managedprincipal/replace_test.go new file mode 100644 index 00000000..77601c9a --- /dev/null +++ b/identity/managedprincipal/replace_test.go @@ -0,0 +1,266 @@ +package managedprincipal + +import ( + "context" + "errors" + "net/http" + "sync" + "testing" + "time" + + "github.com/google/uuid" + + "github.com/plexusone/systemforge/identity/credential" + "github.com/plexusone/systemforge/identity/ent" + "github.com/plexusone/systemforge/identity/ent/hook" + "github.com/plexusone/systemforge/identity/ent/managerrelationship" +) + +func TestReplaceLoginTokens(t *testing.T) { + ctx := context.Background() + f := newVisibilityFixture(t) + second, err := f.svc.IssueLoginToken(ctx, IssueLoginTokenInput{ManagedID: f.learner, ClientID: client, IssuedBy: f.teacher, Name: "spare"}) + if err != nil { + t.Fatal(err) + } + + // A guardian replaces the card the educator issued, and the spare. + got, err := f.svc.ReplaceLoginTokens(ctx, IssueLoginTokenInput{ManagedID: f.learner, ClientID: client, IssuedBy: f.parent, Name: "new card"}) + if err != nil { + t.Fatalf("ReplaceLoginTokens: %v", err) + } + if got.Revoked != 2 || got.Token == nil || got.Token.PlainToken == "" { + t.Fatalf("replaced %+v", got) + } + for _, old := range []string{f.plain, second.PlainToken} { + if _, err := f.svc.creds.VerifyLoginToken(ctx, old, ""); err == nil { + t.Error("a replaced token still verifies") + } + } + if _, err := f.svc.creds.VerifyLoginToken(ctx, got.Token.PlainToken, ""); err != nil { + t.Errorf("the new token does not verify: %v", err) + } + tokens, err := f.svc.ListLoginTokens(ctx, f.teacher, f.learner) + if err != nil || len(tokens) != 1 || tokens[0].ID != got.Token.Credential.ID || tokens[0].Name != "new card" { + t.Fatalf("tokens after replace = %+v, %v", tokens, err) + } + // Sessions signed in with an old token are left to RevokeAll. + if sessions, err := f.svc.ListSessions(ctx, f.teacher, f.learner); err != nil || len(sessions) != 3 { + t.Errorf("sessions after replace = %d, %v", len(sessions), err) + } + + var ev map[string]any + for _, e := range f.rec.events { + if e.EventType == EventTokensReplaced { + ev = e.Details + } + } + if ev == nil || ev["revoked"] != 2 || ev["credential_id"] != got.Token.Credential.ID.String() { + t.Errorf("replace audit details = %v", ev) + } + + // With no live token, a replace just issues one, single-use with its + // shorter default lifetime. + if err := f.svc.RevokeLoginToken(ctx, f.parent, f.learner, got.Token.Credential.ID); err != nil { + t.Fatal(err) + } + before := time.Now() + once, err := f.svc.ReplaceLoginTokens(ctx, IssueLoginTokenInput{ManagedID: f.learner, ClientID: client, IssuedBy: f.parent, SingleUse: true}) + if err != nil { + t.Fatal(err) + } + exp := once.Token.Credential.ExpiresAt + if once.Revoked != 0 || exp == nil || exp.After(before.Add(21*time.Minute)) { + t.Errorf("single-use replace = revoked %d, expires %v", once.Revoked, exp) + } + at := time.Now().Add(7 * 24 * time.Hour) + week, err := f.svc.ReplaceLoginTokens(ctx, IssueLoginTokenInput{ManagedID: f.learner, ClientID: client, IssuedBy: f.parent, ExpiresAt: &at}) + if err != nil { + t.Fatal(err) + } + if week.Revoked != 1 || !week.Token.Credential.ExpiresAt.Equal(at) { + t.Errorf("replace with an expiry = revoked %d, expires %v, want %v", week.Revoked, week.Token.Credential.ExpiresAt, at) + } +} + +func TestReplaceLoginTokensFailureKeepsTheOldTokens(t *testing.T) { + ctx := context.Background() + f := newVisibilityFixture(t) + errCreate := errors.New("create refused") + revokedFirst := false + f.db.Credential.Use(func(next ent.Mutator) ent.Mutator { + return hook.CredentialFunc(func(ctx context.Context, m *ent.CredentialMutation) (ent.Value, error) { + if m.Op().Is(ent.OpCreate) { + // The old token is revoked in the transaction before the + // new one is created; the rollback undoes it. + live, err := m.Client().Credential.Query().Where(liveLoginTokens(f.learner, time.Now())...).Count(ctx) + if err != nil { + return nil, err + } + revokedFirst = live == 0 + return nil, errCreate + } + return next.Mutate(ctx, m) + }) + }) + if _, err := f.svc.ReplaceLoginTokens(ctx, IssueLoginTokenInput{ManagedID: f.learner, ClientID: client, IssuedBy: f.teacher}); !errors.Is(err, errCreate) { + t.Fatalf("ReplaceLoginTokens = %v, want the create's error", err) + } + if !revokedFirst { + t.Error("the old token was still live in the transaction when the new one was created") + } + if _, err := f.svc.creds.VerifyLoginToken(ctx, f.plain, ""); err != nil { + t.Errorf("the old token stopped working after a failed replace: %v", err) + } + for _, typ := range f.rec.types() { + if typ == EventTokensReplaced { + t.Error("a failed replace was audited as done") + } + } +} + +func TestReplaceLoginTokensRefusals(t *testing.T) { + ctx := context.Background() + f := newVisibilityFixture(t) + for _, tc := range []struct { + name string + in IssueLoginTokenInput + want error + }{ + {"not a manager", IssueLoginTokenInput{ManagedID: f.learner, ClientID: client, IssuedBy: f.stranger}, ErrNotManager}, + {"client not allowed", IssueLoginTokenInput{ManagedID: f.learner, ClientID: "other", IssuedBy: f.teacher}, ErrNotAllowed}, + {"unknown principal", IssueLoginTokenInput{ManagedID: uuid.New(), ClientID: client, IssuedBy: f.teacher}, ErrNotManaged}, + } { + t.Run(tc.name, func(t *testing.T) { + if _, err := f.svc.ReplaceLoginTokens(ctx, tc.in); !errors.Is(err, tc.want) { + t.Errorf("ReplaceLoginTokens = %v, want %v", err, tc.want) + } + }) + } + if _, err := f.svc.creds.VerifyLoginToken(ctx, f.plain, ""); err != nil { + t.Errorf("a refused replace revoked the token: %v", err) + } +} + +func TestRemoteReplaceLoginTokens(t *testing.T) { + db := openSQLite(t) + svc, _ := newService(t, db) + c := remoteCaller{t, NewHandler(svc, WithRequestClient(RequestClientFromPrincipal))} + teacher := addVerifiedHuman(t, db, "teacher@example.com") + outsider := addVerifiedHuman(t, db, "outsider@example.com") + learner, err := svc.Create(context.Background(), CreateInput{ClientID: client, DisplayName: "Robin", + Managers: []ManagerInput{{PrincipalID: teacher, Role: RoleEducator}}, ActorID: teacher}) + if err != nil { + t.Fatal(err) + } + path := "/" + learner.String() + "/login-tokens/replace" + + expectStatus(t, c.do(uuid.Nil, client, http.MethodPost, path, IssueLoginTokenRequest{}), http.StatusUnauthorized) + expectStatus(t, c.do(outsider, client, http.MethodPost, path, IssueLoginTokenRequest{}), http.StatusForbidden) + past := time.Now().Add(-time.Hour) + expectStatus(t, c.do(teacher, client, http.MethodPost, path, IssueLoginTokenRequest{ExpiresAt: &past}), http.StatusBadRequest) + + w := c.do(teacher, client, http.MethodPost, "/"+learner.String()+"/login-tokens", IssueLoginTokenRequest{Name: "card"}) + expectStatus(t, w, http.StatusCreated) + var old IssuedLoginToken + decodeBody(t, w, &old) + + w = c.do(teacher, client, http.MethodPost, path, IssueLoginTokenRequest{Name: "new card"}) + expectStatus(t, w, http.StatusCreated) + var got ReplacedLoginTokens + decodeBody(t, w, &got) + if got.Revoked != 1 || got.Token == "" || got.ID == old.ID { + t.Fatalf("replaced %+v", got) + } + if _, err := svc.creds.VerifyLoginToken(context.Background(), old.Token, ""); err == nil { + t.Error("the old token still verifies") + } + if _, err := svc.creds.VerifyLoginToken(context.Background(), got.Token, ""); err != nil { + t.Errorf("the new token does not verify: %v", err) + } +} + +// racingGate stands in for a manager's removal committing between the +// client check and the issuing transaction: it runs removed when the token +// policy is read, which happens in between. +type racingGate struct { + gate + removed func() +} + +func (g racingGate) LoginTokenPolicy(ctx context.Context, clientID string) (credential.LoginTokenPolicy, error) { + g.removed() + return g.gate.LoginTokenPolicy(ctx, clientID) +} + +// TestIssuingRechecksTheIssuerUnderTheLock verifies that a manager removed +// after the client check but before the issuing transaction is refused, +// for both issuing and replacing, and that a refused replace keeps the +// principal's token. +func TestIssuingRechecksTheIssuerUnderTheLock(t *testing.T) { + ctx := context.Background() + f := newVisibilityFixture(t) + remove := func() { + if err := f.db.ManagerRelationship.Update(). + Where(managerrelationship.ManagedPrincipalID(f.learner), managerrelationship.ManagerPrincipalID(f.parent)). + SetActive(false).Exec(ctx); err != nil { + t.Fatalf("deactivate relationship: %v", err) + } + } + svc := NewService(f.db, racingGate{gate: gate{client: true}, removed: remove}, WithDirectManagerAdds()) + in := IssueLoginTokenInput{ManagedID: f.learner, ClientID: client, IssuedBy: f.parent} + if _, err := svc.IssueLoginToken(ctx, in); !errors.Is(err, ErrNotManager) { + t.Errorf("IssueLoginToken by a manager removed meanwhile = %v, want %v", err, ErrNotManager) + } + if _, err := svc.ReplaceLoginTokens(ctx, in); !errors.Is(err, ErrNotManager) { + t.Errorf("ReplaceLoginTokens by a manager removed meanwhile = %v, want %v", err, ErrNotManager) + } + if _, err := f.svc.creds.VerifyLoginToken(ctx, f.plain, ""); err != nil { + t.Errorf("a refused replace revoked the token: %v", err) + } +} + +// TestConcurrentReplacesLeaveOneToken verifies on PostgreSQL that two +// managers replacing a principal's card at the same moment both succeed and +// leave exactly one live token: the second waits for the first's lock and +// revokes its token. +func TestConcurrentReplacesLeaveOneToken(t *testing.T) { + ctx := context.Background() + db := openPostgres(t) + svc, _ := newService(t, db) + for range 10 { + a := addHuman(t, db, uuid.NewString()+"@example.com", true) + b := addHuman(t, db, uuid.NewString()+"@example.com", true) + id, err := svc.Create(ctx, CreateInput{ActorID: a, DisplayName: "Learner", ClientID: client, + Managers: []ManagerInput{{a, RoleGuardian}, {b, RoleEducator}}}) + if err != nil { + t.Fatalf("Create: %v", err) + } + if _, err := svc.IssueLoginToken(ctx, IssueLoginTokenInput{ManagedID: id, ClientID: client, IssuedBy: a}); err != nil { + t.Fatalf("IssueLoginToken: %v", err) + } + var wg sync.WaitGroup + start := make(chan struct{}) + errs := make([]error, 2) + for i, by := range []uuid.UUID{a, b} { + wg.Go(func() { + <-start + _, errs[i] = svc.ReplaceLoginTokens(ctx, IssueLoginTokenInput{ManagedID: id, ClientID: client, IssuedBy: by}) + }) + } + close(start) + wg.Wait() + for i, err := range errs { + if err != nil { + t.Fatalf("replace %d: %v", i, err) + } + } + live, err := db.Credential.Query().Where(liveLoginTokens(id, time.Now())...).Count(ctx) + if err != nil { + t.Fatal(err) + } + if live != 1 { + t.Fatalf("live tokens after two concurrent replaces = %d, want 1", live) + } + } +} diff --git a/identity/managedprincipal/service.go b/identity/managedprincipal/service.go index 04818e9b..d38c1bee 100644 --- a/identity/managedprincipal/service.go +++ b/identity/managedprincipal/service.go @@ -33,6 +33,9 @@ const ( EventManagerAdded = "managed_principal.manager_added" EventManagerRemoved = "managed_principal.manager_removed" EventTokenIssued = "managed_principal.login_token_issued" + // EventTokensReplaced records a replace: live login tokens revoked and + // one issued, with the number revoked. + EventTokensReplaced = "managed_principal.login_tokens_replaced" // EventReleased records that a principal's management ended. EventReleased = "managed_principal.released" ) @@ -377,61 +380,153 @@ func (s *Service) RemoveManager(ctx context.Context, managedID uuid.UUID, m Mana // IssueLoginToken issues a login token for a managed principal on behalf // of one of its active managers holding manage_credentials, which // guardians and educators do. The plain token is returned once. +// The issuer's authority is checked under the principal's lock, in the +// transaction that issues, so a manager removed at the same moment cannot +// be issued a token. func (s *Service) IssueLoginToken(ctx context.Context, in IssueLoginTokenInput) (*credential.GeneratedLoginToken, error) { - allowed, err := s.gate.ManagedPrincipalsAllowed(ctx, in.ClientID) + issue, clamped, err := s.prepareLoginToken(ctx, in) if err != nil { - return nil, fmt.Errorf("managedprincipal: check client: %w", err) + return nil, err } - if !allowed { - return nil, ErrNotAllowed + var tok *credential.GeneratedLoginToken + err = s.withTx(ctx, func(tx *ent.Tx) error { + if err := s.lockAndAuthorizeIssuer(ctx, tx, in); err != nil { + return err + } + tok, err = credential.NewService(tx.Client()).IssueLoginToken(ctx, issue) + if err != nil { + return fmt.Errorf("managedprincipal: %w", err) + } + return nil + }) + if err != nil { + return nil, err } - if err := s.checkIssuer(ctx, in.ManagedID, in.IssuedBy); err != nil { + s.emit(ctx, EventTokenIssued, "issue_login_token", in.IssuedBy, in.ManagedID, issuedDetails(in, issue.ExpiresAt, tok, clamped)) + return tok, nil +} + +// ReplaceLoginTokens revokes every live login token of a managed principal +// and issues one new token, in one transaction, so a lost or shared card +// is retired and a new one printed in one step. It is authorized like +// IssueLoginToken, under the same lock. If issuing fails nothing is revoked, so a failure never +// leaves the principal with no working token. Sessions already signed in +// with an old token keep running; RevokeAll ends them. +func (s *Service) ReplaceLoginTokens(ctx context.Context, in IssueLoginTokenInput) (*LoginTokenReplacement, error) { + issue, clamped, err := s.prepareLoginToken(ctx, in) + if err != nil { return nil, err } - // A login token signs the principal in, so issuing one is a credential - // action: an active manager whose role lacks manage_credentials, such - // as an application-defined role, cannot issue one. - if err := s.authorizeCredentials(ctx, in.IssuedBy, in.ManagedID); err != nil { + out := &LoginTokenReplacement{} + err = s.withTx(ctx, func(tx *ent.Tx) error { + // Two replaces of one principal at once would each leave a token; + // the principal's lock makes the second wait and revoke the first's. + // The locks are taken in RemoveManager's order: the row, then the + // graph. + if err := s.lockAndAuthorizeIssuer(ctx, tx, in); err != nil { + return err + } + if err := graphlock.Lock(ctx, tx, graphlock.Principal(in.ManagedID)); err != nil { + return err + } + n, err := tx.Credential.Update(). + Where(liveLoginTokens(in.ManagedID, s.now())...). + SetRevoked(true). + SetRevokedAt(s.now()). + SetRevokedReason(revokedByReplace). + SetActive(false). + Save(ctx) + if err != nil { + return fmt.Errorf("managedprincipal: revoke login tokens: %w", err) + } + out.Revoked = n + out.Token, err = credential.NewService(tx.Client()).IssueLoginToken(ctx, issue) + if err != nil { + return fmt.Errorf("managedprincipal: %w", err) + } + return nil + }) + if err != nil { return nil, err } + details := issuedDetails(in, issue.ExpiresAt, out.Token, clamped) + details["revoked"] = out.Revoked + s.emit(ctx, EventTokensReplaced, "replace_login_tokens", in.IssuedBy, in.ManagedID, details) + return out, nil +} + +// prepareLoginToken checks that in.ClientID may issue login tokens, and +// resolves the token's expiry. The issuer's authority is checked later, +// under the principal's lock (lockAndAuthorizeIssuer). clamped reports that +// the expiry was cut to the platform ceiling. +func (s *Service) prepareLoginToken(ctx context.Context, in IssueLoginTokenInput) (credential.IssueLoginTokenInput, bool, error) { + var none credential.IssueLoginTokenInput + allowed, err := s.gate.ManagedPrincipalsAllowed(ctx, in.ClientID) + if err != nil { + return none, false, fmt.Errorf("managedprincipal: check client: %w", err) + } + if !allowed { + return none, false, ErrNotAllowed + } policy, err := s.gate.LoginTokenPolicy(ctx, in.ClientID) if err != nil { - return nil, fmt.Errorf("managedprincipal: login token policy: %w", err) + return none, false, fmt.Errorf("managedprincipal: login token policy: %w", err) } expiresAt, clamped, err := policy.ExpiresAt(s.now(), in.ExpiresAt, in.SingleUse) if err != nil { - return nil, err + return none, false, err } if clamped { s.logger.Info("managedprincipal: login token lifetime clamped to the platform ceiling", "principal_id", in.ManagedID, "client_id", in.ClientID, "expires_at", expiresAt) } - - tok, err := s.creds.IssueLoginToken(ctx, credential.IssueLoginTokenInput{ + return credential.IssueLoginTokenInput{ PrincipalID: in.ManagedID, IssuedBy: in.IssuedBy, ExpiresAt: expiresAt, SingleUse: in.SingleUse, Name: in.Name, - }) - if err != nil { - return nil, fmt.Errorf("managedprincipal: %w", err) + }, clamped, nil +} + +// lockAndAuthorizeIssuer takes the managed principal's row lock, as +// RemoveManager does, and then requires in.IssuedBy to be one of its +// active managers holding manage_credentials, reading in tx. A removal of +// that manager has committed, or has not started, by the time it is read. +func (s *Service) lockAndAuthorizeIssuer(ctx context.Context, tx *ent.Tx, in IssueLoginTokenInput) error { + if _, err := lockHuman(ctx, tx, in.ManagedID); err != nil { + if errors.Is(err, ErrNotFound) { + return ErrNotManaged + } + return err } - s.emit(ctx, EventTokenIssued, "issue_login_token", in.IssuedBy, in.ManagedID, map[string]any{ + c := tx.Client() + if err := s.checkIssuer(ctx, c, in.ManagedID, in.IssuedBy); err != nil { + return err + } + // A login token signs the principal in, so issuing one is a credential + // action: an active manager whose role lacks manage_credentials, such + // as an application-defined role, cannot issue one. The authorizer + // reads manager roles in tx (ManagerRoles). + return s.authorizeCredentialsIn(context.WithValue(ctx, txContextKey{}, tx), c, in.IssuedBy, in.ManagedID) +} + +// issuedDetails is the audit detail of an issued login token. +func issuedDetails(in IssueLoginTokenInput, expiresAt time.Time, tok *credential.GeneratedLoginToken, clamped bool) map[string]any { + return map[string]any{ "client_id": in.ClientID, "credential_id": tok.Credential.ID.String(), "prefix": tok.Prefix, "expires_at": expiresAt, "single_use": in.SingleUse, "clamped": clamped, - }) - return tok, nil + } } // checkIssuer requires an active managed principal and an issuer that is // one of its active managers and still a valid manager. -func (s *Service) checkIssuer(ctx context.Context, managedID, issuerID uuid.UUID) error { - ok, err := s.client.Human.Query(). +func (s *Service) checkIssuer(ctx context.Context, c *ent.Client, managedID, issuerID uuid.UUID) error { + ok, err := c.Human.Query(). Where( human.PrincipalID(managedID), hasActiveManager(), @@ -444,7 +539,7 @@ func (s *Service) checkIssuer(ctx context.Context, managedID, issuerID uuid.UUID if !ok { return ErrNotManaged } - ok, err = s.client.ManagerRelationship.Query(). + ok, err = c.ManagerRelationship.Query(). Where( managerrelationship.ManagedPrincipalID(managedID), managerrelationship.ManagerPrincipalID(issuerID), diff --git a/identity/managedprincipal/visibility.go b/identity/managedprincipal/visibility.go index f45fcbc8..96f4ed10 100644 --- a/identity/managedprincipal/visibility.go +++ b/identity/managedprincipal/visibility.go @@ -26,6 +26,7 @@ const ( EventAllRevoked = "managed_principal.all_revoked" revokedByManager = "revoked by manager" + revokedByReplace = "replaced by manager" revokedAllByManager = "all revoked by manager" revokedDeleted = "principal deleted" ) @@ -62,7 +63,13 @@ type Session struct { // authorizeCredentials requires actorID to hold manage_credentials on a // managed principal. func (s *Service) authorizeCredentials(ctx context.Context, actorID, managedID uuid.UUID) error { - managed, err := s.client.Human.Query(). + return s.authorizeCredentialsIn(ctx, s.client, actorID, managedID) +} + +// authorizeCredentialsIn is authorizeCredentials reading through c, such +// as a transaction's client. +func (s *Service) authorizeCredentialsIn(ctx context.Context, c *ent.Client, actorID, managedID uuid.UUID) error { + managed, err := c.Human.Query(). Where(human.PrincipalID(managedID), hasActiveManager()). Exist(ctx) if err != nil { diff --git a/identity/systemauth/social_directory.go b/identity/systemauth/social_directory.go index 0a431445..e9cb17ea 100644 --- a/identity/systemauth/social_directory.go +++ b/identity/systemauth/social_directory.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "slices" "strings" "sync" "time" @@ -81,6 +82,33 @@ type PrincipalDirectory interface { GetPrincipal(ctx context.Context, id uuid.UUID) (*HumanPrincipal, error) } +// LinkedProviderLister is an optional PrincipalDirectory capability: it +// lists the providers whose sign-in is linked to a principal, so the +// sign-in methods page can show them. A directory without it gets a page +// that offers every provider. +type LinkedProviderLister interface { + // LinkedProviders returns the distinct providers linked to principalID, + // sorted, or none. + LinkedProviders(ctx context.Context, principalID uuid.UUID) ([]string, error) +} + +// providersLinkedTo returns the sorted, distinct providers of the external +// identities in byExternal that belong to id. +func providersLinkedTo(byExternal map[string]uuid.UUID, id uuid.UUID) []string { + var out []string + for key, owner := range byExternal { + if owner != id { + continue + } + provider, _, _ := strings.Cut(key, "\x00") + if !slices.Contains(out, provider) { + out = append(out, provider) + } + } + slices.Sort(out) + return out +} + // ResolveExternalLogin finds or creates the principal for an upstream login // using the canonical account-linking model: // @@ -323,6 +351,13 @@ func (d *MemoryPrincipalDirectory) RecordLogin(_ context.Context, principalID uu return nil } +// LinkedProviders implements LinkedProviderLister. +func (d *MemoryPrincipalDirectory) LinkedProviders(_ context.Context, principalID uuid.UUID) ([]string, error) { + d.mu.Lock() + defer d.mu.Unlock() + return providersLinkedTo(d.byExternal, principalID), nil +} + // GetPrincipal implements PrincipalDirectory. func (d *MemoryPrincipalDirectory) GetPrincipal(_ context.Context, id uuid.UUID) (*HumanPrincipal, error) { d.mu.Lock() @@ -330,7 +365,10 @@ func (d *MemoryPrincipalDirectory) GetPrincipal(_ context.Context, id uuid.UUID) return d.copyOf(id) } -var _ PrincipalDirectory = (*MemoryPrincipalDirectory)(nil) +var ( + _ PrincipalDirectory = (*MemoryPrincipalDirectory)(nil) + _ LinkedProviderLister = (*MemoryPrincipalDirectory)(nil) +) func displayNameFor(login ExternalLogin) string { if login.Name != "" { diff --git a/identity/systemauth/social_directory_ent.go b/identity/systemauth/social_directory_ent.go index d2f295d7..37c7b093 100644 --- a/identity/systemauth/social_directory_ent.go +++ b/identity/systemauth/social_directory_ent.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "log/slog" + "slices" "strings" "time" @@ -96,8 +97,9 @@ func (d *EntPrincipalDirectory) syncOrg(ctx context.Context, org *signup.Persona } var ( - _ PrincipalDirectory = (*EntPrincipalDirectory)(nil) - _ PersonalOrgEnsurer = (*EntPrincipalDirectory)(nil) + _ PrincipalDirectory = (*EntPrincipalDirectory)(nil) + _ PersonalOrgEnsurer = (*EntPrincipalDirectory)(nil) + _ LinkedProviderLister = (*EntPrincipalDirectory)(nil) ) // FindByExternalIdentity implements PrincipalDirectory. @@ -360,6 +362,20 @@ func (d *EntPrincipalDirectory) LinkExternalIdentity(ctx context.Context, princi return nil } +// LinkedProviders implements LinkedProviderLister. +func (d *EntPrincipalDirectory) LinkedProviders(ctx context.Context, principalID uuid.UUID) ([]string, error) { + providers, err := d.db.ExternalIdentity.Query(). + Where(externalidentity.PrincipalIDEQ(principalID)). + Unique(true). + Select(externalidentity.FieldProvider). + Strings(ctx) + if err != nil { + return nil, fmt.Errorf("listing linked providers: %w", err) + } + slices.Sort(providers) + return slices.Compact(providers), nil +} + // RecordLogin implements PrincipalDirectory. func (d *EntPrincipalDirectory) RecordLogin(ctx context.Context, principalID uuid.UUID, login ExternalLogin, at time.Time) error { if _, err := d.db.ExternalIdentity.Update(). diff --git a/identity/systemauth/social_directory_memory_storage.go b/identity/systemauth/social_directory_memory_storage.go index fdad8721..29ec7ae6 100644 --- a/identity/systemauth/social_directory_memory_storage.go +++ b/identity/systemauth/social_directory_memory_storage.go @@ -87,4 +87,14 @@ func (d memoryStorageDirectory) GetPrincipal(ctx context.Context, id uuid.UUID) return d.s.GetHuman(ctx, id) } -var _ PrincipalDirectory = memoryStorageDirectory{} +// LinkedProviders implements LinkedProviderLister. +func (d memoryStorageDirectory) LinkedProviders(_ context.Context, principalID uuid.UUID) ([]string, error) { + d.s.mu.RLock() + defer d.s.mu.RUnlock() + return providersLinkedTo(d.s.external, principalID), nil +} + +var ( + _ PrincipalDirectory = memoryStorageDirectory{} + _ LinkedProviderLister = memoryStorageDirectory{} +) diff --git a/identity/systemauth/social_link.go b/identity/systemauth/social_link.go index a1992b8d..aca6223b 100644 --- a/identity/systemauth/social_link.go +++ b/identity/systemauth/social_link.go @@ -6,6 +6,7 @@ import ( "html/template" "net/http" "net/url" + "slices" "strings" "github.com/go-chi/chi/v5" @@ -37,12 +38,18 @@ 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.

+{{with .Email}}

Signed in as {{.}}.

+{{end}}{{with .JustLinked}}

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

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

{{.}}

-{{end}}
+{{end}}{{if .Linked}}

Linked

+ +{{end}}{{if .Providers}}

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

+ +{{else}}

Every available sign-in method is linked to this account.

+{{end}}{{with .Back}}

Back to {{.Name}}

+{{end}} `)) // linkProblems are the page's messages for a refused link, by reason. @@ -52,36 +59,106 @@ var linkProblems = map[string]string{ "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. +// returnTo is the validated return_to: where the page's back link goes, an +// allowed redirect target, or "" for none. An invalid one is dropped, never +// followed. +func (sl *socialLogin) returnTo(raw string) string { + if raw == "" { + return "" + } + v, err := sl.redirects.validate(raw) + if err != nil { + return "" + } + return v +} + +// methodsURL is the sign-in methods page's address, keeping returnTo (already +// validated) and adding extra, such as the outcome of a link. +func methodsURL(returnTo string, extra url.Values) string { + q := url.Values{} + for k, v := range extra { + q[k] = v + } + if returnTo != "" { + q.Set("return_to", returnTo) + } + if len(q) == 0 { + return SignInMethodsPath + } + return SignInMethodsPath + "?" + q.Encode() +} + +// signInFirst sends a browser without a login session to sign in, and back +// to the page with its return_to. +func signInFirst(w http.ResponseWriter, r *http.Request, returnTo string, status int) { + http.Redirect(w, r, LoginPath+"?return_to="+url.QueryEscape(methodsURL(returnTo, nil)), status) +} + +// handleSignInMethods serves GET SignInMethodsPath: the account's email, +// the sign-in methods linked to it, a Link button for each one that is not, +// and, with a valid return_to, a link back to the app. 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) { + ctx := r.Context() noStore(w) - if _, err := sl.currentSession(r); err != nil { - http.Redirect(w, r, LoginPath+"?return_to="+url.QueryEscape(SignInMethodsPath), http.StatusFound) + q := r.URL.Query() + returnTo := sl.returnTo(q.Get("return_to")) + sess, err := sl.currentSession(r) + if err != nil { + signInFirst(w, r, returnTo, http.StatusFound) return } + var email string + if p, err := sl.directory.GetPrincipal(ctx, sess.PrincipalID); err == nil { + email = p.Email + } + var linked []string + if lister, ok := sl.directory.(LinkedProviderLister); ok { + linked, err = lister.LinkedProviders(ctx, sess.PrincipalID) + if err != nil { + LoggerFromContext(ctx).Error("listing linked sign-in methods", "error", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + } type provider struct{ Name, Action string } var providers []provider - var authorizeURLs []string + var linkedNames, authorizeURLs []string for _, p := range sl.providers { + if slices.Contains(linked, p) { + linkedNames = append(linkedNames, displayName(p)) + continue + } 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() + var justLinked string if p := q.Get("linked"); p != "" && sl.connectors[p] != nil { - linked = displayName(p) + justLinked = displayName(p) + } + type back struct{ URL, Name string } + var backLink *back + if returnTo != "" { + name := sl.redirects.signedOutName + if name == "" { + name = hostOf(returnTo, sl.redirects.issuerOrigin) + } + backLink = &back{URL: returnTo, Name: name} + } + if len(authorizeURLs) > 0 { + allowFormTargets(w, formSources(authorizeURLs...)) } - 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) + Email, JustLinked, Problem, ReturnTo string + Linked []string + Providers []provider + Back *back + }{email, justLinked, linkProblems[q.Get("error")], returnTo, linkedNames, providers, backLink}); err != nil { + LoggerFromContext(ctx).Error("rendering sign-in methods page", "error", err) } } @@ -109,9 +186,10 @@ func (sl *socialLogin) handleLinkStart(w http.ResponseWriter, r *http.Request) { http.NotFound(w, r) return } + returnTo := sl.returnTo(r.PostFormValue("return_to")) sess, err := sl.currentSession(r) if err != nil { - http.Redirect(w, r, LoginPath+"?return_to="+url.QueryEscape(SignInMethodsPath), http.StatusSeeOther) + signInFirst(w, r, returnTo, http.StatusSeeOther) return } state, err := oauthclient.GenerateState() @@ -123,7 +201,7 @@ func (sl *socialLogin) handleLinkStart(w http.ResponseWriter, r *http.Request) { verifier := oauth2.GenerateVerifier() if err := sl.states.Put(ctx, state, oauthclient.StateData{ Provider: provider, - RedirectURL: SignInMethodsPath, + RedirectURL: methodsURL(returnTo, nil), Nonce: linkNoncePrefix + sess.PrincipalID.String(), PKCEVerifier: verifier, }, loginStateTTL); err != nil { @@ -148,13 +226,19 @@ func linkingPrincipal(nonce string) (uuid.UUID, bool) { } // completeLink links login's identity to principalID, the account that -// started the link, and returns the browser to the sign-in methods page. +// started the link, and returns the browser to the sign-in methods page, +// with the return_to the link started with (page, from the login state), +// validated again. // 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) { +func (sl *socialLogin) completeLink(ctx context.Context, w http.ResponseWriter, r *http.Request, page string, principalID uuid.UUID, login ExternalLogin) { + var returnTo string + if u, err := url.Parse(page); err == nil { + returnTo = sl.returnTo(u.Query().Get("return_to")) + } back := func(param, value string) { - http.Redirect(w, r, SignInMethodsPath+"?"+param+"="+url.QueryEscape(value), http.StatusFound) + http.Redirect(w, r, methodsURL(returnTo, url.Values{param: {value}}), http.StatusFound) } sess, err := sl.currentSession(r) if err != nil || sess.PrincipalID != principalID { diff --git a/identity/systemauth/social_link_test.go b/identity/systemauth/social_link_test.go index 16590008..9b07866a 100644 --- a/identity/systemauth/social_link_test.go +++ b/identity/systemauth/social_link_test.go @@ -5,15 +5,24 @@ import ( "net/http" "net/http/httptest" "net/url" + "slices" "strings" "testing" + + "github.com/plexusone/systemforge/identity/oauthclient" ) // 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) + return linkStartWith(t, s, sess, url.Values{}) +} + +// linkStartWith is linkStart posting form, such as the page's return_to. +func linkStartWith(t *testing.T, s *Server, sess *http.Cookie, form url.Values) (string, *http.Cookie) { + t.Helper() + w := postForm(s, SignInMethodsPath+"/github", testIssuer, form, sess) if w.Code != http.StatusSeeOther { t.Fatalf("link start: %d %s", w.Code, w.Body.String()) } @@ -49,18 +58,18 @@ func TestLinkSignInMethod(t *testing.T) { t.Fatal(err) } - // The page offers each provider, styled, and lets its form reach the - // provider through the POST's redirect. + // The page, styled, shows the account and GitHub as linked; with every + // provider linked, it offers none. 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) + body := w.Body.String() + if w.Code != http.StatusOK || !strings.Contains(body, pageStyleLink) || + !strings.Contains(body, "Signed in as octo@example.com") || + !strings.Contains(body, "
  • GitHub
    • ") || strings.Contains(body, ">Link GitHub") || + !strings.Contains(body, "Every available sign-in method is linked") { + t.Fatalf("page: %d %s", w.Code, body) } // A cross-origin POST is refused. @@ -142,3 +151,158 @@ func TestLinkSignInMethodRefusals(t *testing.T) { t.Errorf("page messages: %s", body) } } + +// newTwoProviderServer is a social server with the fake GitHub and Google, +// whose authorization endpoint is never reached, and AcademyOS-like +// redirect settings. +func newTwoProviderServer(t *testing.T, f *fakeGitHub, dir PrincipalDirectory, signedOutName string) *Server { + t.Helper() + google := oauthclient.NewGoogleConnector(oauthclient.ProviderConfig{ + ClientID: "g-client", ClientSecret: "g-secret", RedirectURL: testIssuer + "/login/google/callback", + }) + social := &SocialLoginConfig{AllowedRedirectOrigins: []string{"https://app.example.com"}} + if signedOutName != "" { + social.SignedOutURL, social.SignedOutName = "https://app.example.com/", signedOutName + } + s, err := NewEmbedded(Config{Issuer: testIssuer, SocialLogin: social}, + WithSocialConnector(f.connector()), WithSocialConnector(google), WithPrincipalDirectory(dir)) + if err != nil { + t.Fatalf("NewEmbedded: %v", err) + } + return s +} + +func methodsPage(s *Server, target string, cookies ...*http.Cookie) *httptest.ResponseRecorder { + req := httptest.NewRequest(http.MethodGet, target, nil) + for _, c := range cookies { + req.AddCookie(c) + } + w := httptest.NewRecorder() + s.ServeHTTP(w, req) + return w +} + +func TestSignInMethodsPage(t *testing.T) { + const back = "https://app.example.com/settings" + f := newFakeGitHub(t) + s := newTwoProviderServer(t, f, NewMemoryPrincipalDirectory(), "") + page := SignInMethodsPath + "?return_to=" + url.QueryEscape(back) + + // Signed out, the sign-in comes back to the page with its return_to. + w := methodsPage(s, page) + if want := LoginPath + "?return_to=" + url.QueryEscape(page); w.Code != http.StatusFound || w.Header().Get("Location") != want { + t.Fatalf("signed out: %d %q, want %q", w.Code, w.Header().Get("Location"), want) + } + + // Signed in with GitHub: GitHub is listed as linked, only Google is + // offered, its form carries return_to, and the page links back. + sess := socialSession(t, s) + w = methodsPage(s, page, sess) + body := w.Body.String() + for _, want := range []string{ + "
  • GitHub
    • ", + ">Link Google", + ``, + `Back to app.example.com`, + } { + if !strings.Contains(body, want) { + t.Errorf("page lacks %q:\n%s", want, body) + } + } + if strings.Contains(body, ">Link GitHub") { + t.Errorf("page offers the linked GitHub:\n%s", body) + } + if csp := w.Header().Get("Content-Security-Policy"); !strings.Contains(csp, "https://accounts.google.com") { + t.Errorf("form-action does not allow Google: %s", csp) + } + + // A return_to outside the allowed origins is dropped, never linked. + w = methodsPage(s, SignInMethodsPath+"?return_to="+url.QueryEscape("https://evil.example.com/x"), sess) + if body := w.Body.String(); w.Code != http.StatusOK || strings.Contains(body, "evil.example.com") || strings.Contains(body, "Back to") { + t.Errorf("invalid return_to: %d %s", w.Code, body) + } + + // The site's name, when configured, names the back link. + named := newTwoProviderServer(t, f, NewMemoryPrincipalDirectory(), "Example") + if body := methodsPage(named, page, socialSession(t, named)).Body.String(); !strings.Contains(body, ">Back to Example") { + t.Errorf("named back link:\n%s", body) + } +} + +func TestLinkKeepsReturnTo(t *testing.T) { + const back = "https://app.example.com/settings" + f := newFakeGitHub(t) + s := newTwoProviderServer(t, f, NewMemoryPrincipalDirectory(), "") + sess := socialSession(t, s) + + // The link's outcome comes back to the page with return_to. + f.userID, f.email = 5555, "other@example.com" + state, stateCookie := linkStartWith(t, s, sess, url.Values{"return_to": {back}}) + w := callback(s, state, stateCookie, sess) + want := SignInMethodsPath + "?linked=github&return_to=" + url.QueryEscape(back) + if w.Code != http.StatusFound || w.Header().Get("Location") != want { + t.Errorf("link: %d %q, want %q", w.Code, w.Header().Get("Location"), want) + } + + // A refusal keeps it too. + state, stateCookie = linkStartWith(t, s, sess, url.Values{"return_to": {back}}) + w = callback(s, state, stateCookie) + if want := SignInMethodsPath + "?error=session&return_to=" + url.QueryEscape(back); w.Header().Get("Location") != want { + t.Errorf("refused: %q, want %q", w.Header().Get("Location"), want) + } + + // An invalid return_to is dropped at the start. + f.userID, f.email = 6666, "third@example.com" + state, stateCookie = linkStartWith(t, s, sess, url.Values{"return_to": {"https://evil.example.com/x"}}) + if w := callback(s, state, stateCookie, sess); w.Header().Get("Location") != SignInMethodsPath+"?linked=github" { + t.Errorf("invalid return_to: %q", w.Header().Get("Location")) + } + + // Without a session, the start sends the browser to sign in, keeping it. + w = postForm(s, SignInMethodsPath+"/github", testIssuer, url.Values{"return_to": {back}}) + page := SignInMethodsPath + "?return_to=" + url.QueryEscape(back) + if want := LoginPath + "?return_to=" + url.QueryEscape(page); w.Code != http.StatusSeeOther || w.Header().Get("Location") != want { + t.Errorf("signed out start: %d %q, want %q", w.Code, w.Header().Get("Location"), want) + } +} + +func TestLinkedProviders(t *testing.T) { + ctx := context.Background() + dirs := map[string]func(t *testing.T) PrincipalDirectory{ + "memory": func(*testing.T) PrincipalDirectory { return NewMemoryPrincipalDirectory() }, + "memory storage": func(*testing.T) PrincipalDirectory { return NewMemoryStorage().PrincipalDirectory() }, + "ent": func(t *testing.T) PrincipalDirectory { return NewEntPrincipalDirectory(openTestEnt(t)) }, + } + for name, newDir := range dirs { + t.Run(name, func(t *testing.T) { + dir := newDir(t) + lister, ok := dir.(LinkedProviderLister) + if !ok { + t.Fatal("not a LinkedProviderLister") + } + p, err := dir.CreateFromExternal(ctx, githubLogin("1", "a@example.com", true)) + if err != nil { + t.Fatal(err) + } + other, err := dir.CreateFromExternal(ctx, githubLogin("2", "b@example.com", true)) + if err != nil { + t.Fatal(err) + } + for _, login := range []ExternalLogin{ + githubLogin("3", "a@example.com", true), + {Provider: "google", Subject: "g1", Email: "a@example.com", EmailVerified: true}, + } { + if err := dir.LinkExternalIdentity(ctx, p.ID, login); err != nil { + t.Fatal(err) + } + } + got, err := lister.LinkedProviders(ctx, p.ID) + if err != nil || !slices.Equal(got, []string{"github", "google"}) { + t.Errorf("linked to the account: %v %v", got, err) + } + if got, err := lister.LinkedProviders(ctx, other.ID); err != nil || !slices.Equal(got, []string{"github"}) { + t.Errorf("linked to the other account: %v %v", got, err) + } + }) + } +} diff --git a/identity/systemauth/social_login.go b/identity/systemauth/social_login.go index b68cc3f1..4132a6ca 100644 --- a/identity/systemauth/social_login.go +++ b/identity/systemauth/social_login.go @@ -605,7 +605,7 @@ func (sl *socialLogin) handleCallback(w http.ResponseWriter, r *http.Request) { // 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{ + sl.completeLink(ctx, w, r, data.RedirectURL, principalID, ExternalLogin{ Provider: provider, Subject: user.ProviderID, Email: user.Email, diff --git a/mkdocs.yml b/mkdocs.yml index f14b432c..4e89054c 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -169,6 +169,7 @@ nav: - Releases: - Overview: releases/index.md - Changelog: releases/changelog.md + - v0.32.0: releases/v0.32.0.md - v0.31.0: releases/v0.31.0.md - v0.30.5: releases/v0.30.5.md - v0.30.4: releases/v0.30.4.md