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 RestClient Reactive to support registering providers via custom annotations #16522

Closed
sberyozkin opened this issue Apr 14, 2021 · 13 comments · Fixed by #28451
Closed

Comments

@sberyozkin
Copy link
Member

sberyozkin commented Apr 14, 2021

Description

The following is possible with quarkus-rest-client, for example, with OidcClientRequestFilter

@Inject
@OidcClientFilter
MyMpRestClient client;

where the quarkus-oidc-client-filter extension registers OidcClientFilter annotation and OidcClientRequestFilter.class pair, and where quarkus-rest-client's RestClientProcessor discovers such registrations and passes them to RestClientBase.

Note as far as OidcClientRequestFilter is concerned, the alternative solution (explicitly register it with MP RestClient RegisterProvider) is available. This issue is not about OidcClientRequestFilter but about a generic support for registering providers via custom annotations.

Implementation ideas

Do something similar in the reactive rest client to what is done in quarkus-rest-client's RestClientProcessor and RestClientBase.

@sberyozkin sberyozkin added the kind/enhancement New feature or request label Apr 14, 2021
@geoand
Copy link
Contributor

geoand commented Mar 9, 2022

cc @michalszynkiewicz

@quarkus-bot
Copy link

quarkus-bot bot commented Jul 8, 2022

/cc @michalszynkiewicz

@rsvoboda
Copy link
Member

I'm looking into migration of an app that is using quarkus-oidc-client-filter and quarkus-oidc-token-propagation to their reactive variants

Change of extensions doesn't work, code doesn't compile as it is using io.quarkus.oidc.client.filter.OidcClientFilter and io.quarkus.oidc.token.propagation.AccessToken. Those classes are not available in quarkus-oidc-client-reactive-filter and quarkus-oidc-token-propagation-reactive.

Unfortunately it is not possible for RESTEasy Reactive because of this issue.
So with the new reactive options one needs to reg the providers directly for now: https://quarkus.io/guides/security-openid-connect-client-reference#oidc-client-reactive-filter and https://quarkus.io/guides/security-openid-connect-client-reference#reactive-token-propagation

This makes transition to quarkus-oidc-client-reactive-filter and quarkus-oidc-token-propagation-reactive harder / damages user experience a bit.

@rsvoboda
Copy link
Member

/cc @geoand @Sgitario

@geoand
Copy link
Contributor

geoand commented Sep 15, 2022

@sberyozkin what's your take on #16522 (comment)? Is there something we need to do to make the experience better? Or maybe just add some more documentation?

@sberyozkin
Copy link
Member Author

sberyozkin commented Oct 7, 2022

Hey @geoand, apologies, I've just discovered your comment after following on the related comments from Rostislav in QUARKUS-2389.

I think that if RestEasy Reactive could allow supporting the same style of registering OIDC Reactive client filter as the one allowed in RestEasy Classic for OIDC client,

@Inject
@OidcClientFilter
MyMpRestClient client;

in addition to the standard

@Inject
@RegisterProvider(OidcClientRequestReactiveFilter.class)
MyMpRestClient client;

then it would indeed be a bit nicer and as @rsvoboda has indicated, would not even require the code changes during the migration to Resteasy Reactive which Rostislav is quite concerned about.

Have a look at the description please - I've provided links to the RestEasy Classic code showing how @OidcClientFilter is recognized and processed to get (classic) OidcClientFilter registered, I can try to do pretty much the same but in RestEasy Reactive, but I'd appreciate some help. May be Jose can be interested as well ?

thanks

@geoand
Copy link
Contributor

geoand commented Oct 7, 2022

Thanks @sberyozkin, I'll have a look next week.

@sberyozkin
Copy link
Member Author

sberyozkin commented Oct 7, 2022

For example, oidc-client-filter registers a pair of @OidcClientFilter and the associated provider class name here, and then RestEasy Classic checks the list of these build items and if a given REST Client has a matching annotation then the associated provider is added, so it should be simple enough I guess to do the same with RestEasy Reactive :-), and then once this is in place, the users can have their custom extensions provide such annotation shortcuts for their providers as well

@sberyozkin
Copy link
Member Author

@geoand Np at all, yeah, lets see if it can be done without too many problems, ping me please if you'd like me to help somehow, cheers

@geoand
Copy link
Contributor

geoand commented Oct 7, 2022

👍🏼

@geoand
Copy link
Contributor

geoand commented Oct 7, 2022

@sberyozkin can you take a look at #28451?

It should cover this

geoand added a commit to geoand/quarkus that referenced this issue Oct 7, 2022
@sberyozkin
Copy link
Member Author

Hey @geoand what a good Friday, sorry, had to step outside for a very late lunch :-)

@geoand
Copy link
Contributor

geoand commented Oct 7, 2022

No problem at all :)

geoand added a commit to geoand/quarkus that referenced this issue Oct 7, 2022
sberyozkin added a commit that referenced this issue Oct 10, 2022
Introduce a version of @OidcClientFilter for the reactive client
@quarkus-bot quarkus-bot bot added this to the 2.14 - main milestone Oct 10, 2022
@gsmet gsmet modified the milestones: 2.14 - main, 2.13.2.Final Oct 10, 2022
gsmet pushed a commit to gsmet/quarkus that referenced this issue Oct 10, 2022
igorregis pushed a commit to igorregis/quarkus that referenced this issue Oct 16, 2022
igorregis pushed a commit to igorregis/quarkus that referenced this issue Oct 16, 2022
igorregis pushed a commit to igorregis/quarkus that referenced this issue Oct 16, 2022
igorregis pushed a commit to igorregis/quarkus that referenced this issue Oct 17, 2022
tmihalac pushed a commit to tmihalac/quarkus that referenced this issue Oct 27, 2022
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