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

Allow to do a blocking smallrye-jwt authentication #20635

Merged
merged 1 commit into from
Oct 14, 2021

Conversation

sberyozkin
Copy link
Member

@sberyozkin sberyozkin commented Oct 8, 2021

It is related to smallrye/smallrye-jwt#511.

smallrye-jwt uses an internal blocking Jose4j HTTP client and if the keys have to be fetched from a remote endpoint then it is done at the 1st request time and if the remote keys endpoint is slow or unavailable then the problems like #511 will happen.

As far as smallrye-jwt is concerned, the best effort can be made there to read the keys early. However even in that case, when dealing with the JWK keys, they will have to be refreshed now and then remotely, so a risk of blocking will remain.

So, at the quarkus level it makes sense IMHO to let users control that a blocking authentication has to be performed in case of the slow/unstable remote key endpoints.

This is not a problem at the quarkus-oidc level but since we do not control the client at the smallrye-jwt level, we have to find a workaround. Longer term, 1) making smallrye-jwt reactive - the issue exists 2) making Jose4J HTTPS handler accept a client interface as opposed to its Simple HTTP client for Quarkus to register Vertx client will make it work as well but it may take awhile.

If would be good if it could make it to 2.4.0.CR1, CC @aloubyansky

thanks

@sberyozkin
Copy link
Member Author

Let me make it a draft for now

@sberyozkin sberyozkin marked this pull request as draft October 8, 2021 17:24
@sberyozkin
Copy link
Member Author

@stuartwdouglas Hi Stuart - do you think calling IdentityProviderManager.authenticateBlocking can actually work ? I wonder if HttpAuthenticationMechanism can be passed a blocking request context as a RoutingContext attribute ?

@sberyozkin
Copy link
Member Author

Silly me, it can be handled at the identity provider level, one would need to have a class specific blocking request context to call IdentityProviderManager.authenticateBlocking but since smallrye-jwt inlines its own IdentityProvider it can be handled there

@sberyozkin sberyozkin marked this pull request as ready for review October 8, 2021 21:15
@sberyozkin sberyozkin force-pushed the blocking_jwt_authentication branch 2 times, most recently from 3f0444e to 66bf00d Compare October 8, 2021 21:17
@sberyozkin sberyozkin marked this pull request as draft October 8, 2021 21:48
@sberyozkin
Copy link
Member Author

Converting to draft again, Roberto, @radcortez, quarkus.smallrye-jwt.blocking-authentication=true has no effect, SmallRyeJwtConfig is injected, but its blockingAuthentication is always false. Can you spot what I'm missing ? Pretty sure it is something silly on my part. You can add quarkus.smallrye-jwt.blocking-authentication=true to one of application.properties in smallrye-jwt/deployment to check, thanks

@sberyozkin sberyozkin marked this pull request as ready for review October 13, 2021 16:56
@sberyozkin sberyozkin force-pushed the blocking_jwt_authentication branch from 66bf00d to 4531e3c Compare October 13, 2021 16:57
@sberyozkin
Copy link
Member Author

@radcortez thanks, I've also added a test, I spent nearly an hour last Friday staring at the test file checking for a property typo but could not find a problem and now it just works, sorry for the noise

@sberyozkin sberyozkin added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 13, 2021
@sberyozkin sberyozkin merged commit b90701b into quarkusio:main Oct 14, 2021
@quarkus-bot quarkus-bot bot added this to the 2.5 - main milestone Oct 14, 2021
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Oct 14, 2021
@sberyozkin sberyozkin deleted the blocking_jwt_authentication branch October 14, 2021 08:43
@aloubyansky aloubyansky modified the milestones: 2.5 - main, 2.4.0.Final Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants