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

Quarkus JWT producing 401 errors using @RolesAllowed #14450

Closed
mklueh opened this issue Jan 20, 2021 · 9 comments · Fixed by #14815
Closed

Quarkus JWT producing 401 errors using @RolesAllowed #14450

mklueh opened this issue Jan 20, 2021 · 9 comments · Fixed by #14815
Assignees
Labels
Milestone

Comments

@mklueh
Copy link
Contributor

mklueh commented Jan 20, 2021

Describe the bug

I´m running into an issue with Quarkus JWT and I cannot get past the 401 error as none of my requests is getting authorized.

To create my ssh key pair I´m using the following code from the documentation https://quarkus.io/guides/security-jwt#generating-a-jwt :

openssl genrsa -out src/main/resources/privateKey_tmp.pem 2048
openssl rsa -pubout -in src/main/resources/privateKey.pem -out src/main/resources/META-INF/resources/publicKey.pem
openssl pkcs8 -topk8 -nocrypt -inform pem -in src/main/resources/privateKey_tmp.pem -outform pem -out src/main/resources/privateKey.pem

And I´m creating my token with

Jwt
                .issuer("some-id")
                .groups("user")
                .expiresAt(System.currentTimeMillis() + 1000000000L)
                .sign()

While providing the necessary properties in application.properties

I´ve created a small producer where you just need to run the tests.

https://github.com/mklueh/quarkus-jwt-401-reproducer

Am I missing something or do I issue invalid tokens for whatever reason?

Thanks for any help

@mklueh mklueh added the kind/bug Something isn't working label Jan 20, 2021
@ghost
Copy link

ghost commented Jan 20, 2021

/cc @sberyozkin

@sberyozkin
Copy link
Member

@mklueh Hi, can you please add the trace configuration as suggested at:
https://quarkus.io/guides/security-jwt#how-to-check-the-errors-in-the-logs ?

(also add io.smallrye.jwt.auth.principal.PrincipalLogging)

thanks

@sberyozkin
Copy link
Member

sberyozkin commented Jan 20, 2021

this is not related to the issue but you can also optimize

.expiresAt(System.currentTimeMillis() + 1000000000L)

by default it is set to +5 mins so if it suits for the test then you can drop this call - and there should be a new method expiresIn, ex .expiresIn(100000000L) etc

@mklueh
Copy link
Contributor Author

mklueh commented Jan 20, 2021

@sberyozkin thank you for the fast response.

As soon as I create this class the auth is working as expected, which is not what I have expected.

@ApplicationScoped
@Alternative
@Priority(1)
public class TestJWTCallerPrincipalFactory extends JWTCallerPrincipalFactory {

    @Override
    public JWTCallerPrincipal parse(String token, JWTAuthContextInfo authContextInfo) throws ParseException {
        try {
            // Token has already been verified, parse the token claims only
            String json = new String(Base64.getUrlDecoder().decode(token.split("\\.")[1]), StandardCharsets.UTF_8);
            return new DefaultJWTCallerPrincipal(JwtClaims.parse(json));
        } catch (InvalidJwtException ex) {
            throw new ParseException(ex.getMessage());
        }
    }
}

So obviously the DefaultJWTCallerPrincipalFactory does not work in my case.

I´ve debugged into the class JwtConsumer.java and there I´ve seen that the issuer was mismatching.

I did the same in my actual application (where the issuers were correct) and found the reason why I´ve created this reproducer in the first place:

I´ve passed an integer as the subject which is syntactically correct but leads to an exception I did not see in the logs

JWT.claim(Claims.sub.name(), 1)

Passing the number as a string did work for me

JWT.claim(Claims.sub.name(), "1")

Thank you for helping :)

@sberyozkin
Copy link
Member

Hi @mklueh thanks, that original code should work as well, I'll fix it and will close this issue once Quarkus gets updated to smallrye-jwt 2.4.3

@sberyozkin sberyozkin self-assigned this Jan 20, 2021
@sberyozkin sberyozkin added this to the 1.12 - master milestone Jan 20, 2021
@sberyozkin
Copy link
Member

yeah, it is better to do Jwt.subject() :-), but for a generic claim setter it should work...

@sberyozkin
Copy link
Member

Opened smallrye/smallrye-jwt#391

@mklueh
Copy link
Contributor Author

mklueh commented Jan 23, 2021

Thank you :)

@sberyozkin
Copy link
Member

sberyozkin commented Feb 4, 2021

@mklueh Sorry, missed you last comment, thanks :-). I'm going to update to smallrye-jwt 2.4.3 shortly. FYI, after some thoughts I've decided to throw IAE in case of the well-known claims like sub having the illegal value types; the conversion may also hide the bug otherwise. This will ensure that it is consistent, ex, .subject("1") and .claim(Claims.sub, "1"), etc.
We can't avoid the conversion if a claim() Object is not one of the formally supported ones like Jsonp one, etc, but for those which are directly supported at the API level we can enforce the correct type is used

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants