quarkusio / quarkus

Quarkus: Supersonic Subatomic Java.
https://quarkus.io
Apache License 2.0
13.54k stars 2.61k forks source link

TestSecurity behaviour for OIDC extension's local logout changed since 3.11.0.CR1 #41125

Closed cflee closed 2 months ago

cflee commented 3 months ago

Describe the bug

Behaviour of local logout in the Quarkus OIDC extension under @TestSecurity annotation conditions has changed. The test accessing an endpoint that performs a local logout no longer clears the q_session and related cookies, while in practice when the endpoint is invoked externally in dev mode the response does still clear the cookies as expected.

I have confirmed that this is still present against a snapshot build of the current main branch (git rev below). It does not occur with version 3.10.2. I have bisected this change to the commit fcdebde4e131fad3c6ef56503ea78bcb6e175dc4, the merge of PR #40059. Looking at the PR, I don't see any obvious reason why this test should start failing, so I am reporting this as a bug.

Expected behavior

The HTTP response from the local logout endpoint test will clear the q_session and related cookies.

Actual behavior

The HTTP response from the local logout endpoint test does not clear the q_session and related cookies.

How to Reproduce?

Reproducer:

  1. Use the reproducer at https://github.com/cflee/quarkus-reproducer-1/commit/81c5317ffb652be98b93facbda86e6b524bd3f2e which is set to use Quarkus 3.11.1
  2. Run ./mvnw test, it should fail
  3. Modify the quarkus.platform.version property in pom.xml to be 3.10.2
  4. Run ./mvnw test, it should now pass

The test failure message with 3.11.1:

[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0, Time elapsed: 22.54 s <<< FAILURE! -- in org.acme.AuthControllerTest
[ERROR] org.acme.AuthControllerTest.testLogout_authenticated_deletesCookiesSuccessfully -- Time elapsed: 0.582 s <<< FAILURE!
org.opentest4j.AssertionFailedError: Targeted cookie "q_session" was not found in headers ==> expected: <true> but was: <false>

Output of uname -a or ver

Darwin cf-lee-tp92p 23.5.0 Darwin Kernel Version 23.5.0: Wed May 1 20:14:38 PDT 2024; root:xnu-10063.121.3~5/RELEASE_ARM64_T6020 arm64

Output of java -version

openjdk version "21" 2023-09-19
OpenJDK Runtime Environment GraalVM CE 21+35.1 (build 21+35-jvmci-23.1-b15)
OpenJDK 64-Bit Server VM GraalVM CE 21+35.1 (build 21+35-jvmci-23.1-b15, mixed mode, sharing)

Quarkus version or git rev

c29652139610785174dd712bbb410d31a8765ba4

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

------------------------------------------------------------
Gradle 8.5
------------------------------------------------------------

Build time:   2023-11-29 14:08:57 UTC
Revision:     28aca86a7180baa17117e0e5ba01d8ea9feca598

Kotlin:       1.9.20
Groovy:       3.0.17
Ant:          Apache Ant(TM) version 1.10.13 compiled on January 4 2023
JVM:          21 (GraalVM Community 21+35-jvmci-23.1-b15)
OS:           Mac OS X 14.5 aarch64

Additional information

No response

quarkus-bot[bot] commented 3 months ago

/cc @geoand (kotlin), @pedroigor (oidc), @sberyozkin (oidc,security)

sberyozkin commented 3 months ago

I guess this is related to the TEST_SECURITY credential type and now the OIDC mechanism which can clear out the cookie is skipped, but is found if the test security mechanism credential type is null. @michalvavrik, I wonder if it is possible to workaround it somehow...

michalvavrik commented 3 months ago

I guess this is related to the TEST_SECURITY credential type and now the OIDC mechanism which can clear out the cookie is skipped, but is found if the test security mechanism credential type is null. @michalvavrik, I wonder if it is possible to workaround it somehow...

the issue has very nice reproducer and analysis, thanks @cflee , I'll have a look tomorrow

michalvavrik commented 2 months ago

just to clarify actual behavior note is not spot on

The HTTP response from the local logout endpoint test does not clear the q_session and related cookies.

there are no q_session cookie or others now, while previously with 3.10.2 response cookies were like

response cookies {q_auth_49a1d34d-ab0b-4e36-82d5-54fab57e19d8=49a1d34d-ab0b-4e36-82d5-54fab57e19d8, q_session=, q_session_at=, q_session_rt=}

I am not all that sure how they are useful if you are mocking security, but I'll try to fix it as it probably goes down to the mechanism priority; I'll get back to you when I know more or have a fix.

michalvavrik commented 2 months ago

Actually, I know how to fix it as I considered this when I was writing #40059, but I need to do little research to what is proper behavior and write tests to make sure assumptions are correct.