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

quarkus-spring-data-rest + javax.annotation.security annotations behave inconsistently and lack documentation #30358

Closed
jsmrcka opened this issue Jan 13, 2023 · 2 comments · Fixed by #30364
Labels
area/security area/spring Issues relating to the Spring integration kind/bug Something isn't working
Milestone

Comments

@jsmrcka
Copy link
Contributor

jsmrcka commented Jan 13, 2023

Describe the bug

Since #29009, javax.annotation.security annotations are being propagated to generated REST resources when using rest-data-panache.

This applies to the quarkus-spring-data-rest extension as well. But the behavior here is a little bit confusing (compared to e.g. quarkus-hibernate-orm-rest-data-panache).

An example:

package org.acme;

import javax.annotation.security.DenyAll;
import javax.annotation.security.PermitAll;

import org.springframework.data.repository.CrudRepository;

@DenyAll
public interface FruitsRepository extends CrudRepository<Fruit, Long> {
    @Override
    @PermitAll
    Iterable<Fruit> findAll();
}

Here, the @DenyAll gets propagated to the JAX-RS resource, while the @PermitAll does not.

In order to propagate method security annotations, the user has to add @RestResource , even if they would not need it otherwise:

package org.acme;

import javax.annotation.security.DenyAll;
import javax.annotation.security.PermitAll;

import org.springframework.data.repository.CrudRepository;
import org.springframework.data.rest.core.annotation.RestResource;

// No @RepositoryRestResource needed here, the @DenyAll security annotation gets propagated anyway.
@DenyAll
public interface FruitsRepository extends CrudRepository<Fruit, Long> {
    @Override
    @PermitAll
    @RestResource // This must be here in order to propagate the @PermitAll security annotation.
    Iterable<Fruit> findAll();
}

This should be fixed: either require or do not require @RepositoryRestResource/@RestResource annotations both on classes and on methods.

The second issue is lacking documentation. The support for the security annotations should be mentioned in the Spring Data Rest guide for Quarkus 2.13+: https://quarkus.io/version/2.13/guides/spring-data-rest.

Expected behavior

No response

Actual behavior

No response

How to Reproduce?

git clone git@github.com:jsmrcka/spring-data-rest-security-annotations-reproducer.git
mvn -f ./spring-data-rest-security-annotations-reproducer clean test

Output of uname -a or ver

No response

Output of java -version

No response

GraalVM version (if different from Java)

No response

Quarkus version or git rev

2.13.6.Final, 2.15.3.Final

Build tool (ie. output of mvnw --version or gradlew --version)

No response

Additional information

No response

@jsmrcka jsmrcka added the kind/bug Something isn't working label Jan 13, 2023
@quarkus-bot quarkus-bot bot added area/security area/spring Issues relating to the Spring integration labels Jan 13, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Jan 13, 2023

/cc @geoand (spring), @sberyozkin (security)

@jsmrcka
Copy link
Contributor Author

jsmrcka commented Jan 13, 2023

cc @Sgitario

Sgitario added a commit to Sgitario/quarkus that referenced this issue Jan 13, 2023
Before these changes, the security annotations were only propagated to the generated resource if and only if the method was annotated with either the `@RestResource` or `@RepositoryRestResource` annotation. 

In contract, the pure REST Data with Panache does not have this limitation (propagate these annotations regardless the annotation present). 

I've also updated the documentation to mention this feature.
Fix quarkusio#30358
@quarkus-bot quarkus-bot bot added this to the 2.17 - main milestone Jan 13, 2023
@gsmet gsmet modified the milestones: 2.17 - main, 2.16.0.Final Jan 17, 2023
gsmet pushed a commit to gsmet/quarkus that referenced this issue Jan 17, 2023
Before these changes, the security annotations were only propagated to the generated resource if and only if the method was annotated with either the `@RestResource` or `@RepositoryRestResource` annotation.

In contract, the pure REST Data with Panache does not have this limitation (propagate these annotations regardless the annotation present).

I've also updated the documentation to mention this feature.
Fix quarkusio#30358

(cherry picked from commit e1bdd70)
gsmet pushed a commit that referenced this issue Jan 19, 2023
Before these changes, the security annotations were only propagated to the generated resource if and only if the method was annotated with either the `@RestResource` or `@RepositoryRestResource` annotation.

In contract, the pure REST Data with Panache does not have this limitation (propagate these annotations regardless the annotation present).

I've also updated the documentation to mention this feature.
Fix #30358

(cherry picked from commit e1bdd70)
@gsmet gsmet modified the milestones: 2.16.0.Final, 2.13.7.Final Jan 19, 2023
ebullient pushed a commit to maxandersen/quarkus that referenced this issue Jan 24, 2023
Before these changes, the security annotations were only propagated to the generated resource if and only if the method was annotated with either the `@RestResource` or `@RepositoryRestResource` annotation. 

In contract, the pure REST Data with Panache does not have this limitation (propagate these annotations regardless the annotation present). 

I've also updated the documentation to mention this feature.
Fix quarkusio#30358
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security area/spring Issues relating to the Spring integration kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants