diff --git a/internal/web/README.md b/internal/web/README.md index 9ed43cd..cf8786f 100644 --- a/internal/web/README.md +++ b/internal/web/README.md @@ -10,13 +10,14 @@ JSON under `/api/v1` for external tools and map polling. First-party UI is a pro | Page | Routes | Authz | |------|--------|-------| | Login | `GET/POST /login`, `POST /logout` | public / session | -| Dashboard | `GET /dashboard` | session; **server-rendered connection summary** (table/counts from FSD service). Leaflet map is PE only (`credentials: 'same-origin'`) | -| Users (directory) | `GET /usereditor[?q&rating&sort&dir&page&cid&new&flash]`, `POST /usereditor/create`, `POST /usereditor/update` | Instructor1+: directory + rating adjust; Supervisor+: create + name/password. CSRF; URL-owned filters; `dir_*` on POST for PRG | +| Dashboard | `GET /dashboard` | any session (OBS+); **server-rendered connection summary** (table/counts from FSD service). Leaflet map is PE only (`credentials: 'same-origin'`) | +| Account | `GET /account`, `POST /account/password`, `POST /account/delete` | any session; change password (current required) + soft-delete account (optional hard-delete via `ALLOW_PERMANENT_ACCOUNT_DELETE`, default false). CSRF; password step-up on delete | +| Users (directory) | `GET /usereditor[?q&rating&sort&dir&page&cid&new&flash]`, `POST /usereditor/create`, `POST /usereditor/update` | **Supervisor+**; create + name/password + ratings (network ceiling ≤ actor; full pilot scale). CSRF; URL-owned filters; `dir_*` on POST for PRG | | Config editor | `GET/POST /configeditor`, `POST /configeditor/create-token`, `POST /configeditor/reset-secret` | Administrator; CSRF on mutations | -| Sweatbox | `GET /sweatbox`, form POSTs under `/sweatbox/*` | Administrator; CSRF on mutations; proxies FSD service HTTP | +| Sweatbox | `GET /sweatbox`, form POSTs under `/sweatbox/*` | **Instructor1+**; CSRF on mutations; proxies FSD service HTTP | | Airport editor | `GET /airport-editor`, `POST /airport-editor/download-apt`, `POST /airport-editor/download-air` | Administrator; CSRF on download; **echo-only** (no disk/DB persistence of `.apt`/`.air`) | -JSON under `/api/v1` remains for external consumers and map polling. Admin mutations work with **cookie + CSRF only** (no `Authorization` header required). +JSON under `/api/v1` remains for external consumers and map polling. Session dual-accept mutations work with **cookie + CSRF only** (no `Authorization` header required). ### Airport editor validation @@ -41,7 +42,7 @@ The **airport editor** (`/airport-editor`) is a second complexity-gate exception - `POST /logout` clears the session cookie - 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) -- **Rating in the cookie is fixed until expiry.** Demotion/suspension does not revoke existing sessions until `exp` unless the JWT secret is rotated (Configure Server → Reset JWT secret). Prefer shorter TTL if faster revoke is required. +- **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. Residual window after soft-delete is **Bearer access tokens only** (15m TTL). ### Cookie `Secure` flag (`COOKIE_SECURE`) | Condition | Secure | @@ -67,10 +68,10 @@ Authorization: Bearer ## Network Ratings The API enforces role-based access control using `NetworkRating` values defined in `pkg/protocol`. Key thresholds: -- **Instructor1–3 (8–10)**: Can open the Users directory and adjust network/pilot ratings up to their own ceilings (any target). Cannot create users or change name/password. -- **Supervisor (11)**: Full user mutation (create, name, password) when target network rating ≤ own; rating adjust as above; kick active connections. -- **Administrator (12)**: Can manage server configuration, reset JWT secret keys, and create API tokens. -- **Suspended (0) / Inactive (-1)**: Cannot log in to the web UI or obtain FSD JWTs. +- **Instructor1–3 (8–10)**: Sweatbox instructor UI + JSON proxies. Cannot open the Users directory. +- **Supervisor (11)**: User editor (create, name, password, ratings); kick active connections. Network rating assignments capped at own rating; pilot ratings use the full official scale. +- **Administrator (12)**: Server configuration, JWT secret reset, API tokens, airport editor. +- **Suspended (0) / Inactive (-1)**: Cannot log in to the web UI or obtain FSD JWTs; existing session cookies are rejected on revalidation. --- diff --git a/internal/web/api_sweatbox_test.go b/internal/web/api_sweatbox_test.go index 5bd50f1..8f16dbf 100644 --- a/internal/web/api_sweatbox_test.go +++ b/internal/web/api_sweatbox_test.go @@ -7,6 +7,7 @@ import ( "strings" "testing" + "github.com/renorris/openfsd/internal/db" "github.com/renorris/openfsd/internal/serviceapi" "github.com/renorris/openfsd/pkg/protocol" "github.com/stretchr/testify/assert" @@ -30,6 +31,32 @@ func TestAPISweatboxStateForbiddenForObserver(t *testing.T) { require.Equal(t, http.StatusForbidden, w.Code, w.Body.String()) } +func TestAPISweatboxStateAllowedForInstructor(t *testing.T) { + m := &sweatboxMock{} + fsd := httptest.NewServer(m.handler()) + t.Cleanup(fsd.Close) + + env := setupTestAPI(t) + env.server.cfg.FsdHttpServiceAddress = fsd.URL + + i1Pass := "i1pass123" + i1 := &db.User{ + Password: i1Pass, + FirstName: strPtr("Inst"), + LastName: strPtr("One"), + NetworkRating: int(protocol.NetworkRatingInstructor1), + } + require.NoError(t, env.server.dbRepo.UserRepo.CreateUser(i1)) + + access, _ := env.login(t, i1.CID, i1Pass) + w := env.doJSON(t, http.MethodGet, "/api/v1/sweatbox/state", nil, access) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + + var st serviceapi.SweatboxStateJSON + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &st)) + assert.Equal(t, "KBTV", st.ICAO) +} + func TestAPISweatboxStateProxiesFSD(t *testing.T) { m := &sweatboxMock{} fsd := httptest.NewServer(m.handler()) diff --git a/internal/web/pages_account.go b/internal/web/pages_account.go index b28cbe7..919dc02 100644 --- a/internal/web/pages_account.go +++ b/internal/web/pages_account.go @@ -111,7 +111,8 @@ func (s *Server) handleFrontendAccountPassword(c *gin.Context) { return } - // KD-7: re-issue session with rememberMe=false (24h); rotate CSRF. + // KD-7: re-issue session with rememberMe=false (24h); clear CSRF so the + // following GET re-issues a fresh token (issueCSRFToken reuses request cookies). if err := s.setSessionCookie(c, user, false); err != nil { slog.Error("account password session reissue failed", "cid", claims.CID, "err", err) page.FormError = "Password updated but session could not be refreshed; please log in again" @@ -119,7 +120,6 @@ func (s *Server) handleFrontendAccountPassword(c *gin.Context) { return } s.clearCSRFCookie(c) - s.issueCSRFToken(c) slog.Info("account password changed", "cid", claims.CID, @@ -200,7 +200,6 @@ func (s *Server) handleFrontendAccountDelete(c *gin.Context) { "event", "account_deleted", "mode", "soft", ) - _ = permanentDisabled } s.clearSessionCookie(c) diff --git a/internal/web/pe_account_test.go b/internal/web/pe_account_test.go index 5e84ff4..39ec342 100644 --- a/internal/web/pe_account_test.go +++ b/internal/web/pe_account_test.go @@ -88,22 +88,33 @@ func TestChangePasswordSuccess(t *testing.T) { t.Fatal("expected session Set-Cookie on password change") } } - // CSRF rotated (clear + new issue → different value from old) - newCSRF := csrfFromCookies(cookies) - if newCSRF == "" { - // May need GET after rotate - w2, cookies2 := authedGET(t, ts, "/account", cookies) - _ = w2 - newCSRF = csrfFromCookies(cookies2) - cookies = cookies2 + // CSRF cleared on password change response; next GET re-issues a fresh token. + clearedCSRF := false + for _, sc := range w.Result().Header.Values("Set-Cookie") { + if strings.HasPrefix(sc, csrfCookieName+"=") && + (strings.Contains(sc, "Max-Age=0") || strings.Contains(sc, "Max-Age=-1") || strings.Contains(sc, csrfCookieName+"=;")) { + clearedCSRF = true + break + } } + if !clearedCSRF { + // mergeCookies drops empty/cleared values; post-merge CSRF must not equal old. + if csrfFromCookies(cookies) == oldCSRF && oldCSRF != "" { + t.Fatal("expected CSRF clear/rotation after password change") + } + } + w2, cookies2 := authedGET(t, ts, "/account", cookies) + if w2.Code != http.StatusOK { + t.Fatalf("GET /account after password change: %d", w2.Code) + } + newCSRF := csrfFromCookies(cookies2) if newCSRF == "" { - t.Fatal("expected CSRF after password change") + t.Fatal("expected CSRF after password change follow-up GET") } if oldCSRF != "" && newCSRF == oldCSRF { - // Rotation: clearCSRF then issueCSRFToken on same response may set empty then new. - // Accept if session still works with new CSRF. + t.Fatal("CSRF token must change after password change (KD-7)") } + cookies = cookies2 // Old password fails login csrf, loginCookies := getLoginCSRF(t, ts) @@ -225,6 +236,33 @@ func TestChangePasswordCSRF(t *testing.T) { } } +func TestDeleteAccountCSRF(t *testing.T) { + ts := newTestServer(t) + user := createTestUser(t, ts, "pw", int(protocol.NetworkRatingObserver)) + cookies := formLogin(t, ts, user.CID, "pw") + + form := url.Values{} + form.Set("current_password", "pw") + form.Set("confirm_cid", itoa(user.CID)) + form.Set("csrf_token", "not-the-real-token") + req := httptest.NewRequest(http.MethodPost, "/account/delete", strings.NewReader(form.Encode())) + req.Header.Set("Content-Type", "application/x-www-form-urlencoded") + req.Header.Set("Cookie", cookieHeader(cookies)) + w := httptest.NewRecorder() + ts.engine.ServeHTTP(w, req) + if w.Code != http.StatusForbidden { + t.Fatalf("status %d want 403", w.Code) + } + // Account must not be deleted + 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 after CSRF reject", u.NetworkRating) + } +} + func TestDeleteAccountSoft(t *testing.T) { ts := newTestServer(t) user := createTestUser(t, ts, "delete-me1", int(protocol.NetworkRatingObserver)) @@ -390,18 +428,33 @@ func TestSessionRejectedAfterSoftDelete(t *testing.T) { if loc := w.Header().Get("Location"); loc != "/login" { t.Fatalf("Location=%q want /login", loc) } - // Session cookie cleared + // Session cookie cleared on the soft-delete reject response. 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")) { cleared = true + break } } if !cleared { - // mergeCookies should drop empty/cleared - if csrfFromCookies(cookies2) != "" || extractCookie(w.Result(), sessionCookieName) != "" { - // extractCookie may still return empty value for cleared cookie + t.Fatal("expected session cookie Max-Age=0/-1 clear after soft-delete revalidation") + } + if extractCookie(w.Result(), sessionCookieName) != "" { + // extractCookie returns the value part; empty value is OK for clear. + val := extractCookie(w.Result(), sessionCookieName) + if val != "" { + // Some clients may still parse Max-Age=-1 with empty value only. + 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")) { + val = "" + break + } + } + if val != "" { + t.Fatalf("session cookie still has value after clear: %q", val) + } } } // Follow-up without re-login fails diff --git a/internal/web/session.go b/internal/web/session.go index 853631a..6a7970e 100644 --- a/internal/web/session.go +++ b/internal/web/session.go @@ -163,11 +163,20 @@ func (s *Server) parseSessionCookie(c *gin.Context) (*auth.CustomClaims, error) } // optionalSession loads session claims into the gin context when present (no redirect). +// Revalidates against the DB (KD-9): inactive/missing users get cookies cleared and +// are treated as signed-out so landing does not show stale elevated nav. func (s *Server) optionalSession(c *gin.Context) *auth.CustomClaims { claims, err := s.parseSessionCookie(c) if err != nil { return nil } + claims, user, err := s.revalidateSessionFromDB(claims) + if err != nil { + s.clearSessionCookie(c) + s.clearCSRFCookie(c) + return nil + } setJwtContext(c, claims) + c.Set(dbUserContextKey, user) return claims } diff --git a/internal/web/templates/sweatbox_manual.html b/internal/web/templates/sweatbox_manual.html index 33a40ad..cbbc2b2 100644 --- a/internal/web/templates/sweatbox_manual.html +++ b/internal/web/templates/sweatbox_manual.html @@ -111,7 +111,7 @@

4. Typical workflow

    -
  1. Enable sweatbox on FSD and open /sweatbox as Administrator.
    2. +
  2. Enable sweatbox on FSD and open /sweatbox as Instructor1+.
  3. Load an airport (.apt). Use Airport Editor to author/download files if needed