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

Mixing auth-mechanizms fails TestSecurity tests #38298

Closed
01epa opened this issue Jan 19, 2024 · 4 comments · Fixed by #40059
Closed

Mixing auth-mechanizms fails TestSecurity tests #38298

01epa opened this issue Jan 19, 2024 · 4 comments · Fixed by #40059
Labels
area/security kind/bug Something isn't working
Milestone

Comments

@01epa
Copy link

01epa commented Jan 19, 2024

Describe the bug

In case if security is mixed (basic+bearer) accordingly to quarkus doc I have to specify an authentication mechanizm

After that tests with @TestSecurity failed.

Looks like the problem is in the transport method - io.quarkus.test.security.TestHttpAuthenticationMechanism#getCredentialTransport since it does return null for tests and as a result findBestCandidateMechanism here returns baisic auth mechanizm instead of null

Expected behavior

Test passed

Actual behavior

Test failed

How to Reproduce?

To reproduce the issue just replace lines in application.properties file
from

%auth-failed-ex-mapper.quarkus.http.auth.permission.basic.paths=/*
%auth-failed-ex-mapper.quarkus.http.auth.permission.basic.policy=authenticated
%auth-failed-ex-mapper.quarkus.http.auth.proactive=false

to

quarkus.http.test-port=0
quarkus.http.auth.permission.basic.paths=/*
quarkus.http.auth.permission.basic.policy=authenticated
quarkus.http.auth.permission.basic.auth-mechanism=basic

and run test TestSecurityTestCase

If commented out line

quarkus.http.auth.permission.basic.auth-mechanism=basic

then everything will work.

The same problem is with bearer mechanizm.

Output of uname -a or ver

No response

Output of java -version

No response

Quarkus version or git rev

latest

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

No response

Additional information

No response

@01epa 01epa added the kind/bug Something isn't working label Jan 19, 2024
Copy link

quarkus-bot bot commented Jan 19, 2024

/cc @sberyozkin (security)

@sberyozkin sberyozkin changed the title Auth-mechanizm fails security tests Mixing auth-mechanizms fails security tests Jan 19, 2024
@sberyozkin
Copy link
Member

@01epa Thanks, I've just updated the issue name a bit to make the problem clearer

@sberyozkin sberyozkin changed the title Mixing auth-mechanizms fails security tests Mixing auth-mechanizms fails TestSecurity tests Jan 19, 2024
@sberyozkin
Copy link
Member

@01epa One more edit to make it even more clearer. Looks like fixing it will require adding a new TestSecurity attribute

@michalvavrik
Copy link
Member

The TestSecurity is not basic auth, so expectation that you select it with the quarkus.http.auth.permission.basic.auth-mechanism=basic is wrong. Problem is that currently, you can't select the TestSecurity authentication mechanism as you correctly pointed out.

The easiest thing that we can do is to allow to explicitly select test TestSecurity so that you can explicitly select other mechanisms for /*.

I actually run into same situation and here

quarkus.http.auth.permission.use-code-flow-by-default.paths=/web-app*,/web-app2*,/web-app3*,/tenant-autorefresh*,/tenant-https*,/tenant-logout*,/tenant-nonce*,/tenant-refresh*,/public-web-app*,/index.html,/,/tenant-cookie-path-header,/tenant-javascript
I had to list all non TestSecurity paths as we have no exclusions pattern on HTTP Perms (IMO not necessary).

I think io.quarkus.test.security.TestHttpAuthenticationMechanism should have io.quarkus.vertx.http.runtime.security.HttpCredentialTransport#authenticationScheme called test. Alternative proposal would be auth-mechanism=any, but it's bit more complicated. Thoughts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/security kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants