Fix #1127 - Use CredentialProvider in OAuth2 provider #1126

gabriel-farache · 2025-04-30T09:45:03Z

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

introduces tests for the Oauth2 provider (both Classic and Reactive depending on the delegate implementation used)
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 · 2025-04-30T10:01:20Z

@ricardozanini PTAL

...time/src/main/java/io/quarkiverse/openapi/generator/providers/ConfigCredentialsProvider.java

...nt/runtime/src/main/java/io/quarkiverse/openapi/generator/providers/CredentialsProvider.java

...t/runtime/src/main/java/io/quarkiverse/openapi/generator/providers/AbstractAuthProvider.java

...time/src/main/java/io/quarkiverse/openapi/generator/providers/ConfigCredentialsProvider.java

...nt/runtime/src/main/java/io/quarkiverse/openapi/generator/providers/CredentialsProvider.java

.../main/java/io/quarkiverse/openapi/generator/oidc/providers/OAuth2AuthenticationProvider.java

...time/src/main/java/io/quarkiverse/openapi/generator/providers/ConfigCredentialsProvider.java

gabriel-farache · 2025-05-13T09:28:05Z

@ricardozanini I had to use Quarkus test profile to load the custom credentials provider for the IT, now it works, PTAL again :)

ricardozanini

I'll take another look later.

client/integration-tests/auth-provider/src/main/resources/application.properties

ricardozanini

LGTM

gabriel-farache · 2025-05-14T08:22:46Z

@ricardozanini I added a data structure which embed all parameters that must be made available to the methods of the CredentialProvider.

I added lombok dependency to have the getter and builder generated, if that pose a problem, I will remove, let me know

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

ricardozanini · 2025-05-14T14:37:42Z

Hey @gabriel-farache many thanks! Yes, please remove lombok.

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

gabriel-farache · 2025-05-14T18:36:18Z

Hey @gabriel-farache many thanks! Yes, please remove lombok.

@ricardozanini done

...nt/runtime/src/main/java/io/quarkiverse/openapi/generator/providers/CredentialsProvider.java

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

ricardozanini · 2025-05-16T14:20:12Z

I'll ask for more eyes.

...uth-provider/src/test/java/io/quarkiverse/openapi/generator/it/auth/KeycloakServiceMock.java

...ovider/src/test/java/io/quarkiverse/openapi/generator/it/auth/TokenExternalServicesMock.java

...dc/src/test/java/io/quarkiverse/openapi/generator/oidc/OAuth2AuthenticationProviderTest.java

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

hbelmiro

Thank you @gabriel-farache.

...uth-provider/src/test/java/io/quarkiverse/openapi/generator/it/auth/KeycloakServiceMock.java

...ovider/src/test/java/io/quarkiverse/openapi/generator/it/auth/TokenExternalServicesMock.java

...est/java/io/quarkiverse/openapi/generator/it/auth/TokenWithCustomCredentialProviderTest.java

gmunozfe · 2025-05-20T16:48:31Z

...est/java/io/quarkiverse/openapi/generator/oidc/ReactiveOAuth2AuthenticationProviderTest.java

+        provider.filter(reactiveRequestContext);
+        assertHeader(headers, HttpHeaders.AUTHORIZATION, expectedAuthorizationHeader);


filter is a non-blocking method and then, immediately, you are checking the headers in the assertHeader.
Potentially header couldn't be there yet, making the test to be flaky.

Perhaps you can use CountDownLatch or other mechanism to wait until the filter logic finishes, and then assert the headers. Wdyt?

not sure how I could use a CountDownLatch as I do not want to add code just for testing purposes in the filter part handling the response
Plus, the getTokens method is mocked and should return instantly.

But I do agree, in rare case, there may be sync issue and the test may fail; to avoid that, I mocked the RestClientRequestContext that executes the suspend and resume methods which are used in the filter . THen I added a simple while-sleep loop to wait until the request is resumed. The loop may not be the most efficient solution, but for this case I believe it's enough as the impact will be very low, WDYT?

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

ricardozanini · 2025-05-26T21:44:53Z

@gmunozfe mind taking a look?

gabriel-farache requested a review from a team as a code owner April 30, 2025 09:45

gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch 6 times, most recently from 9cb6610 to 565cdd6 Compare April 30, 2025 09:57

hbelmiro added the backport-main-lts label Apr 30, 2025

ricardozanini self-assigned this Apr 30, 2025

ricardozanini added the area:client This item is related to the client extension label Apr 30, 2025

ricardozanini reviewed Apr 30, 2025

View reviewed changes

...time/src/main/java/io/quarkiverse/openapi/generator/providers/ConfigCredentialsProvider.java Outdated Show resolved Hide resolved

...time/src/main/java/io/quarkiverse/openapi/generator/providers/ConfigCredentialsProvider.java Outdated Show resolved Hide resolved

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 changed the title ~~Use CredentialProvider in OAuth2 provider~~ Fix #1127 - Use CredentialProvider in OAuth2 provider Apr 30, 2025

gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch 3 times, most recently from 9c48cc2 to a276dfa Compare April 30, 2025 19:07

github-advanced-security bot found potential problems Apr 30, 2025

View reviewed changes

ricardozanini reviewed Apr 30, 2025

View reviewed changes

gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch 5 times, most recently from 30d851b to 8483233 Compare April 30, 2025 21:50

github-advanced-security bot found potential problems Apr 30, 2025

View reviewed changes

...nt/runtime/src/main/java/io/quarkiverse/openapi/generator/providers/CredentialsProvider.java Fixed Show fixed Hide fixed

...nt/runtime/src/main/java/io/quarkiverse/openapi/generator/providers/CredentialsProvider.java Fixed Show fixed Hide fixed

gabriel-farache requested a review from ricardozanini May 5, 2025 07:09

ricardozanini reviewed May 5, 2025

View reviewed changes

.../main/java/io/quarkiverse/openapi/generator/oidc/providers/OAuth2AuthenticationProvider.java Show resolved Hide resolved

...time/src/main/java/io/quarkiverse/openapi/generator/providers/ConfigCredentialsProvider.java Outdated Show resolved Hide resolved

gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch from 0f1e570 to 6df6a23 Compare May 5, 2025 13:52

gabriel-farache requested a review from ricardozanini May 7, 2025 06:52

gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch 2 times, most recently from a225e19 to 23a4790 Compare May 13, 2025 09:26

ricardozanini reviewed May 13, 2025

View reviewed changes

client/integration-tests/auth-provider/src/main/resources/application.properties Outdated Show resolved Hide resolved

gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch from 23a4790 to 04c31a4 Compare May 13, 2025 13:46

ricardozanini approved these changes May 13, 2025

View reviewed changes

gabriel-farache added 3 commits May 14, 2025 10:27

Add unit tests for OAUTH2 provider

f468379

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

Use CredentialProvider in OAUTH2 provider

20e31c2

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

Add integration tests for custom credential provider

185ebd5

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

gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch from 898fb3b to 6f7ddc0 Compare May 14, 2025 08:27

Add data structure as input parameter for CredentialProvider

d52de6a

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

gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch from 6f7ddc0 to d52de6a Compare May 14, 2025 18:35

ricardozanini reviewed May 14, 2025

View reviewed changes

...nt/runtime/src/main/java/io/quarkiverse/openapi/generator/providers/CredentialsProvider.java Outdated Show resolved Hide resolved

Rename data structure for CredentialProvider

eb73606

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

ricardozanini approved these changes May 16, 2025

View reviewed changes

hbelmiro linked an issue May 17, 2025 that may be closed by this pull request

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

Open

hbelmiro reviewed May 17, 2025

View reviewed changes

Apply feedback

425df44

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

hbelmiro approved these changes May 19, 2025

View reviewed changes

gmunozfe reviewed May 20, 2025

View reviewed changes

gabriel-farache added 2 commits May 21, 2025 16:56

Apply feedback related to sync

06eca21

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

Fix formatting

fcbdbf0

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

gabriel-farache force-pushed the fix/oauth2_provider_credentialProvider branch from 11c0b03 to fcbdbf0 Compare May 21, 2025 15:05

gabriel-farache mentioned this pull request May 22, 2025

Custom CredentialProvider class not loaded #1156

Open

gabriel-farache requested a review from gmunozfe May 26, 2025 07:38

		provider.filter(reactiveRequestContext);
		assertHeader(headers, HttpHeaders.AUTHORIZATION, expectedAuthorizationHeader);

Fix #1127 - Use CredentialProvider in OAuth2 provider #1126

Are you sure you want to change the base?

Fix #1127 - Use CredentialProvider in OAuth2 provider #1126

Uh oh!

Conversation

gabriel-farache commented Apr 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gabriel-farache commented Apr 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gabriel-farache commented May 13, 2025

Uh oh!

ricardozanini left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ricardozanini left a comment

Choose a reason for hiding this comment

Uh oh!

gabriel-farache commented May 14, 2025

Uh oh!

ricardozanini commented May 14, 2025

Uh oh!

gabriel-farache commented May 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

ricardozanini commented May 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

hbelmiro left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gmunozfe May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gabriel-farache May 21, 2025

Choose a reason for hiding this comment

Uh oh!

ricardozanini commented May 26, 2025

Uh oh!

Uh oh!

gabriel-farache commented Apr 30, 2025 •

edited

Loading

gabriel-farache commented May 14, 2025 •

edited

Loading

gmunozfe May 20, 2025 •

edited

Loading