diff --git a/CHANGELOG.md b/CHANGELOG.md index b6551ca3..a973d0bd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,16 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed + +- **Logout failure messages no longer send users to a Settings feature that + does not exist.** When logout could not revoke a session, or could not + confirm that it did, the message told the user to check or revoke their + sessions in Settings. Settings shows only the current session and cannot + list or end others. Each message now says the session may remain valid + until it expires and names the remedy that works: an administrator can + end it by resetting the user's password. + ## [0.8.0-rc.5] Eyrie (2026-09-19) `v0.8.0-rc.4` built, passed every machine gate, and its assets were diff --git a/internal/server/auth_handlers.go b/internal/server/auth_handlers.go index 9acb5186..abf86c88 100644 --- a/internal/server/auth_handlers.go +++ b/internal/server/auth_handlers.go @@ -406,8 +406,10 @@ func (h *handlers) PostAuthLogout(w http.ResponseWriter, r *http.Request) { // Cookies are cleared above regardless, so the browser stops presenting // the credential either way. But if the server could not revoke, say so // rather than reporting a clean logout: the client needs to know the - // credential may still be live so a human can revoke the session - // explicitly or rotate. + // credential may still be live. Each message names the one remedy the + // product has: an administrator's password reset revokes every + // interactive credential. There is no user-facing sessions list, so no + // message may send the user to one. Spec C-46. if lockWaitExceeded { // Nothing was revoked, and the response says so. The cookies are // still cleared above: the web client treats every logout @@ -416,18 +418,18 @@ func (h *handlers) PostAuthLogout(w http.ResponseWriter, r *http.Request) { // retryable: with the cookies cleared, a repeated request may name // no family, or a different one after another sign-in. Spec C-43. writeError(w, http.StatusServiceUnavailable, "server.error", "server", - "signed out on this device, but the account lock could not be acquired in time, so nothing was revoked. The session may remain valid until it expires. Revoke it from Settings.", false) + "signed out on this device, but the account lock could not be acquired in time, so nothing was revoked. The session may remain valid until it expires. An administrator can end it by resetting your password.", false) return } if revokeUnknown { // Not retryable, and no claim either way. Spec C-40. writeError(w, http.StatusServiceUnavailable, "server.error", "server", - "signed out on this device, but revocation of your session could not be confirmed. Check your active sessions in Settings.", false) + "signed out on this device, but revocation of your session could not be confirmed. It may remain valid until it expires. An administrator can end it by resetting your password.", false) return } if revokeFailed { writeError(w, http.StatusInternalServerError, "auth.logout_incomplete", "server", - "signed out on this device, but the server could not revoke the session. It may remain valid until it expires. Revoke it from Settings or contact an administrator.", true) + "signed out on this device, but the server could not revoke the session. It may remain valid until it expires. An administrator can end it by resetting your password.", true) return } w.WriteHeader(http.StatusNoContent) diff --git a/internal/server/logout_remedy_test.go b/internal/server/logout_remedy_test.go new file mode 100644 index 00000000..fdfd4ea4 --- /dev/null +++ b/internal/server/logout_remedy_test.go @@ -0,0 +1,70 @@ +// @spec system-auth-identity +// +// Every logout failure message names a remedy the product has (C-46). + +package server + +import ( + "net/http" + "strings" + "testing" + "time" + + "github.com/Hanalyx/openwatch/internal/identity" +) + +const logoutRemedy = "An administrator can end it by resetting your password." + +// @ac AC-93 +// AC-93: when logout cannot revoke, or cannot confirm that it did, its +// message names the administrator reset, which does end the session, and +// never a Settings sessions feature, which does not exist. +func TestLogout_FailureMessagesNameARealRemedy(t *testing.T) { + t.Run("system-auth-identity/AC-93", func(t *testing.T) { + url, pool, srv := freshAPIServerWithMaxConns(t, lockTestPoolSize) + + check := func(t *testing.T, got apiResult, status int, code string) { + t.Helper() + if got.status != status || got.code != code { + t.Errorf("response = %d %q, want %d %q", got.status, got.code, status, code) + } + if !strings.Contains(got.message, logoutRemedy) { + t.Errorf("message %q does not name the administrator reset", got.message) + } + if strings.Contains(strings.ToLower(got.message), "settings") { + t.Errorf("message %q sends the user to Settings, which cannot list or end sessions", got.message) + } + if !got.clearsCredential() { + t.Error("logout did not clear the cookies") + } + } + + t.Run("revocation failed and rolled back", func(t *testing.T) { + li := loginFresh(t, url, pool, "ac93failed") + restore := failOnWrite(t, pool, "refresh_tokens", "UPDATE") + got := doAPI(t, logoutRequest(url, li.sessionCookie, li.refreshCookie)) + restore() + check(t, got, http.StatusInternalServerError, "auth.logout_incomplete") + }) + + t.Run("revocation outcome unknown", func(t *testing.T) { + li := loginFresh(t, url, pool, "ac93unknown") + srv.handlers.serializer = &indeterminateBeginner{inner: pool} + got := doAPI(t, logoutRequest(url, li.sessionCookie, li.refreshCookie)) + srv.handlers.serializer = nil + check(t, got, http.StatusServiceUnavailable, "server.error") + if !strings.Contains(got.message, "could not be confirmed") { + t.Errorf("message %q does not say the revocation could not be confirmed", got.message) + } + }) + + t.Run("account lock not acquired", func(t *testing.T) { + defer waitPoolIdle(t, pool) + li := loginFresh(t, url, pool, "ac93lock") + release := holdUserLock(t, pool, li.u.ID, identity.LockWaitBound+15*time.Second) + got := doAPI(t, logoutRequest(url, li.sessionCookie, li.refreshCookie)) + release() + check(t, got, http.StatusServiceUnavailable, "server.error") + }) + }) +} diff --git a/specs/system/auth-identity.spec.yaml b/specs/system/auth-identity.spec.yaml index ecadbef7..ab6e3148 100644 --- a/specs/system/auth-identity.spec.yaml +++ b/specs/system/auth-identity.spec.yaml @@ -1,7 +1,7 @@ spec: id: system-auth-identity title: Auth identity primitives (password, session, JWT, MFA) - version: "1.9.0" + version: "1.10.0" status: approved tier: 1 @@ -451,6 +451,18 @@ spec: rows' error before treating an empty result as authoritative. type: security enforcement: error + - id: C-46 + description: > + When logout cannot revoke the session it names, or cannot confirm + that it did, its message MUST name a remedy the product has: an + administrator's password reset, which revokes every interactive + credential (C-36). It MUST NOT direct the user to a Settings + sessions feature. Settings shows only the current session and cannot + list or end others, so a message pointing there tells the user to do + something they cannot do, on the one path where their session may + still be live. + type: security + enforcement: error - id: C-44 description: > Cookie-session verification MUST be bounded and MUST NOT @@ -2289,3 +2301,29 @@ spec: Rows end with an error: returns: "The stream error, not an empty role list and not ErrNoRoles" transaction_effects: "None; a read-only lookup" + + - id: AC-93 + description: > + Every logout failure message names the administrator password reset + as the remedy and never a Settings sessions feature. + priority: high + references_constraints: [C-40, C-43, C-46] + inputs: + initial_state: "A signed-in user; POST /auth/logout with both cookies and a valid CSRF pair" + variants: + - name: "Revocation failed" + setup: "A trigger fails the refresh-token revocation, so the sweep rolls back" + - name: "Outcome unknown" + setup: "The revocation's commit outcome is indeterminate" + - name: "Account lock not acquired" + setup: "A second connection holds the per-user lock past the lock limit" + expected_output: + per_variant: + Revocation failed: {status: 500, code: auth.logout_incomplete} + Outcome unknown: {status: 503, code: server.error, message_also_states: "could not be confirmed"} + Account lock not acquired: {status: 503, code: server.error} + message_states: "An administrator can end it by resetting your password." + message_must_not_mention: "Settings" + cookies: "Both credential cookies cleared" + transaction_effects: "Not asserted here; covered by AC-66, AC-69 and AC-73" + audit: "Not asserted"