-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
smallrye-jwt-extension: Make role mapping configurable and use newer smallrye-jwt apis #7485
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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 { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the moment I see three use cases:
|
||
|
||
@Override | ||
public Set<String> mapRoles(JsonWebToken token) { | ||
return token.getGroups(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
package io.quarkus.smallrye.jwt.runtime.auth; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same question, why is this interface needed ? There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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> mapRoles(JsonWebToken token); | ||
} |
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; | ||
|
||
/** | ||
|
@@ -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 | ||
|
@@ -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()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
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.mapRoles(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; | ||
} | ||
|
There was a problem hiding this comment.
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.