Quarkus: Overriding security context doesn't work anymore

[elamotte7]

Describe the bug
We are facing an issue with the common security annotations @RolesAllowed, @DenyAll, @PermitAll on REST endpoints. These annotations seems not to work with the roles authorized with a specific security context.
PS : I am anticipating the migration from quarkus 0.19.1 to 1.1.0 (I was waiting for the fix of this issue https://github.com/quarkusio/quarkus/issues/3516, that's why we are always using an old version currently)

We are developing an application to provide some REST endpoints. These endpoints are reachable with a valid JWT token with the right roles. The REST resources are annotated with @RolesAllowed.

To handle the security on my REST endpoints, I have implemented a JWT filter :

@Provider
@ApplicationScoped
@JWTTokenNeeded
@Priority(Priorities.AUTHENTICATION)
public class JwtFilterWithCustomSecurityContext implements ContainerRequestFilter {
...

//This JWTFilter check if there is a valid JWT Token in http header Authorization, 
//if yes, I override the security context with a custom behaviour to retrieve the userPrincipal and to //check if  the specific role is well granted for a client : 
if (claimsSet != null && claimsSet.getSubject() != null) {
            final SecurityContext securityContext = requestContext.getSecurityContext();
            requestContext.setSecurityContext(new SecurityContext() {
                @Override
                public Principal getUserPrincipal() {
                    return () -> claimsSet.getSubject();
                }
                @Override
                public boolean isUserInRole(String role) {
                    Map<String, List<String>> mapUser = new HashMap<>();
                    mapUser.put("user1", List.of("role1", "role2"));
                    mapUser.put("user2", List.of("role3"));
                    List<String> userRight = mapUser.get(claimsSet.getSubject());
                    return userRight != null && userRight.contains(role);
                }
                @Override
                public boolean isSecure() {
                    return uriInfo.getAbsolutePath().toString().startsWith("https");
                }
                @Override
                public String getAuthenticationScheme() {
                    return "OIDC-JWT";
                }
            });
        }

}
I have the same behaviour with the unit test.

Expected behavior
If a valid JWT doesn't have the roles then we can't reach the REST Endpoint and get back an http code 403.

Actual behavior
If the JWT is valid, the client can access to all REST endpoint, the annotation RolesAllowed doesn't work, the client can access to all REST endpoint without restriction on the roles.

To Reproduce
I have implemented a reproducer :
the branch for the version working :
https://github.com/elamotte7/quarkus/tree/0.19.1-security-context

the branch with the quarkus upgrade that doesn't work :
https://github.com/elamotte7/quarkus/tree/1.1.0.CR1-security-context

Steps to reproduce the behavior:
With unit test

1. Launch test UserResourceTest in your IDE or mvn test

when the application is running

1. Generate two tokens with TokenUtils.CreateToken("user1") and TokenUtils.CreateToken("user2")
   (user1 has "role1" and "role2", user2 has "role3")
2. launch the application, mvn compile quarkus:dev
3. open postman
   set the url to http://localhost:9000/secure/user
   set the Authorization to Bearer Token
4. Launch requests with the two token already generated in the Authorization Token
5. You can see that both user1 and user2 can access to the user resource
   "The user is granted to access to user resource"

Normally only user1 should be able to access to the user resource
the user2 should retrieve an http code 403 with the message
"Access forbidden: role not allowed"

Classes involved in the behaviour described above :
com.elamotte.quarkus.poc.rest.secure.UserResource
com.elamotte.quarkus.poc.rest.security.filter.JwtFilterWithCustomSecurityContext
com.elamotte.quarkus.poc.rest.security.filter.MockJwtFilterWithCustomSecurityContext
com.elamotte.quarkus.poc.it.rest.UserResourceTest

Configuration
see repo github

Environment (please complete the following information):

• Output of uname -a or ver:
  Darwin macbook-pro-de-colonel-d 19.0.0 Darwin Kernel Version 19.0.0: Thu Oct 17 16:17:15 PDT 2019; root:xnu-6153.41.3~29/RELEASE_X86_64 x86_64
• Output of java -version:
  openjdk version "11.0.4" 2019-07-16
  OpenJDK Runtime Environment AdoptOpenJDK (build 11.0.4+11)
  OpenJDK 64-Bit Server VM AdoptOpenJDK (build 11.0.4+11, mixed mode)
• Quarkus version or git rev:
  1.1.0.CR1

[gsmet]

/cc @stuartwdouglas @sberyozkin

[sberyozkin]

This was reported before, #5948, but I've closed #5948 as a duplicate

[sberyozkin]

@stuartwdouglas Hi Stuart, I think we may have a case for an Epic :-), we now have several issues to do with the disconnect between the JAX-RS chains and the Quarkus security layer (this issue, not possible to use JAX-RS exception mappers to handle the security exceptions)
On the JAX-RS Path we should have SecurityIdentity.getPrincipal == SecurityContext.getUserPrincipal but I'm not sure yet since requiring the users inject SecurityIdentityAugmentor would be a duplication of what they do with the JAX-RS API as far as the security context customization is concerned...

[sberyozkin]

@stuartwdouglas, @gsmet I think what can work is that we register our own ContainerRequestFilter very late in the chain but before the RBAC filter is run, and all this filter will do it will align JAX-RS SecurityContext with Quarkus SecurityIdentity, how does it sound ?

[sberyozkin]

The only problem here is that SecurityIdentityAugmentor has the methods like addRole but the role names are not available from SecurityContext

[elamotte7]

Hi everyone,
when do you think this bug will be fixed? For the next release?
thx for your help.

[elamotte7]

I saw the new quarkus release this morning, but this issue is not embedded in the 1.2.0.Final.
Can you please give me some visibility on the resolution of this problem?
It's a very annoying point because with this flaw, my apis won't be safe anymore.
And we are stil using an old quakus version, 0.19.0 ...

[camboui]

Hi,

I had the same issue and I've been digging in debug mode into your repo.
I found out that there's a default RestEasy RoleBasedSecurityFilter but it's not even registered by default. So you won't manage to override it.

You can enable its registration by adding the following to your "appliccation.properties" file
resteasy.role.based.security=true
and voilà, tests are passing !

[elamotte7]

Hi,

I had the same issue and I've been digging in debug mode into your repo.
I found out that there's a default RestEasy RoleBasedSecurityFilter but it's not even registered by default. So you won't manage to override it.

You can enable its registration by adding the following to your "appliccation.properties" file
resteasy.role.based.security=true
and voilà, tests are passing !

Thx Camboui, indeed it works. Thx again for your help.

@gsmet, it seems it's not a bug only a lack in the documentation.

[gsmet]

There's something missing but that's definitely not only documentation.

This should be wired by Quarkus.

@sberyozkin could you have a look at that one soon? Looks like something we will want fixed for 1.3.

[gsmet]

We probably need Stuart's feedback on that one. Affecting it to him so that we can talk about it when he's back.

[wfrank2509]

Hello ...

what is the planned way forward for supporting Custom SecurityContext implementations?

I have a similar issue. Need a own JWT-based security context. I tried overwriting the current context in a ContainerRequestFilter and enabled the @RolesAllowed annotations. I always get back a 403 and debugging the code shows that my own context is set and also returns my custom Principal - but the "role based" methods for checking them are not called.

@Provider
@Priority(Priorities.AUTHENTICATION)
class SecurityInterceptor : ContainerRequestFilter {
    private val log: Logger = LoggerFactory.getLogger(SecurityInterceptor::class.java)

    @Context
    var request: HttpServerRequest? = null

    override fun filter(context: ContainerRequestContext) {
        val userToken = request!!.getHeader("authorization")
        if (!userToken.isNullOrEmpty()) {
            context.securityContext = GatewaySecurityContext(context.securityContext, userToken)
        }
    }
}

internal class GatewaySecurityContext(val currentSecurityContext: SecurityContext, val userToken: String) : SecurityContext {

    @Context
    var info: UriInfo? = null

    @Inject
    lateinit var authService : AuthService

    override fun isUserInRole(role: String?): Boolean {
        return true // NEVER CALLED
    }

    override fun getAuthenticationScheme(): String {
        return currentSecurityContext.authenticationScheme // NEVER CALLED
    }

    override fun getUserPrincipal(): Principal {
        return GatewayPrincipal(userToken) // CALLED AND WORKING
    }

    override fun isSecure(): Boolean {
        return currentSecurityContext.isSecure
    }
}

    @GET
    @Path("/secured/{articleid}")
    @RolesAllowed("customer")
    fun manualflow(@PathParam("articleid") articleId: String, @Context scx: SecurityContext): String {
        return "helloReply" + articleId + " / " + scx.userPrincipal.name + " / " + scx.userPrincipal.userId()
    }

## SECURITY
quarkus.http.auth.policy.role-policy1.roles-allowed=customer
quarkus.http.auth.permission.roles1.paths=/secured/*
quarkus.http.auth.permission.roles1.policy=role-policy1
resteasy.role.based.security=true

Is my assumption wrong that this should be pluggable or am I missing something?

Thanks and greets,
W.

[stuartwdouglas]

@sberyozkin I am thinking that the best way to fix this would be to:

• Document that the getRoles() method on SecurityIdentity is an optional method. We still need this for augmenting identities where we want to copy the roles (I think, although it could just work by delegation). All security based checks should be changed to use hasRole.
• Introduce a SecurityContext backed SecurityIdentity that would become the current SecurityIdentity if the context is changed.
• Change Quarkus-HTTP to make sure that these changes take effect when Servlet is in use.

[stuartwdouglas]

The reason why the JAX-RS SecurityContext is not taking effect is because security is now provided by a CDI interceptor, that uses our SecurityIdentity (and io.quarkus.security.runtime.SecurityIdentityAssociation) classes to determine the identity. This is wired up to provide the identity to the JAX-RS security context, but the inverse is not wired up yet (i.e. setting the JAX-RS SecurityContect) will not affect the current identity.

For most use cases you can use a SecurityIdentityAugmentor to change the security identity, although it might depend on your use case as to if this works for you, as you will have access to the JsonWebToken but not to arbitrary headers.

[wfrank2509]

I was looking into using the SecurityIdentityAugmentor and implementing a Supplier but I can't figure out how I can access the request scope or to be exact how I can get access to the JWT token (header).

The AuthenticationRequestContex does not offer access to the request context like I expected.

[stuartwdouglas]

If you are using JWT the principal should be an instance of org.eclipse.microprofile.jwt.JsonWebToken

[sberyozkin]

Hi Stuart @stuartwdouglas

Document that the getRoles() method on SecurityIdentity is an optional method. We still need this for augmenting identities where we want to copy the roles (I think, although it could just work by delegation). All security based checks should be changed to use hasRole.

Not sure how is it related. In order to make it consistent with JAX-RS SecurityContext.isUserInRole ?

Introduce a SecurityContext backed SecurityIdentity that would become the current SecurityIdentity if the context is changed.

Sorry, still not following. The JAX-RS users are setting a custom SecurityContext in the current ContainerRequestContext.

Change Quarkus-HTTP to make sure that these changes take effect when Servlet is in use.

Vertx HTTP should also work, right ?

[sberyozkin]

@stuartwdouglas I wonder if quarkus-resteasy should register its own pre-match ContainerRequestFilter (so that it can run last in the pre-match JAX-RS phase) which is when it will make sure the current JAX-RS SecurityContext can be used to back up SecurityIdentity as per your suggestion. And document that it will only work if the custom SecurityContext is set in the pre-match phase with a priority up to a given limit, to ensure our filter is run last.
How does it sound ?

[stuartwdouglas]

I think we can probably do it without those constraints, we should be able to add some logic to check the current SecurityContext when part of a JAX-RS request chain (in terms of resolving the current security identity).

[sberyozkin]

@stuartwdouglas Well, I don't know how this can be done without those constraints. For example, if we try to check the custom SecurityContext at the end of the pre-match chain then we'll miss the filter shown above by @wfrank2509 as it runs in the post match chain. And there would always be a risk, without the priority constraints, that some custom filter will run just in front of our filter.
If instead we'd recommend to have the Quarkus specific code injected into a filter then it would not be better than using SecurityIdentiyAugmentor as far as the portability of those filters is concerned.

Though indeed, in Quarkus we can insert out filter to be the very last in the post-match chain oe wherever needed, so it should work, assuming this is what you have in mind too...

I wonder if we can use some black magic and intercept all the requests to RestEasy implementation of JAX-RS ContainerRequestContext :-), are you thinking along these lines too ? If not then please clarify.

[stuartwdouglas]

Actually it looks like this can only be set in pre-match, so your approach should work fine. I was thinking we could just do some RESTEasy magic to update the current security identity when it was set your filter approach is likely a lot simpler and does not require and changes to RESTEasy.

[camboui]

Hi,

Did you find a way to fix this ?

Thanks !

[elamotte7]

Hi everyone,

any news about this topic?

We were able to implement a workaround to overload the security context by configuring restasy.role.based.security to true in the 1.2.x quarkus release.
Only since release 1.3.x it doesn't work anymore. We have 403 http return code even if the @RolesAllowed({"role1", "role2"}) annotation is set on our resource.

What is the right way to override the security context in the new quarkus release?

Thanks for your help.

[stuartwdouglas]

The first step I mentioned about deprecating getRoles() and moving to hasRole() has gone in, so I am hoping to address the rest of this this week.

[sberyozkin]

@stuartwdouglas Hi Stuart, by the way, what do you think about the idea of optionally starting the whole Quarkus security as the JAX-RS filter ?
Though it can probably be done, if realistic, in the next phase, this is so that we can get a header propagation for free if the service protected by quarkus-oidc or quarkus-smallrye-jwt uses MP JWT client to propagate the token in a header further, or so that the users can intercept the exceptions with their mappers. I thought I created an epic which I wanted to fill in later, but it was probably closed and may be the epic is no longer needed :-). I guess I can open the issue to discuss it further. Update, see #8570, it is a bit orthogonal to this issue, thanks

[Anthony-Janin]

Hi guys,

@wfrank2509 @elamotte7

I use this workaround
(do not add quarkus security dependency)

```
@Provider
@Priority(Priorities.AUTHORIZATION)
public class JWTSecurityInterceptor implements ContainerRequestFilter {

@Inject
IJWTService jwtService;

@Override
public void filter(ContainerRequestContext context) {
    final ResourceMethodInvoker resourceMethodInvoker = (ResourceMethodInvoker) context.getProperty("org.jboss.resteasy.core.ResourceMethodInvoker");
    final Method method = resourceMethodInvoker.getMethod();

    if (!method.isAnnotationPresent(PermitAll.class)) {
        if (method.isAnnotationPresent(DenyAll.class)) {
            context.abortWith(Response.status(Response.Status.FORBIDDEN).build());
            return;
        }

        final String authorization = context.getHeaderString("Authorization");
        if (authorization == null || authorization.isEmpty() || !authorization.startsWith("Bearer ")) {
            context.abortWith(Response.status(Response.Status.UNAUTHORIZED).build());
            return;
        }

        final String token = authorization.substring("Bearer ".length());
        if (authorization == null || authorization.isEmpty()) {
            context.abortWith(Response.status(Response.Status.UNAUTHORIZED).build());
            return;
        }

        final String username = jwtService.getUserNameFromToken(token);
        if (username == null) {
            context.abortWith(Response.status(Response.Status.UNAUTHORIZED).build());
            return;
        }

        final Set<Role> roles = jwtService.getRoles(token);
        if (roles == null || roles.isEmpty()) {
            context.abortWith(Response.status(Response.Status.UNAUTHORIZED).build());
            return;
        }

        if (method.isAnnotationPresent(RolesAllowed.class)) {
            final RolesAllowed rolesAllowedAnnotation = method.getAnnotation(RolesAllowed.class);
            final String[] rolesAllowed = rolesAllowedAnnotation.value();

            if (!Arrays.stream(rolesAllowed).filter(roleAllowed -> roles.contains(Role.valueOf(roleAllowed))).findFirst().isPresent()) {
                context.abortWith(Response.status(Response.Status.UNAUTHORIZED).build());
                return;
            }
        }
    }
}

}
````

Regards
Janin Anthony

[elamotte7]

Thank you @Anthony-Janin for your help.

I have choosen another approach, I delegate the authorization to smallrye for the JWT token part and I manage the authorizations with a custom annotation.
So with this solution I can still use PermitAll and DenyAll and I implemented an @RolesAllowedCustom where I check the user rights.

@RolesAllowedCustom :
@Documented @Retention(RUNTIME) @Target({TYPE, METHOD}) @InterceptorBinding public @interface RolesAllowedCustom { @Nonbinding String[] value() default ""; }

RolesAllowedCustomInterceptor :
`@Interceptor
@RolesAllowedCustom
@Priority(Interceptor.Priority.LIBRARY_BEFORE)
public class RolesAllowedCustomInterceptor {

@Inject
private JsonWebToken jwt;

@Inject
private UserRightsService userRightsService;

@Inject
private SecurityIdentity identity;

public RolesAllowedCustomInterceptor() {
}

@AroundInvoke
public Object intercept(InvocationContext ic) throws Exception {
    if (identity.isAnonymous()) {
        throw new UnauthorizedException();
    }
    List<String> allowedRoles = null;
    RolesAllowedCustom annotation = (RolesAllowedCustom) getAnnotationClass(RolesAllowedCustom.class, ic);
    if (annotation != null) {
        allowedRoles = new ArrayList<>(Arrays.asList(annotation.value()));
    } else {
        return ic.proceed();
    }

    UserRight userRight = userRightsService.getUserRightByUid(jwt.getClaim(Claims.sub.name()));

    if(userRight == null){
        throw new ForbiddenException();
    }

    for (String role : allowedRoles) {
        if (userRight.getUserProfile() == UserProfiles.valueOf(role).getCode()) {
            return ic.proceed();
        }
    }

    throw new ForbiddenException();
}

private Object getAnnotationClass(Class clazz, InvocationContext ctx) {
    Object annotationClass = ctx.getMethod().getAnnotation(clazz);

    // Try to find annotation on class level
    if (annotationClass == null) {
        annotationClass = ctx.getTarget().getClass().getSuperclass().getAnnotation(clazz);
    }

    return annotationClass;
}

}
application.properties : mp.jwt.verify.publickey.location=META-INF/resources/publicKey.pem
mp.jwt.verify.issuer=https://quarkus.io/using-jwt-rbac

quarkus.smallrye-jwt.enabled=true`

And then I can use @RolesAllowedCustom({"MyRole"}) on a class or simply on a method.

Now we can migrate to quarkus 1.3.2.Final and still wait that there is a "proper quarkus solution" to implement my use case (our JWT doesn't have role as claims, roles are retrieved from the DataBase by the application)

[sirf]

Hi guys.

Quick question, is there another way of obtaining the set of roles for a SecurityIdentity, now that getRoles() is deprecated?

Thanks.

[stuartwdouglas]

Thinking about it deprecating is probably a bit much. I might just change it to a warning that it might not always reflect the results of hasRole if the underlying user representation does not support getRoles