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

Scheme(s) should not be discared when alternate header is used #286

Closed
abutic opened this issue Jul 19, 2020 · 4 comments · Fixed by #296
Closed

Scheme(s) should not be discared when alternate header is used #286

abutic opened this issue Jul 19, 2020 · 4 comments · Fixed by #296
Milestone

Comments

@abutic
Copy link

abutic commented Jul 19, 2020

Is it intentional not to check for scheme(s) when alternate header is used or is it a bug?

When default Authorization header is used, smallrye.jwt.token.schemes property is properly taken into account, but when alternate header name is used, for instance like this:
smallrye.jwt.token.header=x-forwarded-authorization
you're then completely discarding smallrye.jwt.token.schemes property.

Furthermore, you don't even allow a header value to contain any scheme.

For instance, if configured to use the default Authorization header, this header value is valid:
Bearer <JWT token>

But if smallrye.jwt.token.header property is set, then
Bearer <JWT token>
is not a valid header value.
Furthermore, any scheme name in front of a JWT itself is illegal, no matter if smallrye.jwt.token.schemes property is being used at all and no matter its value. Only this is valid header value if smallrye.jwt.token.header property is set:
<JWT token>

If this is a bug, it can be easily fixed by changing the io.smallrye.jwt.auth.AbstractBearerTokenExtractor#getBearerToken method here:

As you may guess, this is not an academic discussion: when Google Cloud Endpoints v2 is configured to use Firebase Auth, it forwards original Authorization header value as x-forwarded-authorization header value. So, as the original header value is forwared "as is", it contains the "Bearer " prefix. Hence the trouble...

@sberyozkin
Copy link
Contributor

@abutic Hi
OK, the notion of the authentication scheme is relevant in the context of Authorization because it is known that its value is structured as AuthScheme AuthValue.
If it is not Authorization then the assumption is it contains the value directly, we don't know the value format of the custom headers.
The only exception is Cookie.

Now if we have

X-Forwarded-Authorization: Bearer <JWT Token>

then indeed smallrye-jwt can't handle it now.
I suppose we can split the value of the custom header and if the array length is 2 and the first value is equal to the configured scheme then we tale the 2nd array element...

That said, another alternative is to deal with the forwarded header in the filter dealing with the X-Fowarded-* headers. Are you using Quarkus ?

@abutic
Copy link
Author

abutic commented Jul 20, 2020

@sberyozkin Hi, thanks for the quick reply!

Yes, my guess was that you're making an assumption that alternate header contains the value directly, that's why I didn't categorically say it was a bug. :-) I guess there's no standard dealing with it, so it can go either way.

I can speak for myself: my expectaion was, after reading the docs, that header value would be treated the same way, no matter which header contained it, Authorization or another one.

Their is no mention of smallrye.jwt.token.schemes property being discarded if smallrye.jwt.token.header property was set. Or at least I didn't get it.

I suppose we can split the value of the custom header and if the array length is 2 and the first value is equal to the configured scheme then we tale the 2nd array element...

That sound good! It's an elegant way of dealing with this kind of situation, instead of introducing yet another config param, something like smallrye.jwt.use-schemes-with-alternate-header. Either way, if the docs clarify it - excellent!

Yes, we're using Quarkus. You're suggesting using a request filter to modify the header?

@sberyozkin
Copy link
Contributor

Yes, we're using Quarkus. You're suggesting using a request filter to modify the header?

I'm not 100% sure, but it feels that in this specific case, X-Forwarded-Authorization is only one specific case of X-Forwarded-* which are dealt with in https://github.com/quarkusio/quarkus/blob/master/extensions/vertx-http/runtime/src/main/java/io/quarkus/vertx/http/runtime/ForwardedParser.java. Handling it there would offer a much more generic support for such a case (example, with quarkus-oidc service applications). That said, since smallrye-jwt is not only used with Quarkus it would make sense to support it directly in smallrye-jwt as well

@abutic
Copy link
Author

abutic commented Jul 20, 2020

That said, since smallrye-jwt is not only used with Quarkus it would make sense to support it directly in smallrye-jwt as well

@sberyozkin Are you the man in charge of that? I mean, is it your decision whether it will be supported in smallrye-jwt or not? It seems to me that the possibility of using Quarkus+SmallRye-JWT behind Google Endpoints v2 out-of-the-box, without header modifications made by filters, makes a pretty strong case.

The case is even stronger if we take into account that changes needed for all this are fairly easy to implement.

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 a pull request may close this issue.

2 participants