quarkus-bot[bot]
commented
1 year ago /cc @evanchooly (kotlin), @geoand (kotlin), @sberyozkin (security)
Open JackyAnn opened 1 year ago
quarkus-bot[bot]
commented
1 year ago /cc @evanchooly (kotlin), @geoand (kotlin), @sberyozkin (security)
sberyozkin
commented
1 year ago @JackyAnn Would you like to activate both role based access control and permission based access control at the same time?
JackyAnn
commented
1 year ago yes @sberyozkin
sberyozkin
commented
1 year ago @JackyAnn
Can you please provide a use case ? For example, permissions can be derived from roles, but when we have both @RolesAllowed("admin") and @PermissionAllowed("see:all") - what exactly does that mean ?
If it is only admin role that can lead to a see:all permission, then why just not list @PermissionAllowed("see:all") only ?
If admin role is not necessarily leading to a see:all permission - then does it mean if a user role can be mapped to a see:all permission then a user role should be allowed to invoke the method annotated with @RolesAllowed(admin)?
Is your goal to have an admin role mapped to a see:all permission without having to do it in the configuration ? In this case you need to use a simple custom SecurityIdentityAugmentor which will do it. If we are talking about OIDC tokens - we can handle mapping scope claims to permissions a bit later automatically.
What exactly is your use case ?
Thanks
JackyAnn
commented
1 year ago @sberyozkin https://github.com/JackyAnn/quarkus-devui-issues/blob/main/src/main/java/org/acme/GreetingResource2.java
import io.quarkus.security.PermissionsAllowed;
import jakarta.annotation.security.RolesAllowed;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;
@Path("/hello2")
public class GreetingResource2 {
/**
* Does not work properly, startup error reported
*/
@GET
@RolesAllowed("user")
@PermissionsAllowed("see:read")
@Produces(MediaType.TEXT_PLAIN)
public String hello() {
return "Hello RESTEasy";
}
}```
```2023-05-14 13:19:05,464 ERROR [io.qua.dep.dev.IsolatedDevModeMain] (main) Failed to start quarkus: java.lang.RuntimeException: io.quarkus.builder.BuildException: Build failure: Build failed due to errors
[error]: Build step io.quarkus.security.deployment.SecurityProcessor#gatherSecurityChecks threw an exception: java.lang.IllegalStateException: Method hello of class org.acme.GreetingResource2 is annotated with multiple security annotations
at io.quarkus.security.deployment.PermissionSecurityChecks$PermissionSecurityChecksBuilder.gatherPermissionsAllowedAnnotations(PermissionSecurityChecks.java:216)
at io.quarkus.security.deployment.SecurityProcessor.gatherSecurityAnnotations(SecurityProcessor.java:673)
at io.quarkus.security.deployment.SecurityProcessor.gatherSecurityChecks(SecurityProcessor.java:527)
at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
at java.base/java.lang.reflect.Method.invoke(Method.java:578)
at io.quarkus.deployment.ExtensionLoader$3.execute(ExtensionLoader.java:909)
at io.quarkus.builder.BuildContext.run(BuildContext.java:282)
at org.jboss.threads.ContextHandler$1.runWith(ContextHandler.java:18)
at org.jboss.threads.EnhancedQueueExecutor$Task.run(EnhancedQueueExecutor.java:2513)
at org.jboss.threads.EnhancedQueueExecutor$ThreadBody.run(EnhancedQueueExecutor.java:1538)
at java.base/java.lang.Thread.run(Thread.java:1623)
at org.jboss.threads.JBossThread.run(JBossThread.java:501)```
https://github.com/quarkusio/quarkus/blob/ba59762006946bd8d6c73c249acb355190b903aa/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/PermissionSecurityChecks.java#L213
https://github.com/quarkusio/quarkus/blob/ba59762006946bd8d6c73c249acb355190b903aa/extensions/security/deployment/src/main/java/io/quarkus/security/deployment/PermissionSecurityChecks.java#L256As long as @RolesAllowed is not used in combination with other security annotations on the same method or class, everything works fine
@Path("/hello1")
@RolesAllowed("user")
public class GreetingResource1 {
@GET
/**
* work properly
*/
@PermissionsAllowed("see:read")
@Produces(MediaType.TEXT_PLAIN)
public String hello() {
return "Hello RESTEasy";
}
}This restriction is just too inconvenient.
sberyozkin
commented
1 year ago @JackyAnn Let me reopen to discuss a few details with @michalvavrik
sberyozkin
commented
1 year ago @JackyAnn Right, but what is the actual requirement ? If all you need to do is to derive see:read out of the user role then, it has to be done with SecurityIdentityAugmentor or as you tried earlier - with HTTP security configuration.
If you have an or relationship here, either a user role or see:read permission meaning that someone who has no user role but see:all permission can access the method - then I'm not sure what that means :-).
I suppose, @michalvavrik, the actual requirement here - is to map a role to permissions without having to create SecurityIdentityAugmentor, I was thinking, how about having something like:
@PermissionAllowed(value="see:all", roles=["admin", "user"]), meaning - if the identity has one of these 2 roles, add a see:all permission ? That said, I'm not sure how this would be better instead of only @RolesAllowed("user", "admin") :-)
michalvavrik
commented
1 year ago I suppose, @michalvavrik, the actual requirement here - is to map a role to permissions without having to create
SecurityIdentityAugmentor, I was thinking, how about having something like:@PermissionAllowed(value="see:all", roles=["admin", "user"]), meaning - if the identity has one of these 2 roles, add asee:allpermission ? That said, I'm not sure how this would be better instead of only@RolesAllowed("user", "admin"):-)
hey @sberyozkin , I have strong opinions here, but please feel free to push back :-) Maybe I just don't understand usage?
@PermissionAllowed(value="see:all", roles=["admin", "user"])
Would basically mean we define mapping between roles and permissions for each annotation instance item, right? That is possible, but we actually try to avoid any reference to original annotated method if possible because @RolesAllowed (for which I presume @PermissionsAllowed will be same case) is often repeated with same values, so instead of creating dozen of SecurityCheck instances you create one. Also it is important to stress that we do map roles on level of HTTP Policy maping where we don't know whether this HTTP request is targeting some annotated method, so our only shot would be to augment SecurityIdentity and perform security check on that augmented identity before each call to SecurityIdentity#checkPermissions.
In short, this can be done, but currently I can't see a way to make it as performance and resource efficient as current solution. Opinionated framework :-) what is a gain here?
@RolesAllowed("user", "admin")
RolesAllowed annotation is not ours. My opinion is that we shouldn't give some extra function to standard Jakarta annotations as it might confuse users. WDYT? Apart of that, I stress same arguments as for PermissionsAllowed#roles.
sberyozkin
commented
1 year ago Hey @michalvavrik thanks for the feedback, what you say makes sense.
As far as @RolesAllowed is concerned, agreed, it can only be used for enforcing a Role Based Access Control. If something like @PermissionAllowed(value="see:all", roles=["admin", "user"]) were to be supported, @RolesAllowed would not be involved at all, no RBAC would be required, same way as in the HTTP security policy configuration where we can do the mapping and enable permission checks without enabling RBAC.
I suppose we can keep the issue open for us to keep discussing an auto-mapping from roles to permissions idea. It is not urgent, and might not work, but may be we'll come up with something over the next while :-) Thanks
michalvavrik
commented
1 year ago I have changed issue type from bug to enhancement as this behavior is documented and description shows validation error. The validation error is there to inform you it is not supported. Let's keep the issue open so that others can vote for this enhancement.
Describe the bug
When both PermissionsAllowed and other security annotations are used on the method, the system will throw an exception
error:
When PermissionsAllowed and other security annotations are not used simultaneously on the method, everything works normally
the gudides https://quarkus.io/guides/security-authorize-web-endpoints-reference the CRUDResource example shows that we can use permissions for role mapping and grant role permissions,
Expected behavior
PermissionsAllowed and other security annotations can be used simultaneously on methods.
Actual behavior
PermissionsAllowed and other security annotations can not used simultaneously on methods.
How to Reproduce?
No response
Output of
uname -aorverLinux fedora 6.2.14-300.fc38.x86_64 #1 SMP PREEMPT_DYNAMIC Mon May 1 00:55:28 UTC 2023 x86_64 GNU/Linux
Output of
java -versionopenjdk version "20" 2023-03-21 OpenJDK Runtime Environment (Red_Hat-20.0.0.0.36-1.rolling.fc38) (build 20+36) OpenJDK 64-Bit Server VM (Red_Hat-20.0.0.0.36-1.rolling.fc38) (build 20+36, mixed mode, sharing)
GraalVM version (if different from Java)
No response
Quarkus version or git rev
3.0.1.final
Build tool (ie. output of
mvnw --versionorgradlew --version)------------------------------------------------------------ Gradle 8.1.1 ------------------------------------------------------------ Build time: 2023-04-21 12:31:26 UTC Revision: 1cf537a851c635c364a4214885f8b9798051175b Kotlin: 1.8.10 Groovy: 3.0.15 Ant: Apache Ant(TM) version 1.10.11 compiled on July 10 2021 JVM: 20 (Red Hat, Inc. 20+36) OS: Linux 6.2.14-300.fc38.x86_64 amd64
Additional information
No response