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

Enhance TestSecurity OIDC module to support the injection of UserInfo and OidcConfigurationMetadata #16697

Closed
sberyozkin opened this issue Apr 21, 2021 · 5 comments · Fixed by #17075
Assignees
Milestone

Comments

@sberyozkin
Copy link
Member

Description

OidcConfigurationMetadata can be initialized from quarkus.oidc.jwk-path, etc
UserInfo from a security attribute with a userinfo. prefix

@sberyozkin sberyozkin added the kind/enhancement New feature or request label Apr 21, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Apr 21, 2021

/cc @manovotn, @mkouba, @pedroigor

@quarkus-bot quarkus-bot bot added area/arc Issue related to ARC (dependency injection) area/oidc area/security labels Apr 21, 2021
@sberyozkin sberyozkin self-assigned this Apr 21, 2021
@Marcus-Biel-idnow
Copy link

go go go :)

@hantsy
Copy link
Contributor

hantsy commented Apr 25, 2021

@sberyozkin

It is not only the issue of the test-security and oidc used together.

I tried it more today in my sample application, https://github.com/hantsy/quarkus-sandbox/tree/master/oidc-api-auth0

This is a service application(the default application type is service).

I have an AudienceValidator class to validate the aud claim, see here.

If it is activated, and starts the application, then I used a client to test the APIs, see IntegrationTests, it will raise the OidcConfigurationMetadata can not be injected exceptions. (When disabling the AudienceValidator, it works)

So the final solution is in a service type application, OidcConfigurationMetadata should be provided in CDI context.

@hantsy
Copy link
Contributor

hantsy commented Apr 25, 2021

@sberyozkin BTW, not sure it can be categorized as an issue.

When the quarkus.oidc.client-id is not set, it will throw an exception: Tenant configuration can not be resolved.

As I understand it, a service type application or the Resource server role in OAuth2 protocol, it should not contain a client-id.

The client-id and client-secrets are part of the client application when an Oauth2 Client application role is registered.

I also create a similar service API spring application, just need to declare it as a ResourceServer and specify the issuer-uri configuration, no need others, all work well, see: https://github.com/hantsy/spring-webmvc-auth0-sample/blob/master/src/main/resources/application.yml#L49

@mkouba mkouba removed the area/arc Issue related to ARC (dependency injection) label Apr 26, 2021
@sberyozkin
Copy link
Member Author

sberyozkin commented May 5, 2021

Wow, I totally missed all the comments :-) (the secret is I only check the issues manually to minimize the amount of notifications), anyway, sorry for a delay...

@Marcus-Biel-idnow - OK :-), will try to prioritize soon enough, should be straightforward enough to fix

@hantsy

OidcConfigurationMetadata can not be injected exceptions

The problem is that if the path request is public then OidcConfigurationMetadata can not be injected - but it has just been fixed as part of this #16586. It will be injected if the path is protected.

As I understand it, a service type application or the Resource server role in OAuth2 protocol, it should not contain a client-id.

It has been discussed before - it is not a good idea to drop it (even if somewhere else it is not needed) since, when the token verification fails, all that can be logged is that the token fails - no way to link it to the client which requested this token - we don't log it yet - but I'll make sure the client id is logged - additionally, quarkus-oidc will do the remote token introspection for opaque tokens but also optionally for JWT tokens - where a client id is required.
We should probably do not require it if the token introspection is disabled: #17021

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

Successfully merging a pull request may close this issue.

4 participants