Skip to content

Commit 27180b5

Browse files
veloferenc-csaky
andauthored
Return HTTP 403 when JWT is valid but missing required claims (#1640)
Co-authored-by: Ferenc Csaky <[email protected]>
1 parent 4d26e39 commit 27180b5

File tree

6 files changed

+237
-32
lines changed

6 files changed

+237
-32
lines changed

sqrl-server/sqrl-server-vertx-base/src/main/java/com/datasqrl/graphql/RestBridgeVerticle.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,17 @@ private void createRestEndpoint(ApiOperation operation) {
181181
var fut = bridgeRequestToGraphQL(ctx, operation, variables);
182182
fut.onSuccess(
183183
executionResult -> {
184-
ctx.response()
185-
.setStatusCode(executionResult.getErrors().isEmpty() ? 200 : 400)
186-
.putHeader("content-type", "application/json");
184+
var res = ctx.response();
185+
var statusCode = res.getStatusCode();
186+
187+
// Preserve status code if already set to non-200
188+
// Otherwise use 200 for success, 400 for errors
189+
if (statusCode == 200) {
190+
res.setStatusCode(executionResult.getErrors().isEmpty() ? 200 : 400);
191+
}
192+
193+
res.putHeader("content-type", "application/json");
194+
187195
if (!executionResult.getErrors().isEmpty()) {
188196
var json = new JsonObject();
189197
json.put(

sqrl-server/sqrl-server-vertx-base/src/main/java/com/datasqrl/graphql/auth/AuthMetadataReader.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,23 @@ public Object read(DataFetchingEnvironment env, String name, boolean isRequired)
3636

3737
if (isRequired && !principal.containsKey(name)) {
3838
log.warn(
39-
"Attribute {} is not present on authorization, attributes: {}",
39+
"Required claim '{}' is not present on authorization, attributes: {}",
4040
name,
4141
principal.attributes());
42-
throw new IllegalArgumentException(
43-
"Attribute '%s' is not present on authorization".formatted(name));
42+
43+
// Set the HTTP status to 403 directly
44+
rc.response().setStatusCode(403);
45+
rc.response()
46+
.putHeader(
47+
"WWW-Authenticate",
48+
"Bearer error=\"insufficient_scope\", error_description=\"Required claim missing\"");
49+
50+
throw new MissingRequiredClaimException(
51+
name,
52+
env.getField().getSourceLocation() != null
53+
? java.util.List.of(env.getField().getSourceLocation())
54+
: null,
55+
env.getExecutionStepInfo().getPath());
4456
}
4557

4658
var value = principal.get(name);
Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/*
2+
* Copyright © 2021 DataSQRL ([email protected])
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package com.datasqrl.graphql.auth;
17+
18+
import graphql.GraphQLError;
19+
import graphql.execution.ResultPath;
20+
import graphql.language.SourceLocation;
21+
import java.util.List;
22+
import lombok.Getter;
23+
24+
/**
25+
* Exception thrown when a required JWT claim is missing or not present in the authorization token.
26+
* This exception should result in an HTTP 403 Forbidden response.
27+
*/
28+
@Getter
29+
public class MissingRequiredClaimException extends RuntimeException implements GraphQLError {
30+
31+
private final String claimName;
32+
private final List<SourceLocation> locations;
33+
private final ResultPath path;
34+
35+
public MissingRequiredClaimException(String claimName) {
36+
super("Required claim missing");
37+
this.claimName = claimName;
38+
this.locations = null;
39+
this.path = null;
40+
}
41+
42+
public MissingRequiredClaimException(
43+
String claimName, List<SourceLocation> locations, ResultPath path) {
44+
super("Required claim missing");
45+
this.claimName = claimName;
46+
this.locations = locations;
47+
this.path = path;
48+
}
49+
50+
@Override
51+
public String getMessage() {
52+
return "Forbidden";
53+
}
54+
55+
@Override
56+
public List<SourceLocation> getLocations() {
57+
return locations;
58+
}
59+
60+
@Override
61+
public graphql.ErrorClassification getErrorType() {
62+
return graphql.ErrorType.ValidationError;
63+
}
64+
65+
@Override
66+
public List<Object> getPath() {
67+
return path != null ? path.toList() : null;
68+
}
69+
}

sqrl-testing/sqrl-testing-container/src/test/java/com/datasqrl/container/testing/JwtContainerIT.java

Lines changed: 140 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -64,12 +64,12 @@ static class MyTableResponse {
6464

6565
// Feign client for testing REST endpoints
6666
interface RestClient {
67-
@RequestLine("GET /v1/rest/queries/MyTable?limit={limit}")
68-
MyTableResponse getMyTable(@Param("limit") int limit);
67+
@RequestLine("GET /v1/rest/queries/AuthMyTable?limit={limit}")
68+
MyTableResponse getAuthMyTable(@Param("limit") int limit);
6969

70-
@RequestLine("GET /v1/rest/queries/MyTable?limit={limit}")
70+
@RequestLine("GET /v1/rest/queries/AuthMyTable?limit={limit}")
7171
@Headers("Authorization: Bearer {token}")
72-
MyTableResponse getMyTableWithAuth(@Param("token") String token, @Param("limit") int limit);
72+
MyTableResponse getAuthMyTableWithAuth(@Param("token") String token, @Param("limit") int limit);
7373
}
7474

7575
private PostgreSQLContainer<?> postgresql;
@@ -142,18 +142,24 @@ protected void commonTearDown() {
142142
void givenJwt_whenUnauthenticatedGraphQL_thenReturns401() {
143143
compileAndStartServer("jwt-authorized-base.sqrl", testDir);
144144

145-
var response = executeGraphQLQuery("{\"query\":\"query { __typename }\"}");
145+
var response = executeGraphQLQuery("{\"query\":\"query { AuthMyTable(limit: 5) { val } }\"}");
146146

147147
assertThat(response.getStatusLine().getStatusCode()).isEqualTo(401);
148148
}
149149

150150
@Test
151151
@SneakyThrows
152152
void givenJwt_whenAuthenticatedGraphQL_thenSucceeds() {
153-
compileAndStartServer("jwt-authorized-base.sqrl", testDir);
153+
compileAndStartServerWithDatabase("jwt-authorized-base.sqrl", testDir);
154154

155-
var response = executeGraphQLQuery("{\"query\":\"query { __typename }\"}", generateJwtToken());
156-
validateBasicGraphQLResponse(response);
155+
var response =
156+
executeGraphQLQuery(
157+
"{\"query\":\"query { AuthMyTable(limit: 5) { val } }\"}", generateJwtToken());
158+
159+
assertThat(response.getStatusLine().getStatusCode()).isEqualTo(200);
160+
var responseBody = EntityUtils.toString(response.getEntity());
161+
assertThat(responseBody).contains("\"data\"");
162+
assertThat(responseBody).contains("\"AuthMyTable\"");
157163
}
158164

159165
@Test
@@ -163,7 +169,8 @@ void givenJwt_whenBadToken_thenReturns401() {
163169

164170
// Generate token with RS256 algorithm while server expects HS256
165171
var response =
166-
executeGraphQLQuery("{\"query\":\"query { __typename }\"}", generateRS256JwtToken());
172+
executeGraphQLQuery(
173+
"{\"query\":\"query { AuthMyTable(limit: 5) { val } }\"}", generateRS256JwtToken());
167174

168175
assertThat(response.getStatusLine().getStatusCode()).isEqualTo(401);
169176

@@ -181,7 +188,9 @@ void givenJwt_whenCorruptedToken_thenReturns401() {
181188
compileAndStartServer("jwt-authorized-base.sqrl", testDir);
182189

183190
// Generate token with RS256 algorithm while server expects HS256
184-
var response = executeGraphQLQuery("{\"query\":\"query { __typename }\"}", "dummy-invalid-jwt");
191+
var response =
192+
executeGraphQLQuery(
193+
"{\"query\":\"query { AuthMyTable(limit: 5) { val } }\"}", "dummy-invalid-jwt");
185194

186195
assertThat(response.getStatusLine().getStatusCode()).isEqualTo(401);
187196

@@ -194,6 +203,29 @@ void givenJwt_whenCorruptedToken_thenReturns401() {
194203
assertThat(responseBody).contains("\"IllegalArgumentException: Invalid format for JWT\"");
195204
}
196205

206+
@Test
207+
@SneakyThrows
208+
void givenJwt_whenMissingRequiredClaims_thenReturns403() {
209+
compileAndStartServer("jwt-authorized-base.sqrl", testDir);
210+
211+
try {
212+
// Generate valid JWT token but with empty claims (missing required claims)
213+
var response =
214+
executeGraphQLQuery(
215+
"{\"query\":\"query { AuthMyTable(limit: 5) { val } }\"}",
216+
generateJwtToken(Map.of()));
217+
218+
// Valid token but missing required claims should return 403 Forbidden
219+
var responseBody = EntityUtils.toString(response.getEntity());
220+
assertThat(response.getStatusLine().getStatusCode()).isEqualTo(403);
221+
assertThat(responseBody).contains("\"errors\"");
222+
} finally {
223+
var logs = serverContainer.getLogs();
224+
log.info("Detailed server logs:");
225+
System.out.println(logs);
226+
}
227+
}
228+
197229
@Test
198230
@SneakyThrows
199231
void givenJwt_whenUnauthenticatedMcp_thenFails() {
@@ -251,7 +283,7 @@ void givenJwt_whenAuthenticatedMcp_thenSucceeds() {
251283
// Test calling a tool that doesn't require auth metadata
252284
var myTableTool =
253285
tools.tools().stream()
254-
.filter(tool -> "GetMyTable".equals(tool.name()))
286+
.filter(tool -> "GetAuthMyTable".equals(tool.name()))
255287
.findFirst()
256288
.orElseThrow(() -> new RuntimeException("GetMyTable tool not found"));
257289
log.info("Testing tool call: {}", myTableTool.name());
@@ -274,10 +306,60 @@ void givenJwt_whenAuthenticatedMcp_thenSucceeds() {
274306
}
275307
}
276308

309+
@Test
310+
@SneakyThrows
311+
void givenJwt_whenMissingRequiredClaimsMcp_thenFails() {
312+
compileAndStartServer("jwt-authorized-base.sqrl", testDir);
313+
314+
var mcpUrl =
315+
String.format(
316+
"http://localhost:%d/v1/mcp", serverContainer.getMappedPort(HTTP_SERVER_PORT));
317+
var jwtToken = generateJwtToken(Map.of());
318+
log.info("Testing MCP endpoint with JWT missing claims: {}", mcpUrl);
319+
320+
// Create MCP client with JWT authentication but missing required claims
321+
var transport =
322+
HttpClientStreamableHttpTransport.builder(mcpUrl)
323+
.customizeRequest(r -> r.header("Authorization", "Bearer " + jwtToken))
324+
.endpoint("/v1/mcp")
325+
.build();
326+
327+
try (var client = McpClient.sync(transport).requestTimeout(Duration.ofSeconds(10)).build(); ) {
328+
// Should succeed with proper JWT - authentication passes
329+
client.initialize();
330+
331+
var tools = client.listTools();
332+
333+
log.info("Successfully listed {} tools", tools.tools().size());
334+
assertThat(tools).isNotNull();
335+
assertThat(tools.tools()).isNotNull();
336+
assertThat(tools.tools()).isNotEmpty();
337+
338+
// Test calling a tool that requires auth metadata - should fail with Forbidden
339+
var myTableTool =
340+
tools.tools().stream()
341+
.filter(tool -> "GetAuthMyTable".equals(tool.name()))
342+
.findFirst()
343+
.orElseThrow(() -> new RuntimeException("GetAuthMyTable tool not found"));
344+
log.info("Testing tool call: {}", myTableTool.name());
345+
346+
var callRequest =
347+
McpSchema.CallToolRequest.builder()
348+
.name(myTableTool.name())
349+
.arguments(Map.of("limit", 5))
350+
.build();
351+
352+
// Should throw exception when calling tool with missing required claims
353+
assertThatThrownBy(() -> client.callTool(callRequest))
354+
.isInstanceOf(RuntimeException.class)
355+
.hasMessageContaining("Forbidden");
356+
}
357+
}
358+
277359
@Test
278360
@SneakyThrows
279361
void givenJwt_whenUnauthenticatedRest_thenReturns401() {
280-
compileAndStartServerWithDatabase("jwt-authorized-base.sqrl", testDir);
362+
compileAndStartServer("jwt-authorized-base.sqrl", testDir);
281363

282364
var restUrl =
283365
String.format("http://localhost:%d", serverContainer.getMappedPort(HTTP_SERVER_PORT));
@@ -289,8 +371,8 @@ void givenJwt_whenUnauthenticatedRest_thenReturns401() {
289371
.decoder(new JacksonDecoder())
290372
.target(RestClient.class, restUrl);
291373

292-
// When JWT is enabled, all REST endpoints require authentication
293-
assertThatThrownBy(() -> restClient.getMyTable(5))
374+
// AuthMyTable endpoint requires authentication
375+
assertThatThrownBy(() -> restClient.getAuthMyTable(5))
294376
.isInstanceOf(FeignException.Unauthorized.class)
295377
.hasMessageContaining("JWT auth failed");
296378
}
@@ -312,7 +394,7 @@ void givenJwt_whenAuthenticatedRest_thenSucceeds() {
312394
.target(RestClient.class, restUrl);
313395

314396
// REST endpoint should work with valid JWT
315-
var response = restClient.getMyTableWithAuth(jwtToken, 5);
397+
var response = restClient.getAuthMyTableWithAuth(jwtToken, 5);
316398
log.info("REST endpoint response with JWT: {}", response);
317399

318400
assertThat(response).isNotNull();
@@ -321,19 +403,53 @@ void givenJwt_whenAuthenticatedRest_thenSucceeds() {
321403
assertThat(response.data).allSatisfy(item -> assertThat(item.val).isPositive());
322404
}
323405

406+
@Test
407+
@SneakyThrows
408+
void givenJwt_whenMissingRequiredClaimsRest_thenReturns403() {
409+
compileAndStartServer("jwt-authorized-base.sqrl", testDir);
410+
411+
var restUrl =
412+
String.format("http://localhost:%d", serverContainer.getMappedPort(HTTP_SERVER_PORT));
413+
var jwtToken = generateJwtToken(Map.of());
414+
log.info("Testing REST endpoint with JWT missing claims: {}", restUrl);
415+
416+
try {
417+
var restClient =
418+
Feign.builder()
419+
.encoder(new JacksonEncoder())
420+
.decoder(new JacksonDecoder())
421+
.target(RestClient.class, restUrl);
422+
423+
// Valid token but missing required claims should return 403 Forbidden
424+
assertThatThrownBy(() -> restClient.getAuthMyTableWithAuth(jwtToken, 5))
425+
.isInstanceOf(FeignException.Forbidden.class);
426+
} finally {
427+
var logs = serverContainer.getLogs();
428+
log.info("Detailed MCP Validation Results:");
429+
System.out.println(logs);
430+
}
431+
}
432+
324433
private String generateJwtToken() {
434+
return generateJwtToken(Map.of("val", 1, "values", List.of(1, 2, 3)));
435+
}
436+
437+
private String generateJwtToken(Map<String, Object> claims) {
325438
var now = Instant.now();
326439
var expiration = now.plus(1, ChronoUnit.HOURS);
327440

328-
return Jwts.builder()
329-
.issuer("my-test-issuer")
330-
.audience()
331-
.add("my-test-audience")
332-
.and()
333-
.issuedAt(Date.from(now))
334-
.expiration(Date.from(expiration))
335-
.claim("val", 1)
336-
.claim("values", List.of(1, 2, 3))
441+
var builder =
442+
Jwts.builder()
443+
.issuer("my-test-issuer")
444+
.audience()
445+
.add("my-test-audience")
446+
.and()
447+
.issuedAt(Date.from(now))
448+
.expiration(Date.from(expiration));
449+
450+
claims.forEach(builder::claim);
451+
452+
return builder
337453
.signWith(
338454
new SecretKeySpec(
339455
"testSecretThatIsAtLeast256BitsLong32Chars".getBytes(UTF_8), "HmacSHA256"))

sqrl-testing/sqrl-testing-integration/src/test/java/com/datasqrl/FullUseCaseIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@ public class FullUseCaseIT extends AbstractFullUseCaseTest {
6161
@Disabled("Intended for manual usage")
6262
@Test
6363
void specificUseCase() {
64-
var pkg = USE_CASES.resolve("avro-schema-legacy-ts").resolve("package.json");
64+
var pkg = USE_CASES.resolve("jwt-authorized").resolve("package.json");
6565

6666
var param = new UseCaseParam(pkg);
6767
fullUseCaseTest(param);

sqrl-testing/sqrl-testing-integration/src/test/resources/usecases/jwt-authorized/snapshots/AuthMyTable-valid-no-val.snapshot

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"errors" : [ {
3-
"message" : "Exception while fetching data (/AuthMyTable) : Attribute 'val' is not present on authorization",
3+
"message" : "Exception while fetching data (/AuthMyTable) : Forbidden",
44
"locations" : [ {
55
"line" : 2,
66
"column" : 3

0 commit comments

Comments
 (0)