Support simple expression language for authorization

[sberyozkin]

Description

While Quarkus users do have several options to customize authorization checks such as registering custom permissions and HTTP security policies and binding them to JAX-RS methods with annotations, there is some niche there where really simple authorization checks are required for a large number of methods where creating custom permissions or policy beans can be a bit excessive.

Several colleagues have been mentioning Spring's PreAuthorize but also there are solutions like Google Cloud where simple permissions can be applied using expression languages.

IMHO it makes sense to consider a similar solution for Quarkus.

Implementation ideas

Start with the most basic expression language support. I'd propose to allow only 2 simple expressions at the very start, and then see how it goes and evolve it as necessary.

I think in general, anything that requires more than a request URL or header check should not be dealt with. But if the check is all about asserting that an authenticated user's request URL has a query parameter, or some header is provided, then having something like

@AuthorizationExpression(query.contains="a=${a}") 

(and similarly for headers) or whatever is the easiest option, where ${a} matches a JAX-RS QueryParam("a"), can help users save on typing a lot of code with custom permissions, policies.

IMHO we should start with the most basic expression support, see how it goes, and then invest more time if necessary to get it formalized, richer.

CC @michalvavrik @FroMage @dmlloyd @cescoffier

[quarkus-bot]

/cc @pedroigor (bearer-token)

[FroMage]

I'm sorry I don't understand what the use-case is for this. In general I've found EL to be useless in permission checks, because most permissions are not only user-dependent but target-dependent, and that's impossible to express with EL and annotations, these checks must be implemented using APIs, and in most cases involve DB calls or web services.

For example, you have write permission on a certain github project if you own that project, or you're listed on its teams, or as a dirrect contributor, but only if you have the write permission. All of this logic requires loading the project model, the user, the set of teams, and contributors, all of which come from the DB and require loading and logic that is super hard to express in EL and trivial in code.

[sberyozkin]

@FroMage Agreed, yes, anything that requires even moderate/medium complexity check will be done with custom permissions, policies.

It is really to support very straightforward checks, related to the presence of query parameters or headers where there could be many JAX-RS methods, and a query parameter can be used to determine what kind of permission a given authenticated user has.

[FroMage]

How can a query parameter be used to determine what kind of permission a given authenticated user has? This is user input.

[sberyozkin]

@FroMage Yeah, sorry, it was not a good or rather complete example all right, I've just reread the use case scenario, see below.

I suppose when we have tokens, it is a verified user input though but then may be we'd rather have an annotation like @Claim to assert the presence of some claim, as opposed to the typical roles data.

But the other examples I've seen seem interesting enough, like an IP (range) check in Google Cloud.

Back to the query parameters, the use case I've heard about recently is about several thousands users and projects, where each user has different permissions in different projects. The query parameter supplies a project id and the expression is used to connect this query parameter with the bean which calculates the permissions for the project, so the expression can look like this in pseudo language: rolesAwareBean.hasTheAPermission(${projectid}) and then the methods which require the A permission are protected with these expressions, wrapped in simple custom annotations...

I think supporting cases like this one, linking query or header parameters to some bean method inputs and using the boolean output to drive the authorization can be useful

[sberyozkin]

Something along these lines might work not bad IMHO:

1. Introduce Authorizer marker interface, with name() only, default name value authorizer
2. Any CDI bean with all sort of methods, accepting String, int, long, or other types which JAX-RS QueryParam supports (or header or path or even matrix param) and it implements Authorizer.
3. User writes an expression like @Authorization(authorizer.isThisOrThat("customerid")) where customerid matches QueryParam("customerid") long id, passing this query param's value
4. authorizer.isThisOrThat must return boolean, and the result determines if the authorization check passed or not

[ebullient]

Can't a user do this already with a CDI interceptor (which would have access to available request-scoped attributes)? These can be pretty trivial to write. I would recommend that over an expression language (which will always be not-quite-right, IMO).

[michalvavrik]

Can't a user do this already with a CDI interceptor (which would have access to available request-scoped attributes)?

Yes, they can. Though we would prefer if they run async security checks on IO thread before serialization. But they can access request-scoped attributes from RoutingContext when they annotated endpoints with @AuthorizationPolicy https://quarkus.io/version/main/guides/security-authorize-web-endpoints-reference#custom-http-security-policy or they can use custom @PermissionsAllowed (but that is for complex security).

These can be pretty trivial to write. I would recommend that over an expression language (which will always be not-quite-right, IMO).

+1; @FroMage arguments are appealing to me.

In Spring, I always used EL just to invoke bean and pass arguments there, because I had to learn syntax only to forget it little later (don't write EL every day). I think this feature proposal is not here to add capability, but to give option to users who prefer brevity. IMHO it is completely legit, though I personally would prefer Java.

[ebullient]

Can't a user do this already with a CDI interceptor (which would have access to available request-scoped attributes)?

Yes, they can. Though we would prefer if they run async security checks on IO thread before serialization. But they can access request-scoped attributes from RoutingContext when they annotated endpoints with @AuthorizationPolicy https://quarkus.io/version/main/guides/security-authorize-web-endpoints-reference#custom-http-security-policy or they can use custom @PermissionsAllowed (but that is for complex security).

I am probably doing it wrong, but for my own simple cases, I literally use a plain old interceptor. I see no value to dealing with all of the policy overhead nonsense for what I need to check, which (as @FroMage says), involves a lot of context that isn't available until later. I don't use any of the annotations or named policies or anything that requires a bunch of spec-reading or domain-specific knowledge, I just use an interceptor. That's it.

Our security config + docs are already super complicated (because there is just.. a lot there). More annotations that you have to decide how to use / if you can use is not going to help that.

[sberyozkin]

Hi @ebullient @michalvavrik, so let's take a simple example.

Users: alice, bob, fred
Projects: project-a,project-b,project-c
Permissions:

• project-a: alice-write, bob-read, fred-none
• project-b: alice-read, bob-write fred-read,write
• project-c: alice-read,write, bob-none fred-write

Authenticated user has a project-id query parameter.
Class:

class Service {
   //user must have a read perm in the current project
   String read(@QueryParam("projectid") String projectid) {}
   //user must have a write perm in the current project
   void write(@QueryParam("projectid") String projectid) {}
   // //user must have both read and write perm in the current project
   void String writeAndThenRead(@QueryParam("projectid") String projectid) {} 
}

With the proposed solution above (I'm not saying it can be easy to support :-)), I can do:

@Singleton
class ProjectAuthorizer implements Authorizer {
    @Inject SecurityIdentity identity;
    Map<String, Set<String>> projectIdToRoles;

    @PostConstruct
    void populateProjectIdToRoles() {
    }

   // this may not compile :-) but you see we want the identity and the project have the read role, etc

    boolean isReadAllowed(String projectid) {
       return projectIdToRoles.get(projectid).contains("read") && identity.getRoles().contains("read");
    }
    boolean isWriteAllowed(String projectid) {
       return projectIdToRoles.get(projectid).contains("write" && identity.getRoles().contains("write"));
    }
    boolean isReadAndWriteAllowed(String projectid) {
       return isReadAllowed(projectid) && isWriteAllowed(projectid);
    } 
 
}

and then

class Service {
   @Authorization(authorizer.isReadAllowed("projectid"))  
   String read(@QueryParam("projectid") String projectid) {}
   // write perm
   @Authorization(authorizer.isWriteAllowed("projectid")
   void write(@QueryParam("projectid") String projectid) {}
   // read and write perms
   @Authorization(authorizer.isReadAndWriteAllowed("projectid")
   void String writeAndThenRead(@QueryParam("projectid") String projectid) {} 
}

We have a single extra piece of Java code to support it all, ProjectAuthorizer CDI bean.

Now, we have other options, permissions, policies, CDI interceptors, which can be bound to specific methods and which can be very powerful, but how can what we have can compare to the suggested prototype above ?

My motivation here is keep trying finding optimal setups for our users for specific authorization setups :-).

[michalvavrik]

We have a single extra piece of Java code to support it all, ProjectAuthorizer CDI bean.

As far as I am concerned, your proposal is just another option for users. You can achieve same check with AuthorizationPolicy annotation. I don't say there are not users that will want that. Probably users migrating from Spring @geoand ?

I'm not saying it can be easy to support :-)

That is least of my worries. Your current proposal can be generated during buildtime as authorization policy class, I can do it over weekend if there is ever agreement. It would be whole different story once other arguments than HTTP request ones are required far in the future.

[geoand]

I don't say there are not users that will want that. Probably users migrating from Spring @geoand ?

We've had little to no such requests along those lines to be honest.

I'm personally -0.5 on having ELs

[sberyozkin]

I'm not even worried about Spring migrations. It is for Quarkus, I don't mind what is happening elsewhere, though I have to acknowledge I've had a feedback from Spring users.

@michalvavrik

As far as I am concerned, your proposal is just another option for users.

Well, true, but I'm not proposing it just to have N+1 options instead of N.

Let's do a proper analysis, of how much cost is involved in supporting a simple case suggested above (recall the original case - a thousand or so users, with each user having different roles in a thousand or so projects). It is nothing to do with Spring really, lets imagine someone is just starting with Quarkus.

I actually don't understand how we can easily do a task suggested above with the current annotations we have... If we can get one more option which can make coding a task like that a breeze than it would be worth it IMHO...

[sberyozkin]

I mean, it is not critical, and as far as EL is concerned, now that I think about it, the only expression I have in mind is bean.somemethod(param1, param2) :-)

I think I know how users can use for ex HttpSecurityPolicy bound to JAX-RS methods with @AuthorizationPolicy : users would get RoutingContext query project id and use it - Michal, is it what you had in mind ? But then for 3 different types of access control checks (read, write or readAndWrite), we'd need 3 separate HttpSecurityPolicy classes, for 10 different permissions - 10 classes.

Custom permission classes can accept method parameters - but then the user needs to write an augmentor to bind permissions first and how does the user know which permissions to bind in the example above, I'm not sure.

Dev exp looks a bit suboptimal in two last cases.

So, let's keep it as a take our time discussion, see if it makes sense at all, if users can get some noticeable savings with larger projects...

I'll be happy to close this issue if we find it is all may end up to be too much effort for negligible savings

thanks

[michalvavrik]

I think I know how users can use for ex HttpSecurityPolicy bound to JAX-RS methods with @AuthorizationPolicy : users would get RoutingContext query project id and use it - Michal, is it what you had in mind ?

yes

But then for 3 different types of access control checks (read, write or readAndWrite), we'd need 3 separate HttpSecurityPolicy classes, for 10 different permissions - 10 classes. Custom permission classes can accept method parameters - but then the user needs to write an augmentor to bind permissions first and how does the user know which permissions to bind in the example above, I'm not sure.

yes, that is how it is; it is possible, but more verbose

So, let's keep it as a take our time discussion, see if it makes sense at all, if users can get some noticeable savings with larger projects...

I am sorry, I don't know what you mean by this. We would need a feedback from users, but that would be based on implemented feature. I think without this being implemented, we can only gather opinions here and so far, they are rather negative.

I'll be happy to close this issue if we find it is all may end up to be too much effort for negligible savings

I don't want to be too optimistic, as I probably am, but I still don't see implementation issue. This is transferable to named HttpSecurityPolicy as long as logical operators (if any) and passed method params stays very limited.

I just think you opened discussion so there must be more users asking for it or agreeing. Thanks for patience in feature explanation.

[sberyozkin]

@michalvavrik, thanks, I mean, was I correct with

But then for 3 different types of access control checks (read, write or readAndWrite), we'd need 3 separate HttpSecurityPolicy classes, for 10 different permissions - 10 classes.

?

Is it worth considering a prototyped solution if no matter how many permissions we have there will only be a single java authorizer bean, as opposed to one bean per permission.

we can only gather opinions here and so far, they are rather negative.

That is not a problem, but I prefer to talk about a concrete case I prototyped based on a much more complex setup from one of our colleagues which you were CC-ed too.

If the transition to Quarkus requires N beans instead of one for every permission type then it is not a good example we can talk aloud about IMHO.

So indeed, let's keep gathering feedback.

Cheers

[sberyozkin]

If the variation is just about 3-5 beans extra beans max then it is all right I guess, a few extra beans but with clearer annotations etc, is OK. Let me ask for out colleague to comment here if possible

[sberyozkin]

Asked with CC to Michal. Let's keep discussing over the next while. Thanks everyone for the feedback so far

[FroMage]

What's the improvement in:

   @Authorization("authorizer.isReadAllowed('projectid')")  
   String read(@QueryParam("projectid") String projectid) {}

Over:

   String read(@QueryParam("projectid") String projectid) {
      checkAuthorization(isReadAllowed(projectId)); // throws if false
   }

I fail to see it. And I've written hundreds of such methods. Not only that, but also, in most cases you do want to run the auth checks in the same TX as the endpoint method, and also, reuse the objects loaded by the auth code in your method.

In this example we're loading the Project entity more than once (in auth check, and later there's a good chance in the endpoint method), altough if we're in the same TX it should be cached.

Now, add to that the question of input validation (query parameters are optional, what happens if null/empty?) and type-checking to make sure that the EL method call to authorizer.isReadAllowed() has the right type of parameter, and we've got ourselves a fine mess.

I've looked long and hard for an improvement over writing Java methods for determining auth permissions in endpoint methods, and have never found anything better. Most of the proposals, or even my POCs were just deceptive and too limited.

At this point, I'm -1 on this, but ready to change my mind if something really good comes along.

[FroMage]

Ah, found the previous discussion, which has a good summary of the use-cases, why we didn't want EL, and ideas on what alternatives might be better than EL if we had the time to implement them: #5479

[michalvavrik]

I incline to @FroMage arguments.

What's the improvement in

• @Authorization("authorizer.isReadAllowed('projectid')") we can easily detected during the build time. I have seen in issue reproducers that some users use default JAX-RS security to deny endpoint not secured with security annotations. You cannot do that with checkAuthorization(isReadAllowed(projectId)); // throws if false.
• Except for custom permissions (even there we require authentication eagerly), all our security annotations run as the first thing after match (HTTP request <-> actual endpoint with annotation). If the request is denied, there is less processing (like serialization etc.). Now, I don't think most users care. See Erin's comments. Also I had colleagues that wrote exactly the code checkAuthorization(isReadAllowed(projectId));. But still, there is more code executed.

[FroMage]

I have seen in issue reproducers that some users use default JAX-RS security to deny endpoint not secured with security annotations

Most such endpoints will required a logged in user before they attempt any authorization, so at the very least they will have @Authenticated.

[sberyozkin]

Thanks @FroMage, sure, I'm still hoping we can get the user's feedback, which would've helped to have a bit more visibility. I'm easy with this enhancement not going ahead. For now, a minor comment:

String read(@QueryParam("projectid") String projectid) {
checkAuthorization(isReadAllowed(projectId)); // throws if false
}

It is fine of course, but this is an alternative to an annotation based solution, where users don't want to have any security logic in the actual endpoint method implementations, and we know some users have strong preferences around it.

An expression like @Authorization("authorizer.isReadAllowed('projectid')") can be wrapped into a simple @CanRead() only...

But in any case, I'd like to see the user's feedback for us to try to estimate more precisely is there any code cost saving with the basic EL there at all or not, thanks

[jasoncsmith7]

I initially reached out to the Quarkus seeking an equivalent to Spring's @PreAuthorize annotation for authorization in Quarkus. @michalvavrik and @sberyozkin advised exploring CustomPermission and SecurityIdentityAugmentor. Concurrently, Sergey investigated a Quarkus-level solution to address this issue. I've successfully implemented authorization using the CustomPermission and SecurityIdentityAugmentor approach.

Original Approach (Spring)

• Used @PreAuthorize annotation
• Endpoint defined as:

  @PreAuthorize("hasAnyAuthority(@authorityHelper.getHasWriteAuthority(#project))")

• Request param specified the project
• Annotation config defined required access level
• Authorization based on correct access (or greater) for the provided project
• Code ensured access only to that project's data/features

New Approach (Quarkus)

Based on @michalvavrik and @sberyozkin's advice:

1. Implemented using CustomPermission and SecurityIdentityAugmentor
2. Plans for a custom annotation:
   • Will create @CanWrite annotation
   • Build step to convert @CanWrite to:

     @PermissionsAllowed(value = "write", permission = CustomPermission.class)

   • Ensure request parameter is provided
   • Authorization based on correct access (or greater) for the provided project
   • Code ensured access only to that project's data/features

I've successfully replicated the @PreAuthorize functionality using SecurityIdentityAugmentor and I will proceed with this solution. However, I remain open to adopting any official Quarkus-specific approach that may be developed in the future.

[sberyozkin]

Thanks @jasoncsmith7, it is interesting. If we can also eventually make it easy to do annotations like @CanWrite as in #39663, then it can become quite compact and neat.

My only minor concern here is that a user has to create a custom SecurityIdentityAugmentor in order to register implied permissions, ideally it would not be necessary.

Please also have a look at @AuthorizationPolicy which @michalvavrik introduced, which you can use to link to named HttpSecurityPolicy policies. Quarkus 3.16 CR1 will have it early next week, I wonder how would you find it comparing with the SecurityIdentityAugmentor and custom permissions option.

Thanks

[gsmet]

FWIW, we used a lot the Spring security annotations at previous job and also the EL for some of them.

What's nice with it is that you have a clear definition of your security contract for these methods. Sure you can always write code in the body of the method to handle it but that makes the security contract a lot less visible and clear.

Also when you review code changes, you are a lot more careful when you see changes there rather than changes to an if in the body of the method (which most of the time won't be related to security).

And when it's about security, I think visibility and clarity is a feature.

[geoand]

What I personally hate about ELs is that they are completely un-debuggable. Sure, they work really nice when the expressions is correct, but when anything is not written properly (which is super easy since there is no tooling support), it's guesswork all the way to try and understand what the real issue is.

[FroMage]

An expression like @authorization("authorizer.isReadAllowed('projectid')") can be wrapped into a simple @CanRead() only...

This is stringly-typed code. In a language which is type-safe. It makes zero sense IMO. Even Hibernate and Jakarta-Data when they went to allow HQL in annotations went to great lengths to make it type-safe using quite complex mechanisms and tooling. And in that specific case, they had good reasons to move from HQL in code to HQL in annotations: making it type-safe where it could not be type-safe in code. Via tooling. Because it's not Java, it's a different language.

Turning type-safe, refactorable, easy to modify, read, comprehend, debug Java code into stringly-typed EL is something that IMO doesn't achieve anything of value and worse, opens the door to many issues around UX, and obfuscates security.

Hiding EL behind a @CanRead annotation doesn't make the problems go away. All the original problems remain and will be located on that annotation.

What's nice with it is that you have a clear definition of your security contract for these methods. Sure you can always write code in the body of the method to handle it but that makes the security contract a lot less visible and clear

I disagree that it's clear, because it's not code: it won't be indented, colored, clickable, debuggable, refactorable, it won't be composable, can't be easily edited. It's just as bad as code in javadoc, and they went to great lengths to move it out into files where it can be type-checked.

Adding it in the method body makes it code, with all the advantages that code has. It also makes it easier to edit, compose, tweak, reuse, refactor…

I think I'm just rehashing my arguments, I've already said them all. That doesn't mean I'm right and we should block this, but even with an open mind, I haven't seen a single argument in favour of EL for security that would counter-balance all the arguments I've listed against it. So I'm this against this, and in fact we've shown several better alternatives, existing or possible to implement, that are better than EL, IMO. Obviously I can't block this if people overwhelmingly want to add EL to security, but I'm still firmly in the camp that says this is a really bad idea. Sorry :(

We could ask @gavinking for his opinion, as someone familiar with EL (back in Seam days) and the HQL in annotations example that I mentioned. He's probably going to be more articulate about the pros and cons of this topic, and perhaps might come up with good arguments in favour of EL in security annotations?

[sberyozkin]

Sure, the immediate improvement idea from Michal is to support meta annotations like @CanWrite to help @jasoncsmith7 wrap longer @PermissionAllowed expressions which he had success with during the migration from Spring's PreAuthorize. This was independently requested awhile back by another user.

As far as the EL is concerned, I agree with @gsmet re putting the security checks in the code but I guess it is not only about using EL as such; I was looking at it as a possible tool to simplify the migration for users doing large projects, but @jasoncsmith7's feedback suggests users can do quite well so far with Quarkus's existing annotations and we'll look at tuning it further.

So right now, I suppose, we should just give it a bit more time and collect some more feedback to see if there could be any savings made with EL at all, for example, while supporting a single expression type only, just to link to some existing beans, etc. Otherwise looks like we are good so far without EL.

[sberyozkin]

I think we can close it now that @PermissionChecker enhancement is going ahead