Create smallrye-jwt-http-mechanism and smallrye-jwt-cdi-extension modules #381
Conversation
There was a problem hiding this comment.
Just a question on the JAX-RS change, but otherwise it looks good to me. Thanks @sberyozkin .
| context.register(JWTAuthorizationFilterRegistrar.class); | ||
|
|
||
| if (!SmallRyeJWTAuthCDIExtension.isHttpAuthMechanismEnabled()) { | ||
| context.register(JWTAuthenticationFilter.class); | ||
|
|
||
| JAXRSLogging.log.eeSecurityNotInUseButRegistered(JWTAuthenticationFilter.class.getSimpleName()); | ||
| } | ||
| context.register(JWTAuthenticationFilter.class); |
There was a problem hiding this comment.
@sberyozkin - I'm trying to get my head around the impact of registering the filter unconditionally. Suppose the HttpAuthMechanism is registered via the CDI extension. I assume (but I am not certain) that the filter would then execute second, and the principal would be an instance of JsonWebToken, bypassing the code in filter. What are your thoughts?
There was a problem hiding this comment.
Hi @MikeEdgar I think it would be simplest to copy that code from the feature checking for HttpAuthMechanism just to preserve the original logic. We can fine tune it further as required... Otherwise, indeed, I'm not 100% sure of the side-effects introduced by this update - it does not look like this feature is used right now, but it is better to keep the check in place
There was a problem hiding this comment.
@MikeEdgar I've tried to do this copy but found it was not easy to do. I suppose what I said in the closed PR probably makes sense. The user should be now responsible for making sure no ambiguities happen.
This JAX-RS feature is not annotated as Provider and it is not loaded in the CDI extension. So if some users would like to use this feature - they'd have to enable it in their Application or via DynamicFeature.
And if the users choose to use the CDI extension then all is good - it will load the HTTP mechanism first if EESec is enabled and if not - it will register the JAX-RS filter directly.
So I think all is good :-) - do you agree ?
There was a problem hiding this comment.
@sberyozkin - I agree. Also, for historical perspective, much of this code was originally intended to allow using SmallRye JWT in Wildfly prior to it's official inclusion.
There was a problem hiding this comment.
@MikeEdgar OK, thanks. Ok then I'll go ahead with the merge now that Darran also checked it for us to do RC2 today
| <?xml version="1.0" encoding="UTF-8"?> | ||
|
|
||
| <!-- | ||
| ~ Copyright 2017 Red Hat, Inc. |
There was a problem hiding this comment.
As a new file should this be 2021?
| <?xml version="1.0" encoding="UTF-8"?> | ||
|
|
||
| <!-- | ||
| ~ Copyright 2017 Red Hat, Inc. |
|
@sberyozkin my day is a little bit disrupted today but do you want me to give this a go with WildFly and double check no unintended consequences? Otherwise looks good to me as well. |
|
@darranl Thanks, I update the year in a few poms. As discussed, we can do RC2 - it also makes sense to try in Quarkus before the final, we can do RC2 later today |
* Update the version to 3.0.0-SNAPSHOT * JWE Server support * Fixed compile issues. * Removed failing tests, to fix with new implementations. * Support MP 1.2 mp.jwt.decrypt.key.location. * Ignore failing tests to fix later. * Baseline to implement new configuration properties. * Support MP JWT 1.2 new configurations (#240) * Support mp.jwt.token.header and mp.jwt.token.cookie configurations. * Support mp.jwt.verify.audiences. * Fixed issues with sign and decrypt. (#245) * Readd fixed TCK tests. * Throw NotAuthorizedException if a JWT is sent to an unauthenticated endpoint and is not valid. (#241) * Support mp.jwt.verify.publickey.algorithm. (#247) * Remove properties and methods which have been deprecated before 2.1.2 * Remove deprecated requireIssuer * Get all TCK tests passing * Remove unneeded test. * Added mpjwt12 branch to build. * Use MP JWT SNAPSHOT version. * Include all TCK tests. * Remove unneeded file. * Fix formatter errors. * TCK compatible MP JWT 1.2. * Release 3.0.0-RC1. (#338) * [maven-release-plugin] prepare release 3.0.0-RC1 * [maven-release-plugin] prepare for next development iteration * Fix the coverage module and smallrye config versions * Move the JAX-RS related code into smallrye-jwt-jaxrs (#367) * Fix JWTAuthenticationFilter and logging/messages resolution issues * Create smallrye-jwt-http-mechanism and smallrye-jwt-cdi-extension modules (#381) * Fix a javadoc problem in the jwt-jaxrs module * Release Smallrye JWT 3.0.0-RC2 (#386) * [maven-release-plugin] prepare release 3.0.0-RC2 * [maven-release-plugin] prepare for next development iteration * Fix check in spec configuration for the decrypt key location. * Remove Optional for the config properties with the default values (#389) * Update MP Config version to 2.0 (#392) * Release Smallrye JWT 3.0.0-RC4 (#412) * [maven-release-plugin] prepare release 3.0.0-RC4 * [maven-release-plugin] prepare for next development iteration * Remove deprecated JWT Build API inner-sign none signature support and key location properties (#413) * Complete the removal of smallrye.jwt.sign.key-location and smallrye.jwt.encrypt.key-location properties (#416) * Release Smallrye JWT 3.0.0-RC5 (#417) * [maven-release-plugin] prepare release 3.0.0-RC5 * [maven-release-plugin] prepare for next development iteration * Release SmallRye JWT 3.0.0 (#418) * [maven-release-plugin] prepare release 3.0.0 * [maven-release-plugin] prepare for next development iteration * Remove the mpjwt12 branch from build.yml Co-authored-by: Roberto Cortez <radcortez@yahoo.com> Co-authored-by: SmallRye CI <smallrye@googlegroups.com>
Hi All, here is a new PR, thanks
Fixes #332