Skip to content
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

Support injection of the @BeanParam parameters into a custom permission referenced in the @PermissionAllowed security annotation #43353

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 128 additions & 8 deletions docs/src/main/asciidoc/security-authorize-web-endpoints-reference.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -830,8 +830,8 @@
In this scenario, the permission `list` is added to the `SecurityIdentity` instance as `new CustomPermission("list")`.

You can also create a custom `java.security.Permission` class with additional constructor parameters.
These additional parameters get matched with arguments of the method annotated with the `@PermissionsAllowed` annotation.
These additional parameters names get matched with arguments names of the method annotated with the `@PermissionsAllowed` annotation.
Later, Quarkus instantiates your custom permission with actual arguments, with which the method annotated with the `@PermissionsAllowed` has been invoked.

Check warning on line 834 in docs/src/main/asciidoc/security-authorize-web-endpoints-reference.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsSuggestions] Depending on the context, consider using ', which (non restrictive clause preceded by a comma)' or 'that (restrictive clause without a comma)' rather than 'which'. Raw Output: {"message": "[Quarkus.TermsSuggestions] Depending on the context, consider using ', which (non restrictive clause preceded by a comma)' or 'that (restrictive clause without a comma)' rather than 'which'.", "location": {"path": "docs/src/main/asciidoc/security-authorize-web-endpoints-reference.adoc", "range": {"start": {"line": 834, "column": 79}}}, "severity": "INFO"}

.Example of a custom `java.security.Permission` class that accepts additional arguments

Expand Down Expand Up @@ -910,12 +910,12 @@
public class LibraryService {

@PermissionsAllowed(value = "tv:write", permission = LibraryPermission.class) <1>
public Library updateLibrary(String newDesc, Library update) {
update.description = newDesc;
return update;
public Library updateLibrary(String newDesc, Library library) {
library.description = newDesc;
return library;
}

@PermissionsAllowed(value = "tv:write", permission = LibraryPermission.class, params = "library") <2>
@PermissionsAllowed(value = "tv:write", permission = LibraryPermission.class) <2>
@PermissionsAllowed(value = {"tv:read", "tv:list"}, permission = LibraryPermission.class)
public Library migrateLibrary(Library migrate, Library library) {
// migrate libraries
Expand All @@ -924,10 +924,11 @@

}
----
<1> The formal parameter `update` is identified as the first `Library` parameter and gets passed to the `LibraryPermission` class.
<1> The formal parameter `library` is identified as the parameter matching same-named `LibraryPermission` constructor parameter.

Check warning on line 927 in docs/src/main/asciidoc/security-authorize-web-endpoints-reference.adoc

View workflow job for this annotation

GitHub Actions / Linting with Vale

[vale] reported by reviewdog 🐶 [Quarkus.TermsSuggestions] Depending on the context, consider using 'because' or 'while' rather than 'as'. Raw Output: {"message": "[Quarkus.TermsSuggestions] Depending on the context, consider using 'because' or 'while' rather than 'as'.", "location": {"path": "docs/src/main/asciidoc/security-authorize-web-endpoints-reference.adoc", "range": {"start": {"line": 927, "column": 50}}}, "severity": "INFO"}
Therefore, Quarkus will pass the `library` parameter to the `LibraryPermission` class constructor.
However, the `LibraryPermission` must be instantiated each time the `updateLibrary` method is invoked.
<2> Here, the first `Library` parameter is `migrate`; therefore, the `library` parameter gets marked explicitly through `PermissionsAllowed#params`.
The permission constructor and the annotated method must have the parameter `library` set; otherwise, validation fails.
<2> Here, the second `Library` parameter matches the name `library`,
while the `migrate` parameter is ignored during the `LibraryPermission` permission instantiation.

.Example of a resource secured with the `LibraryPermission`

Expand Down Expand Up @@ -1078,6 +1079,125 @@
----
<1> Any method or class annotated with the `@CanWrite` annotation is secured with this `@PermissionsAllowed` annotation instance.

[[permission-bean-params]]
=== Pass `@BeanParam` parameters into a custom permission

Quarkus can map fields of a secured method parameters to a custom permission constructor parameters.
You can use this feature to pass `jakarta.ws.rs.BeanParam` parameters into your custom permission.
Let's consider following Jakarta REST resource:

[source,java]
----
package org.acme.security.rest.resource;

import io.quarkus.security.PermissionsAllowed;
import jakarta.ws.rs.BeanParam;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.Path;

@Path("/hello")
public class SimpleResource {

@PermissionsAllowed(value = "say:hello", permission = BeanParamPermission.class,
params = "beanParam.securityContext.userPrincipal.name") <1>
sberyozkin marked this conversation as resolved.
Show resolved Hide resolved
@GET
public String sayHello(@BeanParam SimpleBeanParam beanParam) {
return "Hello from " + beanParam.uriInfo.getPath();
}

}
----
<1> The `params` annotation attribute specifies that user principal name should be passed to the `BeanParamPermission` constructor.
Other `BeanParamPermission` constructor parameters like `customAuthorizationHeader` and `query` are matched automatically.
Quarkus identifies the `BeanParamPermission` constructor parameters among `beanParam` fields and their public accessors.
To avoid ambiguous resolution, automatic detection only works for the `beanParam` fields.
For that reason, we had to specify path to the user principal name explicitly.

Where the `SimpleBeanParam` class is declared like in the example below:

[source,java]
----
package org.acme.security.rest.dto;

import java.util.List;

import jakarta.ws.rs.HeaderParam;
import jakarta.ws.rs.QueryParam;
import jakarta.ws.rs.core.Context;
import jakarta.ws.rs.core.SecurityContext;
import jakarta.ws.rs.core.UriInfo;

public class SimpleBeanParam {

@HeaderParam("CustomAuthorization")
private String customAuthorizationHeader;

@Context
SecurityContext securityContext;

@Context
public UriInfo uriInfo;

@QueryParam("query")
public String query; <1>

public SecurityContext getSecurityContext() { <2>
return securityContext;
}

public String customAuthorizationHeader() { <3>
return customAuthorizationHeader;
}
}
----
<1> Quarkus Security can only pass public fields to a custom permission constructor.
<2> Quarkus Security automatically uses public getter methods if they are available.
<3> The `customAuthorizationHeader` field is not public, therefore Quarkus access this field with the `customAuthorizationHeader` accessor.
That is particularly useful with Java records, where generated accessors are not prefixed with `get`.

Here is an example of the `BeanParamPermission` permission that checks user principal, custom header and query parameter:

[source,java]
----
package org.acme.security.permission;

import java.security.Permission;

public class BeanParamPermission extends Permission {

private final String actions;

public BeanParamPermission(String permissionName, String customAuthorizationHeader, String name, String query) {
michalvavrik marked this conversation as resolved.
Show resolved Hide resolved
sberyozkin marked this conversation as resolved.
Show resolved Hide resolved
super(permissionName);
this.actions = computeActions(customAuthorizationHeader, name, query);
}

@Override
public boolean implies(Permission p) {
boolean nameMatches = getName().equals(p.getName());
boolean actionMatches = actions.equals(p.getActions());
return nameMatches && actionMatches;
}

private static String computeActions(String customAuthorizationHeader, String name, String query) {
boolean queryParamAllowedForPermissionName = checkQueryParams(query);
boolean usernameWhitelisted = isUserNameWhitelisted(name);
boolean customAuthorizationMatches = checkCustomAuthorization(customAuthorizationHeader);
var isAuthorized = queryParamAllowedForPermissionName && usernameWhitelisted && customAuthorizationMatches;
if (isAuthorized) {
return "hello";
} else {
return "goodbye";
}
}

...
}
----

michalvavrik marked this conversation as resolved.
Show resolved Hide resolved
NOTE: You can pass `@BeanParam` directly into a custom permission constructor and access its fields programmatically in the constructor instead.
Ability to reference `@BeanParam` fields with the `@PermissionsAllowed#params` attribute is useful when you have multiple differently structured `@BeanParam` classes.

== References

* xref:security-overview.adoc[Quarkus Security overview]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package io.quarkus.resteasy.reactive.server.test.security;

import java.security.Permission;

import jakarta.enterprise.context.ApplicationScoped;

import io.quarkus.security.StringPermission;
import io.quarkus.security.identity.AuthenticationRequestContext;
import io.quarkus.security.identity.SecurityIdentity;
import io.quarkus.security.identity.SecurityIdentityAugmentor;
import io.quarkus.security.runtime.QuarkusSecurityIdentity;
import io.smallrye.mutiny.Uni;

@ApplicationScoped
public class BeanParamPermissionIdentityAugmentor implements SecurityIdentityAugmentor {

@Override
public Uni<SecurityIdentity> augment(SecurityIdentity securityIdentity,
AuthenticationRequestContext authenticationRequestContext) {
var possessedPermission = createPossessedPermission(securityIdentity);
var augmentedIdentity = QuarkusSecurityIdentity
.builder(securityIdentity)
.addPermissionChecker(requiredPermission -> Uni
.createFrom()
.item(requiredPermission.implies(possessedPermission)))
.build();
return Uni.createFrom().item(augmentedIdentity);
}

private Permission createPossessedPermission(SecurityIdentity securityIdentity) {
// here comes your business logic
return securityIdentity.isAnonymous() ? new StringPermission("list") : new StringPermission("read");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
package io.quarkus.resteasy.reactive.server.test.security;

import jakarta.ws.rs.BeanParam;

import org.jboss.resteasy.reactive.RestHeader;
import org.jboss.resteasy.reactive.RestQuery;

public record MyBeanParam(@RestQuery String queryParam, @BeanParam Headers headers) {
public record Headers(@RestHeader String authorization) {
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
package io.quarkus.resteasy.reactive.server.test.security;

import java.security.Permission;
import java.util.Objects;

public class MyPermission extends Permission {

static final MyPermission EMPTY = new MyPermission("my-perm", null, null);

private final String authorization;
private final String queryParam;

public MyPermission(String permissionName, String authorization, String queryParam) {
super(permissionName);
this.authorization = authorization;
this.queryParam = queryParam;
}

@Override
public boolean implies(Permission permission) {
if (permission instanceof MyPermission myPermission) {
return myPermission.authorization != null && "query1".equals(myPermission.queryParam);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once again a permission where this. members are never used. I don't understand this model.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The model comes from the https://docs.oracle.com/en/java/javase/21/docs/api/java.base/java/security/Permission.html#implies(java.security.Permission). In Quarkus terms, the identity has permission checker that can check required permissions. Here it is:

So technically, you only need one required permission and one checker. You can do requiredPermission.implies(null) or whatever, but it feels very hacky, so I always expect users will declare both requiredPermission and possessedPermission and do: possessedPermission.implies(requiredPermission).

I expect this can be simplified with your @RequiredPermission or @PermissionChecker or whatever later.

BTW this particular MyPermission is here to test that bean parameters were passed correctly, I don't think it needs to use this for that purpose. I wouldn't write this to the docs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can understand implies as it's documented, which is about figuring out if, given an already granted permission, another one is implied by it. For example, given that the current user has the admin permission, is the read permission implied? Probably, yes.

But this is not what we're describing here with MyPermission. I see no relation.

SecurityIdentity.checkPermission(Permission) I also understand: does the current user have the right to do X? Fine. That makes sense.

But again, this does not look like what MyPermission.implies is doing. Is this something related to the current PR, or the general security model in Quarkus?

Copy link
Member Author

@michalvavrik michalvavrik Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this is not what we're describing here with MyPermission. I see no relation.

ad describing: it's a test, not docs; but sure, I'll answer

SecurityIdentity.checkPermission(Permission) I also understand: does the current user have the right to do X? Fine. That makes sense.

When you specify @PermissionsAllowed(permission=CustomPermission) then CustomPermission instance is created and passed to the SecurityIdentity.checkPermission(Permission) because it is fact checkPermission(requiredPermission).

It depends on how user writes SecurityIdentityAugmentor whether this is used. I am using one and the same permission in the all tests for making it easy, but you can also do this (meta code ...):

@Override
public Uni<SecurityIdentity> augment(SecurityIdentity identity, AuthenticationRequestContext context) {
    return QuarkusSecurityIdentity.builder(identity).addPermissionChecker(requiredPermission -> {
        if (requiredPermission.implies(new StringPermission("read"))) {
            return Uni.createFrom().item(Boolean.TRUE);
        }
        return Uni.createFrom().item(Boolean.FALSE);
    }).build();
}

and this augmentor part can be simplified with #43238.

I cannot simplify this model in this PR.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, so wait, users have to implement a SecurityIdentityAugmentor to make this work?

Copy link
Member Author

@michalvavrik michalvavrik Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is one option. There are other ways to add permission checker on your security identity. For example we map token claims as StringPermission. Or you can remap SecurityIdentity roles like this:

map identity role 'user' to String permission 'read'
quarkus.http.auth.policy.role-policy2.permissions.user=read  
quarkus.http.auth.permission.roles2.paths=*
quarkus.http.auth.permission.roles2.policy=role-policy2

Which gives you var possessed = new StringPermission("read")

Problem with this is that we add permission check like possessedPermisison.implies(requiredPermission). It felt right, but I suppose for purpose of this particular situation, requiredPermisison.implies(possessedPermission) is equivalent. Though it is not from POV of formal logic.

So ATM you do need augmentor, but this part could be changed in this PR if it is requested.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be yet another breaking change though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And if you wonder, if user doesn't specify any permission checker but @PermissionsAllowed is applied, the QuarkusSecurityIdentity responds with false by default

so you don't have the permission unless you posses at least one that complies it. I think we shouldn't change it in this PR. I'll continue updating the docs with the augmentor.

}
return false;
}

@Override
public boolean equals(Object o) {
if (this == o)
return true;
if (o == null || getClass() != o.getClass())
return false;
MyPermission that = (MyPermission) o;
return Objects.equals(authorization, that.authorization)
&& Objects.equals(queryParam, that.queryParam);
}

@Override
public int hashCode() {
return Objects.hash(authorization, queryParam);
}

@Override
public String getActions() {
return "";
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package io.quarkus.resteasy.reactive.server.test.security;

import jakarta.ws.rs.HeaderParam;
import jakarta.ws.rs.QueryParam;
import jakarta.ws.rs.core.Context;
import jakarta.ws.rs.core.SecurityContext;
import jakarta.ws.rs.core.UriInfo;

public class OtherBeanParam {

@HeaderParam("CustomAuthorization")
private String customAuthorizationHeader;

@Context
SecurityContext securityContext;

@Context
public UriInfo uriInfo;

@QueryParam("query")
public String query;

public SecurityContext getSecurityContext() {
return securityContext;
}

public String customAuthorizationHeader() {
return customAuthorizationHeader;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package io.quarkus.resteasy.reactive.server.test.security;

import java.security.Permission;

public class OtherBeanParamPermission extends Permission {

private final String actions;

public OtherBeanParamPermission(String permissionName, String customAuthorizationHeader, String name, String query) {
super(permissionName);
this.actions = computeActions(customAuthorizationHeader, name, query);
}

@Override
public String getActions() {
return actions;
}

@Override
public boolean implies(Permission p) {
boolean nameMatches = getName().equals(p.getName());
boolean actionMatches = getActions().equals(p.getActions());
return nameMatches && actionMatches;
}

@Override
public boolean equals(Object obj) {
return false;
}

@Override
public int hashCode() {
return 0;
}

private static String computeActions(String customAuthorizationHeader, String name, String query) {
boolean queryParamAllowedForPermissionName = checkQueryParams(query);
boolean usernameWhitelisted = isUserNameWhitelisted(name);
boolean customAuthorizationMatches = checkCustomAuthorization(customAuthorizationHeader);
var isAuthorized = queryParamAllowedForPermissionName && usernameWhitelisted && customAuthorizationMatches;
if (isAuthorized) {
return "hello";
} else {
return "goodbye";
}
}

private static boolean checkCustomAuthorization(String customAuthorization) {
return "customAuthorization".equals(customAuthorization);
}

private static boolean isUserNameWhitelisted(String userName) {
return "admin".equals(userName);
}

private static boolean checkQueryParams(String queryParam) {
return "myQueryParam".equals(queryParam);
}

}
Loading