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.

Sign up for GitHub

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

Jump to bottom

smallrye-jwt-extension: Make role mapping configurable and use newer smallrye-jwt apis #7485

Closed
wants to merge 4 commits into from
Closed

smallrye-jwt-extension: Make role mapping configurable and use newer smallrye-jwt apis #7485

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
db2501d
Make JwtParser and JwtRolesMapper injectable
andreas-eberle Feb 28, 2020
c78dbb7
Use newer smallrye-jwt apis
andreas-eberle Feb 28, 2020
dfd67a4
Remove JwtTokenUtils
andreas-eberle Feb 28, 2020
cd7eb56
Fix method name
andreas-eberle Mar 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion bom/runtime/pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@
<smallrye-open-api.version>1.2.1</smallrye-open-api.version>
<smallrye-opentracing.version>1.3.4</smallrye-opentracing.version>
<smallrye-fault-tolerance.version>4.1.0</smallrye-fault-tolerance.version>
<smallrye-jwt.version>2.0.13</smallrye-jwt.version>
<smallrye-jwt.version>2.1.0-SNAPSHOT</smallrye-jwt.version>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't merge as is, there's a SNAPSHOT version.

<smallrye-context-propagation.version>1.0.11</smallrye-context-propagation.version>
<smallrye-reactive-streams-operators.version>1.0.10</smallrye-reactive-streams-operators.version>
<smallrye-converter-api.version>1.0.10</smallrye-converter-api.version>
Expand Down
4 changes: 4 additions & 0 deletions ...jwt/deployment/src/main/java/io/quarkus/smallrye/jwt/deployment/SmallRyeJwtProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import io.quarkus.deployment.builditem.nativeimage.NativeImageResourceBuildItem;
import io.quarkus.deployment.builditem.nativeimage.ReflectiveClassBuildItem;
import io.quarkus.security.deployment.JCAProviderBuildItem;
import io.quarkus.smallrye.jwt.runtime.auth.DefaultJwtRolesMapper;
import io.quarkus.smallrye.jwt.runtime.auth.JWTAuthMechanism;
import io.quarkus.smallrye.jwt.runtime.auth.JwtPrincipalProducer;
import io.quarkus.smallrye.jwt.runtime.auth.MpJwtValidator;
Expand All @@ -39,6 +40,7 @@
import io.smallrye.jwt.auth.cdi.CommonJwtProducer;
import io.smallrye.jwt.auth.cdi.JsonValueProducer;
import io.smallrye.jwt.auth.cdi.RawClaimTypeProducer;
import io.smallrye.jwt.auth.principal.DefaultJWTParser;
import io.smallrye.jwt.config.JWTAuthContextInfoProvider;

/**
Expand Down Expand Up @@ -79,6 +81,8 @@ void registerAdditionalBeans(BuildProducer<AdditionalBeanBuildItem> additionalBe
removable.addBeanClass(RawClaimTypeProducer.class);
removable.addBeanClass(JsonValueProducer.class);
removable.addBeanClass(JwtPrincipalProducer.class);
removable.addBeanClass(DefaultJwtRolesMapper.class);
removable.addBeanClass(DefaultJWTParser.class);
removable.addBeanClass(Claim.class);
additionalBeans.produce(removable.build());

Expand Down
10 changes: 9 additions & 1 deletion extensions/smallrye-jwt/deployment/src/test/java/io/quarkus/jwt/test/TokenRealmUnitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,12 @@
import io.quarkus.security.identity.request.TokenAuthenticationRequest;
import io.quarkus.security.runtime.AnonymousIdentityProvider;
import io.quarkus.security.runtime.QuarkusIdentityProviderManagerImpl;
import io.quarkus.smallrye.jwt.runtime.auth.DefaultJwtRolesMapper;
import io.quarkus.smallrye.jwt.runtime.auth.JwtRolesMapper;
import io.quarkus.smallrye.jwt.runtime.auth.MpJwtValidator;
import io.smallrye.jwt.auth.principal.DefaultJWTParser;
import io.smallrye.jwt.auth.principal.JWTAuthContextInfo;
import io.smallrye.jwt.auth.principal.JWTParser;

/**
* Validate usage of the bearer token based realm
Expand All @@ -29,8 +33,12 @@ public void testAuthenticator() throws Exception {
KeyPair keyPair = generateKeyPair();
PublicKey pk1 = keyPair.getPublic();
PrivateKey pk1Priv = keyPair.getPrivate();

JWTAuthContextInfo contextInfo = new JWTAuthContextInfo((RSAPublicKey) pk1, "https://server.example.com");
MpJwtValidator jwtValidator = new MpJwtValidator(contextInfo);
JWTParser jwtParser = new DefaultJWTParser(contextInfo);
JwtRolesMapper jwtRolesMapper = new DefaultJwtRolesMapper();
MpJwtValidator jwtValidator = new MpJwtValidator(jwtParser, jwtRolesMapper);

QuarkusIdentityProviderManagerImpl authenticator = QuarkusIdentityProviderManagerImpl.builder()
.addProvider(new AnonymousIdentityProvider())
.setBlockingExecutor(new Executor() {
Expand Down
19 changes: 19 additions & 0 deletions ...jwt/runtime/src/main/java/io/quarkus/smallrye/jwt/runtime/auth/DefaultJwtRolesMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package io.quarkus.smallrye.jwt.runtime.auth;

import java.util.Set;

import javax.enterprise.context.ApplicationScoped;

import org.eclipse.microprofile.jwt.JsonWebToken;

import io.quarkus.arc.DefaultBean;

@DefaultBean
@ApplicationScoped
public class DefaultJwtRolesMapper implements JwtRolesMapper {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What the existing smallrye.jwt properties such as groups path for example can't do that this interface will help with ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the moment I see three use cases:

  1. the current implementation cannot handle JWTs like I showed in the example at the top. Sometimes tokens do not contain a groups field but contain role information in the form of other fields. In that example in the form of a boolean "admin" claim. With the JwtRolesMapper, I can write custom logic for my custom behavior to map the roles and then using the standard @RolesAllowed annotation.
  2. It might well be that I do not want to use the group names as they are used in the JWT, which I might have to work with but have no control over. Sometimes the group names in the JWT are just not what they mean in my application. In that case, I can use the JwtRolesMapper to map the groups to roles that make sense in my context and thus make the code more readable.
  3. I have a case where I have to handle tokens from different issuers, all using different groups. With the JwtRolesMapper, I can map their groups depending on the issuer of the token and unify it to something usable in my application. Again, I don't have power over those issuers, which is why I have to make it work in my application.


@Override
public Set<String> mapGroupsAndRoles(JsonWebToken token) {
return token.getGroups();
}
}
2 changes: 1 addition & 1 deletion ...lrye-jwt/runtime/src/main/java/io/quarkus/smallrye/jwt/runtime/auth/JWTAuthMechanism.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
import io.smallrye.jwt.auth.AbstractBearerTokenExtractor;
import io.smallrye.jwt.auth.cdi.PrincipalProducer;
import io.smallrye.jwt.auth.principal.JWTAuthContextInfo;
import io.vertx.ext.web.Cookie;
import io.vertx.core.http.Cookie;
import io.vertx.ext.web.RoutingContext;

/**
Expand Down
9 changes: 9 additions & 0 deletions ...allrye-jwt/runtime/src/main/java/io/quarkus/smallrye/jwt/runtime/auth/JwtRolesMapper.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package io.quarkus.smallrye.jwt.runtime.auth;
Copy link
Member

@sberyozkin sberyozkin Feb 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same question, why is this interface needed ?

Copy link
Member

@sberyozkin sberyozkin Feb 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@andreas-eberle If there is a real case where the roles info is spread across multiple claims and when the existing smallrye properties can't really help with the auto extraction of these roles, then lets consider this interface (that should live then in smallrye-jwt I guess) but having it just in case would be redundant. If you have some specific requirement/mapping case in mind then lets review, thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interface is there to allow for easy injection of own implementations of this roles mapping. Please also see my comment to your previous remark. I'm happy to move this interface to smallrye-swt. Should I prepare a PR?


import java.util.Set;

import org.eclipse.microprofile.jwt.JsonWebToken;

public interface JwtRolesMapper {
Set<String> mapGroupsAndRoles(JsonWebToken token);
}
43 changes: 13 additions & 30 deletions ...allrye-jwt/runtime/src/main/java/io/quarkus/smallrye/jwt/runtime/auth/MpJwtValidator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,25 +1,21 @@
package io.quarkus.smallrye.jwt.runtime.auth;

import java.util.HashSet;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CompletionStage;

import javax.enterprise.context.ApplicationScoped;
import javax.inject.Inject;

import org.eclipse.microprofile.jwt.JsonWebToken;
import org.jboss.logging.Logger;
import org.jose4j.jwt.JwtClaims;
import org.jose4j.jwt.MalformedClaimException;
import org.jose4j.jwt.consumer.JwtContext;

import io.quarkus.security.AuthenticationFailedException;
import io.quarkus.security.identity.AuthenticationRequestContext;
import io.quarkus.security.identity.IdentityProvider;
import io.quarkus.security.identity.SecurityIdentity;
import io.quarkus.security.identity.request.TokenAuthenticationRequest;
import io.quarkus.security.runtime.QuarkusSecurityIdentity;
import io.smallrye.jwt.auth.principal.DefaultJWTTokenParser;
import io.smallrye.jwt.auth.principal.JWTAuthContextInfo;
import io.smallrye.jwt.auth.principal.JWTParser;
import io.smallrye.jwt.auth.principal.ParseException;

/**
Expand All @@ -30,17 +26,13 @@ public class MpJwtValidator implements IdentityProvider<TokenAuthenticationReque

private static final Logger log = Logger.getLogger(MpJwtValidator.class);

final JWTAuthContextInfo authContextInfo;

private final DefaultJWTTokenParser parser = new DefaultJWTTokenParser();

public MpJwtValidator() {
authContextInfo = null;
}
final JWTParser parser;
final JwtRolesMapper jwtRolesMapper;

@Inject
public MpJwtValidator(JWTAuthContextInfo authContextInfo) {
this.authContextInfo = authContextInfo;
public MpJwtValidator(JWTParser parser, JwtRolesMapper jwtRolesMapper) {
this.parser = parser;
this.jwtRolesMapper = jwtRolesMapper;
}

@Override
Expand All @@ -52,25 +44,16 @@ public Class<TokenAuthenticationRequest> getRequestType() {
public CompletionStage<SecurityIdentity> authenticate(TokenAuthenticationRequest request,
AuthenticationRequestContext context) {
try {
JwtContext jwtContext = parser.parse(request.getToken().getToken(), authContextInfo);
JsonWebToken jwt = parser.parse(request.getToken().getToken());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are these changes necessary ? I'd like to keep an option open to have the factory used instead of the parser so that people can plugin custom factories.

Copy link
Contributor Author

@andreas-eberle andreas-eberle Mar 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see why this should not be possible any more. People can now easily inject the JwtParser implementation they like. And with the changes I proposed in smallrye/smallrye-jwt#194, it would also easily be possible to inject a different JWTCallerPrincipalFactory into the DefaultJwtParser.


JwtClaims claims = jwtContext.getJwtClaims();
String name = claims.getClaimValue("upn", String.class);
if (name == null) {
name = claims.getClaimValue("preferred_username", String.class);
if (name == null) {
name = claims.getSubject();
}
}
QuarkusJwtCallerPrincipal principal = new QuarkusJwtCallerPrincipal(name, claims);
return CompletableFuture
.completedFuture(QuarkusSecurityIdentity.builder().setPrincipal(principal)
.addRoles(new HashSet<>(claims.getStringListClaimValue("groups")))
.addAttribute(SecurityIdentity.USER_ATTRIBUTE, principal).build());
.completedFuture(QuarkusSecurityIdentity.builder().setPrincipal(jwt)
.addRoles(jwtRolesMapper.mapGroupsAndRoles(jwt))
.addAttribute(SecurityIdentity.USER_ATTRIBUTE, jwt).build());

} catch (ParseException | MalformedClaimException e) {
} catch (ParseException e) {
log.debug("Authentication failed", e);
CompletableFuture<SecurityIdentity> cf = new CompletableFuture<SecurityIdentity>();
CompletableFuture<SecurityIdentity> cf = new CompletableFuture<>();
cf.completeExceptionally(new AuthenticationFailedException(e));
return cf;
}
Expand Down