From 9e7756a421aead819335f64154441d87c9fa6e38 Mon Sep 17 00:00:00 2001 From: Shelley Shen Date: Thu, 17 Sep 2026 18:49:54 -0700 Subject: [PATCH 1/2] fix(event-ledger): return 401 locally for requests with no Authorization header newPolicyMiddleware forwarded requests with a missing or malformed Authorization header to the policy evaluator with an empty API key, relying on the evaluator to deny it. The evaluator has no dedicated "no credential" verdict and returns 403, which event-ledger relayed as-is, misrepresenting an unauthenticated request as an unauthorized one. It also spent a network round trip on a request that can never succeed. Fail fast with 401 when Authorization is absent, unless JWT claims are already in the request context: in managed mode, jwtVerify clears the header after a successful local JWT verification before handing off to this middleware, so that case must still reach the policy evaluator. Co-Authored-By: Claude Sonnet 5 --- .../internal/middleware/policy.go | 10 +- .../internal/middleware/policy_test.go | 101 +++++++++++------- 2 files changed, 72 insertions(+), 39 deletions(-) diff --git a/src/control-plane-services/event-ledger/internal/middleware/policy.go b/src/control-plane-services/event-ledger/internal/middleware/policy.go index 65d97a4f43..26ab0f3e86 100644 --- a/src/control-plane-services/event-ledger/internal/middleware/policy.go +++ b/src/control-plane-services/event-ledger/internal/middleware/policy.go @@ -209,11 +209,19 @@ func newPolicyMiddleware(policyClient policy.Authorizer, serviceName string, log // 1. Extract the token (simple bearer token extraction) token := "" authHeader := r.Header.Get("Authorization") + _, hasJWTClaims := r.Context().Value(claimsContextKey).(jwt.MapClaims) if strings.HasPrefix(authHeader, "Bearer ") { token = strings.TrimPrefix(authHeader, "Bearer ") logger.InfoContext(traceCtx, "policy: token extracted", zap.String("token_length", strconv.Itoa(len(token)))) - } else { + } else if !hasJWTClaims { + // No credential of any kind: fail fast instead of asking the + // policy evaluator to deny an empty API key. hasJWTClaims + // covers the managed-mode JWT-then-policy chain, where + // jwtVerify already consumed and cleared this header after a + // successful local verification. logger.WarnContext(traceCtx, "policy: no bearer token found in authorization header") + http.Error(w, "Unauthorized", http.StatusUnauthorized) + return } authCtx := map[string]interface{}{ diff --git a/src/control-plane-services/event-ledger/internal/middleware/policy_test.go b/src/control-plane-services/event-ledger/internal/middleware/policy_test.go index 864af39c9e..d6d5f0eac9 100644 --- a/src/control-plane-services/event-ledger/internal/middleware/policy_test.go +++ b/src/control-plane-services/event-ledger/internal/middleware/policy_test.go @@ -335,24 +335,27 @@ func TestPolicyAuthInputFields(t *testing.T) { func TestNewPolicyMiddleware(t *testing.T) { tests := []struct { - name string - client *stubPolicyClient - token string - expectedStatusCode int - expectedActorID string - expectedOrgName string - expectedActorType string - expectedRoles []string + name string + client *stubPolicyClient + token string + claims jwt.MapClaims + expectedStatusCode int + expectedClientCalled bool + expectedActorID string + expectedOrgName string + expectedActorType string + expectedRoles []string }{ { - name: "Successful Authorization", - client: &stubPolicyClient{result: allowResult(nil)}, - token: "valid-token", - expectedStatusCode: http.StatusOK, - expectedActorID: "user123", - expectedOrgName: "org123", - expectedActorType: "user", - expectedRoles: []string{"admin", "user"}, + name: "Successful Authorization", + client: &stubPolicyClient{result: allowResult(nil)}, + token: "valid-token", + expectedStatusCode: http.StatusOK, + expectedClientCalled: true, + expectedActorID: "user123", + expectedOrgName: "org123", + expectedActorType: "user", + expectedRoles: []string{"admin", "user"}, }, { name: "Failed Authorization", @@ -361,35 +364,54 @@ func TestNewPolicyMiddleware(t *testing.T) { "statusCode": 403, "reasons": []interface{}{"unauthorized access"}, }}, - token: "invalid-token", - expectedStatusCode: http.StatusForbidden, + token: "invalid-token", + expectedStatusCode: http.StatusForbidden, + expectedClientCalled: true, }, { - name: "Policy Service Error", - client: &stubPolicyClient{err: errors.New("service unavailable")}, - token: "token", - expectedStatusCode: http.StatusUnauthorized, + name: "Policy Service Error", + client: &stubPolicyClient{err: errors.New("service unavailable")}, + token: "token", + expectedStatusCode: http.StatusUnauthorized, + expectedClientCalled: true, }, { - name: "Empty Result From Policy", - client: &stubPolicyClient{empty: true}, - token: "token", - expectedStatusCode: http.StatusUnauthorized, + name: "Empty Result From Policy", + client: &stubPolicyClient{empty: true}, + token: "token", + expectedStatusCode: http.StatusUnauthorized, + expectedClientCalled: true, }, { - name: "No Token Provided", + // No Authorization header and no prior JWT verification: fail + // fast locally instead of sending an empty API key to the + // policy evaluator. + name: "No Token Provided", + client: &stubPolicyClient{result: allowResult(nil)}, + token: "", + expectedStatusCode: http.StatusUnauthorized, + expectedClientCalled: false, + }, + { + // Simulates the managed-mode JWT-then-policy chain: jwtVerify + // already validated the token and cleared the Authorization + // header, but stashed claims in the request context. The policy + // evaluator must still be consulted using those claims. + name: "No Header, But Already-Verified JWT Claims", client: &stubPolicyClient{result: allowResult(map[string]interface{}{ - "actorId": "anonymous", - "orgName": "anonymous", - "actorType": "anonymous", - "roles": []interface{}{"guest"}, + "actorId": "user456", + "orgName": "org456", + "actorType": "user", + "roles": []interface{}{"user"}, })}, - token: "", - expectedStatusCode: http.StatusOK, - expectedActorID: "anonymous", - expectedOrgName: "anonymous", - expectedActorType: "anonymous", - expectedRoles: []string{"guest"}, + claims: jwt.MapClaims{"sub": "user456"}, + token: "", + expectedStatusCode: http.StatusOK, + expectedClientCalled: true, + expectedActorID: "user456", + expectedOrgName: "org456", + expectedActorType: "user", + expectedRoles: []string{"user"}, }, } @@ -399,10 +421,13 @@ func TestNewPolicyMiddleware(t *testing.T) { if tt.token != "" { req.Header.Set("Authorization", "Bearer "+tt.token) } + if tt.claims != nil { + req = req.WithContext(context.WithValue(req.Context(), claimsContextKey, tt.claims)) + } recorder, capturedCtx := servePolicy(t, tt.client, req) assert.Equal(t, tt.expectedStatusCode, recorder.Code) - assert.True(t, tt.client.called) + assert.Equal(t, tt.expectedClientCalled, tt.client.called) if tt.expectedStatusCode != http.StatusOK { return From ad3a04c6c924c133dfb0ed29766109e3ab91f521 Mon Sep 17 00:00:00 2001 From: Shelley Shen Date: Thu, 17 Sep 2026 19:29:38 -0700 Subject: [PATCH 2/2] fix(event-ledger): reject empty and whitespace-only Bearer tokens The fail-fast check for a missing Authorization header only looked at the branch where the header wasn't Bearer-prefixed. A header of "Bearer " (or Bearer followed by only whitespace) still matched the Bearer-prefixed branch, so it skipped the check and reached the policy evaluator with an empty API key, same as the bug being fixed. Extract and trim the token first, then fail fast on an empty result regardless of which branch produced it, unless JWT claims are already in the request context. Verified end-to-end against a locally running instance (Cassandra + a stub policy evaluator): missing, empty, and whitespace-only Authorization headers all return 401 without reaching the evaluator; a real credential still reaches the evaluator and its allow/deny verdict (including 403 on deny) is unaffected. Co-Authored-By: Claude Sonnet 5 --- .../internal/middleware/policy.go | 20 ++++++++------ .../internal/middleware/policy_test.go | 26 ++++++++++++++++++- 2 files changed, 37 insertions(+), 9 deletions(-) diff --git a/src/control-plane-services/event-ledger/internal/middleware/policy.go b/src/control-plane-services/event-ledger/internal/middleware/policy.go index 26ab0f3e86..ab78dc2423 100644 --- a/src/control-plane-services/event-ledger/internal/middleware/policy.go +++ b/src/control-plane-services/event-ledger/internal/middleware/policy.go @@ -211,18 +211,22 @@ func newPolicyMiddleware(policyClient policy.Authorizer, serviceName string, log authHeader := r.Header.Get("Authorization") _, hasJWTClaims := r.Context().Value(claimsContextKey).(jwt.MapClaims) if strings.HasPrefix(authHeader, "Bearer ") { - token = strings.TrimPrefix(authHeader, "Bearer ") - logger.InfoContext(traceCtx, "policy: token extracted", zap.String("token_length", strconv.Itoa(len(token)))) - } else if !hasJWTClaims { - // No credential of any kind: fail fast instead of asking the - // policy evaluator to deny an empty API key. hasJWTClaims - // covers the managed-mode JWT-then-policy chain, where - // jwtVerify already consumed and cleared this header after a - // successful local verification. + token = strings.TrimSpace(strings.TrimPrefix(authHeader, "Bearer ")) + } + if token == "" && !hasJWTClaims { + // No usable credential (header absent, not Bearer-prefixed, or + // an empty/whitespace-only Bearer value): fail fast instead of + // asking the policy evaluator to deny an empty API key. + // hasJWTClaims covers the managed-mode JWT-then-policy chain, + // where jwtVerify already consumed and cleared this header + // after a successful local verification. logger.WarnContext(traceCtx, "policy: no bearer token found in authorization header") http.Error(w, "Unauthorized", http.StatusUnauthorized) return } + if token != "" { + logger.InfoContext(traceCtx, "policy: token extracted", zap.String("token_length", strconv.Itoa(len(token)))) + } authCtx := map[string]interface{}{ "path": r.URL.Path, diff --git a/src/control-plane-services/event-ledger/internal/middleware/policy_test.go b/src/control-plane-services/event-ledger/internal/middleware/policy_test.go index d6d5f0eac9..56b27902fa 100644 --- a/src/control-plane-services/event-ledger/internal/middleware/policy_test.go +++ b/src/control-plane-services/event-ledger/internal/middleware/policy_test.go @@ -338,6 +338,8 @@ func TestNewPolicyMiddleware(t *testing.T) { name string client *stubPolicyClient token string + rawAuthHeader string + setRawAuthHeader bool claims jwt.MapClaims expectedStatusCode int expectedClientCalled bool @@ -392,6 +394,26 @@ func TestNewPolicyMiddleware(t *testing.T) { expectedStatusCode: http.StatusUnauthorized, expectedClientCalled: false, }, + { + // "Bearer " with nothing after it is Bearer-prefixed, so it must + // not slip past the fail-fast check via the happy-path branch. + name: "Empty Bearer Token", + client: &stubPolicyClient{result: allowResult(nil)}, + rawAuthHeader: "Bearer ", + setRawAuthHeader: true, + expectedStatusCode: http.StatusUnauthorized, + expectedClientCalled: false, + }, + { + // Whitespace-only value after "Bearer " must be treated the same + // as an empty token, not forwarded to the policy evaluator. + name: "Whitespace-Only Bearer Token", + client: &stubPolicyClient{result: allowResult(nil)}, + rawAuthHeader: "Bearer ", + setRawAuthHeader: true, + expectedStatusCode: http.StatusUnauthorized, + expectedClientCalled: false, + }, { // Simulates the managed-mode JWT-then-policy chain: jwtVerify // already validated the token and cleared the Authorization @@ -418,7 +440,9 @@ func TestNewPolicyMiddleware(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { req := httptest.NewRequest(http.MethodGet, "/test", nil) - if tt.token != "" { + if tt.setRawAuthHeader { + req.Header.Set("Authorization", tt.rawAuthHeader) + } else if tt.token != "" { req.Header.Set("Authorization", "Bearer "+tt.token) } if tt.claims != nil {