diff --git a/CHANGELOG.md b/CHANGELOG.md index a8750821..beca0a2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,13 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ### Changed +- Auth: an anonymous request to a protected endpoint (no credentials, or a + session cookie that expired in the browser and is no longer sent) now returns + **401 `auth.required`** instead of 403. The SPA redirects to login on a 401, + so an expired session surfaces as a clean re-login prompt rather than a + dead-end "failed to load." An authenticated caller whose role lacks the + permission still gets 403 `authz.permission_denied`. + - CI release safety: the release workflow now fails closed on a `v*` tag push when no GPG signing key is configured, rather than publishing unsigned packages. Manual `workflow_dispatch` trial builds stay permissive (warn + diff --git a/api/error_codes.yaml b/api/error_codes.yaml index 8e6d7905..33f164b2 100644 --- a/api/error_codes.yaml +++ b/api/error_codes.yaml @@ -69,6 +69,17 @@ errors: # ========================================================================= # auth - authentication # ========================================================================= + - code: auth.required + http_status: 401 + fault: client + retryable: false + description: > + The request reached a protected endpoint without a usable session (no + credentials presented, or the session cookie expired in the browser and + was not sent). The caller must sign in. Distinct from + authz.permission_denied (403), which is an authenticated caller whose role + lacks the permission. The SPA redirects to login on this 401. + - code: auth.invalid_credentials http_status: 401 fault: client diff --git a/internal/auth/middleware.go b/internal/auth/middleware.go index 1936aec5..0ac8d693 100644 --- a/internal/auth/middleware.go +++ b/internal/auth/middleware.go @@ -60,15 +60,28 @@ func RequirePermission(p Permission) func(http.Handler) http.Handler { } } -// denyPermission writes the canonical 403 envelope and emits the +// denyPermission writes the RBAC denial envelope and emits the // authz.permission_denied audit event with detail.required_permission // set to the permission id. Per system-rbac AC-09 + AC-11 + C-04. +// +// The HTTP status distinguishes the two denial classes so the SPA can react +// correctly: an ANONYMOUS caller (no or expired credentials — the request +// arrived without a usable session) gets 401 auth.required so the client +// redirects to login; an AUTHENTICATED caller whose role lacks the permission +// gets 403 authz.permission_denied. The audit record is identical either way — +// a denial is a denial. func denyPermission(w http.ResponseWriter, r *http.Request, p Permission, id Identity) { + status, code, fault, msg := http.StatusForbidden, "authz.permission_denied", "policy", + "this operation requires a permission your role does not grant" + if id.IsAnonymous { + status, code, fault, msg = http.StatusUnauthorized, "auth.required", "client", + "authentication required; sign in to continue" + } errBody := map[string]any{ - "code": "authz.permission_denied", - "fault": "policy", + "code": code, + "fault": fault, "retryable": false, - "human_message": "this operation requires a permission your role does not grant", + "human_message": msg, "detail": map[string]any{ "required_permission": string(p), }, @@ -78,8 +91,11 @@ func denyPermission(w http.ResponseWriter, r *http.Request, p Permission, id Ide } envelope := map[string]any{"error": errBody} body, _ := json.Marshal(envelope) + if status == http.StatusUnauthorized { + w.Header().Set("WWW-Authenticate", "Bearer") + } w.Header().Set("Content-Type", "application/json") - w.WriteHeader(http.StatusForbidden) + w.WriteHeader(status) _, _ = w.Write(body) actorID := id.ID diff --git a/internal/server/api_alerts_test.go b/internal/server/api_alerts_test.go index bc12b6af..d63e5026 100644 --- a/internal/server/api_alerts_test.go +++ b/internal/server/api_alerts_test.go @@ -89,8 +89,8 @@ func TestAPI_Alerts_List_Anonymous_Forbidden(t *testing.T) { t.Fatalf("GET: %v", err) } defer resp.Body.Close() - if resp.StatusCode != http.StatusForbidden { - t.Errorf("status=%d, want 403", resp.StatusCode) + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("status=%d, want 401 auth.required (anonymous)", resp.StatusCode) } }) } diff --git a/internal/server/api_audit_query_test.go b/internal/server/api_audit_query_test.go index eb38fded..5ad03672 100644 --- a/internal/server/api_audit_query_test.go +++ b/internal/server/api_audit_query_test.go @@ -354,8 +354,8 @@ func TestAPI_AuditEvents_RequiresAuditRead(t *testing.T) { // Anonymous → 403, and no event body leaks. anon := doReq(t, asRole(t, "GET", url+"/api/v1/audit/events", "", nil)) defer anon.Body.Close() - if anon.StatusCode != http.StatusForbidden { - t.Fatalf("anonymous GET /audit/events = %d, want 403", anon.StatusCode) + if anon.StatusCode != http.StatusUnauthorized { + t.Fatalf("anonymous GET /audit/events = %d, want 401", anon.StatusCode) } body, _ := io.ReadAll(anon.Body) if strings.Contains(string(body), "\"items\"") { diff --git a/internal/server/api_fleet_test.go b/internal/server/api_fleet_test.go index 418e7898..55b211c9 100644 --- a/internal/server/api_fleet_test.go +++ b/internal/server/api_fleet_test.go @@ -470,13 +470,13 @@ func TestAPI_Fleet_Anonymous_Returns403(t *testing.T) { req, _ := http.NewRequest("GET", url+"/api/v1/fleet/score", nil) resp := doReq(t, req) defer resp.Body.Close() - if resp.StatusCode != http.StatusForbidden { + if resp.StatusCode != http.StatusUnauthorized { b, _ := io.ReadAll(resp.Body) - t.Fatalf("status = %d, want 403; body=%s", resp.StatusCode, b) + t.Fatalf("status = %d, want 401 (anonymous); body=%s", resp.StatusCode, b) } b, _ := io.ReadAll(resp.Body) - if !strings.Contains(string(b), "authz.permission_denied") { - t.Errorf("body lacks authz.permission_denied: %s", b) + if !strings.Contains(string(b), "auth.required") { + t.Errorf("body lacks auth.required: %s", b) } }) } diff --git a/internal/server/api_host_system_info_test.go b/internal/server/api_host_system_info_test.go index 8959f8e5..aa784469 100644 --- a/internal/server/api_host_system_info_test.go +++ b/internal/server/api_host_system_info_test.go @@ -170,8 +170,8 @@ func TestAPI_HostSystemInfo_GET_Anonymous_Forbidden(t *testing.T) { t.Fatalf("GET: %v", err) } defer resp.Body.Close() - if resp.StatusCode != http.StatusForbidden { - t.Fatalf("expected 403 anon, got %d", resp.StatusCode) + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("expected 401 anon, got %d", resp.StatusCode) } }) } diff --git a/internal/server/api_intelligence_test.go b/internal/server/api_intelligence_test.go index a967f4ea..f54f211a 100644 --- a/internal/server/api_intelligence_test.go +++ b/internal/server/api_intelligence_test.go @@ -115,8 +115,8 @@ func TestAPI_Intelligence_Events_Anonymous_Forbidden(t *testing.T) { t.Fatalf("GET: %v", err) } defer resp.Body.Close() - if resp.StatusCode != http.StatusForbidden { - t.Errorf("expected 403 anon, got %d", resp.StatusCode) + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("expected 401 anon, got %d", resp.StatusCode) } }) } diff --git a/internal/server/api_rbac_test.go b/internal/server/api_rbac_test.go index edbdb131..b6fa36d9 100644 --- a/internal/server/api_rbac_test.go +++ b/internal/server/api_rbac_test.go @@ -53,13 +53,29 @@ func TestAPI_RBAC_DeniesWithoutPermission(t *testing.T) { // No session cookie → anonymous. resp := doReq(t, req) defer resp.Body.Close() - if resp.StatusCode != http.StatusForbidden { + if resp.StatusCode != http.StatusUnauthorized { b, _ := io.ReadAll(resp.Body) - t.Fatalf("status = %d, want 403; body=%s", resp.StatusCode, b) + t.Fatalf("anonymous status = %d, want 401; body=%s", resp.StatusCode, b) } b, _ := io.ReadAll(resp.Body) - if !strings.Contains(string(b), "authz.permission_denied") { - t.Errorf("body lacks authz.permission_denied: %s", b) + if !strings.Contains(string(b), "auth.required") { + t.Errorf("anonymous body lacks auth.required: %s", b) + } + + // Authenticated caller whose role lacks the permission still gets 403 + // authz.permission_denied (a viewer holds host:read but not host:write). + vreq := asRole(t, "POST", url+"/api/v1/diagnostics:require-host-write", auth.RoleViewer, + map[string]any{"message": "rbac-deny-authed"}) + vreq.Header.Set("Idempotency-Key", "rbac-deny-authed-key") + vresp := doReq(t, vreq) + defer vresp.Body.Close() + if vresp.StatusCode != http.StatusForbidden { + vb, _ := io.ReadAll(vresp.Body) + t.Fatalf("authenticated viewer status = %d, want 403; body=%s", vresp.StatusCode, vb) + } + vb, _ := io.ReadAll(vresp.Body) + if !strings.Contains(string(vb), "authz.permission_denied") { + t.Errorf("authenticated-denial body lacks authz.permission_denied: %s", vb) } }) } @@ -297,8 +313,8 @@ func TestAPI_RBAC_AdminRolesDeniesAnonymous(t *testing.T) { url, _ := freshAPIServer(t) resp := doGet(t, url+"/api/v1/roles") defer resp.Body.Close() - if resp.StatusCode != http.StatusForbidden { - t.Errorf("status = %d, want 403 (no role)", resp.StatusCode) + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("status = %d, want 401 (no role / anonymous)", resp.StatusCode) } }) } diff --git a/internal/server/api_system_connectivity_test.go b/internal/server/api_system_connectivity_test.go index 05a62f59..1a845e7e 100644 --- a/internal/server/api_system_connectivity_test.go +++ b/internal/server/api_system_connectivity_test.go @@ -192,8 +192,8 @@ func TestAPI_SystemConnectivity_Config_PUT_Anonymous_Forbidden(t *testing.T) { t.Fatalf("PUT: %v", err) } defer resp.Body.Close() - if resp.StatusCode != http.StatusForbidden { - t.Fatalf("expected 403 anon, got %d", resp.StatusCode) + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("expected 401 anon, got %d", resp.StatusCode) } }) } @@ -239,8 +239,8 @@ func TestAPI_SystemConnectivity_Status_Anonymous_Forbidden(t *testing.T) { t.Fatalf("GET: %v", err) } defer resp.Body.Close() - if resp.StatusCode != http.StatusForbidden { - t.Fatalf("expected 403 anon, got %d", resp.StatusCode) + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("expected 401 anon, got %d", resp.StatusCode) } }) } @@ -387,8 +387,8 @@ func TestAPI_FleetConnectivity_Breakdown_Anonymous_Forbidden(t *testing.T) { t.Fatalf("GET: %v", err) } defer resp.Body.Close() - if resp.StatusCode != http.StatusForbidden { - t.Fatalf("expected 403 anon, got %d", resp.StatusCode) + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("expected 401 anon, got %d", resp.StatusCode) } }) } diff --git a/internal/server/api_system_discovery_config_test.go b/internal/server/api_system_discovery_config_test.go index fcb794f3..09dec496 100644 --- a/internal/server/api_system_discovery_config_test.go +++ b/internal/server/api_system_discovery_config_test.go @@ -89,8 +89,8 @@ func TestAPI_SystemDiscoveryConfig_GET_AsAnonymous_Forbidden(t *testing.T) { t.Fatalf("GET: %v", err) } defer resp.Body.Close() - if resp.StatusCode != http.StatusForbidden { - t.Fatalf("expected 403 anon, got %d", resp.StatusCode) + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("expected 401 anon, got %d", resp.StatusCode) } }) } diff --git a/internal/server/api_system_intelligence_config_test.go b/internal/server/api_system_intelligence_config_test.go index 7d6d8b1e..0b142e1a 100644 --- a/internal/server/api_system_intelligence_config_test.go +++ b/internal/server/api_system_intelligence_config_test.go @@ -89,8 +89,8 @@ func TestAPI_SystemIntelligenceConfig_GET_AsAnonymous_Forbidden(t *testing.T) { t.Fatalf("GET: %v", err) } defer resp.Body.Close() - if resp.StatusCode != http.StatusForbidden { - t.Fatalf("expected 403 anon, got %d", resp.StatusCode) + if resp.StatusCode != http.StatusUnauthorized { + t.Fatalf("expected 401 anon, got %d", resp.StatusCode) } }) } diff --git a/specs/api/alerts.spec.yaml b/specs/api/alerts.spec.yaml index fabf9494..858ffc4c 100644 --- a/specs/api/alerts.spec.yaml +++ b/specs/api/alerts.spec.yaml @@ -79,7 +79,7 @@ spec: priority: critical references_constraints: [C-01] - id: AC-02 - description: 'GET /api/v1/alerts without an authenticated session returns 403' + description: 'GET /api/v1/alerts as an anonymous caller (no/expired session) returns 401 auth.required' priority: critical references_constraints: [C-01] - id: AC-03 diff --git a/specs/api/audit-events-query.spec.yaml b/specs/api/audit-events-query.spec.yaml index e86f6b00..6276f3fd 100644 --- a/specs/api/audit-events-query.spec.yaml +++ b/specs/api/audit-events-query.spec.yaml @@ -48,7 +48,7 @@ spec: type: technical enforcement: error - id: C-06 - description: GET /audit/events MUST require the audit:read permission. The audit trail is security-sensitive (actor ids, IPs, resource ids, action detail); an anonymous or unauthorized caller MUST receive 403 and MUST NOT receive any events. Enforcement is via auth.EnforcePermission(AuditRead) as the first statement of the handler, matching every sibling data handler. + description: GET /audit/events MUST require the audit:read permission. The audit trail is security-sensitive (actor ids, IPs, resource ids, action detail); an anonymous caller MUST receive 401 auth.required and an authenticated-but-unauthorized caller 403 and MUST NOT receive any events. Enforcement is via auth.EnforcePermission(AuditRead) as the first statement of the handler, matching every sibling data handler. type: security enforcement: error @@ -87,6 +87,6 @@ spec: description: Stored detail with redacted fields shows "" placeholders; redactions lists scrubbed names. priority: critical - id: AC-11 - description: GET /audit/events with no session/credential (anonymous) returns 403 and no event body — the deny path runs before any query. A caller holding audit:read returns 200. (A role lacking audit:read is denied by the same auth.EnforcePermission mechanism, covered generically by system-rbac.) + description: GET /audit/events with no session/credential (anonymous) returns 401 auth.required and no event body — the deny path runs before any query. A caller holding audit:read returns 200. (A role lacking audit:read is denied by the same auth.EnforcePermission mechanism, covered generically by system-rbac.) priority: critical references_constraints: [C-06] diff --git a/specs/api/fleet-connectivity-breakdown.spec.yaml b/specs/api/fleet-connectivity-breakdown.spec.yaml index dc94669b..1e651e38 100644 --- a/specs/api/fleet-connectivity-breakdown.spec.yaml +++ b/specs/api/fleet-connectivity-breakdown.spec.yaml @@ -55,7 +55,7 @@ spec: type: technical enforcement: error - id: C-03 - description: The endpoint requires system:read; an anonymous caller is rejected. + description: The endpoint requires system:read; an anonymous caller is rejected with 401 auth.required. type: security enforcement: error @@ -73,6 +73,6 @@ spec: priority: high references_constraints: [C-01] - id: AC-07 - description: GET /api/v1/fleet/connectivity/breakdown by an anonymous caller is rejected (not authenticated). + description: GET /api/v1/fleet/connectivity/breakdown by an anonymous caller is rejected with 401 auth.required. priority: high references_constraints: [C-03] diff --git a/specs/api/fleet-observability.spec.yaml b/specs/api/fleet-observability.spec.yaml index a776ec13..89a9a221 100644 --- a/specs/api/fleet-observability.spec.yaml +++ b/specs/api/fleet-observability.spec.yaml @@ -53,7 +53,7 @@ spec: type: technical enforcement: error - id: C-02 - description: Every endpoint MUST require the system:read permission via auth.EnforcePermission. Both anonymous callers and authenticated callers without system:read return 403 authz.permission_denied (matches the established RBAC pattern — see system-rbac AC-09) + description: Every endpoint MUST require the system:read permission via auth.EnforcePermission. Anonymous callers return 401 auth.required; authenticated callers without system:read return 403 authz.permission_denied (matches the established RBAC pattern — see system-rbac AC-09) type: security enforcement: error - id: C-03 diff --git a/specs/api/host-system-info.spec.yaml b/specs/api/host-system-info.spec.yaml index 44a725e9..c3fa7d03 100644 --- a/specs/api/host-system-info.spec.yaml +++ b/specs/api/host-system-info.spec.yaml @@ -84,7 +84,7 @@ spec: priority: critical references_constraints: [C-03] - id: AC-04 - description: 'GET /api/v1/hosts/{id}/system-info as anonymous (no session) returns 403 authz.permission_denied. host:read failure surfaces the same envelope used by GET /hosts/{id}' + description: 'GET /api/v1/hosts/{id}/system-info as an anonymous caller (no/expired session) returns 401 auth.required. host:read failure surfaces the same envelope used by GET /hosts/{id}' priority: critical references_constraints: [C-01] - id: AC-05 diff --git a/specs/api/os-intelligence.spec.yaml b/specs/api/os-intelligence.spec.yaml index c7ede66c..1c4cd80d 100644 --- a/specs/api/os-intelligence.spec.yaml +++ b/specs/api/os-intelligence.spec.yaml @@ -81,7 +81,7 @@ spec: priority: critical references_constraints: [C-01] - id: AC-02 - description: 'GET /api/v1/intelligence/events without an authenticated session returns 403' + description: 'GET /api/v1/intelligence/events as an anonymous caller (no/expired session) returns 401 auth.required' priority: critical references_constraints: [C-01] - id: AC-03 diff --git a/specs/api/system-connectivity.spec.yaml b/specs/api/system-connectivity.spec.yaml index 9b290769..9fe310a7 100644 --- a/specs/api/system-connectivity.spec.yaml +++ b/specs/api/system-connectivity.spec.yaml @@ -64,7 +64,7 @@ spec: type: technical enforcement: error - id: C-04 - description: Reads require system:read and the mutating PUT requires system:write; a caller lacking the permission gets 403, and an anonymous caller is rejected. + description: Reads require system:read and the mutating PUT requires system:write; a caller lacking the permission gets 403, and an anonymous caller is rejected with 401 auth.required. type: security enforcement: error @@ -90,7 +90,7 @@ spec: priority: high references_constraints: [C-04] - id: AC-07 - description: PUT /api/v1/system/connectivity/config by an anonymous caller is rejected (not authenticated). + description: PUT /api/v1/system/connectivity/config by an anonymous caller is rejected with 401 auth.required. priority: high references_constraints: [C-04] - id: AC-08 @@ -98,6 +98,6 @@ spec: priority: high references_constraints: [C-03] - id: AC-09 - description: GET /api/v1/system/connectivity/status by an anonymous caller is rejected (not authenticated). + description: GET /api/v1/system/connectivity/status by an anonymous caller is rejected with 401 auth.required. priority: high references_constraints: [C-04] diff --git a/specs/api/system-discovery-config.spec.yaml b/specs/api/system-discovery-config.spec.yaml index 52837283..26232c87 100644 --- a/specs/api/system-discovery-config.spec.yaml +++ b/specs/api/system-discovery-config.spec.yaml @@ -102,7 +102,7 @@ spec: priority: critical references_constraints: [C-01] - id: AC-02 - description: 'GET /api/v1/system/discovery/config with a caller missing system:read returns 403 authz.permission_denied' + description: 'GET /api/v1/system/discovery/config as an anonymous caller returns 401 auth.required (an authenticated caller missing system:read returns 403 authz.permission_denied)' priority: critical references_constraints: [C-02] - id: AC-03 diff --git a/specs/api/system-intelligence-config.spec.yaml b/specs/api/system-intelligence-config.spec.yaml index a30d0c7c..a642b037 100644 --- a/specs/api/system-intelligence-config.spec.yaml +++ b/specs/api/system-intelligence-config.spec.yaml @@ -85,7 +85,7 @@ spec: priority: critical references_constraints: [C-01] - id: AC-02 - description: 'GET /api/v1/system/intelligence/config with a caller missing system:read returns 403 authz.permission_denied' + description: 'GET /api/v1/system/intelligence/config as an anonymous caller returns 401 auth.required (an authenticated caller missing system:read returns 403 authz.permission_denied)' priority: critical references_constraints: [C-02] - id: AC-03 diff --git a/specs/system/rbac.spec.yaml b/specs/system/rbac.spec.yaml index 27bc10ed..510e01fb 100644 --- a/specs/system/rbac.spec.yaml +++ b/specs/system/rbac.spec.yaml @@ -97,7 +97,7 @@ spec: description: RequirePermission middleware allows the request when the identity grants the permission and the license gate passes. priority: critical - id: AC-09 - description: RequirePermission middleware denies with 403 authz.permission_denied when the identity lacks the permission. + description: "RequirePermission / EnforcePermission distinguishes the two denial classes by HTTP status. An ANONYMOUS caller (no credentials, or a session cookie that expired in the browser and is no longer sent) is denied 401 auth.required so the SPA redirects to login. An AUTHENTICATED caller whose role lacks the permission is denied 403 authz.permission_denied. The authz.permission.denied audit event is emitted in both cases." priority: critical references_constraints: [C-03, C-04] - id: AC-10