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

OIDC AccessToken can lead to AmbiguousResolutionException #36994

Closed
mocenas opened this issue Nov 10, 2023 · 7 comments · Fixed by #39132
Closed

OIDC AccessToken can lead to AmbiguousResolutionException #36994

mocenas opened this issue Nov 10, 2023 · 7 comments · Fixed by #39132
Labels
area/oidc kind/bug Something isn't working
Milestone

Comments

@mocenas
Copy link
Contributor

mocenas commented Nov 10, 2023

Describe the bug

Using io.quarkus.oidc.token.propagation.AccessToken annotation on a OIDC client leads to AmbiguousResolutionException if any custom AccessTokenRequestFilter is defined. example of it's definition

Exception is: jakarta.enterprise.inject.AmbiguousResolutionException: Beans: [CLASS bean [class=io.quarkus.ts.security.keycloak.oidcclient.extended.restclient.ping.filters.CustomTokenRequestFilter, id=oJQuCeYWIxp6j-Oe322IFp9iDbk], CLASS bean [class=io.quarkus.oidc.token.propagation.AccessTokenRequestFilter, id=uT_pr1MaC1nPZjXJ_ZSEFBRfYeY]]

This exception occurs, when I try to inject the client.

Expected behavior

Quarkus will inject the client without problem. Annotation @accesstoken should work, even if custom filters are defined.

Actual behavior

Injecting client will fail, because dependency injection can't resolve the filter class.

How to Reproduce?

  1. Define any client with @accesstoken annotation. e.g.
@RegisterRestClient
@AcessToken
public interface TokenClient{}
  1. Define custom AccessTokenRequestFilter, e.g.
public class CustomAccessTokenRequestFilter extends AccessTokenRequestFilter{}
  1. Try to inject the client into a http resource, e.g.
@Inject
@RestClient
TokenClient tokenClient;
  1. Try to call the client.

Output of uname -a or ver

No response

Output of java -version

openjdk 11.0.19

Quarkus version or git rev

999-SNAPSHOT

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

No response

Additional information

No response

@mocenas mocenas added the kind/bug Something isn't working label Nov 10, 2023
@quarkus-bot
Copy link

quarkus-bot bot commented Nov 10, 2023

/cc @pedroigor (oidc), @sberyozkin (oidc)

@sberyozkin
Copy link
Member

@mocenas I think this issue is invalid. @AccessToken registers the default filter, while you add a custom filter which should be registered with a register provider annotation instead

@mocenas
Copy link
Contributor Author

mocenas commented Nov 10, 2023

@sberyozkin I think there is a misunderstanding.
@RegisterProvider annotation works as expected. Problem is, that once I define custom filter, and I want to use @AccessToken annotation elsewhere (independently on my custom filter), this annotation fails on ambiguous resolution exception.

Workaround is to define other "defaultFilter" (without any customization) filter. And to use @RegisterProvider(Defaultfilter.class).

Even using annotation @RegisterProvider(AccessTokenRequestFilter.class) fail on ambiguous exception, once custom filter is defined.

@sberyozkin
Copy link
Member

sberyozkin commented Nov 10, 2023

@mocenas

@RegisterProvider annotation works as expected. Problem is, that once I define custom filter, and I want to use @accesstoken annotation elsewhere (independently on my custom filter), this annotation fails on ambiguous resolution exception.

Can you please explain why this exception is reported with this combination ? CC @michalvavrik

@michalvavrik
Copy link
Contributor

@sberyozkin I didn't have a time to investigate when @mocenas wrote about it in QE TS PR, but if it's not a bug, it is something that can be solved anyway. Why should users not be able to use @AccessToken or @RegisterProvider(AccessTokenRequestFilter.class) if they need to use custom client as well? At least we can be more user friendly and set qualifiers or byte-generate class for them to avoid this. I can take care of it. Shout if you still think it's not a bug.

@mocenas
Copy link
Contributor Author

mocenas commented Nov 10, 2023

@mocenas

@RegisterProvider annotation works as expected. Problem is, that once I define custom filter, and I want to use @accesstoken annotation elsewhere (independently on my custom filter), this annotation fails on ambiguous resolution exception.

Can you please explain why this exception is reported with this combination ? CC @michalvavrik

I understand this is probably not a common scenario. But still IMHO mere definition of custom filter should not break @AccessToken or make it impossible to use @RegisterProvider(AccessTokenRequestFilter.class). I agree on this with @michalvavrik that this could me more user friendly.

@sberyozkin
Copy link
Member

@michalvavrik Thanks,

At least we can be more user friendly and set qualifiers or byte-generate class for them to avoid this. I can take care of it.

Please add it to your queue, I agree it is a valid test case scenario, but I'm not too worried about it from the practical point of view as I'm not sure what kind of scenario it covers.

@quarkus-bot quarkus-bot bot added this to the 3.9 - main milestone Mar 4, 2024
@gsmet gsmet modified the milestones: 3.9.0.CR1, 3.8.5 May 15, 2024
jedla97 added a commit to jedla97/quarkus-test-suite that referenced this issue May 30, 2024
jedla97 added a commit to jedla97/quarkus-test-suite that referenced this issue May 30, 2024
jedla97 added a commit to jedla97/quarkus-test-suite that referenced this issue May 30, 2024
jedla97 added a commit to jedla97/quarkus-test-suite that referenced this issue May 31, 2024
jedla97 added a commit to jedla97/quarkus-test-suite that referenced this issue Jun 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/oidc kind/bug Something isn't working
Projects
None yet
4 participants