From b179e6b2e5bbc09581eb8bb74d9652a6cb3b6ab8 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Thu, 5 Nov 2026 17:54:21 +0100 Subject: [PATCH] fix: activate-by-code asks the login gates before it signs in After the mailed code verifies, ActivateByCode consults the login gates with source password, as Login does. A gate answer (for example a second-factor challenge) replaces the token response and sets no session cookie; a gate error answers the opaque 500. The account is activated either way. Without a gate, or when every gate allows, the answer is unchanged. With require_activation off an account can enable a second factor before it activates, so the mailed code no longer stands in for it. --- README.md | 8 +- classes/login_gate.go | 11 ++- controllers/api_controller.go | 33 ++++++++- controllers/login_gates.go | 5 +- login_gate_test.go | 136 ++++++++++++++++++++++++++++++++++ 5 files changed, 181 insertions(+), 12 deletions(-) diff --git a/README.md b/README.md index 7829f60..8041c3f 100644 --- a/README.md +++ b/README.md @@ -16,9 +16,9 @@ It owns the `users` table extensions, `user_throttle`, `jwt_blacklist`, `user_ap ## Features -- **Registration and activation.** `POST /register`, `POST /activate`, `POST /activate-by-code`. Registration can be switched off and throttled; `activate_mode` is `auto`, `user` (a mailed code) or `admin`, with codes from `IssueActivationCode` and `VerifyActivationCode`. A `/register` body sets only the `User.RegisterFillable` columns: `name`, `surname`, `email`, `password` (stored as its hash), `preferred_locale` and `marketing_consent`. It cannot set `organisation_id` or `organisation_role` (membership is the administrator's or the application's), `is_onboarded`, `must_change_password`, or the activation, reset, address proof, handle and permission columns; other keys are ignored (they still reach `RegisterEvent.Payload`). `created_ip_address` and `last_ip_address` are server-set from the client address only. With `require_activation` on (the default), `POST /login` refuses an account that is not activated with the same 401 invalid-login answer as a wrong password and counts the attempt for the login throttle. Such an account is activated by the mailed code (`POST /activate-by-code` answers with a token), by the invitation link or by an administrator. `POST /activate` serves the sessions of not-activated accounts, which exist when `require_activation` is off. `Login` checks in this order: a ban or an active suspension (`RejectIfThrottled`), the password, the activation, then the deactivation restore. +- **Registration and activation.** `POST /register`, `POST /activate`, `POST /activate-by-code`. Registration can be switched off and throttled; `activate_mode` is `auto`, `user` (a mailed code) or `admin`, with codes from `IssueActivationCode` and `VerifyActivationCode`. A `/register` body sets only the `User.RegisterFillable` columns: `name`, `surname`, `email`, `password` (stored as its hash), `preferred_locale` and `marketing_consent`. It cannot set `organisation_id` or `organisation_role` (membership is the administrator's or the application's), `is_onboarded`, `must_change_password`, or the activation, reset, address proof, handle and permission columns; other keys are ignored (they still reach `RegisterEvent.Payload`). `created_ip_address` and `last_ip_address` are server-set from the client address only. With `require_activation` on (the default), `POST /login` refuses an account that is not activated with the same 401 invalid-login answer as a wrong password and counts the attempt for the login throttle. Such an account is activated by the mailed code (`POST /activate-by-code` answers with a token unless a login gate holds the sign-in back, for example a second-factor challenge the user completes through the gate's plugin; the account is activated either way), by the invitation link or by an administrator. `POST /activate` serves the sessions of not-activated accounts, which exist when `require_activation` is off. `Login` checks in this order: a ban or an active suspension (`RejectIfThrottled`), the password, the activation, then the deactivation restore. - **JWT sessions.** `POST /login`, `GET /fetch`, `POST /refresh`, `POST /logout`. Boot registers the `jwt` guard with the framework's `bouncer.Registry` and exposes it as the `jwt.auth` middleware for other plugins. The guard reads the session cookies `classes.SessionCookieNames()` lists and resolves the published `bouncer.BlacklistStore` on every request. Boot installs the plugin's GORM hooks (`classes.RegisterHooks`), publishes the Postgres `jwt_blacklist` store (`bouncer.NewPostgresBlacklist`) and starts its background sweep, which prunes the blacklist, through `lagoon.OnDatabase`. Under `summer serve`, which publishes the database after Boot, they take effect when `lagoon.Publish` runs, before the router is built; when the database is already published they take effect at once. While no store is published, no token counts as blacklisted. `POST /logout` ends the session however the token travels: the bearer header, or only the `token` or `auth_token` cookie the guard also reads. Every session token the request carries goes to the Postgres `jwt_blacklist` until its refresh window closes (the later of `exp` and `iat` plus `refresh_ttl`, plus a minute), so the guard refuses it on the next request and `POST /refresh` cannot revive it after the access token expired; a token whose access lifetime passed is revoked too while it could still be refreshed. The answer, a 401 included, expires the `auth.cookie.*` token cookie (the one `golem15.sociallogin`'s `token_cookie` sets), and any other session cookie the request carries, with that cookie's path, domain, SameSite and Secure. `POST /refresh` loads the token's user and refuses (401, `Could not refresh token`, `User not found`) a deactivated or banned user and a token issued before the user's `tokens_valid_after`; a suspension alone does not stop a refresh. `GET /fetch`, `POST /refresh`, `POST /update` and `POST /change-password`, like the plugin's other signed-in routes, read the session token as the `jwt` guard does: the bearer header first, then the `token` and `auth_token` cookies, so a cookie-only session works on them and a bearer header wins when both are sent. A `POST /refresh` whose token came from the `auth.cookie.*` cookie (`auth.cookie.name`, default `token`) with no bearer header gets that cookie back with the rotated token: the cookie has the `bouncer.LoadCookieConfig` attributes, HttpOnly and Max-Age `refresh_ttl`, and the answer is `{}`, so the token never reaches JavaScript. This holds whoever wrote the cookie (this plugin's `golem15.user.jwt.token_cookie`, `golem15.sociallogin`'s `token_cookie` or the application); a frontend that writes the `token` cookie itself and refreshes through it gets it back HttpOnly, without a body token, so such a frontend sends the bearer header to keep rotating in the body. A refresh carried by the bearer header (a cookie sent with it included) or by the other session cookie answers `{"token": …}` and writes no cookie; the old token is blacklisted after `golem15.user.jwt.blacklist_grace`, so that caller stores the new token itself. A refused refresh writes and forgets no cookie. When `bouncer.LoadCookieConfig` refuses the `auth.cookie` block, a cookie-carried refresh answers the opaque 500 before anything is rotated; a bearer refresh never reads that block. Keep `auth.cookie.name` at `token` or `auth_token`, the cookies the guard reads. With `golem15.user.jwt.token_cookie` on, `POST /login`, `POST /register` (when it signs the new account in) and `POST /activate-by-code` set that HttpOnly session cookie (`bouncer.LoadCookieConfig` attributes, Max-Age `refresh_ttl`) and leave `token` out of the body; `user`, and `message` on activate-by-code, stay. With it off (the default) the token is in the body and no cookie is set. SameSite stays the `auth.cookie` setting (`lax` by default) and there is no CSRF token, so do not set `same_site: none` for this cookie. With the key on, a broken `auth.cookie` block answers those three routes with the opaque 500 before they sign in, create or activate anything. Personal API tokens (`POST /tokens`) are not sessions and are unaffected. A cookie-carried session relies on the cookie's SameSite attribute against cross-site requests (the `auth.cookie` default is `lax`), as on the `jwt.auth` routes. -- **Login gates.** Register publishes a `*classes.LoginGates` registry. Another plugin (a two-factor plugin, for example) looks it up with `app.Lookup[*classes.LoginGates]()` and adds a `classes.LoginGate` in its Boot. `POST /login` consults the gates through `controllers.CheckLoginGates` after the password, activation and throttle checks (and the bcrypt rehash) and before it restores a deactivated account, sends the reactivate mail, stamps `last_seen` and mints the token. A gate sees a `classes.LoginAttempt` (`User`, `Source`, the already-read login body as `Fields`, `Request`). A nil result allows the sign-in; a `classes.LoginGateResult` replaces the token response with its `Status` (0 means 200) and `Body`, for example `{"two_factor_required":true,"challenge_token":…,"available_methods":[…],"expires_at":…}`, and no token is issued, no account is restored and no login event fires. A gate error fails closed with the opaque 500. Without a gate, `/login` answers exactly as before. `POST /refresh` never consults the gates. +- **Login gates.** Register publishes a `*classes.LoginGates` registry. Another plugin (a two-factor plugin, for example) looks it up with `app.Lookup[*classes.LoginGates]()` and adds a `classes.LoginGate` in its Boot. `POST /login` consults the gates through `controllers.CheckLoginGates` after the password, activation and throttle checks (and the bcrypt rehash) and before it restores a deactivated account, sends the reactivate mail, stamps `last_seen` and mints the token. `POST /activate-by-code` consults them after the mailed code verified and before it mints the token, with source `password`: with `require_activation` off an account can enable a second factor before it activates, so the mailed code must not stand in for that factor. A gate sees a `classes.LoginAttempt` (`User`, `Source`, the already-read login or activation body as `Fields`, `Request`). A nil result allows the sign-in; a `classes.LoginGateResult` replaces the token response with its `Status` (0 means 200) and `Body`, for example `{"two_factor_required":true,"challenge_token":…,"available_methods":[…],"expires_at":…}`, and no token is issued, no session cookie is set, no account is restored and no login event fires. A gate error fails closed with the opaque 500. An activation a gate held back or failed stays committed. Without a gate, `/login` and `/activate-by-code` answer exactly as before. `POST /refresh` never consults the gates. - **Login event.** Every sign-in that issued a session token fires `classes.LoginEvent` (`classes.LoginEventName`, `golem15.user.login`, PHP's name) with `User` and `Source` (`LoginSourcePassword`, `LoginSourceSocial`, `LoginSourcePasskey`, `LoginSourceTwoFactor`, `LoginSourceDevice`), after the token was minted. `POST /login` fires it with `password`; another plugin that completes a sign-in fires it through `controllers.FireLoginEvent`. A listener error is logged and never fails the sign-in. Refresh and `/register` fire none. - **Password-change hook.** A password reset, the user's own `POST /change-password` and an administrator setting a password on the Users screen fire `classes.AccessRevokedEvent` inside the write's transaction (see Access revocation); `(*AccessRevokedEvent).PasswordChanged()` reports these three reasons. A two-factor plugin listens to it to revoke trusted devices, and this plugin revokes the users' pending and confirmed device sessions (see Device authorization); a listener error rolls the password write back. The bcrypt rehash on login and a new account fire nothing. - **Password reset and change.** `POST /forgot-password` answers the same body for known and unknown addresses; `POST /reset-password` consumes a code from `IssueResetCode`. `POST /change-password` requires the current password. With `golem15.user.password.public_routes` false, `POST /login`, `POST /forgot-password` and `POST /reset-password` are not registered, so they answer the router's own 404 like any unknown path (any method, so no 405 shows they exist), while the `controllers` handlers stay exported. @@ -233,7 +233,7 @@ Packages: - `BackendLinkShow`, `BackendLinkShowPost`, `BackendLinkConfirm` and `TokenMe`, the handlers of the account link and `/token/me` routes. - `APIArray(ctx, app, user)` builds the user payload, extended by `GetApiArrayEvent` listeners. - `FinishLogin(ctx, app, db, user, ip)` runs the steps `POST /login` takes after authentication, for a caller that authenticated the user another way (a social sign-in): it restores a soft-deleted account and sends the reactivate mail, records `ip` as `last_ip_address` and touches `last_seen` (a failure there is only logged). It checks no password, throttle, activation or ban and fires no event; the caller does those checks, consults the login gates before it and fires the login event after it minted the token. - - `CheckLoginGates(ctx, app, attempt)` consults the published `*classes.LoginGates` (none published allows the sign-in) and returns its answer: nil allows, a `*classes.LoginGateResult` replaces the token response, an error must fail the sign-in closed. A social-login plugin calls it before it issues a token. + - `CheckLoginGates(ctx, app, attempt)` consults the published `*classes.LoginGates` (none published allows the sign-in) and returns its answer: nil allows, a `*classes.LoginGateResult` replaces the token response, an error must fail the sign-in closed. `Login` and `ActivateByCode` call it before they mint; a social-login plugin calls it before it issues a token. - `FireLoginEvent(ctx, app, user, source)` fires `classes.LoginEvent` and returns the listeners' error; call it after the token was minted and log the error. - `DeviceInitiate`, `DeviceStatus`, `DeviceAuthorize`, `DeviceIndex` and `DeviceDestroy`, the device authorization handlers (see Device authorization). - `RegistrationSettings(app)` returns `allow_registration`, `use_register_throttle`, `activate_mode` and `require_activation` with their defaults (`true`, `true`, `auto`, `true`), and `RegistrationsFromIP(ctx, db, ip)` counts the accounts created from an IP in the last 60 minutes (`/register` refuses at 3 with the throttle on). @@ -357,7 +357,7 @@ The mail language is pinned by `TestNormalizeMailLocale`, `TestResolveMailLocale `TestMemberTokenScopesBoot` (no Docker) checks that the scope Boot step fails for a name two plugins declare (naming both), for a redeclared `read`, `write` or `ai`, for a bad name, an empty label and a missing phrase key. With Docker: `TestMemberTokenScopes` walks the member token scope acceptance list item by item (mint and `middleware.Scope`, the 403 message, the 422 for an undeclared scope, the list, discovery in en and pl, the per-application catalog and the legacy fallback); `TestBackendLinkAuthTime` checks the link proof against login, request, refresh and re-login orders, a token without `auth_time` and another user's session token; `TestBackendLinkShowPost` checks that `POST /backend-link/show` answers like the GET route with `Cache-Control: no-store, private`, the fragment URL and `backend_links.page_path`; `TestAccessRevocation` checks `AccessRevokedEvent` from the ban action, the password reset, the password change and an administrator setting a password (ids, `Before`, reason, the transaction, the subject event after commit, the rollback on a failing listener) and that a banned user loses the backend link until unbanned; `TestUserMCPGuardClientIP` checks `last_used_ip` on the MCP personal-token path; `TestAPITokenRevokeEvent` checks the token-specific revoked event of `DELETE /tokens/{id}`; `TestOrganisationLogo` checks the logo field, the upload, `OrganisationLogoFields` with and without a logo, the URL fallback when the thumbnail fails, and that deleting an organisation removes its avatar and logo attachments, the rows and, once the delete commits, the blobs and thumbnails. -The login-gate seam, the password-change hook and the Reset 2FA slot have their own tests. `TestLoginGates` (in `-short`, clean under `-race`) pins the registry: a nil or empty registry allows, gates run in the order they were added, the first result or error wins and stops the chain, nil gates are ignored, and `controllers.CheckLoginGates` allows when no registry is published. `TestLoginGateOrder` pins where `POST /login` asks the gates: refused first factors (wrong password, unknown email, not activated, banned) never reach them; the gate sees the attempt with the body already read; the bcrypt rehash happens before a challenge and fires no password-change event; a challenged deactivated account stays deactivated with no mail and no login event, while an allowed one is restored with one reactivate mail and one login event; a gate's status and body pass through; `/refresh` never asks a gate. `TestPasswordChangedSources` checks that the reset, the user's own change and an administrator setting a password each fire one `AccessRevokedEvent` with `PasswordChanged()` true inside the write's transaction (a listener's write through `ev.Tx` commits with it, a listener error rolls both back), and that the rehash and a registration fire none. `TestResetTwoFactorAction` covers the `reset_two_factor` record action: not offered without a published `classes.TwoFactorAdmin`, offered with the danger confirmation when one reports a second factor, hidden when it reports none, refused without `golem15.users.reset_two_factor`, `Run` refused outside the action's transaction, `Reset` committed with the action and rolled back by its error, and a privileged-group member refused with 403 before `Reset` without `golem15.users.manage_privileged_groups`. The plan 01 smoke tests (`TestLoginGatesSmoke`, `TestLoginGateSmoke`, `TestPasswordChangedEventSmoke`, `TestResetTwoFactorSlotSmoke`) stay. The Postgres tests need Docker. +The login-gate seam, the password-change hook and the Reset 2FA slot have their own tests. `TestLoginGates` (in `-short`, clean under `-race`) pins the registry: a nil or empty registry allows, gates run in the order they were added, the first result or error wins and stops the chain, nil gates are ignored, and `controllers.CheckLoginGates` allows when no registry is published. `TestLoginGateOrder` pins where `POST /login` asks the gates: refused first factors (wrong password, unknown email, not activated, banned) never reach them; the gate sees the attempt with the body already read; the bcrypt rehash happens before a challenge and fires no password-change event; a challenged deactivated account stays deactivated with no mail and no login event, while an allowed one is restored with one reactivate mail and one login event; a gate's status and body pass through; `/refresh` never asks a gate. `TestActivateByCodeGates` pins `POST /activate-by-code`: a challenge or a refusal replaces the token with the gate's status and body and sets no cookie, the gate sees source `password` and the code in `Fields`, an allowing gate gets the usual token answer, a gate error answers the opaque 500, an invalid code never reaches a gate, and the account is activated whatever the gate answers. `TestPasswordChangedSources` checks that the reset, the user's own change and an administrator setting a password each fire one `AccessRevokedEvent` with `PasswordChanged()` true inside the write's transaction (a listener's write through `ev.Tx` commits with it, a listener error rolls both back), and that the rehash and a registration fire none. `TestResetTwoFactorAction` covers the `reset_two_factor` record action: not offered without a published `classes.TwoFactorAdmin`, offered with the danger confirmation when one reports a second factor, hidden when it reports none, refused without `golem15.users.reset_two_factor`, `Run` refused outside the action's transaction, `Reset` committed with the action and rolled back by its error, and a privileged-group member refused with 403 before `Reset` without `golem15.users.manage_privileged_groups`. The plan 01 smoke tests (`TestLoginGatesSmoke`, `TestLoginGateSmoke`, `TestPasswordChangedEventSmoke`, `TestResetTwoFactorSlotSmoke`) stay. The Postgres tests need Docker. The device authorization flow is pinned by `TestDeviceCodes` (in `classes`, `-short`: 10,000 tokens and short codes, the alphabet in every position, no modulo bias, the normalization table), `TestDeviceAuthURL` and `TestDeviceDisplayName` (both `-short`: the `auth_url` template with `{short_code}` as the default, the TTL setting and PHP's user-agent order), and on Postgres through the assembled routes: `TestDeviceInitiate` (PHP's answer with an `auth_url` that carries the short code and not the token, the stored pending row, the TTL, the 255-character `device_name` bound, the cleanup of at most 500 stale pending or expired sessions per call, the retry of an insert whose short code collides with a stored one); `TestDeviceStatus` (unknown, pending, PHP's flat expired body for a pending or confirmed session past `expires_at`, used and revoked sessions that never expire); `TestDeviceAuthorize` (short-code forms, the 422 and 400 refusals, the token path, the short code in the token field, body-only fields, sessions bound to another user, a repeated authorize, expired, used and revoked sessions, concurrent authorizes by two users binding one, and the 401 for a missing, invalid, foreign-signed or ended session); `TestDeviceHandOverConcurrent` (50 concurrent polls hand one token over with one login event of source `device`; a poll with another `User-Agent` than the initiating device's takes nothing and the device still collects its token; a deactivated or banned user's session becomes revoked); `TestDeviceAccessRevocation` (a password change revokes the user's confirmed sessions and keeps used ones and other users' sessions, and a session authorized before `tokens_valid_after` hands nothing over without the event); `TestDeviceListAndRevoke` (only confirmed and used sessions, newest activity first, `is_current`, the display name, an empty list as `[]`, 404 for foreign and unknown ids, a repeated revoke, a revoked device signed out); `TestDeviceRevokeAfterRefresh` (the row follows one and two refreshes, a revoke ends the whole token lineage and the blacklist entry outlives the refresh window, a failed refresh leaves the row); `TestDeviceRevokeRacesRefresh` (a refresh of a session revoked after the blacklist check is refused without a token, and a device refreshing in a tight loop while it is revoked ends with its last token refused); `TestDeviceTokensAtRest` (the token column is the SHA-256 hex, no column holds the token or the session JWT). `TestDeviceAuthSessionsMigration` (in `updates`) checks the table's PHP column names plus `jwt_jti`, the types, the unique `token` and `short_code`, the status CHECK, the indexes, the cascade on user delete, the rollback and a second up. The plan 01 smoke tests `TestDeviceCodesSmoke`, `TestDeviceFlowSmoke` and `TestDeviceRevokeSmoke` stay. diff --git a/classes/login_gate.go b/classes/login_gate.go index 610d9fc..ace51c4 100644 --- a/classes/login_gate.go +++ b/classes/login_gate.go @@ -12,7 +12,9 @@ import ( // Login sources name the first factor a LoginAttempt or a LoginEvent comes // from. const ( - // LoginSourcePassword is POST /_user/api/v1/login (email and password). + // LoginSourcePassword is POST /_user/api/v1/login (email and password) + // and POST /_user/api/v1/activate-by-code (the mailed activation code, + // the first factor of that sign-in). LoginSourcePassword = "password" // LoginSourceSocial is a social sign-in completed by a social-login // plugin. @@ -42,7 +44,7 @@ type LoginAttempt struct { // request body again, it is consumed). It carries keys a gate needs, // such as trusted_device_token. It is nil on a social sign-in. Treat it // as read-only, untrusted input; it holds the password on a password - // login. + // login and the activation code on an activation. Fields map[string]any // Request is the sign-in request, for the client address and the user // agent. @@ -78,8 +80,9 @@ type LoginGate interface { // looks it up with app.Lookup[*classes.LoginGates]() and adds its gate in // its Boot: every Register runs before every Boot. Password Login consults // it after the password, activation and throttle checks and before it -// restores a deactivated account and mints the token; JWT refresh never -// consults it. It is safe for concurrent use. +// restores a deactivated account and mints the token; activate-by-code +// consults it after the code verified and before it mints the token; JWT +// refresh never consults it. It is safe for concurrent use. type LoginGates struct { mu sync.RWMutex gates []LoginGate diff --git a/controllers/api_controller.go b/controllers/api_controller.go index da3e053..fc47860 100644 --- a/controllers/api_controller.go +++ b/controllers/api_controller.go @@ -588,8 +588,15 @@ func Activate(app *backpack.App) http.HandlerFunc { } } -// ActivateByCode activates from a public "{id}!{code}" value and mints a token. -// With golem15.user.jwt.token_cookie on, the token goes into the auth.cookie +// ActivateByCode activates from a public "{id}!{code}" value, consults the +// login gates and mints a token. The gates run as on password Login, with +// source password, because the mailed code is the first factor here: with +// golem15.user.activation.require_activation off an account can enable a +// second factor before it activates, and the code must not stand in for it. +// A gate answer replaces the token response with its status and body and a +// gate error answers the opaque 500; neither mints a token or sets a +// session cookie, and the account stays activated either way. With +// golem15.user.jwt.token_cookie on, the token goes into the auth.cookie // session cookie instead of the body. func ActivateByCode(app *backpack.App) http.HandlerFunc { return func(w http.ResponseWriter, r *http.Request) { @@ -638,6 +645,28 @@ func ActivateByCode(app *backpack.App) http.HandlerFunc { writeJSON(w, http.StatusUnprocessableEntity, map[string]any{"error": "This activation link is invalid or has expired"}) return } + // The activation above is committed whatever the gates answer. A + // gate answer replaces the token response; a gate error fails + // closed. + gate, err := CheckLoginGates(r.Context(), app, classes.LoginAttempt{ + User: user, + Source: classes.LoginSourcePassword, + Fields: fields, + Request: r, + }) + if err != nil { + pluginLog.Error("golem15.user: login gate", "user_id", user.ID, "err", err) + writeOpaque500(w) + return + } + if gate != nil { + status := gate.Status + if status == 0 { + status = http.StatusOK + } + writeJSON(w, status, gate.Body) + return + } token, err := mintFor(app, secret, user, requestURL(r)) if err != nil { writeOpaque500(w) diff --git a/controllers/login_gates.go b/controllers/login_gates.go index 26d2240..d184d36 100644 --- a/controllers/login_gates.go +++ b/controllers/login_gates.go @@ -12,8 +12,9 @@ import ( // nil allows the sign-in, a result replaces the token response, and an error // must fail the sign-in closed (answer the opaque 500, issue no token). It // looks the registry up on every call and allows the sign-in when none is -// published. Password Login calls it; a social-login plugin calls it before -// it issues a token. +// published. Password Login calls it, activate-by-code calls it after the +// mailed code verified (source password), and a social-login plugin calls +// it before it issues a token. func CheckLoginGates(ctx context.Context, app *backpack.App, a classes.LoginAttempt) (*classes.LoginGateResult, error) { if app == nil { return nil, nil diff --git a/login_gate_test.go b/login_gate_test.go index b2770e5..3b03fef 100644 --- a/login_gate_test.go +++ b/login_gate_test.go @@ -6,6 +6,9 @@ import ( "errors" "fmt" "net/http" + "net/http/httptest" + "strconv" + "strings" "sync" "sync/atomic" "testing" @@ -399,3 +402,136 @@ func TestLoginGateOrder(t *testing.T) { } }) } + +// TestActivateByCodeGates pins where POST /activate-by-code consults the +// login gates: after the mailed code verifies, with source password (the +// code is the first factor here), and before the mint. A gate answer +// replaces the token response with its status and body and sets no session +// cookie, a gate error answers the opaque 500, and the account is activated +// whatever the gate answers. An invalid code never reaches a gate. +func TestActivateByCodeGates(t *testing.T) { + if testing.Short() { + t.Skip("Postgres") + } + challenge := &classes.LoginGateResult{Body: map[string]any{ + "two_factor_required": true, + "challenge_token": "c", + "available_methods": []string{"totp"}, + "expires_at": "2026-10-26T12:00:00+00:00", + }} + inactive := func(t *testing.T, env *adminEnv, label string) (*models.User, string) { + t.Helper() + user := env.seedUser(t, label, func(u *models.User) { u.IsActivated = false }) + code, err := classes.IssueActivationCode(t.Context(), env.gdb, user) + if err != nil { + t.Fatal(err) + } + return user, strconv.FormatUint(uint64(user.ID), 10) + "!" + code + } + activate := func(t *testing.T, env *adminEnv, packed string) *httptest.ResponseRecorder { + t.Helper() + return env.call(t, http.MethodPost, "/_user/api/v1/activate-by-code", "", map[string]string{"code": packed}) + } + noSession := func(t *testing.T, rec *httptest.ResponseRecorder) { + t.Helper() + if cookies := rec.Header().Values("Set-Cookie"); len(cookies) != 0 { + t.Fatalf("a held-back activation set cookies %v", cookies) + } + if strings.Contains(rec.Body.String(), `"token"`) { + t.Fatalf("a held-back activation answered a token: %s", rec.Body.String()) + } + } + + t.Run("challenge_replaces_the_token", func(t *testing.T) { + env, gate := gatedEnv(t, challenge) + user, packed := inactive(t, env, "abc-challenge") + rec := activate(t, env, packed) + want := `{"available_methods":["totp"],"challenge_token":"c","expires_at":"2026-10-26T12:00:00+00:00","two_factor_required":true}` + if rec.Code != http.StatusOK || rec.Body.String() != want { + t.Fatalf("challenged activation = %d %s, want 200 %s", rec.Code, rec.Body.String(), want) + } + noSession(t, rec) + if gate.count() != 1 { + t.Fatalf("gate calls = %d, want 1", gate.count()) + } + seen := gate.seen[0] + if seen.Source != classes.LoginSourcePassword || seen.User == nil || seen.User.ID != user.ID || seen.Request == nil { + t.Fatalf("attempt = %+v", seen) + } + if seen.Fields["code"] != packed { + t.Fatalf("attempt fields = %#v", seen.Fields) + } + if !env.reload(t, user.ID).IsActivated { + t.Fatal("a challenged activation left the account not activated") + } + }) + + t.Run("refusal_passes_through", func(t *testing.T) { + env, _ := gatedEnv(t, &classes.LoginGateResult{Status: http.StatusUnauthorized, Body: map[string]any{"error": "Invalid or expired challenge."}}) + user, packed := inactive(t, env, "abc-refusal") + rec := activate(t, env, packed) + if rec.Code != http.StatusUnauthorized || rec.Body.String() != `{"error":"Invalid or expired challenge."}` { + t.Fatalf("refused activation = %d %s", rec.Code, rec.Body.String()) + } + noSession(t, rec) + if !env.reload(t, user.ID).IsActivated { + t.Fatal("a refused activation left the account not activated") + } + }) + + t.Run("allowing_gate_mints_as_before", func(t *testing.T) { + env, gate := gatedEnv(t, nil) + user, packed := inactive(t, env, "abc-allow") + rec := activate(t, env, packed) + body := decodeBody(t, rec.Body.Bytes()) + userBody, _ := body["user"].(map[string]any) + token, _ := body["token"].(string) + if rec.Code != http.StatusOK || token == "" || body["message"] != "Account activated" || userBody["is_activated"] != true { + t.Fatalf("allowed activation = %d %s", rec.Code, rec.Body.String()) + } + if gate.count() != 1 { + t.Fatalf("gate calls = %d, want 1", gate.count()) + } + if !env.reload(t, user.ID).IsActivated { + t.Fatal("an allowed activation left the account not activated") + } + }) + + t.Run("gate_error_fails_closed", func(t *testing.T) { + env := newAdminEnv(t) + gates, ok := env.app.Lookup[*classes.LoginGates]() + if !ok || gates == nil { + t.Fatal("the plugin's Register published no *classes.LoginGates") + } + gates.Add(gateFunc(func(context.Context, classes.LoginAttempt) (*classes.LoginGateResult, error) { + return nil, errors.New("challenge store down") + })) + user, packed := inactive(t, env, "abc-error") + rec := activate(t, env, packed) + if rec.Code != http.StatusInternalServerError { + t.Fatalf("erroring gate = %d %s", rec.Code, rec.Body.String()) + } + noSession(t, rec) + if !env.reload(t, user.ID).IsActivated { + t.Fatal("an erroring gate left the account not activated") + } + // The code was spent: the account is activated, so a second try + // answers the Winter error page as before. + assertWinterErrorPage(t, activate(t, env, packed)) + }) + + t.Run("bad_code_never_reaches_the_gate", func(t *testing.T) { + env, gate := gatedEnv(t, challenge) + user, _ := inactive(t, env, "abc-bad") + rec := activate(t, env, strconv.FormatUint(uint64(user.ID), 10)+"!nope") + if rec.Code != http.StatusUnprocessableEntity || rec.Body.String() != `{"error":"This activation link is invalid or has expired"}` { + t.Fatalf("bad code = %d %s", rec.Code, rec.Body.String()) + } + if gate.count() != 0 { + t.Fatalf("the gate ran %d times for a bad code", gate.count()) + } + if env.reload(t, user.ID).IsActivated { + t.Fatal("a bad code activated the account") + } + }) +}