Skip to content

Conversation

@velo
Copy link
Collaborator

@velo velo commented Oct 16, 2025

Summary

  • Implements proper HTTP 403 Forbidden response when JWT authentication succeeds but required claims are missing
  • Distinguishes between authentication failures (401 Unauthorized) and authorization failures (403 Forbidden)

Changes

  • Created MissingRequiredClaimException for handling missing required JWT claims
  • Updated AuthMetadataReader to set HTTP 403 status and WWW-Authenticate header when required claims are missing
  • Modified RestBridgeVerticle to preserve 403 status code (previously overridden to 400)
  • Added comprehensive tests for GraphQL, REST, and MCP endpoints with missing claims

Behavior

Before: Valid JWT with missing required claims returned 200 OK with GraphQL error
After: Valid JWT with missing required claims returns 403 Forbidden

Response Format

HTTP/1.1 403 Forbidden
Content-Type: application/json
WWW-Authenticate: Bearer error="insufficient_scope", error_description="Required claim missing"

{
  "errors": [...],
  "data": null
}

Test Coverage

  • GraphQL endpoint: givenJwt_whenMissingRequiredClaims_thenReturns403
  • REST endpoint: givenJwt_whenMissingRequiredClaimsRest_thenReturns403
  • MCP endpoint: givenJwt_whenMissingRequiredClaimsMcp_thenFails

@velo velo force-pushed the jwt-403-forbidden-response branch 2 times, most recently from f80934f to c50271a Compare October 16, 2025 14:54
@ferenc-csaky ferenc-csaky force-pushed the jwt-403-forbidden-response branch from c50271a to e0b037d Compare October 29, 2025 15:41
@ferenc-csaky ferenc-csaky added this to the 0.8.4 milestone Oct 29, 2025
@ferenc-csaky ferenc-csaky added the enhancement New feature or request label Oct 29, 2025
@codecov
Copy link

codecov bot commented Oct 29, 2025

Codecov Report

❌ Patch coverage is 48.14815% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.36%. Comparing base (4d26e39) to head (a2d6459).
⚠️ Report is 3 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...rl/graphql/auth/MissingRequiredClaimException.java 42.85% 8 Missing ⚠️
.../java/com/datasqrl/graphql/RestBridgeVerticle.java 0.00% 5 Missing ⚠️
.../com/datasqrl/graphql/auth/AuthMetadataReader.java 87.50% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main    #1640      +/-   ##
============================================
- Coverage     62.36%   62.36%   -0.01%     
- Complexity     2745     2746       +1     
============================================
  Files           482      483       +1     
  Lines         15013    15035      +22     
  Branches       1826     1828       +2     
============================================
+ Hits           9363     9376      +13     
- Misses         4788     4797       +9     
  Partials        862      862              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ferenc-csaky ferenc-csaky merged commit 27180b5 into main Oct 29, 2025
11 checks passed
@ferenc-csaky ferenc-csaky deleted the jwt-403-forbidden-response branch October 29, 2025 16:11
@ferenc-csaky ferenc-csaky changed the title Return HTTP 403 Forbidden when JWT is valid but missing required claims Return HTTP 403 when JWT is valid but missing required claims Oct 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants