Quarkus Security: PermissionAllowed

[Sergey Beryozkin]

Michal Vavrik has completed a major enhancement for Permission based access control:

https://github.com/quarkusio/quarkus/blob/main/docs/src/main/asciidoc/security-authorize-web-endpoints-reference.adoc#permission-annotation

Please provide an early feedback, more enhancements are on the way to make it easier for users to handle

Cheers Sergey

[Stephane Epardaud]

OK, so first, this is great, and much needed.

But I've feedback.

First, Permission is an annoying type to implement, I have to implement 5 methods just to be able to declare essentially what should be one logic method:

 static class Checker extends Permission {

public Checker(String name) {
super(name);
}

@Override
public boolean implies(Permission permission) {
// TODO Auto-generated method stub
return false;
}

@Override
public boolean equals(Object obj) {
// TODO Auto-generated method stub
return false;
}

@Override
public int hashCode() {
// TODO Auto-generated method stub
return 0;
}

@Override
public String getActions() {
// TODO Auto-generated method stub
return null;
}

}

Second, it's frustrating that RESTEasy Reactive won't let you pass deserialised arguments to it. Is this only for body parameters, or all parameters? We could probably relax that and change the order in which security is invoked by turning this from a CDI interceptor to a RR RestHandler, no?

Third, I find it very confusing what Permission does. Apparently I implement a permission checker, via an instance of Permission. So far, so good. But then why do I get another Permission instance passed as parameter? Why are there two permissions? In most cases, I've had to check whether the current user had permission to do something on something else. But there was always a single permission per check.

Four, this makes it easy-ish to access parameters (except for RR but we should fix that) but unnecessarily hard because "injection" of those params is done in the constructor, which is separate from where the check logic is (in "implies"), and also makes this hard to inject CDI parameters. Can I suggest the following instead?

public @interface CustomPermission{}
public @interface Checker{
public Class<? extends Annotation> value();
}

@CustomPermission
public @interface MyPermission {
}

public class App extends ControllerWithUser<User> {

 @MyPermission
 @Path("/hello/{user}")
 public String helloForUser(@RestPath String user) {
 }

 // only allow a user to say hello to itself
 @Checker(MyPermission.class)
 public boolean isAllowed(String user) {
  return getUser().username.equals(user);
 }
}

So, we'd have a meta-annotation @CustomPermission, which we put on user-defined permission annotations, such as MyPermission, which say that "this thing is a permission annotation", and then we can place a @Checker annotation on a method and we specify which permission it can check. That method is a lot like the methods for RESTEasy Reactive server filters: it must return a boolean, and it can take any parameters as it likes, including parameters of the checked method, RESTEasy Reactive injectable types, and (if we implemented https://github.com/quarkusio/quarkus/issues/13593 like we should have years ago) also CDI types.

This allows me to use injection, context and lots of stuff, for a much simpler implementation, no? Or am I missing something?

And last, these permissions, ideally should play a part in how we tell the clients what they can or not invoke. I'm thinking of APIs like https://quarkus.io/guides/rest-data-panache where the client will have UI to do some things, but these things depend on whether they have permission for it or not. No permission to delete object X? Then you get no button for it.

In these scenarios, there's the server-side check, and we have a story for that. The missing bit is how the client can tell whether an action is allowed or not. I don't yet have ideas for how to solve this.

[Sergey Beryozkin]

Hey Steph

Thanks for a fast feedback, Michal and Stuart can offer more feedback, but some comments are inline

On Tue, Mar 21, 2023 at 11:24 AM Stephane Epardaud <stephane...@gmail.com> wrote:

OK, so first, this is great, and much needed.

But I've feedback.

First, Permission is an annoying type to implement, I have to implement 5 methods just to be able to declare essentially what should be one logic method:

 static class Checker extends Permission {

public Checker(String name) {
super(name);
}

@Override
public boolean implies(Permission permission) {
// TODO Auto-generated method stub
return false;
}

@Override
public boolean equals(Object obj) {
// TODO Auto-generated method stub
return false;
}

@Override
public int hashCode() {
// TODO Auto-generated method stub
return 0;
}

@Override
public String getActions() {
// TODO Auto-generated method stub
return null;
}

}

Good point, `StringPermission` is available for simple cases, but indeed, in IDEs it can be a bit annoying, may be in addition to `StringPermission` we can offer `AbstractPermission`

Second, it's frustrating that RESTEasy Reactive won't let you pass deserialised arguments to it. Is this only for body parameters, or all parameters? We could probably relax that and change the order in which security is invoked by turning this from a CDI interceptor to a RR RestHandler, no?

It can work with RestEasy Reactive, it is a doc issue probably, you need to have Permission on the CDI bean injected to ResteasyEndpoint and have this CDI bean accepting the extra parameters incl the body I believe

 

Third, I find it very confusing what Permission does. Apparently I implement a permission checker, via an instance of Permission. So far, so good. But then why do I get another Permission instance passed as parameter? Why are there two permissions? In most cases, I've had to check whether the current user had permission to do something on something else. But there was always a single permission per check.

I'll let Michal clarify it - but I think what you get passed to the checker is a Permission instance representing something in the request, added currently via SecurityidentityAugmentor - but later via other declarative mechanisms

 

Four, this makes it easy-ish to access parameters (except for RR but we should fix that) but unnecessarily hard because "injection" of those params is done in the constructor, which is separate from where the check logic is (in "implies"), and also makes this hard to inject CDI parameters. Can I suggest the following instead?

public @interface CustomPermission{}
public @interface Checker{
public Class<? extends Annotation> value();
}

@CustomPermission
public @interface MyPermission {
}

public class App extends ControllerWithUser<User> {

 @MyPermission
 @Path("/hello/{user}")
 public String helloForUser(@RestPath String user) {
 }

 // only allow a user to say hello to itself
 @Checker(MyPermission.class)
 public boolean isAllowed(String user) {
  return getUser().username.equals(user);
 }
}

So, we'd have a meta-annotation @CustomPermission, which we put on user-defined permission annotations, such as MyPermission, which say that "this thing is a permission annotation", and then we can place a @Checker annotation on a method and we specify which permission it can check. That method is a lot like the methods for RESTEasy Reactive server filters: it must return a boolean, and it can take any parameters as it likes, including parameters of the checked method, RESTEasy Reactive injectable types, and (if we implemented https://github.com/quarkusio/quarkus/issues/13593 like we should have years ago) also CDI types.

This allows me to use injection, context and lots of stuff, for a much simpler implementation, no? Or am I missing something?

I'll let Michal, Stuart clarify here, but given what I said above is that passing the extra parameters can also work for Resteasy Reactive, will it change your proposal ?

 

And last, these permissions, ideally should play a part in how we tell the clients what they can or not invoke. I'm thinking of APIs like https://quarkus.io/guides/rest-data-panache where the client will have UI to do some things, but these things depend on whether they have permission for it or not. No permission to delete object X? Then you get no button for it.

In these scenarios, there's the server-side check, and we have a story for that. The missing bit is how the client can tell whether an action is allowed or not. I don't yet have ideas for how to solve this.

Please open an enhancement request/

Perhaps, Qute templates can use `permission:implies("something")` handlers to modify the button visibility, similar to how templates can have for ex CSRF tokens injected ?

 

Thanks Sergey

[Michal Vavřík]

note: I hope I'm not spamming whole lot of people, no sure how these Google groups work :-)

 

Hey Steph

Thanks for a fast feedback, Michal and Stuart can offer more feedback, but some comments are inline

OK, so first, this is great, and much needed.

But I've feedback.

First, Permission is an annoying type to implement, I have to implement 5 methods just to be able to declare essentially what should be one logic method:

 static class Checker extends Permission {

public Checker(String name) {
super(name);
}

@Override
public boolean implies(Permission permission) {
// TODO Auto-generated method stub
return false;
}

@Override
public boolean equals(Object obj) {
// TODO Auto-generated method stub
return false;
}

@Override
public int hashCode() {
// TODO Auto-generated method stub
return 0;
}

@Override
public String getActions() {
// TODO Auto-generated method stub
return null;
}

}

Good point, `StringPermission` is available for simple cases, but indeed, in IDEs it can be a bit annoying, may be in addition to `StringPermission` we can offer `AbstractPermission`

You `Checker` is same as `new java.security.BasicPermission("name") {}`. I think we went with `java.security.Permission` because it is already used by KeycloakAuthorizer and SecurityIdentity has `checkPermission` for quite some time now.

 

Second, it's frustrating that RESTEasy Reactive won't let you pass deserialised arguments to it. Is this only for body parameters, or all parameters? We could probably relax that and change the order in which security is invoked by turning this from a CDI interceptor to a RR RestHandler, no?

It can work with RestEasy Reactive, it is a doc issue probably, you need to have Permission on the CDI bean injected to ResteasyEndpoint and have this CDI bean accepting the extra parameters incl the body I believe

It doesn't work on endpoints now because the check is done by RESTEasy Reactive `org.jboss.resteasy.reactive.server.spi.ServerRestHandler` (more specifically `io.quarkus.resteasy.reactive.server.runtime.security.EagerSecurityHandler` https://github.com/quarkusio/quarkus/pull/19598), not by an interceptor. The handler runs as first thing after the pre-match interceptors and I don't think we can consume request body there or postpone it after deserializers as it could raise potential for DoS etc. (sending unreasonable bodies for requests that are denied anyway? I suppose there are limits though).

 

 

Third, I find it very confusing what Permission does. Apparently I implement a permission checker, via an instance of Permission. So far, so good. But then why do I get another Permission instance passed as parameter? Why are there two permissions? In most cases, I've had to check whether the current user had permission to do something on something else. But there was always a single permission per check.

I'll let Michal clarify it - but I think what you get passed to the checker is a Permission instance representing something in the request, added currently via SecurityidentityAugmentor - but later via other declarative mechanisms

I'm working on a way to add permissions via configuration properties https://github.com/quarkusio/quarkus/issues/12219. I agree that permission checker is not easy to understand, but contract here is set by `java.security.Permission#implies`. The advantage is that we are already using in Quarkus (Keycloak Authorizer mostly) and it's something users may know as it belongs to `java.security. Maybe we can make it even clearer in docs instead of re-implementing it? Ideally users will avoid SecurityIdentityAugmetnor and use config properties in most cases.

I'll try to improve this, thank you.

 

 

Four, this makes it easy-ish to access parameters (except for RR but we should fix that) but unnecessarily hard because "injection" of those params is done in the constructor, which is separate from where the check logic is (in "implies"), and also makes this hard to inject CDI parameters. Can I suggest the following instead?

public @interface CustomPermission{}
public @interface Checker{
public Class<? extends Annotation> value();
}

@CustomPermission
public @interface MyPermission {
}

public class App extends ControllerWithUser<User> {

 @MyPermission
 @Path("/hello/{user}")
 public String helloForUser(@RestPath String user) {
 }

 // only allow a user to say hello to itself
 @Checker(MyPermission.class)
 public boolean isAllowed(String user) {
  return getUser().username.equals(user);
 }
}

So, we'd have a meta-annotation @CustomPermission, which we put on user-defined permission annotations, such as MyPermission, which say that "this thing is a permission annotation", and then we can place a @Checker annotation on a method and we specify which permission it can check. That method is a lot like the methods for RESTEasy Reactive server filters: it must return a boolean, and it can take any parameters as it likes, including parameters of the checked method, RESTEasy Reactive injectable types, and (if we implemented https://github.com/quarkusio/quarkus/issues/13593 like we should have years ago) also CDI types.

This allows me to use injection, context and lots of stuff, for a much simpler implementation, no? Or am I missing something?

I'll let Michal, Stuart clarify here, but given what I said above is that passing the extra parameters can also work for Resteasy Reactive, will it change your proposal ?

@Checker provides same capabilities in regards of argument injection and limitations as @PermissionsAllowed, but you need to inject SecurityIdentity if you want to check user name. I think @PermissionsAllowed should be easier to use as long as you stick with default StringPermission and avoid defining SecurityIdentityAugmentor (Sergey is working on something for OIDC token claims and I'm working on role -> permission mapping), with args you are right, it is harder.

However, I agree, @Checker seems easier to write and understand. Don't know what to do about it...

 

And last, these permissions, ideally should play a part in how we tell the clients what they can or not invoke. I'm thinking of APIs like https://quarkus.io/guides/rest-data-panache where the client will have UI to do some things, but these things depend on whether they have permission for it or not. No permission to delete object X? Then you get no button for it.

In these scenarios, there's the server-side check, and we have a story for that. The missing bit is how the client can tell whether an action is allowed or not. I don't yet have ideas for how to solve this.

Please open an enhancement request/

Perhaps, Qute templates can use `permission:implies("something")` handlers to modify the button visibility, similar to how templates can have for ex CSRF tokens injected ?

Qute already supports CDI bean injection, so we can optionally provide such bean that allows you to `permission:implies("something")`, +1 for open issue. Please ping me there.

 

 

Thanks Sergey

Thanks Michal 

[Stuart Douglas]

Even though the extra methods are annoying, that is easily solved with an abstract type, and IMHO it is generally better to use existing parts of the JDK unless there is a really good reason not to.

 

 

Second, it's frustrating that RESTEasy Reactive won't let you pass deserialised arguments to it. Is this only for body parameters, or all parameters? We could probably relax that and change the order in which security is invoked by turning this from a CDI interceptor to a RR RestHandler, no?

It can work with RestEasy Reactive, it is a doc issue probably, you need to have Permission on the CDI bean injected to ResteasyEndpoint and have this CDI bean accepting the extra parameters incl the body I believe

It doesn't work on endpoints now because the check is done by RESTEasy Reactive `org.jboss.resteasy.reactive.server.spi.ServerRestHandler` (more specifically `io.quarkus.resteasy.reactive.server.runtime.security.EagerSecurityHandler` https://github.com/quarkusio/quarkus/pull/19598), not by an interceptor. The handler runs as first thing after the pre-match interceptors and I don't think we can consume request body there or postpone it after deserializers as it could raise potential for DoS etc. (sending unreasonable bodies for requests that are denied anyway? I suppose there are limits though).

I think you could run it later, I don't think that  there would be any expectation that these would take effect before serialization. If you are worried about DOS you could add an 'isAuthenticated' check earlier, so unauthenticated users are denied eagerly (as they will never have permission).

 

 

 

Third, I find it very confusing what Permission does. Apparently I implement a permission checker, via an instance of Permission. So far, so good. But then why do I get another Permission instance passed as parameter? Why are there two permissions? In most cases, I've had to check whether the current user had permission to do something on something else. But there was always a single permission per check.

I'll let Michal clarify it - but I think what you get passed to the checker is a Permission instance representing something in the request, added currently via SecurityidentityAugmentor - but later via other declarative mechanisms

I'm working on a way to add permissions via configuration properties https://github.com/quarkusio/quarkus/issues/12219. I agree that permission checker is not easy to understand, but contract here is set by `java.security.Permission#implies`. The advantage is that we are already using in Quarkus (Keycloak Authorizer mostly) and it's something users may know as it belongs to `java.security. Maybe we can make it even clearer in docs instead of re-implementing it? Ideally users will avoid SecurityIdentityAugmetnor and use config properties in most cases.

I'll try to improve this, thank you.

The idea is that permissions do not have to be exactly equal to grant access. e.g. say I have HttpPermission(path, method).

if I have a method that requires HttpPermission(path="/admin", method="GET"), I can have a user with a permission HttpPermission(path="/admin", method="*"), and the implies method can figure out that a user with "*" permissions can also access GET.

Basically this implies approach allows for any amount of flexibility in how permissions are defined and enforced.

 

 

 

Four, this makes it easy-ish to access parameters (except for RR but we should fix that) but unnecessarily hard because "injection" of those params is done in the constructor, which is separate from where the check logic is (in "implies"), and also makes this hard to inject CDI parameters. Can I suggest the following instead?

public @interface CustomPermission{}
public @interface Checker{
public Class<? extends Annotation> value();
}

@CustomPermission
public @interface MyPermission {
}

public class App extends ControllerWithUser<User> {

 @MyPermission
 @Path("/hello/{user}")
 public String helloForUser(@RestPath String user) {
 }

 // only allow a user to say hello to itself
 @Checker(MyPermission.class)
 public boolean isAllowed(String user) {
  return getUser().username.equals(user);
 }
}

So, we'd have a meta-annotation @CustomPermission, which we put on user-defined permission annotations, such as MyPermission, which say that "this thing is a permission annotation", and then we can place a @Checker annotation on a method and we specify which permission it can check. That method is a lot like the methods for RESTEasy Reactive server filters: it must return a boolean, and it can take any parameters as it likes, including parameters of the checked method, RESTEasy Reactive injectable types, and (if we implemented https://github.com/quarkusio/quarkus/issues/13593 like we should have years ago) also CDI types.

This allows me to use injection, context and lots of stuff, for a much simpler implementation, no? Or am I missing something?

I'll let Michal, Stuart clarify here, but given what I said above is that passing the extra parameters can also work for Resteasy Reactive, will it change your proposal ?

@Checker provides same capabilities in regards of argument injection and limitations as @PermissionsAllowed, but you need to inject SecurityIdentity if you want to check user name. I think @PermissionsAllowed should be easier to use as long as you stick with default StringPermission and avoid defining SecurityIdentityAugmentor (Sergey is working on something for OIDC token claims and I'm working on role -> permission mapping), with args you are right, it is harder.

However, I agree, @Checker seems easier to write and understand. Don't know what to do about it...

How is this any different to just writing a standard CDI Interceptor, and implementing a custom permission scheme? You can basically get the same result by making MyPermission an interceptor binding and your Checker an interceptor.

Stuart

 

 

And last, these permissions, ideally should play a part in how we tell the clients what they can or not invoke. I'm thinking of APIs like https://quarkus.io/guides/rest-data-panache where the client will have UI to do some things, but these things depend on whether they have permission for it or not. No permission to delete object X? Then you get no button for it.

In these scenarios, there's the server-side check, and we have a story for that. The missing bit is how the client can tell whether an action is allowed or not. I don't yet have ideas for how to solve this.

Please open an enhancement request/

Perhaps, Qute templates can use `permission:implies("something")` handlers to modify the button visibility, similar to how templates can have for ex CSRF tokens injected ?

Qute already supports CDI bean injection, so we can optionally provide such bean that allows you to `permission:implies("something")`, +1 for open issue. Please ping me there.

 

 

Thanks Sergey

Thanks Michal 

--
You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/5ebd9d4d-a470-469e-8724-d948ab89a9cen%40googlegroups.com.

[Michal Vavřík]

I think you could run it later, I don't think that  there would be any expectation that these would take effect before serialization. If you are worried about DOS you could add an 'isAuthenticated' check earlier, so unauthenticated users are denied eagerly (as they will never have permission).

I opened issue for myself https://github.com/quarkusio/quarkus/issues/32030 so that I don't forget. Will do that, thanks. Michal 

[Stephane Epardaud]

On Wed, 22 Mar 2023 at 06:47, Stuart Douglas <sdou...@redhat.com> wrote:

Even though the extra methods are annoying, that is easily solved with an abstract type, and IMHO it is generally better to use existing parts of the JDK unless there is a really good reason not to.

Sure, I agree with that. But then we should provide the abstract class and tell people to implement it. I have no idea why equals/hashCode are required, and it's hard enough to understand why I need a constructor and an implies method, so no idea what the getActions() is for.

 

The idea is that permissions do not have to be exactly equal to grant access. e.g. say I have HttpPermission(path, method).

if I have a method that requires HttpPermission(path="/admin", method="GET"), I can have a user with a permission HttpPermission(path="/admin", method="*"), and the implies method can figure out that a user with "*" permissions can also access GET.

Basically this implies approach allows for any amount of flexibility in how permissions are defined and enforced.

OK. So, perhaps that's also a way to say that the "admin" permission implies every other permission? So does this mean that if I have an "edit-order" permission for a given order, that normally would check if the current user "owns" the specific order, we'd have the admin permission imply it?

I was going to try to write this using the BasicPermission/implies for two permissions, but realised I think I found what confuses the hell out of me. With this model, we have an annotation that describes what permission we must have (fine), then a class with a constructor that describes a permission request, not a permission we have: we build it by passing its constructor parameters that we take from the method we check, and finally, we pass a second instance of Permission to the "implies" method, and that one represents the permission we described in the annotation.

I find this model confusing, because I see it as completely upside-down. There are two instances of Permission, so that's confusing enough and a huge red flag to me. We build a permission request, and later pass it permission queries.

I find the model where we have a stateless logic permission method, and pass it request parameters much more intuitive. Perhaps that's just me? 

 

How is this any different to just writing a standard CDI Interceptor, and implementing a custom permission scheme? You can basically get the same result by making MyPermission an interceptor binding and your Checker an interceptor.

Well, every security check can be implemented using a CDI interceptor, no? Except the mechanism i'm describing is more like RESTEasy Reactive (annotated) filters, because the method can take any number of parameters which we auto-inject from the request. We can think of them like security permission filters, indeed.

I find this model much more intuitive, but again, perhaps that's just me?

Also, I find I can't relate to any of the given examples of permissions. As a web application builder and maintainer, my experience and view on permissions can be easily explained by thinking about GitHub issues, since I assume everyone is familiar with them.

Given {org}/{project}/issues/{id}, can I have the `edit-issue` permission?

- If there is no current user: no

- If the user is admin: yes

- If the user is not allowed to view {org}: no

- Same for {org}/{project}

- Is the user participating in the issue?

- Is the user the owner of the repo?

- Is the user a member of a maintainer list of the repo?

-- Does that list have the required permission?

- Is the user impersonating another user (this is required by customer support)? If yes, run the same checks against the impersonated user.

- etc…

So, I've never written any endpoint that didn't require loading stuff from the DB (the user, the objects worked against) and doing a ton of logic just to figure out if I could run an endpoint or not. Most of the times I see permissions as simple strings with no parameter or url matching or what, I can't relate to it, because I've never seen such checks in (my) production code. That very well may mean that I haven't worked in the right projects ;)

But to sum it up, my code always tends to look like this:

public class Orders extends Controller {

 @Path("{company}/orders/{id}/close")
 public void closeOrder(String company, String id){
  checkUser();
  Order order = Order.findById(company, id);
  notFoundIfNull(order);
  checkEditOrder(order);
  // now we can work on order
 }

 private void checkEditOrder(Order order){
  User user = getUser();
  if(user.admin) return;
  if(order.isParticipating(user)) return;
  Company company = order.company;
  if(company.isSuperuper(user)) return;
  for(Team team : user.teams){
   if(team.canEdit(order)) return;
  }
  unauthorized();
 }

 private void checkUser(){
  if(getUser() == null) unauthorized();
 }
}

So, I express my permission checks as methods that take request parameters to do their jobs. I prefer annotations in general, but I never found a framework that allowed me this sort of flexibility. Also, note that this doesn't allow me to easily query from the template views (in Qute, say) whether I have that permission.

To me, this represents 100% of permission checks in the web apps I've written over the years. In logistics, in law repositories, in software module repositories, in conference websites, in wikis… Again, I may be accidentally have been cornered into lots of niche applications, I don't know…

But the problem I would have with permissions in annotations, as you can see from the example, is that the method parameters alone won't get you the current user (well, you can inject it, in Renarde, so fine), but you'll be lacking the Order, you only have its `id`. You can load it from the annotation check method, but that's in another transaction!!! And to make it worse: you'll be loading it twice, because you'll need it in the method too.

I think this sort of permission checking is common (see github, but really a lot of online websites and services you're using), and so we should think of this use-case when adding support for permissions. I haven't found a solution, but I've just started thinking about this :)

I'm sure we can do better than what we currently have which confuses the hell out of me, even though it's a great feature and great improvement overall :)

--

Stéphane Épardaud

[David Lloyd]

On Wed, Mar 22, 2023 at 9:24 AM Stephane Epardaud <stephane...@gmail.com> wrote:

On Wed, 22 Mar 2023 at 06:47, Stuart Douglas <sdou...@redhat.com> wrote:

Even though the extra methods are annoying, that is easily solved with an abstract type, and IMHO it is generally better to use existing parts of the JDK unless there is a really good reason not to.

Sure, I agree with that. But then we should provide the abstract class and tell people to implement it. I have no idea why equals/hashCode are required, and it's hard enough to understand why I need a constructor and an implies method, so no idea what the getActions() is for.

There is a comprehensive and (IIRC) reasonably well-documented hierarchy of abstract permission classes within WildFly Elytron, which I believe is included by at least one extension if not the core code. Look for `org.wildfly.security.permission.AbstractPermission` and its subclasses, which include permissions which are boolean in nature (you either have it or you don't), have a name, an optional action set, etc.

--

- DML • he/him

[Stephane Epardaud]

I've looked at the docs and subclasses, but these seem more like JDK permissions, where they're not dependent on the instance on which they operate. For example: you can either access the FS or not, but it doesn't depend on which file you're looking at, no?

I've seen actions as bit sets, but they didn't tell me what they're for. I'm probably looking at this the wrong way.

--

Stéphane Épardaud

[Sergey Beryozkin]

Hi Steph

On Wed, Mar 22, 2023 at 2:24 PM Stephane Epardaud <stephane...@gmail.com> wrote:

On Wed, 22 Mar 2023 at 06:47, Stuart Douglas <sdou...@redhat.com> wrote:

Even though the extra methods are annoying, that is easily solved with an abstract type, and IMHO it is generally better to use existing parts of the JDK unless there is a really good reason not to.

Sure, I agree with that. But then we should provide the abstract class and tell people to implement it. I have no idea why equals/hashCode are required, and it's hard enough to understand why I need a constructor and an implies method, so no idea what the getActions() is for.

 

The idea is that permissions do not have to be exactly equal to grant access. e.g. say I have HttpPermission(path, method).

if I have a method that requires HttpPermission(path="/admin", method="GET"), I can have a user with a permission HttpPermission(path="/admin", method="*"), and the implies method can figure out that a user with "*" permissions can also access GET.

Basically this implies approach allows for any amount of flexibility in how permissions are defined and enforced.

OK. So, perhaps that's also a way to say that the "admin" permission implies every other permission? So does this mean that if I have an "edit-order" permission for a given order, that normally would check if the current user "owns" the specific order, we'd have the admin permission imply it?

Unless I'm misunderstanding, this can be structured for example like `@PermissionAllowed("order:edit")` where `edit` is an action. `admin` is not really a permission, it is a role, so, at the HTTP security policy level or for example at the OIDC level, a mapping would be done between a role `admin` and permission `order:edit`, the incoming credential would have to have this `admin` role only set up (in the token or if it is basic auth - in security JPA, etc)

Sergey

 

--

You received this message because you are subscribed to the Google Groups "Quarkus Development mailing list" group.
To unsubscribe from this group and stop receiving emails from it, send an email to quarkus-dev...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/CAKU9E9scPf%2BHbAa7qUJdpB%2B5tSHe%3D_C5WP%3DkSOOBYek-ca%3Dqqg%40mail.gmail.com.

[Stephane Epardaud]

But "order:edit" doesn't tell me anything unless there's logic to look at the order being edited. That's what I don't get.

--

Stéphane Épardaud

[Sergey Beryozkin]

Right now `Order` can be passed to a permission checker constructor if it is on a CDI bean, see the LibraryResource in the docks which has to delegate to the injected LibraryService bean, it is a bit of indirection which won't be necessary once Michal does another enhancement

Sergey

[Michal Vavřík]

re I'm sure we can do better than what we currently have which confuses the hell out of me, even though it's a great feature and great improvement overall :)

• now it is possible to completely avoid SecurityIdentityAugmentor, you can leverage HTTP security policy to map roles to permissions
• there is a PR that enables passing of secured method arguments directly into custom permission for RESTEasy Reactive endpoints https://github.com/quarkusio/quarkus/pull/32534

Regarding more relatable doc examples - I shall make docs suggestions when #32534 is merged.

I hope this improved feature usability in your eyes and keep opposite feedback coming, it's well appreciated, thanks

Michal

[Michal Vavřík]

https://quarkus.io/version/main/guides/security-authorize-web-endpoints-reference#permission-annotation :-)

[Sergey Beryozkin]

Thanks Michal for driving this major security enhancement (Permissions)

IMHO the PR is good to be merged

Cheers Sergey

To view this discussion on the web visit https://groups.google.com/d/msgid/quarkus-dev/69096df4-58c5-45bd-a377-466458dbe6fen%40googlegroups.com.