Fix HTTP Security policies when OIDC tenant is selected with the @Tenant annotation on Jakarta REST resources

[michalvavrik]

• Fixes: OIDC multi-tenancy @Tenant annotation not selecting tenant #37485
• Unblocks Allow authentication mechanism selection for a REST endpoint with annotation #36504

We are in the difficult situation with HTTP Security policies as we need to run authentication and authorization as the first thing, but when the policies are used with the Jakarta REST stack and authentication feature that require endpoint match is used, we would need to postpone authN/authZ to when RESTEasy Reactive starts processing. That can be done, but there can (and are) filters with higher priority that can preceed RR processing and needs to be secured as well. Just imagine that request is consumed by gRPC but we postponed check to JAX-RS phase. That is why for now, this PR propose explicitly marking HTPT Security policies that are only applied on JAX-RS. This feature is closely couple with use cases - it will work for @Tenant and for other annotations resolved to EagerSecurityInterceptors, but it doesn't make sense and is not supported to just delay it for no reason (in short: we need build time detection of annotations to know we should add respective REST handlers).

[github-actions]

🙈 The PR is closed and the preview is expired.

[sberyozkin]

Thanks @michalvavrik, I did look earlier, and thought it was good, but I'll need to go through it again, thanks

[michalvavrik]

Thanks @michalvavrik, I did look earlier, and thought it was good, but I'll need to go through it again, thanks

If you have any questions, please shoot. Personally, I'd suggest area covered by EagerSecurityHandler and EagerSecurityFilter has already had good test coverage before this PR, therefore I have confidence I'm not breaking anything. Please take your time.

[gsmet]

Probably a good idea to have @geoand having a look.

[michalvavrik]

Probably a good idea to have @geoand having a look.

@geoand please have a look if you find a time

[geoand]

Sure, I'll have a look tomorrow

…

On Wed, Feb 14, 2024, 20:48 Michal Vavřík ***@***.***> wrote: Probably a good idea to have @geoand <https://github.com/geoand> having a look. @geoand <https://github.com/geoand> please have a look if you find a time — Reply to this email directly, view it on GitHub <#38772 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABBMDP7QCKBMDV4E75LGRH3YTUBIHAVCNFSM6AAAAABDHLST5OVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNBUGQYDGMZZGI> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[geoand]

RR part LGTM

[michalvavrik]

RR part LGTM

Thanks for the review

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 81386fd.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

⚠️ There are other check runs running, make sure you don't need to wait for their status before merging.

[sberyozkin]

Thanks @michalvavrik Let me do one more round in the next few days, enjoy the weekend.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 81386fd.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

---

Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 extensions/smallrye-health/deployment

✖ io.quarkus.smallrye.health.test.HealthCheckProducerDefaultScopeTest.testHealth - History

• `3 expectations failed.
  JSON path status doesn't match.
  Expected: is "UP"
  Actual: DOWN

JSON path checks.status doesn't match.
Expected: iterable containing ["UP", "UP"]
Actual: <[DOWN, UP]>

JSON path checks.name doesn't match.
Expected: iterable with items ["alpha1", "bravo1"] in any order
Actual: <[org.eclipse.microprofile.health.HealthCheckProducerDefaultScopeTest_HealthCheckProducers_ProducerMethod_bravo_HmeFAUq6LfthXl21HTUmJPTZoCA_ClientProxy, alpha1]>
` - java.lang.AssertionError

java.lang.AssertionError: 
3 expectations failed.
JSON path status doesn't match.
Expected: is "UP"
  Actual: DOWN

JSON path checks.status doesn't match.
Expected: iterable containing ["UP", "UP"]

📦 extensions/smallrye-reactive-messaging-kafka/deployment

✖ io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase.sseStream - History

• Assertion condition Expecting size of: [] to be greater than or equal to 2 but was 0 within 10 seconds. - org.awaitility.core.ConditionTimeoutException

org.awaitility.core.ConditionTimeoutException: 
Assertion condition 
Expecting size of:
  []
to be greater than or equal to 2 but was 0 within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)

📦 integration-tests/opentelemetry

✖ io.quarkus.it.opentelemetry.EndUserEnabledTest.baseTest - History

• AttributesMap{data={http.route=/otel/enduser, code.namespace=io.quarkus.it.opentelemetry.util.EndUserResource, http.method=GET, http.scheme=http, net.protocol.name=http, http.target=/otel/enduser, net.host.port=8081, code.function=dummy, http.response_content_length=0, net.host.name=localhost, user_agent.original=Apache-HttpClient/4.5.14 (Java/21.0.2), http.status_code=200, http.client_ip=127.0.0.1}, capacity=128, totalAddedValues=13} ==> expected: <testUser> but was: <null> - org.opentest4j.AssertionFailedError

org.opentest4j.AssertionFailedError: AttributesMap{data={http.route=/otel/enduser, code.namespace=io.quarkus.it.opentelemetry.util.EndUserResource, http.method=GET, http.scheme=http, net.protocol.name=http, http.target=/otel/enduser, net.host.port=8081, code.function=dummy, http.response_content_length=0, net.host.name=localhost, user_agent.original=Apache-HttpClient/4.5.14 (Java/21.0.2), http.status_code=200, http.client_ip=127.0.0.1}, capacity=128, totalAddedValues=13} ==> expected: <testUser> but was: <null>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
	at org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
	at org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1156)
	at io.quarkus.it.opentelemetry.EndUserEnabledTest.evaluateAttributes(...

[sberyozkin]

@michalvavrik I've looked again, I believe a good part of this PR is about the general refactoring as well, the code related to checking if the JAXRS permission checker should be applied or not appears to be quite simple, thanks