Skip to content

fix: build redirect endpoint URLs locally instead of making HTTP requests#358

Merged
gjtorikian merged 3 commits intomainfrom
url-builder-redirect-endpoints
Apr 14, 2026
Merged

fix: build redirect endpoint URLs locally instead of making HTTP requests#358
gjtorikian merged 3 commits intomainfrom
url-builder-redirect-endpoints

Conversation

@workos-sdk-automation
Copy link
Copy Markdown
Contributor

Summary

  • getAuthorizationUrl() and getLogoutUrl() on SSO and UserManagement were making real HTTP requests to redirect endpoints, receiving HTML instead of JSON, and throwing a TypeError via fromArray(null). These methods now construct the URL client-side and return a string, matching the Python and .NET SDKs.
  • HttpClient::decodeResponse() now throws ApiException on non-JSON success bodies instead of silently returning null, so any future misuse surfaces a clear error.
  • HttpClient::buildUrl() respects RequestOptions::$baseUrl overrides.
  • V5 migration guide updated to reflect the correct behavior.

Test plan

  • script/ci passes (php-cs-fixer, PHPStan, PHPUnit 233 tests)
  • SSO and UserManagement redirect endpoint tests assert string URLs
  • oagen-emitters unit tests pass (20 tests)

Closes #357

…ests

getAuthorizationUrl() and getLogoutUrl() on SSO and UserManagement were
making real HTTP requests to redirect endpoints, receiving HTML instead
of JSON, and throwing a TypeError via fromArray(null). These methods now
construct the URL client-side and return a string, matching the Python
and .NET SDKs.

Also fixes decodeResponse() to throw ApiException on non-JSON success
bodies instead of silently returning null, and updates the V5 migration
guide to reflect the correct behavior.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@workos-sdk-automation workos-sdk-automation bot requested review from a team as code owners April 14, 2026 19:07
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Apr 14, 2026

Greptile Summary

This PR fixes getAuthorizationUrl() and getLogoutUrl() on SSO and UserManagement to build redirect URLs client-side via the new HttpClient::buildUrl() instead of making HTTP requests that returned HTML and caused a TypeError. It also hardens HttpClient::decodeResponse() to throw ApiException on non-JSON success bodies, and correctly propagates RequestOptions::$baseUrl through buildUrl().

  • P1 – Migration guide getProfileAndToken() example is wrong: The "After" code block shows clientId:, clientSecret:, and grantType: as named arguments, but those parameters don't exist in the v5 method signature; following the example as written will throw a PHP fatal error.

Confidence Score: 4/5

Safe to merge after correcting the migration guide's getProfileAndToken() example, which shows non-existent named arguments that would cause a PHP fatal error.

The core fix is correct and well-tested: URLs are built locally, the empty-query guard prevents a bare ?, decodeResponse hardens non-JSON handling, and tests now verify critical OAuth parameters. One P1 issue remains: the migration guide's getProfileAndToken "After" example uses named arguments (clientId, clientSecret, grantType) that don't exist in the v5 method signature.

docs/V5_MIGRATION_GUIDE.md — the getProfileAndToken() example needs to be corrected

Important Files Changed

Filename Overview
lib/HttpClient.php Adds buildUrl() for client-side URL construction; decodeResponse() now throws ApiException on non-JSON success bodies; resolveUrl() correctly respects RequestOptions::$baseUrl. All logic is sound.
lib/Service/SSO.php getAuthorizationUrl() and getLogoutUrl() now delegate to HttpClient::buildUrl() instead of making HTTP requests. response_type=code and client_id are always included correctly.
lib/Service/UserManagement.php Same URL-building fix applied to getAuthorizationUrl() and getLogoutUrl(). Logic matches SSO pattern. client_id appended after filter to avoid being dropped.
tests/HttpClientTest.php New tests cover: decodeResponse throwing on non-JSON body, buildUrl not emitting ? on empty/empty-array queries, and correct query-string formatting.
tests/Service/SSOTest.php URL tests now parse and assert individual query parameters including response_type=code and client_id presence; previously flagged coverage gaps are addressed.
tests/Service/UserManagementTest.php URL tests parse and verify query params including session_id, return_to, redirect_uri, response_type, and client_id; coverage is solid.
docs/V5_MIGRATION_GUIDE.md Auth-URL sections updated correctly; however the getProfileAndToken() "After" example shows non-existent named arguments (clientId, clientSecret, grantType) that would cause a PHP fatal error if used.

Sequence Diagram

sequenceDiagram
    participant App
    participant SSO/UM as SSO / UserManagement
    participant HttpClient
    participant WorkOS as WorkOS API

    Note over App,WorkOS: Before fix — HTTP request made
    App->>SSO/UM: getAuthorizationUrl(...)
    SSO/UM->>HttpClient: request(GET, sso/authorize, ...)
    HttpClient->>WorkOS: HTTP GET /sso/authorize
    WorkOS-->>HttpClient: 302 HTML redirect page
    HttpClient->>HttpClient: decodeResponse() → null
    HttpClient-->>SSO/UM: null
    SSO/UM-->>App: TypeError (fromArray(null))

    Note over App,WorkOS: After fix — URL built locally
    App->>SSO/UM: getAuthorizationUrl(...)
    SSO/UM->>HttpClient: requireClientId()
    HttpClient-->>SSO/UM: "client_123"
    SSO/UM->>HttpClient: buildUrl(sso/authorize, query, options)
    HttpClient->>HttpClient: resolveUrl() + http_build_query()
    HttpClient-->>SSO/UM: "https://api.workos.com/sso/authorize?..."
    SSO/UM-->>App: string URL
Loading

Comments Outside Diff (1)

  1. docs/V5_MIGRATION_GUIDE.md, line 362-373 (link)

    P1 Incorrect named arguments in migration guide example

    The "After" snippet for getProfileAndToken() shows clientId:, clientSecret:, and grantType: as named arguments, but those parameters do not exist in the v5 method signature (string $code, ?RequestOptions $options). Any developer who copies this example verbatim will hit a PHP fatal error: Unknown named argument 'clientId'.

    The actual v5 call (confirmed by SSOTest::testGetProfileAndToken) is:

    $result = $workos->sso()->getProfileAndToken(code: $code);

    client_id, client_secret, and grant_type are set internally from the instantiated client — they are not parameters callers pass.

Reviews (3): Last reviewed commit: "cleanup" | Re-trigger Greptile

Comment thread lib/HttpClient.php Outdated
Comment thread tests/Service/SSOTest.php
@gjtorikian gjtorikian merged commit eae3949 into main Apr 14, 2026
8 checks passed
@gjtorikian gjtorikian deleted the url-builder-redirect-endpoints branch April 14, 2026 19:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

WorkOS\Resource\SSOAuthorizeUrlResponse::fromArray(): Argument #1 ($data) must be of type array, null given

1 participant