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

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

Closed
cflee opened this issue Jun 11, 2024 · 5 comments · Fixed by #41174
Closed

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

cflee opened this issue Jun 11, 2024 · 5 comments · Fixed by #41174
Assignees
Milestone

Comments

@cflee
Copy link

cflee commented Jun 11, 2024

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 fcdebde, 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 cflee/quarkus-reproducer-1@81c5317 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

c296521

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

Copy link

quarkus-bot bot commented Jun 11, 2024

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

@sberyozkin
Copy link
Member

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
Copy link
Member

michalvavrik commented Jun 11, 2024

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 michalvavrik self-assigned this Jun 11, 2024
@michalvavrik
Copy link
Member

michalvavrik commented Jun 12, 2024

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
Copy link
Member

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants