-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Custom Exception in SecurityIdentityAugmentor #39155
Comments
/cc @sberyozkin (security) |
@PierreAdam I can suggest a couple of things to try. |
Hello @sberyozkin, thank you for your quick answer ! I did some tests and it seems like However, if I disable the proactive authentication Also noteworthy, it seems that the behavior after disabling the proactive authentication when throwing an exception is also working when calling I think this kind of half solves the issue but forcing the proactive authentication off for only this purpose seems a bit weird to me. |
+1 to everything @sberyozkin suggested, that's a right way to achieve your goals, just make sure no security
with disabled proactive auth and no HTTP perms defined should work
that's implementation detail :-)
I don't get this comment, can you elaborate what you mean?
We were being careful and we only propagate security exceptions because there could be quite a noise. Not just your exception would be propagated, but any other exception as well and RESTEasy Reactive started just to invoke exception mappers. I'm not sure it's a good idea.
I'm honestly not sure we should solve this, but it is possible. Anyway, you have plenty of other options, like you can use HTTP security policy and just end with 404 or throw that exception and define Vert.x failure handler. |
BTW there are security and performance concerns if we enable this for other exceptions as there will be more processing that does not belong to RR |
One else thing again
I think it is a good think that we run security before anything else and if there is an issue for which we don't know that we need more processing, we end request promptly |
btw I'd expect that inside augmentor the security exception is thrown. you can also throw security exception, define exception mapper and if exception reason matches, you return 404, if not, you delegate to standard exception mapper? |
I just wanted to point out that with public Uni<SecurityIdentity> augment(@NonNull final SecurityIdentity securityIdentity,
@NonNull final AuthenticationRequestContext context) {
return Uni.createFrom().failure(new ResourceNotFoundException());
// OR
throw new ResourceNotFoundException();
// OR
return context.runBlocking(() -> {
throw new ResourceNotFoundException();
});
}
You are probably right on this part. I also tried having own exception extending
This is actually an excellent point ! That kinda fully solve my problem using a trick of course. However, wouldn't it be nice to not have to "Override" |
After some tests, the most elegant solution is as follow : public class ResourceNotFoundException extends AuthenticationCompletionException {
// Standard constructor
} @Provider
public class CustomAuthenticationCompletionExceptionMapper implements ExceptionMapper<AuthenticationCompletionException> {
private final AuthenticationCompletionExceptionMapper originalMapper;
public CustomAuthenticationCompletionExceptionMapper() {
this.originalMapper = new AuthenticationCompletionExceptionMapper();
}
@Override
public Response toResponse(final AuthenticationCompletionException exception) {
if (exception instanceof ResourceNotFoundException) {
return Response.status(Response.Status.NOT_FOUND).build();
}
return this.originalMapper.toResponse(exception);
}
} Even with In the So I think it would be more of an improvement to be able to have an exception that would be "user based" to allow this sort of things 😄 |
Alternatively, you can have
understood
got it, but as you know, blocking is a bad thing :-) I'd go with option number 1.
let's see how my suggestion with the |
@PierreAdam Hi, as I said, the security chain runs earlier by default, the Quarkus Security flow is Quarkus specific and it runs before JAX-RS by default, for the JAX-RS exception mapping to be effective one just needs to delay the security check until it is known the security is in fact required at a given JAX-RS resource, this is what disabling the proactive authentication is about, it does not reduce the security properties of the application. I believe now that you have it working, we can close this issue, right ? |
hey @sberyozkin , I believe that having documented |
@michalvavrik seems like you're absolutely right about public class CustomException extends RuntimeException implements AuthenticationException {
// Constructor etc.
} @Provider
public class CustomExceptionMapper implements ExceptionMapper<CustomException> {
@Override
public Response toResponse(final CustomExceptionexception) {
// Process your exception here.
return Response.status(Response.Status.NOT_FOUND).build();
}
} Thank you very much for the information absolutely worth documenting in my mind 😄 |
Describe the bug
I'm currently making an implementation of
SecurityIdentityAugmentor
that extracts data from the headers and use it to validate that the user currently logged has access to the ressource.If something goes wrong in analyzing the header value such as a wrong value, I want to throw a custom exception
ResourceNotFoundException
and send a 404 not found since the ressource being asked is not valid.It kinda looks like this
It looks like the exception is always causing a 500 internal server error instead of going through the
ExceptionMapper
.For information, when I'm trying to throw the same exception from a controller, it's working just fine and I get my 404 without any issues.
Expected behavior
I would except my custom exception to behave in the same way as exceptions such as
io.quarkus.security.ForbiddenException
but with my custom exception.Actual behavior
An error 500 is always sent to the client despite the
ExceptionMapper
.How to Reproduce?
SecurityIdentityAugmentor
ExceptionMapper
ExceptionMapper
Output of
uname -a
orver
Windows
Output of
java -version
OpenJDK 21
Quarkus version or git rev
3.8.1
Build tool (ie. output of
mvnw --version
orgradlew --version
)Maven 3.9.5
Additional information
No response
The text was updated successfully, but these errors were encountered: