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

Idiomic way to customize HTTP_UNAUTHENTICATED(401) response? #231

Open
commodis opened this issue Dec 16, 2020 · 13 comments
Open

Idiomic way to customize HTTP_UNAUTHENTICATED(401) response? #231

commodis opened this issue Dec 16, 2020 · 13 comments
Milestone

Comments

@commodis
Copy link

commodis commented Dec 16, 2020

Greetings!

I am running mp-jwt on Widlfly 22 using this quickstart

https://github.com/wildfly/quickstart/tree/master/microprofile-jwt

Running the command

curl -i -H "Authorization: Bearer eyJraWQiOiJUZXN0IEtleSIsInR5cCI6Imp3dCIsImFsZyI6IlJTMjU2In0.eyJzdWIiOiJ0ZXN0VXNlciIsInVwbiI6InRlc3RVc2VyIiwiaXNzIjoicXVpY2tzdGFydC1qd3QtaXNzdWVyIiwiYXVkIjoiand0LWF1ZGllbmNlIiwiZ3JvdXBzIjpbIkVjaG9lciIsIlN1YnNjcmliZXIiXSwiYmlydGhkYXRlIjoiMjAxNy0wOS0xNSIsImp0aSI6Ijk5ZWNiOTBiLTNkNmEtNDRjOC1hNWQ2LTQ3MzFiNWVkYzk4MyIsImlhdCI6MTYwODEzMDQzMCwiZXhwIjoxNjA4MTQ0ODMwfQ.jKkFi3076_M9CFL5FYpkhrSOYX7ZzCzas0XQtoKHjml2-pfMnGuPG0Xyv1a-Ii17o6pzggt95-PuqN48zfOzYHXL0qNagXrVntn7e-WLansTd3WfVh9RLRSOpN9Jw2sxIQGQYhvR5j27wXn2czN8aRw1Qs_ORJU0krxK9f3ZZFycshCg5ytN0mKObfSupTKqDKRtwfpVsxwflY3WNXpC99TADQStYatgpzQLj1rG4NWHQ8zoGJrAIuGVwOXMg55duribL_K_NelLEtUaHi2slkfd2R9IlZbgieMI1w-4ITGPWb1coAQqhIzaOHXv4NA4rkmcxsHz7lkYhx_TOPHLZQ" http://localhost:8080/microprofile-jwt/Sample/subscription

results in HTTP 200 and in 401 if I modify the bearer token (as expected). The response is

HTTP/1.1 401 Unauthorized
Connection: keep-alive
Content-Type: text/html;charset=UTF-8
Content-Length: 71
Date: Wed, 16 Dec 2020 16:45:44 GMT

<html><head><title>Error</title></head><body>Unauthorized</body></html>

having a HTML-body of <html><head><title>Error</title></head><body>Unauthorized</body></html>. I tried to overwrite
this behavior using a javax.ws.rs.container.ContainerResponseFilter but the filter-method is invoked for HTTP status code 200 but not for 401.

Here is my filter implementation

package org.wildfly.quickstarts.mpjwt;

import java.io.IOException;

import javax.ws.rs.container.ContainerRequestContext;
import javax.ws.rs.container.ContainerResponseContext;
import javax.ws.rs.container.ContainerResponseFilter;
import javax.ws.rs.ext.Provider;

@Provider
public class UnauthorizedResponseFilter implements ContainerResponseFilter {

    public UnauthorizedResponseFilter() {
        System.out.println("init unauthorized response filter");
    }

    @Override
    public void filter(ContainerRequestContext requestContext, ContainerResponseContext responseContext) throws IOException {
        System.out.println("responseContext.getStatus() = " + responseContext.getStatus());

        if (responseContext.getStatus() == 401) {
            responseContext.setEntity("test");
        }
    }

}

I looked into the latest documentation how the rejection is handled and WebApplicationException was not mentioned thus I cannot use an javax.ws.rs.ext.ExceptionMapper.

How do I hook a ContainerResponseFilter into the response cycle of mp jwt authentication process?

@sberyozkin
Copy link
Contributor

@commodis thanks for this issue, I believe it is better be discussed at the WildFly or possibly smallrye-jwt forums/issues, CC @darranl

@commodis
Copy link
Author

This issue could also start a discussion about clarifying this aspect within the specification

@darranl
Copy link

darranl commented Dec 17, 2020

+1 this is definitely an area I would like to discuss further both at the specification and SmallRye level.

The MicroProfile JWT specification is built on top of existing RFCs which if I am recalling correctly did have the provision to provide a real challenge for clients to understand.

But as @sberyozkin for the WildFly specific you may be better in the WildFly Google Group for discussions.

@rdebusscher
Copy link
Member

rdebusscher commented Dec 17, 2020 via email

@sberyozkin
Copy link
Contributor

sberyozkin commented Dec 17, 2020

Hi @darranl I'm not aware of any specification in the OAuth2 or OIDC space which would recommend returning anything beyond 401.
Hence some resource servers may return 401, some 403, 401 is most used. May be somewhere some text does mention 401, I think I saw it somewhere.
The best I believe we can add here is a requirement in 2.0 to also return WWW-Authenticate: Bearer which is recommended for the OIDC providers when the client id is invalid during the grant requests, so I suppose we can follow the suite and make it a requirement, it won't harm.

The 401 itself is generally not really recommended to have more that the status itself plus may be a few headers. Adding more than what 401 already implies by definition, example some extra details why the token verification has failed, would not be great :-). Also human users don't really use bearer tokens so I guess HTML is not suited well.

But in any case, I'd say that how and what data should be returned can not be meaningfully generalized at the spec level, unless I'm missing something :-)
thanks

@darranl
Copy link

darranl commented Dec 18, 2020

@sberyozkin In this case it is more the optional nature of the challenge header I am talking about so a client can understand why a 401 was received, additionally there is an option for this challenge to contain the required scopes so the client understands the minimal set required on the token it recieves.

One aspect related to this however is I think we are going to find a general gap that needs to be plugged.

In relation to WildFly I can take our MicroProfile JWT quickstart and I can demonstrate how easy it is to get started, with virtually no configuration (Just a couple of MP-Config properties) I can have a JAX-RS endpoint secured and my demo client can call it. This is all great but I think as micro service type deployments increase we are going to end up with a more complex mesh of interactions.

So I think there will be a need at some point when calling an endpoint to identify the authentication requirements of that endpoint (they may not all be token based).

There are always windows where lost tokens could be misused so should likely recommend using tokens with the minimal permissions needed for the operation, some of this may be identifiable from the path but other context could be relevant.

In addition to that it may be the case that tokens are issued from multiple sources, this in turn could end up being recursive allowing a token from one identity provider to be used with another etc.. making this recursive.

So some of these issues I am thinking about are quite a bit higher up than the individual challenge, but the challenge may have some role to play in this. The risk with anything in the challenge is by definition it is after a call where authentication was not successful so you don't in turn want the challenge to leak information it should not be leaking.

To summarise I think it would be a case of "Being able to write a deployed JAX-RS client with zero security code and dynamically handle the differing authentication requirements of many endpoints".

@sberyozkin
Copy link
Contributor

sberyozkin commented Dec 18, 2020

Hey @darranl

There are always windows where lost tokens could be misused so should likely recommend using tokens with the minimal permissions needed for the operation, some of this may be identifiable from the path but other context could be relevant.

Indeed. IDPs should be managing it, it can detect how many times the token has been used, etc - but MP JWT 1.2 verification is always local. Perhaps we should support a remote token introspection - so there IDP can tell the endpoint if the token is still active based on its own checks (number of calls, etc)

So some of these issues I am thinking about are quite a bit higher up than the individual challenge, but the challenge may have some role to play in this. The risk with anything in the challenge is by definition it is after a call where authentication was not successful so you don't in turn want the challenge to leak information it should not be leaking.

This was my point as well; if we start formalizing that 401 response should be accompanied by the reasons the verification failed, it can become a leak.

Section 4.2.1.2 here, https://tools.ietf.org/html/rfc6749#section-4.1.2, is interesting - but there it is returned as a query parameter via the front channel to the confidential client, i.i the endpoint.
Returning similar details, example in a JSON format, can pose some risk or offer some hints where to start focusing as far as trying to break through is concerned.

To summarise I think it would be a case of "Being able to write a deployed JAX-RS client with zero security code and dynamically handle the differing authentication requirements of many endpoints".

This sounds good; but it can be hard to write a client code which can adapt dynamically to various requirements :-)

That said, I guess we can indeed consider what can be optionally returned upon a token verification failure,

{
  "error_description": "",
  "error_code": ""
}

where for example error_code would not be 401 but a code indicating why it happened.
If it is a signature verification failure then it should be just 401. If the failure happened with a correctly signed or signed/encryped token then I guess there is some good scope there to help the dynamic clients with some hints

thanks

@sberyozkin sberyozkin added this to the MPJWT-2.0 milestone Dec 18, 2020
@commodis
Copy link
Author

I agree that leaking more information than necessary is a bad practice
and it should not be enabled by default.
What I am asking for is a way to overwrite this behavior if my business demands it.

In JAX-RS you can do it by using a ContainerResponseFilter. MP-JWT builds on top of JAX-RS and I believe an implementation of MP-JWT should not reduce the degree of freedom of JAX-RS.

@sberyozkin
Copy link
Contributor

sberyozkin commented Dec 21, 2020

@commodis

MP-JWT builds on top of JAX-RS

This is not really correct, there is no specification level requirement for MP JWT implementation be JAX-RS-based . But as Darran implied, it can be useful to report the additional information about the reason behind a verification failure

@sberyozkin
Copy link
Contributor

sberyozkin commented Dec 21, 2020

@commodis See this code. If WildFly uses this JAX-RS filter then you can intercept it with ExceptionMapper and provide your own error response - but again it is not an MP-JWT level concern.
Note I've linked this issue to the MP JWT 2.0 milestone - but it won't be about customizing the error response in the JAX-RS flows but about optionally providing some additional information about a verification failure,
thanks

@commodis
Copy link
Author

Thanks for sharing the code and I already tried it.
When Smallrye throws the NotAuthorizedException my custom ExceptionMapper will not be triggered.
When I throw the exception "manually" by throw new NotAuthorizedException(response); it is triggered.

So I guess this might be a bug?

@sberyozkin
Copy link
Contributor

@commodis - this is probably because WildFly does not use those JAX-RS interceptors but HTTP auth mechanism - please discuss it in the WildFly forum - there should be a way to intercept the response at that level

@cghislai
Copy link

cghislai commented Apr 6, 2023

Handling of those errors should be specified, exceptions should be passed to mappers, and responses to response filters.
For instance, i need to add cors headers so my frontends can handle 401 errors correctly/differently than 503. When my backend is unavailable, i expect a responses with no cors headers so the browser can show me a 0 status code. When my backend is reachable, i expect my response filter adds the cors headers, and that my frontend can read the status code. Unfortunately this does not work out of the box for those 401 on wildfly.

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

No branches or pull requests

5 participants