fix(localega-tsd-proxy): return 403 when export token verification fails - #868
Open
yasinmiran wants to merge 1 commit into
Open
fix(localega-tsd-proxy): return 403 when export token verification fails#868yasinmiran wants to merge 1 commit into
yasinmiran wants to merge 1 commit into
Conversation
…ils #patch_localega-tsd-proxy Map JWT verification failures on GDI export to 403 so a rejected credential is not reported as a server error. parseVerified already throws on a forged or expired access token; after #862 a forged visa does too. Both used to land on the controller catch-all. Closes #863.
There was a problem hiding this comment.
Pull request overview
This PR fixes the export endpoint’s error mapping so that JWT verification failures (access token or visa/passport token verification) return HTTP 403 via GenericException, instead of falling through to the controller catch-all and returning HTTP 500.
Changes:
- Wraps
parseVerified(...)andgetControlledAccessGrantsVisas(...)inExportRequestService.exportRequestGDI(...)to translateJwtExceptionintoGenericException(FORBIDDEN, "Token verification failed"). - Adds unit tests covering forged access token, forged visa, empty visa list, and blank subject behavior for
exportRequestGDI.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| services/localega-tsd-proxy/src/main/java/no/elixir/fega/ltp/services/ExportRequestService.java | Maps JWT verification exceptions to 403 Forbidden to avoid controller catch-all 500. |
| services/localega-tsd-proxy/src/test/java/no/elixir/fega/ltp/services/ExportRequestServiceTest.java | Adds regression tests ensuring token/visa verification failures and edge cases map to the intended responses. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+139
to
+140
| // A token we cannot verify is the caller's problem rather than a server fault. Without this | ||
| // the controller's catch-all at ExportRequestController:48 answers 500. See #863. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
POST /export/gdimaps a token that fails verification onto the controller catch-all and answers 500, which tells the caller the fault is ours. After #862 a forged visa in an otherwise valid passport starts throwingSignatureExceptionfrom clearinghouse, so this is the path that would regress. A forged or expired access token already 500s on main today, becauseparseVerifiedthrowsJwtExceptionwith nobody mapping it, and that half is live whether or not #862 lands first.The wrap lives in
ExportRequestService.exportRequestGDI.JwtExceptionfromparseVerifiedand fromgetControlledAccessGrantsVisasbecomesGenericException(FORBIDDEN, "Token verification failed"), which the controller already turns into 403, and an empty visa list stays 403 with"No valid visas found for this resource". Call order is the original one: verify the access token, reject a missing subject with 400, then fetch visas./export/feganever parses a token, so it is left alone.Status is 403, matching this module's
AAIAspect(present-but-bad token is 403; missing header is 401) and the empty-visa case already on this endpoint. sda-doa answers 401 for the same failure, which is a fair reading of a credential that does not verify. Distinguishing a forgery from an empty passport, which is what #790 asked for, then sits on the response body rather than on two different statuses.Closes #863.
Test plan
./gradlew :services:localega-tsd-proxy:test --tests no.elixir.fega.ltp.services.ExportRequestServiceTestThe four tests pin forged access token and forged visa to 403 with
"Token verification failed", empty visa list to 403 with the existing empty-passport message, and a blank subject to 400 without callinggetControlledAccessGrantsVisas.