Expose auth policy resolution utilities for framework integrations#1512
Merged
Conversation
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR exposes small, reusable auth-policy utilities to support framework integrations (per #1511), by extracting policy resolution and OAuth2/OIDC unwrapping logic from DefaultAuthProviderFactory into public, stateless helpers.
Changes:
- Added
DefaultAuthProviderFactory.resolvePolicy(Workflow, ReferenceableAuthenticationPolicy)to resolve inline vs referenced auth policies. - Introduced public
OAuthPolicyDatarecord withfrom(AuthenticationPolicyUnion)to normalize OAuth2 vs OIDC extraction. - Refactored OAuth provider construction to use
OAuthPolicyData, and added focused unit tests for both helpers.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| impl/core/src/main/java/io/serverlessworkflow/impl/auth/DefaultAuthProviderFactory.java | Adds resolvePolicy and refactors auth provider creation to use OAuthPolicyData. |
| impl/core/src/main/java/io/serverlessworkflow/impl/auth/OAuthPolicyData.java | New public record providing a single extraction path for OAuth2/OIDC policy data. |
| impl/core/src/main/java/io/serverlessworkflow/impl/auth/OAuth2AuthProvider.java | Switches constructor input from policy type to OAuthPolicyData. |
| impl/core/src/main/java/io/serverlessworkflow/impl/auth/OpenIdAuthProvider.java | Switches constructor input from policy type to OAuthPolicyData. |
| impl/test/src/test/java/io/serverlessworkflow/impl/test/ResolvePolicyTest.java | Adds tests covering inline vs reference resolution and null cases. |
| impl/test/src/test/java/io/serverlessworkflow/impl/test/OAuthPolicyDataTest.java | Adds tests covering OAuth2/OIDC extraction (inline vs secret) and non-OAuth inputs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
fjtirado
reviewed
Jul 2, 2026
Comment on lines
+49
to
+50
| if (oidc != null && oidc.getOidc() != null) { | ||
| OpenIdConnectAuthenticationPolicyConfiguration config = oidc.getOidc(); |
Collaborator
There was a problem hiding this comment.
I think that one of the benefits of using multipe return for this method is that it allow to write thing like this
Suggested change
| if (oidc != null && oidc.getOidc() != null) { | |
| OpenIdConnectAuthenticationPolicyConfiguration config = oidc.getOidc(); | |
| if (oidc != null) { | |
| OpenIdConnectAuthenticationPolicyConfiguration config = oidc.getOidc(); | |
| if (config != null) { | |
| } |
fjtirado
reviewed
Jul 2, 2026
fjtirado
reviewed
Jul 2, 2026
fjtirado
reviewed
Jul 2, 2026
fjtirado
left a comment
fjtirado
left a comment
Collaborator
There was a problem hiding this comment.
Also a very good one, just usual picky comments ;)
Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
fjtirado
reviewed
Jul 2, 2026
Comment on lines
+40
to
+41
| public record OAuthPolicyData( | ||
| OAuth2AuthenticationData data, SecretBasedAuthenticationPolicy secret, OAuthScheme scheme) {} |
Collaborator
There was a problem hiding this comment.
hmmm, I think is better to not declare this as inner record, but in a separate class
fjtirado
reviewed
Jul 2, 2026
|
|
||
| private OAuthUtils() {} | ||
|
|
||
| public enum OAuthScheme { |
Collaborator
There was a problem hiding this comment.
Same for this enum, it is bettes to have it in a separate class
fjtirado
reviewed
Jul 2, 2026
| return Optional.empty(); | ||
| } | ||
|
|
||
| public static AuthenticationPolicyUnion resolvePolicy( |
Collaborator
There was a problem hiding this comment.
I do not know where this method is used, but should not return Optional.empty rather than null?
Replace null returns with Optional in OAuthUtils.resolvePolicy to align with the existing from() method style, and extract OAuthScheme to its own top-level file. Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
Move OAuthPolicyData out of OAuthUtils into its own file so integrations can reference it directly, and add a static from(AuthenticationPolicyUnion) convenience factory that delegates to OAuthUtils.from(). Signed-off-by: Matheus Cruz <matheuscruz.dev@gmail.com>
fjtirado
approved these changes
Jul 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Many thanks for submitting your Pull Request ❤️!
What this PR does / why we need it:
Special notes for reviewers:
Additional information (if needed):
Closes #1511