Skip to content

Conversation

@gabriel-farache
Copy link
Contributor

@gabriel-farache gabriel-farache commented Apr 30, 2025

Many thanks for submitting your Pull Request ❤️!

Please make sure that your PR meets the following requirements:

  • You have read the contributors guide
  • Your code is properly formatted according to our code style
  • Pull Request title contains the target branch if not targeting main: [0.9.x] Subject
  • Pull Request contains link to the issue
  • Pull Request contains link to any dependent or related Pull Request
  • Pull Request contains description of the issue
  • Pull Request does not include fixes for issues other than the main ticket

This PR solves #1127

  1. introduces tests for the Oauth2 provider (both Classic and Reactive depending on the delegate implementation used)
  2. make use of the newly introduced CredentialProvider interface instance when setting the token in the header
  • the behaviour is not a get as for the other providers but a set as the access token is first generated by the delegate and then set in the header. The reactive delegate and its Mutiny (async) usage force us to have all in one (getting the token and setting the header) to avoid losing the benefit of releasing the thread while waiting for the token. If we are fine with losing it, a getToken method can instead be defined in the OidcClientRequestFilterDelegate interface and then use it in a CredentialProvider get method for oauth2 instead of calling the filter from the delegate

@gabriel-farache gabriel-farache requested a review from a team as a code owner April 30, 2025 09:45
@gabriel-farache gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch 6 times, most recently from 9cb6610 to 565cdd6 Compare April 30, 2025 09:57
@gabriel-farache
Copy link
Contributor Author

@ricardozanini PTAL

@ricardozanini ricardozanini self-assigned this Apr 30, 2025
@ricardozanini ricardozanini added the area:client This item is related to the client extension label Apr 30, 2025
@gabriel-farache gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch 2 times, most recently from d0c6537 to aad13a7 Compare April 30, 2025 18:54
@gabriel-farache gabriel-farache changed the title Use CredentialProvider in OAuth2 provider Fix #1127 - Use CredentialProvider in OAuth2 provider Apr 30, 2025
@gabriel-farache gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch 3 times, most recently from 9c48cc2 to a276dfa Compare April 30, 2025 19:07
@gabriel-farache gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch 5 times, most recently from 30d851b to 8483233 Compare April 30, 2025 21:50
@gabriel-farache gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch from 0f1e570 to 6df6a23 Compare May 5, 2025 13:52
Copy link
Contributor

@hbelmiro hbelmiro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @gabriel-farache.

@ricardozanini
Copy link
Member

@gmunozfe mind taking a look?

@gabriel-farache gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch 2 times, most recently from 01ff9ab to aaf4919 Compare June 5, 2025 13:36
@gabriel-farache
Copy link
Contributor Author

@ricardozanini should this part of the doc https://github.com/quarkiverse/quarkus-openapi-generator/blob/main/docs/modules/ROOT/pages/includes/custom-auth-provider.adoc?plain=1#L30 be updated? In our IT the priority is higher than the default one and it appears that our custom classes are used instead of the default one

@gabriel-farache gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch from 643f843 to bbd3c5f Compare June 5, 2025 19:15
@ricardozanini
Copy link
Member

@ricardozanini should this part of the doc https://github.com/quarkiverse/quarkus-openapi-generator/blob/main/docs/modules/ROOT/pages/includes/custom-auth-provider.adoc?plain=1#L30 be updated? In our IT the priority is higher than the default one and it appears that our custom classes are used instead of the default one

Yes, the docs are wrong. I forgot to update on my PR. Can you update it here?

Signed-off-by: gabriel-farache <[email protected]>
@gabriel-farache
Copy link
Contributor Author

@ricardozanini should this part of the doc https://github.com/quarkiverse/quarkus-openapi-generator/blob/main/docs/modules/ROOT/pages/includes/custom-auth-provider.adoc?plain=1#L30 be updated? In our IT the priority is higher than the default one and it appears that our custom classes are used instead of the default one

Yes, the docs are wrong. I forgot to update on my PR. Can you update it here?

done

@ricardozanini ricardozanini merged commit 8a2fcb3 into quarkiverse:main Jun 6, 2025
11 checks passed
Copy link
Member

@ricardozanini ricardozanini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gabriel-farache can you please follow up a new PR with my late considerations? I'm sorry, I've merged too soon.

gabriel-farache added a commit to gabriel-farache/quarkus-openapi-generator that referenced this pull request Jun 6, 2025
@gabriel-farache gabriel-farache deleted the fix/oauth2_provider_credentialProvider branch June 6, 2025 13:35
gabriel-farache added a commit to gabriel-farache/quarkus-openapi-generator that referenced this pull request Jun 6, 2025
gabriel-farache added a commit to gabriel-farache/quarkus-openapi-generator that referenced this pull request Jun 6, 2025
gabriel-farache added a commit to gabriel-farache/quarkus-openapi-generator that referenced this pull request Jun 10, 2025
ricardozanini pushed a commit that referenced this pull request Jun 10, 2025
Signed-off-by: gabriel-farache <[email protected]>
ricardozanini pushed a commit to ricardozanini/quarkus-openapi-generator that referenced this pull request Jun 18, 2025
…rkiverse#1126)

* Use CredentialProvider in OAUTH2 provider and update data structure for
CrendentialProvider

Signed-off-by: gabriel-farache <[email protected]>

* Fix Custom auth provider doc

Signed-off-by: gabriel-farache <[email protected]>

---------

Signed-off-by: gabriel-farache <[email protected]>
ricardozanini pushed a commit to ricardozanini/quarkus-openapi-generator that referenced this pull request Jun 18, 2025
fjtirado added a commit that referenced this pull request Jun 19, 2025
[main-lts] Manually cherry-pick PRs since #1126 (CredentialsProvider feat)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:client This item is related to the client extension backport-main-lts

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OAuth2 provider does not use the CrendentialProvider to get/set the token

4 participants