[Fix #1805] Refactor oauth code by fjtirado · Pull Request #1830 · open-workflow-specification/sdk-java

fjtirado · 2026-11-11T16:30:33Z

Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>

Copilot

🟡 Changes recommended

Unresolved content-type and encoding defects can misserialize or abort HTTP requests and alter query values, while cached OAuth secrets leave reused workflows with stale credentials after rotation.

5 open issues

Reload OAuth secrets on each authentication attempt · New
Encode null form values as empty strings · New
Preserve %20 encoding for URI query values · New
Ignore content-type parameters when selecting the encoder · New
Match Content-Type header names case-insensitively · New

What changed in this PR

Moves OAuth token acquisition into core and routes it through the shared HTTP request abstraction, addressing #1805’s goal of removing the Jakarta HTTP dependency from this path.

Changes:

Replaces Jakarta-specific token providers with a core implementation.
Adds form-urlencoded request bodies to both HTTP implementations and shares response handling.
Updates JWT conversion to accept token-response bytes and runs OAuth/OpenID tests with both HTTP backends.

File	Description
`impl/test/src/test/java/org/openworkflow/sdk/impl/test/OpenIDCHTTPWorkflowDefinitionTest.java`	Runs OpenID tests with both HTTP backends.
`impl/test/src/test/java/org/openworkflow/sdk/impl/test/OAuthHTTPWorkflowDefinitionTest.java`	Runs OAuth tests with both HTTP backends.
`impl/test/src/test/java/org/openworkflow/sdk/impl/auth/OAuthRequestBuilderTest.java`	Removes a blank line only.
`impl/jwt-impl/src/test/java/org/openworkflow/sdk/impl/http/jwt/JacksonJWTImplTest.java`	Uses response-byte fixtures and removes malformed-token cases.
`impl/jwt-impl/src/main/java/org/openworkflow/sdk/impl/executors/http/oauth/jackson/JacksonJWTConverter.java`	Extracts `access_token` from JSON response bytes.
`impl/jwt-impl/pom.xml`	Switches the JUnit dependency to the engine.
`impl/http/src/main/resources/META-INF/services/org.openworkflow.sdk.impl.auth.AccessTokenProviderFactory`	Removes Jakarta token-factory registration.
`impl/http/src/main/java/org/openworkflow/sdk/impl/jakarta/http/JakartaRequestExecutor.java`	Adds form-body serialization.
`impl/http/src/main/java/org/openworkflow/sdk/impl/jakarta/auth/JaxRSAccessTokenProviderFactory.java`	Removes the Jakarta-specific factory.
`impl/http/src/main/java/org/openworkflow/sdk/impl/jakarta/auth/JaxRSAccessTokenProvider.java`	Removes the Jakarta-specific token provider.
`impl/core/src/main/java/org/openworkflow/sdk/impl/resources/HttpResource.java`	Delegates requests to shared HTTP helpers.
`impl/core/src/main/java/org/openworkflow/sdk/impl/http/WorkflowHttpRequestInfo.java`	Adds content-type lookup and individual header insertion.
`impl/core/src/main/java/org/openworkflow/sdk/impl/http/WorkflowHttpRequestHelper.java`	Adds raw-response and byte-response helpers.
`impl/core/src/main/java/org/openworkflow/sdk/impl/http/WorkflowHttpContentType.java`	Defines supported JSON and form content types.
`impl/core/src/main/java/org/openworkflow/sdk/impl/http/URIDecodedBuilder.java`	Changes space encoding from `%20` to `+`.
`impl/core/src/main/java/org/openworkflow/sdk/impl/http/DefaultHttpRequest.java`	Adds form bodies and reuses map encoding.
`impl/core/src/main/java/org/openworkflow/sdk/impl/auth/JWTConverter.java`	Changes converter input to bytes.
`impl/core/src/main/java/org/openworkflow/sdk/impl/auth/JwtClientAssertion.java`	Uses the renamed auth request builder.
`impl/core/src/main/java/org/openworkflow/sdk/impl/auth/CommonOAuthProvider.java`	Constructs the core token provider directly.
`impl/core/src/main/java/org/openworkflow/sdk/impl/auth/ClientSecretPost.java`	Uses the renamed auth request builder.
`impl/core/src/main/java/org/openworkflow/sdk/impl/auth/ClientSecretHandler.java`	Updates the shared builder type.
`impl/core/src/main/java/org/openworkflow/sdk/impl/auth/ClientSecretBasic.java`	Uses the renamed auth request builder.
`impl/core/src/main/java/org/openworkflow/sdk/impl/auth/AuthHttpRequestInfoBuilder.java`	Renames the builder and adopts typed content types.
`impl/core/src/main/java/org/openworkflow/sdk/impl/auth/AuthHttpRequestInfo.java`	Stores content type as an enum.
`impl/core/src/main/java/org/openworkflow/sdk/impl/auth/AccessTokenProviderFactory.java`	Removes the token-provider factory interface.
`impl/core/src/main/java/org/openworkflow/sdk/impl/auth/AccessTokenProvider.java`	Implements token acquisition through shared HTTP requests.
`impl/core/src/main/java/org/openworkflow/sdk/impl/auth/AbstractAuthRequestBuilder.java`	Instantiates the renamed auth request builder.

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

A critical OAuth concurrency flaw can lose token parameters or fail requests, while issuer-state, null-form handling, header-mutability, and transport-dependency defects also remain unresolved.

2 open issues

2 previously missed issues

In code that hasn't changed since last review

JWT module dependency pulls in Jakarta HTTP transport

impl/core/src/main/java/org/openworkflow/sdk/impl/auth/CommonOAuthProvider.java:76

OAuth still requires a JWTConverter, and the only implementation is in impl/jwt-impl, which depends on openworkflow-impl-http (impl/jwt-impl/pom.xml:14–17). Using OAuth with the default JDK transport therefore still brings in the Jakarta HTTP module, contrary to #1805's library-independence goal. Separate the HTTP-specific model adapter from the JWT module and remove that dependency.

Copy caller headers before adding incremental headers

impl/core/src/main/java/org/openworkflow/sdk/impl/http/WorkflowHttpRequestInfo.java:87

The new incremental header method mutates the map passed to headers(...). Consequently, builder.headers(Map.of("Accept", "application/json")).header("Content-Type", "application/json") throws UnsupportedOperationException, and mutable caller-owned maps are modified unexpectedly. Copy the supplied map in headers(...) so the builder owns a mutable map.

5 resolved since last review

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

OAuth still requires the optional Jakarta HTTP stack, and concurrent token requests can corrupt shared mutable authentication-builder state.

5 open issues

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

Concurrent OAuth requests reuse a non-thread-safe builder, and the default OAuth path still lacks a JWT converter that works without additional Jakarta HTTP dependencies.

4 open issues

3 resolved since last review

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Active defects can make header additions fail or mutate caller maps, retain stale issuer restrictions after secret updates, and produce conflicting Content-Type headers that mismatch authentication request encoding.

3 open issues

1 resolved since last review

Provide a default JWT converter without the Jakarta HTTP stack

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

Unresolved defects retain stale issuer restrictions after secret reloads, cause header configuration to fail or mutate caller-owned maps, and mismatch OAuth request body encoding with Content-Type.

4 open issues

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

Unresolved defects can produce payloads that mismatch declared content types, retain stale issuer allowlists after secret refreshes, and cause header additions to throw or mutate caller-owned maps.

6 open issues

1 resolved since last review

Avoid adding a default content type when one is already supplied

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

Authentication errors are incorrectly remapped to HTTP 500, breaking status-specific handling, and OpenAPI paths with hyphenated parameter names now fail before requests are sent.

1 open issue

Preserve WorkflowException status when converting request failures · New

1 resolved since last review

Wrap malformed token responses in a communication WorkflowException

_{Review effort: Balanced}

fjtirado · 2026-11-12T13:39:51Z

This is comment for copilot, please do not mention the hyphenated issue anymore, there is no evidence that it is a problem, as already reported here

Copilot

🟡 Changes recommended

HTTP error handling can turn connection failures into a NullPointerException that bypasses workflow catch/retry handlers, while OpenAPI routing rejects valid hyphenated parameter names.

1 open issue

Avoid null dereference when wrapping communication failures · New

1 resolved since last review

Preserve WorkflowException status when converting request failures

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Required path arguments that resolve to null are silently omitted, potentially sending requests to an unintended endpoint instead of rejecting invalid input.

No open issues 🎉

1 resolved since last review

Avoid null dereference when wrapping communication failures

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

The shared resolver now encodes slashes in path-parameter values, changing requested endpoints and breaking nested-path APIs in both the default and Jakarta implementations.

No open issues 🎉

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

The resolver now throws for omitted optional URI-template variables or empty collections, breaking existing HTTP workflows before requests are sent.

1 open issue

Optional URI-template expansions fail for omitted or empty variables · New

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

The resolver now throws for undefined optional URI-template variables instead of omitting them under RFC 6570, breaking ordinary HTTP endpoints that rely on optional parameters.

1 open issue

Preserve omission semantics for undefined URI-template variables · New

1 resolved since last review

Optional URI-template expansions fail for omitted or empty variables

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Path expressions are evaluated twice through the lazy map, so a stateful value can pass validation and then become null, silently producing an incorrect request path.

1 open issue

Preserve omission semantics for undefined URI-template variables

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

The path-parameter resolver evaluates expressions twice, adding redundant work and potentially sending time-dependent values different from those checked for null.

No open issues 🎉

1 resolved since last review

Preserve omission semantics for undefined URI-template variables

_{Review effort: Balanced}

Copilot

🟢 Approval recommended

No active review issues block the OAuth and OpenAPI consolidation into core.

No open issues 🎉

_{Review effort: Balanced}

Copilot

🔵 Needs a closer look

Required path arguments supplied as expressions can resolve to null without validation, causing requests such as /pet/ to be sent instead of rejecting the missing identifier.

No open issues 🎉

_{Review effort: Balanced}

Copilot

🟡 Changes recommended

Content-Type detection casts object-valued headers to String, causing ClassCastException for valid values and preventing requests with bodies from being sent.

1 open issue

Avoid casting object-valued Content-Type headers to String · New

_{Review effort: Balanced}

Copilot

🟢 Approval recommended

No active review issues are reported for the consolidation of OAuth requests and base OpenAPI support into core.

1 open issue

Avoid casting object-valued Content-Type headers to String

_{Review effort: Balanced}

Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>

Copilot

🟢 Approval recommended

No active review issues are reported for the OAuth and OpenAPI refactor into core.

No open issues 🎉

1 resolved since last review

Avoid casting object-valued Content-Type headers to String

_{Review effort: Balanced}

[Fix open-workflow-specification#1805] Refactor oauth code

e0e3f57

Signed-off-by: Francisco Javier Tirado Sarti <ftirados@ibm.com>

Copilot AI balanced review requested due to automatic review settings November 11, 2026 16:30

Copilot started reviewing on behalf of fjtirado November 11, 2026 16:31

View session

fjtirado requested review from gmunozfe and ricardozanini November 11, 2026 16:38

Copilot AI reviewed Nov 11, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings November 11, 2026 17:23

Copilot started reviewing on behalf of fjtirado November 11, 2026 17:23

View session

fjtirado force-pushed the Fix_#1805 branch from 7b081d1 to 017b3e1 Compare November 11, 2026 17:26

Copilot AI reviewed Nov 11, 2026

View reviewed changes

Comment thread impl/core/src/main/java/org/openworkflow/sdk/impl/auth/CommonOAuthProvider.java Resolved

Comment thread impl/http/src/main/java/org/openworkflow/sdk/impl/jakarta/http/JakartaRequestExecutor.java Resolved

Copilot AI balanced review requested due to automatic review settings November 11, 2026 17:32

Copilot started reviewing on behalf of fjtirado November 11, 2026 17:33

View session

Copilot AI reviewed Nov 11, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings November 11, 2026 18:03

fjtirado force-pushed the Fix_#1805 branch from 017b3e1 to 2ea4815 Compare November 11, 2026 18:03

Copilot started reviewing on behalf of fjtirado November 11, 2026 18:04

View session

Copilot AI reviewed Nov 11, 2026

View reviewed changes

Comment thread impl/core/src/main/java/org/openworkflow/sdk/impl/auth/AccessTokenProvider.java Resolved

Comment thread impl/model/src/test/java/org/openworkflow/sdk/impl/auth/jwt/jackson/JacksonJWTImplTest.java Resolved

fjtirado force-pushed the Fix_#1805 branch from 2ea4815 to 70e745b Compare November 11, 2026 18:26

Copilot AI balanced review requested due to automatic review settings November 11, 2026 18:26

Copilot started reviewing on behalf of fjtirado November 11, 2026 18:27

View session

fjtirado force-pushed the Fix_#1805 branch from 70e745b to 3f96334 Compare November 11, 2026 18:28

Copilot AI reviewed Nov 11, 2026

View reviewed changes

Copilot AI balanced review requested due to automatic review settings November 11, 2026 18:33

Copilot started reviewing on behalf of fjtirado November 11, 2026 18:33

View session

Copilot AI reviewed Nov 11, 2026

View reviewed changes

Comment thread impl/README.md Resolved

fjtirado force-pushed the Fix_#1805 branch from 3f96334 to 2a2cc72 Compare November 11, 2026 18:45

Copilot AI balanced review requested due to automatic review settings November 11, 2026 18:45

Copilot started reviewing on behalf of fjtirado November 11, 2026 18:46

View session

fjtirado force-pushed the Fix_#1805 branch from 2a2cc72 to 4d6b8a0 Compare November 11, 2026 18:51

Copilot AI reviewed Nov 11, 2026

View reviewed changes

Comment thread impl/core/src/main/java/org/openworkflow/sdk/impl/http/WorkflowHttpContentType.java Resolved

Comment thread impl/README.md Resolved

Comment thread impl/core/src/main/java/org/openworkflow/sdk/impl/auth/JWTConverter.java Resolved

fjtirado force-pushed the Fix_#1805 branch from b078d48 to 366db2f Compare November 12, 2026 13:26

Copilot AI balanced review requested due to automatic review settings November 12, 2026 13:26

Copilot started reviewing on behalf of fjtirado November 12, 2026 13:27

View session