Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle ResourceMapperError in AsertoAuthorizationManager.check #12

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

ronenh
Copy link
Contributor

@ronenh ronenh commented Aug 5, 2024

Proposed fix for #11.

ResourceMapper.getResource may throw exceptions of type ResourceMapperError but AsertoAuthorizationManager.check doesn't handle those errors. The result is a 500 response.

With this proposed change, the AsertoAuthorizationManager catches the ResourceMapperError, logs it, and returns AuthorizationDecision(false) to deny access.

The ideal outcome when a request is made to a path that matches no routes would be a 404, but the authorization manager has no definitive way of determining the underlying cause of the ResourceMapperError and even if it did, @PreAuthorize requires a boolean value so the only viable options are to either allow or deny the call.

@ronenh ronenh force-pushed the resource-mapper-errors branch from 1a2055a to a0d8870 Compare August 5, 2024 13:43
@ronenh ronenh requested a review from gertd August 5, 2024 13:44
@ronenh ronenh merged commit 07677a1 into main Aug 7, 2024
2 checks passed
@ronenh ronenh deleted the resource-mapper-errors branch August 7, 2024 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants