From 668cd52ff4a150736c9b65b3b05495524c9bc1d9 Mon Sep 17 00:00:00 2001 From: Reese Norris Date: Tue, 28 Jul 2026 11:38:55 -0400 Subject: [PATCH 1/2] web: Provisional JSON account password + delete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Password parity with HTML (§C): confirm_password, validateNewPassword, new≠current; incorrect current → 400. Cookie dual-accept may re-issue session; Bearer has no session side effects. Delete: soft by default; permanent:true hard-deletes only when ALLOW_PERMANENT_ACCOUNT_DELETE is set. Fail-closed 400 when permanent requested but hard-delete disabled (intentional JSON divergence from HTML soft-fallback). Dual-accept OBS+ self via useAPIV1Protected. OpenAPI x-openfsd-stability: provisional; README operator framing. --- internal/web/README.md | 75 ++++- internal/web/api_account.go | 209 ++++++++++++ internal/web/api_account_test.go | 487 +++++++++++++++++++++++++++ internal/web/openapi/openapi.v1.yaml | 160 +++++++++ internal/web/routes.go | 3 +- 5 files changed, 930 insertions(+), 4 deletions(-) create mode 100644 internal/web/api_account.go create mode 100644 internal/web/api_account_test.go diff --git a/internal/web/README.md b/internal/web/README.md index 6c78255..a74adc6 100644 --- a/internal/web/README.md +++ b/internal/web/README.md @@ -45,7 +45,7 @@ The **airport editor** (`/airport-editor`) is a second complexity-gate exception - Cookie-authenticated API mutations require a CSRF synchronizer token (`csrf_token` form field or `X-CSRF-Token` header matching the `openfsd_csrf` cookie) - Suspended/inactive ratings cannot open a web session (same as FSD policy) - **Session cookies are revalidated against the DB on every use** (HTML + dual-accept API): missing or inactive/suspended certificates clear cookies and are rejected; claims (network rating + names) are overlaid from the DB so demotions take effect immediately. -- **Bearer access tokens on dual-accept resource groups** (`/api/v1/user|config|fsdconn|sweatbox|editor/*`) are revalidated the same way (KD-18): demotion, suspension, and soft-delete take effect on the next request. Login/refresh/fsd-jwt remain credential-based and are outside this middleware. +- **Bearer access tokens on dual-accept resource groups** (`/api/v1/user|config|fsdconn|sweatbox|editor|account/*`) are revalidated the same way (KD-18): demotion, suspension, and soft-delete take effect on the next request. Login/refresh/fsd-jwt remain credential-based and are outside this middleware. ### Cookie `Secure` flag (`COOKIE_SECURE`) | Condition | Secure | @@ -101,9 +101,9 @@ OpenFSD-API-Version: 2026-07-28 Canonical OpenAPI file: `internal/web/openapi/openapi.v1.yaml` (`//go:embed`). No mirrored copy under `docs/`. -**Outside microversion reject:** `/api/v1/data/*`, `/api/v1/fsd-jwt`, auth login/refresh, discovery, OpenAPI. Resource groups (`/user`, `/config`, `/fsdconn`, `/sweatbox`, `/editor`) reject unknown/invalid pins with **400** envelope. +**Outside microversion reject:** `/api/v1/data/*`, `/api/v1/fsd-jwt`, auth login/refresh, discovery, OpenAPI. Resource groups (`/user`, `/config`, `/fsdconn`, `/sweatbox`, `/editor`, `/account`) reject unknown/invalid pins with **400** envelope. -**Stability tiers:** existing enveloped user/config/fsdconn/editor routes are **Stable** (goldens under `testdata/api_v1//`). New expansion routes may ship **Provisional** (shape may change without a microversion bump while Provisional). Design: `docs/design/rest-api-versioning.md`. +**Stability tiers:** existing enveloped user/config/fsdconn/editor routes are **Stable** (goldens under `testdata/api_v1//`). New expansion routes may ship **Provisional** (shape may change without a microversion bump while Provisional). **Account self-service JSON is Provisional** until post-release maintainer sign-off. Design: `docs/design/rest-api-versioning.md`. Baseline pin (first supported): **`2026-07-28`**. @@ -511,6 +511,75 @@ Create a new API access token with a specified expiry (max 90 days). --- +### Account self-service (Provisional) + +Operator automation for the same self-service flows as `GET/POST /account` HTML. +**Provisional** until post-release maintainer sign-off — shapes may change without a microversion bump while still Provisional. Dual-accept (Bearer or session cookie + CSRF); self only (OBS+ after revalidation). + +#### POST /api/v1/account/password +Change the authenticated actor's password. + +**Request Body**: +```json +{ + "current_password": "string", + "new_password": "string", + "confirm_password": "string" +} +``` + +| Rule | Behavior | +|------|----------| +| `current_password` | Required; incorrect → **400** `"incorrect password"` | +| `new_password` | Min **8** chars; must not contain `:` (`validateNewPassword`) | +| `new_password != current_password` | Else **400** | +| `confirm_password` | Required; must equal `new_password` | + +**Response (200 OK)**: envelope with `data: null`. + +**Auth side effects:** Bearer — none (no session cookies). Cookie dual-accept — may re-issue 24h session + clear CSRF (HTML parity). + +**Permissions**: Dual-accept; any active self (OBS+). + +--- + +#### POST /api/v1/account/delete +Soft- or hard-delete the authenticated actor's account (password step-up + CID confirm). + +**Request Body**: +```json +{ + "current_password": "string", + "confirm_cid": 12345, + "permanent": false +} +``` + +| Rule | Behavior | +|------|----------| +| Current password | Required + verified (same as password change) | +| `confirm_cid` | Must equal actor CID | +| Soft-delete (default / `permanent` false or omitted) | `network_rating = Inactive`; `data.status = "soft_deleted"` | +| `permanent: true` + `ALLOW_PERMANENT_ACCOUNT_DELETE` | Hard-delete row; `data.status = "hard_deleted"` | +| `permanent: true` + hard-delete **disabled** | **400** `"permanent delete is disabled"`; **no mutation** | + +**Intentional JSON divergence from HTML:** HTML falls back to soft-delete and redirects with `permanent=disabled` when permanent was requested but disabled. JSON is **fail-closed** so automation does not silently get a different delete mode — retry with `permanent: false` or enable hard-delete server-side. + +**Response (200 OK)**: +```json +{ + "version": "v1", + "err": null, + "data": { "status": "soft_deleted" } +} +``` + +**Auth side effects:** Bearer — none (client drops token). Cookie dual-accept — clears session + CSRF like HTML. After success, drop credentials. + +**Permissions**: Dual-accept; any active self (OBS+). + +--- + ### FSD Connection Management #### POST /api/v1/fsdconn/kickuser diff --git a/internal/web/api_account.go b/internal/web/api_account.go new file mode 100644 index 0000000..992fb9a --- /dev/null +++ b/internal/web/api_account.go @@ -0,0 +1,209 @@ +package web + +import ( + "log/slog" + "net/http" + + "github.com/gin-gonic/gin" + "github.com/renorris/openfsd/pkg/protocol" +) + +// setupAccountAPIRoutes mounts Provisional account self-service JSON under +// /api/v1/account (password change + soft/hard delete). Dual-accept OBS+ self. +// Stability: Provisional until post-release maintainer sign-off +// (docs/design/rest-api-versioning.md §C). +func (s *Server) setupAccountAPIRoutes(parent *gin.RouterGroup) { + g := parent.Group("/account") + s.useAPIV1Protected(g) + g.POST("/password", s.handleAPIAccountPassword) + g.POST("/delete", s.handleAPIAccountDelete) +} + +// handleAPIAccountPassword POST /api/v1/account/password +// +// Request: { current_password, new_password, confirm_password } +// Validation parity with pages_account.go / validateNewPassword. +// Bearer: password update only (no session cookie side effects). +// Cookie dual-accept: re-issue 24h session + clear CSRF like HTML. +func (s *Server) handleAPIAccountPassword(c *gin.Context) { + claims, ok := requireJwtContext(c) + if !ok { + return + } + + var reqBody struct { + CurrentPassword string `json:"current_password"` + NewPassword string `json:"new_password"` + ConfirmPassword string `json:"confirm_password"` + } + if !bindJSONOrAbort(c, &reqBody) { + return + } + + user, err := s.dbRepo.UserRepo.GetUserByCID(claims.CID) + if err != nil { + // Actor was valid at middleware; treat as unauthorized if gone mid-request. + res := newAPIV1Failure("unauthorized") + writeAPIV1Response(c, http.StatusUnauthorized, &res) + return + } + + if reqBody.CurrentPassword == "" { + res := newAPIV1Failure("current password is required") + writeAPIV1Response(c, http.StatusBadRequest, &res) + return + } + if !s.dbRepo.UserRepo.VerifyPasswordHash(reqBody.CurrentPassword, user.Password) { + slog.Debug("api password change rejected bad current", "cid", claims.CID) + res := newAPIV1Failure("incorrect password") + writeAPIV1Response(c, http.StatusBadRequest, &res) + return + } + if msg := validateNewPassword(reqBody.NewPassword); msg != "" { + res := newAPIV1Failure(msg) + writeAPIV1Response(c, http.StatusBadRequest, &res) + return + } + if reqBody.ConfirmPassword == "" || reqBody.NewPassword != reqBody.ConfirmPassword { + res := newAPIV1Failure("Passwords do not match") + writeAPIV1Response(c, http.StatusBadRequest, &res) + return + } + if reqBody.NewPassword == reqBody.CurrentPassword { + res := newAPIV1Failure("New password must be different from the current password") + writeAPIV1Response(c, http.StatusBadRequest, &res) + return + } + + user.Password = reqBody.NewPassword + if err := s.dbRepo.UserRepo.UpdateUser(user); err != nil { + slog.Error("api account password update failed", "cid", claims.CID, "err", err) + writeAPIV1Response(c, http.StatusInternalServerError, &genericAPIV1InternalServerError) + return + } + + // Cookie dual-accept: parity with HTML (KD-7 session re-issue). Bearer: none. + if isSessionAuth(c) { + if err := s.setSessionCookie(c, user, false); err != nil { + slog.Error("api account password session reissue failed", "cid", claims.CID, "err", err) + // Password already updated; report success (mutation cannot roll back cleanly). + } else { + s.clearCSRFCookie(c) + } + } + + slog.Info("account password changed", + "cid", claims.CID, + "event", "account_password_changed", + "auth_method", authMethodOf(c), + ) + res := newAPIV1Success(nil) + writeAPIV1Response(c, http.StatusOK, &res) +} + +// handleAPIAccountDelete POST /api/v1/account/delete +// +// Request: { current_password, confirm_cid, permanent? } +// Soft-delete default. permanent:true is fail-closed when hard-delete is +// disabled (intentional JSON divergence from HTML — no silent soft-delete). +// Bearer: no cookie clear. Cookie dual-accept: clear session + CSRF like HTML. +func (s *Server) handleAPIAccountDelete(c *gin.Context) { + claims, ok := requireJwtContext(c) + if !ok { + return + } + + var reqBody struct { + CurrentPassword string `json:"current_password"` + ConfirmCID int `json:"confirm_cid"` + Permanent bool `json:"permanent"` + } + if !bindJSONOrAbort(c, &reqBody) { + return + } + + user, err := s.dbRepo.UserRepo.GetUserByCID(claims.CID) + if err != nil { + res := newAPIV1Failure("unauthorized") + writeAPIV1Response(c, http.StatusUnauthorized, &res) + return + } + + if reqBody.CurrentPassword == "" { + res := newAPIV1Failure("current password is required") + writeAPIV1Response(c, http.StatusBadRequest, &res) + return + } + if !s.dbRepo.UserRepo.VerifyPasswordHash(reqBody.CurrentPassword, user.Password) { + slog.Debug("api account delete rejected bad password", "cid", claims.CID) + res := newAPIV1Failure("incorrect password") + writeAPIV1Response(c, http.StatusBadRequest, &res) + return + } + if reqBody.ConfirmCID != user.CID { + res := newAPIV1Failure("confirm_cid must match your CID") + writeAPIV1Response(c, http.StatusBadRequest, &res) + return + } + + allowHard := s.cfg != nil && s.cfg.AllowPermanentAccountDelete + if reqBody.Permanent && !allowHard { + // Fail-closed: automation must not silently get soft-delete when it + // requested permanent. Retry with permanent:false for soft-delete. + res := newAPIV1Failure("permanent delete is disabled") + writeAPIV1Response(c, http.StatusBadRequest, &res) + return + } + + status := "soft_deleted" + if reqBody.Permanent && allowHard { + if err := s.dbRepo.UserRepo.DeleteUser(user.CID); err != nil { + slog.Error("api account hard-delete failed", "cid", claims.CID, "err", err) + writeAPIV1Response(c, http.StatusInternalServerError, &genericAPIV1InternalServerError) + return + } + status = "hard_deleted" + slog.Info("account deleted", + "cid", claims.CID, + "event", "account_deleted", + "mode", "hard", + "auth_method", authMethodOf(c), + ) + } else { + // Soft-delete: Inactive rating; empty Password keeps existing hash (UpdateUser). + user.NetworkRating = int(protocol.NetworkRatingInactive) + user.Password = "" + if err := s.dbRepo.UserRepo.UpdateUser(user); err != nil { + slog.Error("api account soft-delete failed", "cid", claims.CID, "err", err) + writeAPIV1Response(c, http.StatusInternalServerError, &genericAPIV1InternalServerError) + return + } + slog.Info("account deleted", + "cid", claims.CID, + "event", "account_deleted", + "mode", "soft", + "auth_method", authMethodOf(c), + ) + } + + if isSessionAuth(c) { + s.clearSessionCookie(c) + s.clearCSRFCookie(c) + } + + res := newAPIV1Success(map[string]string{"status": status}) + writeAPIV1Response(c, http.StatusOK, &res) +} + +func isSessionAuth(c *gin.Context) bool { + m, _ := c.Get(authMethodContextKey) + return m == authMethodSession +} + +func authMethodOf(c *gin.Context) string { + m, _ := c.Get(authMethodContextKey) + if s, ok := m.(string); ok && s != "" { + return s + } + return "unknown" +} diff --git a/internal/web/api_account_test.go b/internal/web/api_account_test.go new file mode 100644 index 0000000..b3c1ca3 --- /dev/null +++ b/internal/web/api_account_test.go @@ -0,0 +1,487 @@ +package web + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/renorris/openfsd/pkg/protocol" +) + +func apiAccountJSON(t *testing.T, ts *testServer, method, path string, body any, bearer string, cookies []*http.Cookie) *httptest.ResponseRecorder { + t.Helper() + var rdr *bytes.Reader + if body != nil { + b, err := json.Marshal(body) + if err != nil { + t.Fatal(err) + } + rdr = bytes.NewReader(b) + } else { + rdr = bytes.NewReader(nil) + } + req := httptest.NewRequest(method, path, rdr) + if body != nil { + req.Header.Set("Content-Type", "application/json") + } + if bearer != "" { + req.Header.Set("Authorization", "Bearer "+bearer) + } + if len(cookies) > 0 { + req.Header.Set("Cookie", cookieHeader(cookies)) + if csrf := csrfFromCookies(cookies); csrf != "" { + req.Header.Set("X-CSRF-Token", csrf) + } + } + w := httptest.NewRecorder() + ts.engine.ServeHTTP(w, req) + return w +} + +func loginAccessToken(t *testing.T, ts *testServer, cid int, password string) string { + t.Helper() + body := map[string]any{"cid": cid, "password": password} + b, _ := json.Marshal(body) + req := httptest.NewRequest(http.MethodPost, "/api/v1/auth/login", bytes.NewReader(b)) + req.Header.Set("Content-Type", "application/json") + w := httptest.NewRecorder() + ts.engine.ServeHTTP(w, req) + if w.Code != http.StatusOK { + t.Fatalf("login status %d body %s", w.Code, w.Body.String()) + } + var res APIV1Response + if err := json.Unmarshal(w.Body.Bytes(), &res); err != nil { + t.Fatal(err) + } + data, _ := json.Marshal(res.Data) + var tokens struct { + AccessToken string `json:"access_token"` + } + if err := json.Unmarshal(data, &tokens); err != nil { + t.Fatal(err) + } + if tokens.AccessToken == "" { + t.Fatal("empty access_token") + } + return tokens.AccessToken +} + +func decodeAccountEnvelope(t *testing.T, w *httptest.ResponseRecorder) APIV1Response { + t.Helper() + var res APIV1Response + if err := json.Unmarshal(w.Body.Bytes(), &res); err != nil { + t.Fatalf("decode: %v body=%s", err, w.Body.String()) + } + return res +} + +func errString(res APIV1Response) string { + if res.Err == nil { + return "" + } + return *res.Err +} + +func TestAPIAccountPasswordBearerSuccess(t *testing.T) { + ts := newTestServer(t) + user := createTestUser(t, ts, "oldpassword", int(protocol.NetworkRatingObserver)) + token := loginAccessToken(t, ts, user.CID, "oldpassword") + + w := apiAccountJSON(t, ts, http.MethodPost, "/api/v1/account/password", map[string]any{ + "current_password": "oldpassword", + "new_password": "newpassword1", + "confirm_password": "newpassword1", + }, token, nil) + if w.Code != http.StatusOK { + t.Fatalf("status %d body %s", w.Code, w.Body.String()) + } + res := decodeAccountEnvelope(t, w) + if res.Err != nil { + t.Fatalf("err=%v", *res.Err) + } + // Bearer: no session cookie side effects. + for _, sc := range w.Result().Header.Values("Set-Cookie") { + if strings.HasPrefix(sc, sessionCookieName+"=") { + t.Fatalf("Bearer password change must not Set-Cookie session: %s", sc) + } + if strings.HasPrefix(sc, csrfCookieName+"=") { + t.Fatalf("Bearer password change must not touch CSRF cookie: %s", sc) + } + } + + u, err := ts.dbRepo.UserRepo.GetUserByCID(user.CID) + if err != nil { + t.Fatal(err) + } + if !ts.dbRepo.UserRepo.VerifyPasswordHash("newpassword1", u.Password) { + t.Fatal("password not updated") + } + if ts.dbRepo.UserRepo.VerifyPasswordHash("oldpassword", u.Password) { + t.Fatal("old password still valid") + } +} + +func TestAPIAccountPasswordCookieSessionReissue(t *testing.T) { + ts := newTestServer(t) + user := createTestUser(t, ts, "oldpassword", int(protocol.NetworkRatingObserver)) + cookies := formLogin(t, ts, user.CID, "oldpassword") + // Ensure CSRF is present (from HTML GET). + wGET, cookies := authedGET(t, ts, "/account", cookies) + if wGET.Code != http.StatusOK { + t.Fatalf("GET /account %d", wGET.Code) + } + + w := apiAccountJSON(t, ts, http.MethodPost, "/api/v1/account/password", map[string]any{ + "current_password": "oldpassword", + "new_password": "newpassword1", + "confirm_password": "newpassword1", + }, "", cookies) + if w.Code != http.StatusOK { + t.Fatalf("status %d body %s", w.Code, w.Body.String()) + } + // Session re-issued for cookie dual-accept. + foundSession := false + for _, sc := range w.Result().Header.Values("Set-Cookie") { + if strings.HasPrefix(sc, sessionCookieName+"=") && !strings.Contains(sc, "Max-Age=0") && !strings.Contains(sc, "Max-Age=-1") { + foundSession = true + break + } + } + if !foundSession { + t.Fatal("expected session Set-Cookie re-issue on cookie password change") + } +} + +func TestAPIAccountPasswordValidation(t *testing.T) { + ts := newTestServer(t) + user := createTestUser(t, ts, "oldpassword", int(protocol.NetworkRatingObserver)) + token := loginAccessToken(t, ts, user.CID, "oldpassword") + + cases := []struct { + name string + body map[string]any + want string + }{ + { + name: "empty current", + body: map[string]any{ + "current_password": "", + "new_password": "newpassword1", + "confirm_password": "newpassword1", + }, + want: "current password is required", + }, + { + name: "wrong current", + body: map[string]any{ + "current_password": "nope-nope", + "new_password": "newpassword1", + "confirm_password": "newpassword1", + }, + want: "incorrect password", + }, + { + name: "short new", + body: map[string]any{ + "current_password": "oldpassword", + "new_password": "short", + "confirm_password": "short", + }, + want: "at least 8 characters", + }, + { + name: "colon new", + body: map[string]any{ + "current_password": "oldpassword", + "new_password": "bad:colon1", + "confirm_password": "bad:colon1", + }, + want: "colon", + }, + { + name: "mismatch confirm", + body: map[string]any{ + "current_password": "oldpassword", + "new_password": "newpassword1", + "confirm_password": "newpassword2", + }, + want: "Passwords do not match", + }, + { + name: "missing confirm", + body: map[string]any{ + "current_password": "oldpassword", + "new_password": "newpassword1", + }, + want: "Passwords do not match", + }, + { + name: "same as current", + body: map[string]any{ + "current_password": "oldpassword", + "new_password": "oldpassword", + "confirm_password": "oldpassword", + }, + want: "different from the current password", + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + w := apiAccountJSON(t, ts, http.MethodPost, "/api/v1/account/password", tc.body, token, nil) + if w.Code != http.StatusBadRequest { + t.Fatalf("status %d want 400 body %s", w.Code, w.Body.String()) + } + res := decodeAccountEnvelope(t, w) + if !strings.Contains(errString(res), tc.want) { + t.Fatalf("err=%q want substring %q", errString(res), tc.want) + } + // Password unchanged on every validation failure. + u, err := ts.dbRepo.UserRepo.GetUserByCID(user.CID) + if err != nil { + t.Fatal(err) + } + if !ts.dbRepo.UserRepo.VerifyPasswordHash("oldpassword", u.Password) { + t.Fatal("password must not change on validation failure") + } + }) + } +} + +func TestAPIAccountPasswordUnauthenticated(t *testing.T) { + ts := newTestServer(t) + w := apiAccountJSON(t, ts, http.MethodPost, "/api/v1/account/password", map[string]any{ + "current_password": "x", + "new_password": "newpassword1", + "confirm_password": "newpassword1", + }, "", nil) + if w.Code != http.StatusUnauthorized { + t.Fatalf("status %d want 401", w.Code) + } +} + +func TestAPIAccountPasswordCookieRequiresCSRF(t *testing.T) { + ts := newTestServer(t) + user := createTestUser(t, ts, "oldpassword", int(protocol.NetworkRatingObserver)) + cookies := formLogin(t, ts, user.CID, "oldpassword") + _, cookies = authedGET(t, ts, "/account", cookies) + + b, _ := json.Marshal(map[string]any{ + "current_password": "oldpassword", + "new_password": "newpassword1", + "confirm_password": "newpassword1", + }) + req := httptest.NewRequest(http.MethodPost, "/api/v1/account/password", bytes.NewReader(b)) + req.Header.Set("Content-Type", "application/json") + req.Header.Set("Cookie", cookieHeader(cookies)) + // Intentionally omit X-CSRF-Token + w := httptest.NewRecorder() + ts.engine.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + t.Fatalf("status %d want 403 body %s", w.Code, w.Body.String()) + } +} + +func TestAPIAccountDeleteSoftBearer(t *testing.T) { + ts := newTestServer(t) + user := createTestUser(t, ts, "delete-me1", int(protocol.NetworkRatingObserver)) + token := loginAccessToken(t, ts, user.CID, "delete-me1") + + w := apiAccountJSON(t, ts, http.MethodPost, "/api/v1/account/delete", map[string]any{ + "current_password": "delete-me1", + "confirm_cid": user.CID, + }, token, nil) + if w.Code != http.StatusOK { + t.Fatalf("status %d body %s", w.Code, w.Body.String()) + } + res := decodeAccountEnvelope(t, w) + if res.Err != nil { + t.Fatalf("err=%v", *res.Err) + } + data, _ := json.Marshal(res.Data) + if !strings.Contains(string(data), `"soft_deleted"`) { + t.Fatalf("data=%s want soft_deleted", data) + } + // Bearer: no cookie clear side effects required (none set). + for _, sc := range w.Result().Header.Values("Set-Cookie") { + if strings.HasPrefix(sc, sessionCookieName+"=") { + t.Fatalf("Bearer delete must not Set-Cookie session: %s", sc) + } + } + + u, err := ts.dbRepo.UserRepo.GetUserByCID(user.CID) + if err != nil { + t.Fatal(err) + } + if u.NetworkRating != int(protocol.NetworkRatingInactive) { + t.Fatalf("rating=%d want Inactive", u.NetworkRating) + } + + // Subsequent Bearer use is rejected (revalidation). + w2 := apiAccountJSON(t, ts, http.MethodPost, "/api/v1/account/password", map[string]any{ + "current_password": "delete-me1", + "new_password": "newpassword1", + "confirm_password": "newpassword1", + }, token, nil) + if w2.Code != http.StatusUnauthorized { + t.Fatalf("post-soft-delete bearer status %d want 401", w2.Code) + } +} + +func TestAPIAccountDeleteHardWhenEnabled(t *testing.T) { + ts := newTestServer(t) + ts.cfg.AllowPermanentAccountDelete = true + user := createTestUser(t, ts, "hard-del1", int(protocol.NetworkRatingObserver)) + token := loginAccessToken(t, ts, user.CID, "hard-del1") + + w := apiAccountJSON(t, ts, http.MethodPost, "/api/v1/account/delete", map[string]any{ + "current_password": "hard-del1", + "confirm_cid": user.CID, + "permanent": true, + }, token, nil) + if w.Code != http.StatusOK { + t.Fatalf("status %d body %s", w.Code, w.Body.String()) + } + res := decodeAccountEnvelope(t, w) + data, _ := json.Marshal(res.Data) + if !strings.Contains(string(data), `"hard_deleted"`) { + t.Fatalf("data=%s want hard_deleted", data) + } + _, err := ts.dbRepo.UserRepo.GetUserByCID(user.CID) + if err == nil { + t.Fatal("expected row gone after hard delete") + } +} + +func TestAPIAccountDeletePermanentFailClosed(t *testing.T) { + ts := newTestServer(t) + // AllowPermanentAccountDelete remains false (default). + user := createTestUser(t, ts, "soft-only1", int(protocol.NetworkRatingObserver)) + token := loginAccessToken(t, ts, user.CID, "soft-only1") + + w := apiAccountJSON(t, ts, http.MethodPost, "/api/v1/account/delete", map[string]any{ + "current_password": "soft-only1", + "confirm_cid": user.CID, + "permanent": true, + }, token, nil) + if w.Code != http.StatusBadRequest { + t.Fatalf("status %d want 400 body %s", w.Code, w.Body.String()) + } + res := decodeAccountEnvelope(t, w) + if !strings.Contains(errString(res), "permanent delete is disabled") { + t.Fatalf("err=%q", errString(res)) + } + // No mutation — intentional JSON divergence from HTML soft-fallback. + u, err := ts.dbRepo.UserRepo.GetUserByCID(user.CID) + if err != nil { + t.Fatal(err) + } + if u.NetworkRating != int(protocol.NetworkRatingObserver) { + t.Fatalf("rating=%d want OBS (no silent soft-delete)", u.NetworkRating) + } +} + +func TestAPIAccountDeleteValidation(t *testing.T) { + ts := newTestServer(t) + user := createTestUser(t, ts, "keep-me1x", int(protocol.NetworkRatingObserver)) + token := loginAccessToken(t, ts, user.CID, "keep-me1x") + + cases := []struct { + name string + body map[string]any + want string + }{ + { + name: "empty password", + body: map[string]any{ + "current_password": "", + "confirm_cid": user.CID, + }, + want: "current password is required", + }, + { + name: "wrong password", + body: map[string]any{ + "current_password": "wrong-pass", + "confirm_cid": user.CID, + }, + want: "incorrect password", + }, + { + name: "cid mismatch", + body: map[string]any{ + "current_password": "keep-me1x", + "confirm_cid": 999999, + }, + want: "confirm_cid", + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + w := apiAccountJSON(t, ts, http.MethodPost, "/api/v1/account/delete", tc.body, token, nil) + if w.Code != http.StatusBadRequest { + t.Fatalf("status %d want 400 body %s", w.Code, w.Body.String()) + } + res := decodeAccountEnvelope(t, w) + if !strings.Contains(errString(res), tc.want) { + t.Fatalf("err=%q want %q", errString(res), tc.want) + } + u, err := ts.dbRepo.UserRepo.GetUserByCID(user.CID) + if err != nil { + t.Fatal(err) + } + if u.NetworkRating != int(protocol.NetworkRatingObserver) { + t.Fatalf("must remain active, rating=%d", u.NetworkRating) + } + }) + } +} + +func TestAPIAccountDeleteCookieClearsSession(t *testing.T) { + ts := newTestServer(t) + user := createTestUser(t, ts, "delete-me2", int(protocol.NetworkRatingObserver)) + cookies := formLogin(t, ts, user.CID, "delete-me2") + _, cookies = authedGET(t, ts, "/account", cookies) + + w := apiAccountJSON(t, ts, http.MethodPost, "/api/v1/account/delete", map[string]any{ + "current_password": "delete-me2", + "confirm_cid": user.CID, + "permanent": false, + }, "", cookies) + if w.Code != http.StatusOK { + t.Fatalf("status %d body %s", w.Code, w.Body.String()) + } + cleared := false + for _, sc := range w.Result().Header.Values("Set-Cookie") { + if strings.HasPrefix(sc, sessionCookieName+"=") && + (strings.Contains(sc, "Max-Age=0") || strings.Contains(sc, "Max-Age=-1") || strings.Contains(sc, sessionCookieName+"=;")) { + cleared = true + break + } + } + if !cleared { + t.Fatal("expected session cookie clear on cookie dual-accept delete") + } +} + +func TestAPIAccountDeleteOmitsPermanentIsSoft(t *testing.T) { + ts := newTestServer(t) + user := createTestUser(t, ts, "delete-me3", int(protocol.NetworkRatingSupervisor)) + token := loginAccessToken(t, ts, user.CID, "delete-me3") + + w := apiAccountJSON(t, ts, http.MethodPost, "/api/v1/account/delete", map[string]any{ + "current_password": "delete-me3", + "confirm_cid": user.CID, + }, token, nil) + if w.Code != http.StatusOK { + t.Fatalf("status %d body %s", w.Code, w.Body.String()) + } + res := decodeAccountEnvelope(t, w) + data, _ := json.Marshal(res.Data) + if !strings.Contains(string(data), `"soft_deleted"`) { + t.Fatalf("data=%s", data) + } +} diff --git a/internal/web/openapi/openapi.v1.yaml b/internal/web/openapi/openapi.v1.yaml index 8e740ff..1ef3406 100644 --- a/internal/web/openapi/openapi.v1.yaml +++ b/internal/web/openapi/openapi.v1.yaml @@ -39,6 +39,11 @@ tags: description: APT/AIR validate (Stable; Instructor1+) - name: Sweatbox description: Read-only PE proxies (raw; outside envelope goldens) + - name: Account + description: | + Self-service password change and account delete (Provisional). + Any OBS+ dual-accept actor; self only. Shape may change without a + microversion bump until promoted to Stable (post-release sign-off). # Data feeds (/data/*) are public and outside microversion reject; documented in # internal/web/README.md. Not listed here until a full inventory is needed. @@ -467,6 +472,114 @@ paths: "400": $ref: "#/components/responses/BadAPIVersion" + /account/password: + post: + tags: [Account] + summary: Change own password (self-service) + description: | + Change the authenticated actor's password. Validation parity with the + HTML `/account/password` form (`validateNewPassword`, confirm match, + new ≠ current). + + **Stability: Provisional** — request/response shape may change without a + microversion bump until maintainer sign-off promotes this route to Stable. + + **Auth side effects:** Bearer success does **not** set or clear session + cookies. Cookie dual-accept may re-issue a 24h session cookie and clear + CSRF (parity with HTML). + operationId: postAccountPassword + x-openfsd-stability: provisional + security: + - bearerAuth: [] + - cookieAuth: [] + parameters: + - $ref: "#/components/parameters/OpenFSDAPIVersion" + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/AccountPasswordRequest" + responses: + "200": + description: Password updated (`data` is null) + content: + application/json: + schema: + $ref: "#/components/schemas/APIV1Envelope" + "400": + description: | + Validation failure (empty current, incorrect password, weak new + password, confirm mismatch, new equals current) or bad API version. + content: + application/json: + schema: + $ref: "#/components/schemas/APIV1Envelope" + "401": + description: Missing/invalid credentials + content: + application/json: + schema: + $ref: "#/components/schemas/APIV1Envelope" + + /account/delete: + post: + tags: [Account] + summary: Soft- or hard-delete own account + description: | + Delete the authenticated actor's account after password step-up and + `confirm_cid` match. + + - Default / `permanent: false`: soft-delete (`network_rating = Inactive`). + - `permanent: true` and `ALLOW_PERMANENT_ACCOUNT_DELETE=true`: hard-delete row. + - **`permanent: true` when hard-delete is disabled → 400** + `"permanent delete is disabled"` and **no mutation**. + + **Intentional JSON divergence from HTML:** the HTML form falls back to + soft-delete and redirects with `permanent=disabled` when permanent was + requested but disabled. Automation must not silently receive a different + delete mode than requested — retry with `permanent: false` for soft-delete, + or enable hard-delete server-side. + + **Stability: Provisional.** Bearer success does not clear cookies (client + drops the token). Cookie dual-accept clears session + CSRF like HTML. + After success, clients must drop credentials. + operationId: postAccountDelete + x-openfsd-stability: provisional + security: + - bearerAuth: [] + - cookieAuth: [] + parameters: + - $ref: "#/components/parameters/OpenFSDAPIVersion" + requestBody: + required: true + content: + application/json: + schema: + $ref: "#/components/schemas/AccountDeleteRequest" + responses: + "200": + description: Account deleted + content: + application/json: + schema: + $ref: "#/components/schemas/APIV1EnvelopeAccountDelete" + "400": + description: | + Validation failure, permanent delete disabled (fail-closed), or bad + API version. No account mutation when permanent is requested but + disabled. + content: + application/json: + schema: + $ref: "#/components/schemas/APIV1Envelope" + "401": + description: Missing/invalid credentials + content: + application/json: + schema: + $ref: "#/components/schemas/APIV1Envelope" + components: responses: BadAPIVersion: @@ -580,3 +693,50 @@ components: properties: data: $ref: "#/components/schemas/CreateTokenData" + + AccountPasswordRequest: + type: object + required: [current_password, new_password, confirm_password] + properties: + current_password: + type: string + description: Existing password (required; incorrect → 400 "incorrect password") + new_password: + type: string + description: Min 8 characters; must not contain ":"; must differ from current + confirm_password: + type: string + description: Must equal new_password (required for JSON parity with HTML) + + AccountDeleteRequest: + type: object + required: [current_password, confirm_cid] + properties: + current_password: + type: string + confirm_cid: + type: integer + description: Must equal the actor CID + minimum: 1 + permanent: + type: boolean + default: false + description: | + When true and ALLOW_PERMANENT_ACCOUNT_DELETE is enabled, hard-delete. + When true and hard-delete is disabled, 400 fail-closed (no mutation). + + AccountDeleteData: + type: object + required: [status] + properties: + status: + type: string + enum: [soft_deleted, hard_deleted] + + APIV1EnvelopeAccountDelete: + allOf: + - $ref: "#/components/schemas/APIV1Envelope" + - type: object + properties: + data: + $ref: "#/components/schemas/AccountDeleteData" diff --git a/internal/web/routes.go b/internal/web/routes.go index f63d295..3f7e43d 100644 --- a/internal/web/routes.go +++ b/internal/web/routes.go @@ -44,12 +44,13 @@ func (s *Server) setupRoutes() (*gin.Engine, error) { s.setupAuthRoutes(apiV1Group) // login/refresh: soft version headers only s.setupDataRoutes(apiV1Group) // never version-reject - // Dual-accept JSON resource groups (jwt + csrf + apiVersion). + // Dual-accept JSON resource groups (jwt + csrf + apiVersion + bearer revalidate). s.setupUserRoutes(apiV1Group) s.setupConfigRoutes(apiV1Group) s.setupFsdConnRoutes(apiV1Group) s.setupSweatboxAPIRoutes(apiV1Group) s.setupEditorAPIRoutes(apiV1Group) + s.setupAccountAPIRoutes(apiV1Group) // Frontend groups s.setupFrontendRoutes(e.Group("")) From b4fc0bb1f18408070e96dfdf5ef0aabc331bcd25 Mon Sep 17 00:00:00 2001 From: Reese Norris Date: Tue, 28 Jul 2026 11:58:35 -0400 Subject: [PATCH 2/2] fix: address review feedback for account JSON API Harden dual-accept side-effect tests: cookie password asserts hash update + 24h session re-issue + CSRF clear; cookie delete asserts CSRF clear; Bearer password asserts data:null; cookie fail-closed permanent path keeps session (no silent logout). --- internal/web/api_account_test.go | 111 ++++++++++++++++++++++++++++--- 1 file changed, 102 insertions(+), 9 deletions(-) diff --git a/internal/web/api_account_test.go b/internal/web/api_account_test.go index b3c1ca3..033c321 100644 --- a/internal/web/api_account_test.go +++ b/internal/web/api_account_test.go @@ -85,6 +85,25 @@ func errString(res APIV1Response) string { return *res.Err } +// setCookieIsClear reports whether a Set-Cookie header line clears the named cookie. +func setCookieIsClear(sc, name string) bool { + if !strings.HasPrefix(sc, name+"=") { + return false + } + return strings.Contains(sc, "Max-Age=0") || + strings.Contains(sc, "Max-Age=-1") || + strings.Contains(sc, name+"=;") || + strings.HasPrefix(sc, name+"=;") +} + +// setCookieIsLiveSession reports a non-clear session Set-Cookie (re-issue). +func setCookieIsLiveSession(sc string) bool { + if !strings.HasPrefix(sc, sessionCookieName+"=") { + return false + } + return !setCookieIsClear(sc, sessionCookieName) +} + func TestAPIAccountPasswordBearerSuccess(t *testing.T) { ts := newTestServer(t) user := createTestUser(t, ts, "oldpassword", int(protocol.NetworkRatingObserver)) @@ -102,6 +121,13 @@ func TestAPIAccountPasswordBearerSuccess(t *testing.T) { if res.Err != nil { t.Fatalf("err=%v", *res.Err) } + // Design §C: 200 envelope data: null. + if res.Data != nil { + t.Fatalf("data=%v want null", res.Data) + } + if !strings.Contains(w.Body.String(), `"data":null`) { + t.Fatalf("body missing \"data\":null: %s", w.Body.String()) + } // Bearer: no session cookie side effects. for _, sc := range w.Result().Header.Values("Set-Cookie") { if strings.HasPrefix(sc, sessionCookieName+"=") { @@ -142,17 +168,36 @@ func TestAPIAccountPasswordCookieSessionReissue(t *testing.T) { if w.Code != http.StatusOK { t.Fatalf("status %d body %s", w.Code, w.Body.String()) } - // Session re-issued for cookie dual-accept. + // Password must actually change (not only cookie side effects). + u, err := ts.dbRepo.UserRepo.GetUserByCID(user.CID) + if err != nil { + t.Fatal(err) + } + if !ts.dbRepo.UserRepo.VerifyPasswordHash("newpassword1", u.Password) { + t.Fatal("password not updated on cookie dual-accept path") + } + + // Session re-issued for cookie dual-accept (rememberMe=false → Max-Age=86400). foundSession := false + clearedCSRF := false for _, sc := range w.Result().Header.Values("Set-Cookie") { - if strings.HasPrefix(sc, sessionCookieName+"=") && !strings.Contains(sc, "Max-Age=0") && !strings.Contains(sc, "Max-Age=-1") { + if setCookieIsLiveSession(sc) { foundSession = true - break + // sessionDefaultTTL = 24h when rememberMe=false. + if !strings.Contains(sc, "Max-Age=86400") { + t.Fatalf("session re-issue want Max-Age=86400 (24h), got %s", sc) + } + } + if setCookieIsClear(sc, csrfCookieName) { + clearedCSRF = true } } if !foundSession { t.Fatal("expected session Set-Cookie re-issue on cookie password change") } + if !clearedCSRF { + t.Fatal("expected CSRF cookie clear on cookie password change (HTML parity)") + } } func TestAPIAccountPasswordValidation(t *testing.T) { @@ -384,6 +429,49 @@ func TestAPIAccountDeletePermanentFailClosed(t *testing.T) { } } +// Cookie dual-accept fail-closed: must not clear session (HTML permanent-disabled +// soft-deletes and logs out; JSON must keep the session and leave rating OBS). +func TestAPIAccountDeletePermanentFailClosedCookie(t *testing.T) { + ts := newTestServer(t) + user := createTestUser(t, ts, "soft-only2", int(protocol.NetworkRatingObserver)) + cookies := formLogin(t, ts, user.CID, "soft-only2") + _, cookies = authedGET(t, ts, "/account", cookies) + + w := apiAccountJSON(t, ts, http.MethodPost, "/api/v1/account/delete", map[string]any{ + "current_password": "soft-only2", + "confirm_cid": user.CID, + "permanent": true, + }, "", cookies) + if w.Code != http.StatusBadRequest { + t.Fatalf("status %d want 400 body %s", w.Code, w.Body.String()) + } + res := decodeAccountEnvelope(t, w) + if !strings.Contains(errString(res), "permanent delete is disabled") { + t.Fatalf("err=%q", errString(res)) + } + u, err := ts.dbRepo.UserRepo.GetUserByCID(user.CID) + if err != nil { + t.Fatal(err) + } + if u.NetworkRating != int(protocol.NetworkRatingObserver) { + t.Fatalf("rating=%d want OBS (no silent soft-delete)", u.NetworkRating) + } + // Fail-closed must not log the user out. + for _, sc := range w.Result().Header.Values("Set-Cookie") { + if setCookieIsClear(sc, sessionCookieName) { + t.Fatalf("fail-closed must not clear session: %s", sc) + } + if setCookieIsClear(sc, csrfCookieName) { + t.Fatalf("fail-closed must not clear CSRF: %s", sc) + } + } + // Session still usable for a follow-up HTML GET. + w2, _ := authedGET(t, ts, "/account", cookies) + if w2.Code != http.StatusOK { + t.Fatalf("session should remain valid after fail-closed, GET /account status %d", w2.Code) + } +} + func TestAPIAccountDeleteValidation(t *testing.T) { ts := newTestServer(t) user := createTestUser(t, ts, "keep-me1x", int(protocol.NetworkRatingObserver)) @@ -454,17 +542,22 @@ func TestAPIAccountDeleteCookieClearsSession(t *testing.T) { if w.Code != http.StatusOK { t.Fatalf("status %d body %s", w.Code, w.Body.String()) } - cleared := false + clearedSession := false + clearedCSRF := false for _, sc := range w.Result().Header.Values("Set-Cookie") { - if strings.HasPrefix(sc, sessionCookieName+"=") && - (strings.Contains(sc, "Max-Age=0") || strings.Contains(sc, "Max-Age=-1") || strings.Contains(sc, sessionCookieName+"=;")) { - cleared = true - break + if setCookieIsClear(sc, sessionCookieName) { + clearedSession = true + } + if setCookieIsClear(sc, csrfCookieName) { + clearedCSRF = true } } - if !cleared { + if !clearedSession { t.Fatal("expected session cookie clear on cookie dual-accept delete") } + if !clearedCSRF { + t.Fatal("expected CSRF cookie clear on cookie dual-accept delete (HTML parity)") + } } func TestAPIAccountDeleteOmitsPermanentIsSoft(t *testing.T) {