diff --git a/src/Helpers/OAuth2/OAuthConfig.php b/src/Helpers/OAuth2/OAuthConfig.php index b3ab2c59..4fc0ad4e 100644 --- a/src/Helpers/OAuth2/OAuthConfig.php +++ b/src/Helpers/OAuth2/OAuthConfig.php @@ -60,6 +60,12 @@ class OAuthConfig */ protected array $defaultScopes = []; + /** + * When true, OAuth endpoints (authorize, token, user) may be full URLs that differ from the connector base. + * Do not enable with user-controlled endpoint values (SSRF / credential leakage). + */ + protected bool $allowBaseUrlOverride = false; + /** * Get the Client ID */ @@ -120,6 +126,27 @@ public function setRedirectUri(string $redirectUri): static return $this; } + /** + * Whether absolute URLs are allowed for OAuth authorize, token, and user endpoints. + */ + public function getAllowBaseUrlOverride(): bool + { + return $this->allowBaseUrlOverride; + } + + /** + * Allow OAuth endpoints to be absolute URLs (different host than the connector base). + * Do not enable when endpoint values are user-controlled. + * + * @return $this + */ + public function setAllowBaseUrlOverride(bool $allow = true): static + { + $this->allowBaseUrlOverride = $allow; + + return $this; + } + /** * Get the authorization endpoint. */ diff --git a/src/Helpers/URLHelper.php b/src/Helpers/URLHelper.php index ad8f00a9..bfb2ae45 100644 --- a/src/Helpers/URLHelper.php +++ b/src/Helpers/URLHelper.php @@ -22,18 +22,23 @@ public static function matches(string $pattern, string $value): bool /** * Join a base url and an endpoint together. * - * When the connector has a base URL, the endpoint must be a relative path (e.g. "/users" or "users"). - * Absolute URLs in the endpoint are not allowed in that case (SSRF / credential leakage). + * When the connector has a base URL, the endpoint must be a relative path (e.g. "/users" or "users"), + * unless $allowBaseUrlOverride is true (e.g. OAuth provider URLs). Allowing override with user-controlled + * endpoints reintroduces SSRF and credential leakage. * When the base URL is empty (e.g. Solo Request), the endpoint may be an absolute URL. * - * @throws InvalidArgumentException When the endpoint is an absolute URL and the base URL is not empty + * @throws InvalidArgumentException When the endpoint is an absolute URL, the base URL is not empty, and override is not allowed */ - public static function join(string $baseUrl, string $endpoint): string + public static function join(string $baseUrl, string $endpoint, bool $allowBaseUrlOverride = false): string { $baseTrimmed = trim($baseUrl, '/ '); if ($baseTrimmed !== '' && static::isValidUrl($endpoint)) { + if ($allowBaseUrlOverride) { + return $endpoint; + } + throw new InvalidArgumentException( - 'Absolute URLs are not allowed in the endpoint. The endpoint must be a relative path to prevent SSRF and credential leakage. To request a different host, use a connector with that host as the base URL.' + 'Absolute URLs are not allowed in the endpoint. The endpoint must be a relative path to prevent SSRF and credential leakage. To request a different host, use a connector with that host as the base URL, or enable allowBaseUrlOverride on the connector, request, or OAuth configuration when the endpoint is trusted.' ); } diff --git a/src/Http/Connector.php b/src/Http/Connector.php index d0970bc0..87d5b8fb 100644 --- a/src/Http/Connector.php +++ b/src/Http/Connector.php @@ -36,6 +36,13 @@ abstract class Connector use Makeable; use HasTries; + /** + * When true, resolveEndpoint() may return an absolute URL (different host than base). + * Set on the connector instance or declare e.g. `public bool $allowBaseUrlOverride = true` on your subclass. + * Enabling with user-controlled endpoints risks SSRF and credential leakage. + */ + public bool $allowBaseUrlOverride = false; + /** * Define the base URL of the API. */ diff --git a/src/Http/PendingRequest.php b/src/Http/PendingRequest.php index 02fcf0bd..fa749508 100644 --- a/src/Http/PendingRequest.php +++ b/src/Http/PendingRequest.php @@ -14,21 +14,27 @@ use Saloon\Contracts\FakeResponse; use Saloon\Http\Faking\MockClient; use Saloon\Contracts\Authenticator; +use Saloon\Helpers\OAuth2\OAuthConfig; +use Saloon\Http\OAuth2\GetUserRequest; use Saloon\Contracts\Body\BodyRepository; use Saloon\Http\PendingRequest\MergeBody; use Saloon\Http\PendingRequest\MergeDelay; use Saloon\Http\Middleware\DelayMiddleware; use Saloon\Http\PendingRequest\BootPlugins; +use Saloon\Http\OAuth2\GetAccessTokenRequest; use Saloon\Traits\Auth\AuthenticatesRequests; use Saloon\Http\Middleware\ValidateProperties; +use Saloon\Http\OAuth2\GetRefreshTokenRequest; use Saloon\Http\Middleware\DetermineMockResponse; use Saloon\Exceptions\InvalidResponseClassException; use Saloon\Exceptions\Request\FatalRequestException; use Saloon\Traits\PendingRequest\ManagesPsrRequests; use Saloon\Http\PendingRequest\MergeRequestProperties; use Saloon\Http\PendingRequest\BootConnectorAndRequest; +use Saloon\Http\OAuth2\GetClientCredentialsTokenRequest; use Saloon\Traits\RequestProperties\HasRequestProperties; use Saloon\Http\PendingRequest\AuthenticatePendingRequest; +use Saloon\Http\OAuth2\GetClientCredentialsTokenBasicAuthRequest; class PendingRequest { @@ -89,7 +95,11 @@ public function __construct(Connector $connector, Request $request, ?MockClient $this->connector = $connector; $this->request = $request; $this->method = $request->getMethod(); - $this->url = URLHelper::join($this->connector->resolveBaseUrl(), $this->request->resolveEndpoint()); + $this->url = URLHelper::join( + $this->connector->resolveBaseUrl(), + $this->request->resolveEndpoint(), + $this->resolveAllowBaseUrlOverrideForUrl(), + ); $this->authenticator = $request->getAuthenticator() ?? $connector->getAuthenticator(); $this->mockClient = $mockClient ?? $request->getMockClient() ?? $connector->getMockClient() ?? MockClient::getGlobal(); @@ -316,4 +326,35 @@ protected function tap(callable $callable): static return $this; } + + /** + * Resolve whether an absolute URL may be used when joining the connector base with the request endpoint. + * Uses the request flag when set, else OAuth config for token/user internal requests, else the connector flag. + */ + protected function resolveAllowBaseUrlOverrideForUrl(): bool + { + if ($this->request->allowBaseUrlOverride !== null) { + return $this->request->allowBaseUrlOverride; + } + + if ($this->usesOAuthConfigTokenOrUserEndpoint() && method_exists($this->connector, 'oauthConfig') && $this->connector->oauthConfig() instanceof OAuthConfig) { + return $this->connector->oauthConfig()->getAllowBaseUrlOverride(); + } + + return $this->connector->allowBaseUrlOverride; + } + + /** + * True for internal OAuth2 requests (token exchange, refresh, client credentials, or user info). + */ + protected function usesOAuthConfigTokenOrUserEndpoint(): bool + { + $request = $this->request; + + return $request instanceof GetAccessTokenRequest + || $request instanceof GetRefreshTokenRequest + || $request instanceof GetUserRequest + || $request instanceof GetClientCredentialsTokenRequest + || $request instanceof GetClientCredentialsTokenBasicAuthRequest; + } } diff --git a/src/Http/Request.php b/src/Http/Request.php index 641b62ec..6f145133 100644 --- a/src/Http/Request.php +++ b/src/Http/Request.php @@ -36,6 +36,12 @@ abstract class Request use Bootable; use Makeable; + /** + * When non-null, overrides connector / OAuth resolution for absolute endpoints in resolveEndpoint(). + * null = inherit. Set true/false on the instance or declare on your request subclass. + */ + public ?bool $allowBaseUrlOverride = null; + /** * Define the HTTP method. */ diff --git a/src/Http/SoloRequest.php b/src/Http/SoloRequest.php index 31889f49..11cac619 100644 --- a/src/Http/SoloRequest.php +++ b/src/Http/SoloRequest.php @@ -11,6 +11,12 @@ abstract class SoloRequest extends Request { use HasConnector; + /** + * When true, OAuth endpoints (authorize, token, user) may be full URLs that differ from the connector base. + * Do not enable with user-controlled endpoint values (SSRF / credential leakage). + */ + public ?bool $allowBaseUrlOverride = true; + /** * Create a new connector instance. */ diff --git a/src/Traits/OAuth2/AuthorizationCodeGrant.php b/src/Traits/OAuth2/AuthorizationCodeGrant.php index 8db18a81..a6d9a6a8 100644 --- a/src/Traits/OAuth2/AuthorizationCodeGrant.php +++ b/src/Traits/OAuth2/AuthorizationCodeGrant.php @@ -60,7 +60,8 @@ public function getAuthorizationUrl(array $scopes = [], ?string $state = null, s $query = http_build_query($queryParameters, '', '&', PHP_QUERY_RFC3986); $query = trim($query, '?&'); - $url = URLHelper::join($this->resolveBaseUrl(), $config->getAuthorizeEndpoint()); + $allows = $config->getAllowBaseUrlOverride() || $this->allowBaseUrlOverride; + $url = URLHelper::join($this->resolveBaseUrl(), $config->getAuthorizeEndpoint(), $allows); $glue = str_contains($url, '?') ? '&' : '?'; diff --git a/tests/Feature/Oauth2/AbsoluteOAuthEndpointTest.php b/tests/Feature/Oauth2/AbsoluteOAuthEndpointTest.php new file mode 100644 index 00000000..646548aa --- /dev/null +++ b/tests/Feature/Oauth2/AbsoluteOAuthEndpointTest.php @@ -0,0 +1,49 @@ +oauthConfig()); + + $pending = $connector->createPendingRequest($request); + + expect($pending->getUrl())->toBe('https://auth.external.com/oauth/token'); +}); + +test('OAuth client credentials throws when token endpoint is absolute and OAuthConfig does not allow override', function () { + $connector = new AbsoluteTokenOAuthConnectorWithoutAllow; + $request = new GetClientCredentialsTokenRequest($connector->oauthConfig()); + + expect(fn () => $connector->createPendingRequest($request)) + ->toThrow(InvalidArgumentException::class); +}); + +test('request allowBaseUrlOverride false overrides OAuthConfig allow for token URL', function () { + $connector = new AbsoluteTokenOAuthConnector; + $request = new GetClientCredentialsTokenRequest($connector->oauthConfig()); + $request->allowBaseUrlOverride = false; + + expect(fn () => $connector->createPendingRequest($request)) + ->toThrow(InvalidArgumentException::class); +}); + +test('getAuthorizationUrl works with absolute authorize endpoint when OAuthConfig allows override', function () { + $connector = new AuthorizeUrlOAuthConnector; + $url = $connector->getAuthorizationUrl(); + + expect($url)->toStartWith('https://login.provider.com/authorize'); +}); + +test('getAuthorizationUrl works with absolute authorize when connector allows override only', function () { + $connector = new AuthorizeUrlOAuthConnectorWithConnectorAllowOnly; + $url = $connector->getAuthorizationUrl(); + + expect($url)->toStartWith('https://login.provider.com/authorize'); +}); diff --git a/tests/Fixtures/Connectors/AbsoluteTokenOAuthConnector.php b/tests/Fixtures/Connectors/AbsoluteTokenOAuthConnector.php new file mode 100644 index 00000000..f1d533c6 --- /dev/null +++ b/tests/Fixtures/Connectors/AbsoluteTokenOAuthConnector.php @@ -0,0 +1,28 @@ +setClientId('id') + ->setClientSecret('secret') + ->setTokenEndpoint('https://auth.external.com/oauth/token') + ->setAllowBaseUrlOverride(); + } +} diff --git a/tests/Fixtures/Connectors/AbsoluteTokenOAuthConnectorWithoutAllow.php b/tests/Fixtures/Connectors/AbsoluteTokenOAuthConnectorWithoutAllow.php new file mode 100644 index 00000000..34267fd0 --- /dev/null +++ b/tests/Fixtures/Connectors/AbsoluteTokenOAuthConnectorWithoutAllow.php @@ -0,0 +1,27 @@ +setClientId('id') + ->setClientSecret('secret') + ->setTokenEndpoint('https://auth.external.com/oauth/token'); + } +} diff --git a/tests/Fixtures/Connectors/AuthorizeUrlOAuthConnector.php b/tests/Fixtures/Connectors/AuthorizeUrlOAuthConnector.php new file mode 100644 index 00000000..0690e47a --- /dev/null +++ b/tests/Fixtures/Connectors/AuthorizeUrlOAuthConnector.php @@ -0,0 +1,29 @@ +setClientId('id') + ->setClientSecret('secret') + ->setRedirectUri('https://app.com/cb') + ->setAuthorizeEndpoint('https://login.provider.com/authorize') + ->setAllowBaseUrlOverride(); + } +} diff --git a/tests/Fixtures/Connectors/AuthorizeUrlOAuthConnectorWithConnectorAllowOnly.php b/tests/Fixtures/Connectors/AuthorizeUrlOAuthConnectorWithConnectorAllowOnly.php new file mode 100644 index 00000000..10baa949 --- /dev/null +++ b/tests/Fixtures/Connectors/AuthorizeUrlOAuthConnectorWithConnectorAllowOnly.php @@ -0,0 +1,36 @@ +allowBaseUrlOverride = true; + } + + public function resolveBaseUrl(): string + { + return 'https://api.app.com'; + } + + protected function defaultOauthConfig(): OAuthConfig + { + return OAuthConfig::make() + ->setClientId('id') + ->setClientSecret('secret') + ->setRedirectUri('https://app.com/cb') + ->setAuthorizeEndpoint('https://login.provider.com/authorize'); + } +} diff --git a/tests/Unit/Oauth2/OAuthConfigTest.php b/tests/Unit/Oauth2/OAuthConfigTest.php index 51775bc5..7a72c883 100644 --- a/tests/Unit/Oauth2/OAuthConfigTest.php +++ b/tests/Unit/Oauth2/OAuthConfigTest.php @@ -15,6 +15,7 @@ expect($config->getTokenEndpoint())->toEqual('token'); expect($config->getUserEndpoint())->toEqual('user'); expect($config->getDefaultScopes())->toEqual([]); + expect($config->getAllowBaseUrlOverride())->toBeFalse(); $clientId = 'client-id'; $clientSecret = 'client-secret'; @@ -31,6 +32,7 @@ expect($config->setTokenEndpoint($tokenEndpoint))->toEqual($config); expect($config->setUserEndpoint($userEndpoint))->toEqual($config); expect($config->setDefaultScopes($defaultScopes))->toEqual($config); + expect($config->setAllowBaseUrlOverride(true))->toEqual($config); expect($config->getClientId())->toEqual($clientId); expect($config->getClientSecret())->toEqual($clientSecret); @@ -39,6 +41,10 @@ expect($config->getTokenEndpoint())->toEqual($tokenEndpoint); expect($config->getUserEndpoint())->toEqual($userEndpoint); expect($config->getDefaultScopes())->toEqual($defaultScopes); + expect($config->getAllowBaseUrlOverride())->toBeTrue(); + + $config->setAllowBaseUrlOverride(false); + expect($config->getAllowBaseUrlOverride())->toBeFalse(); }); test('make method creates an instance of OAuthConfig', function () { diff --git a/tests/Unit/URLHelperTest.php b/tests/Unit/URLHelperTest.php index f28fb47b..e36b2763 100644 --- a/tests/Unit/URLHelperTest.php +++ b/tests/Unit/URLHelperTest.php @@ -25,6 +25,16 @@ ->toThrow(InvalidArgumentException::class, 'Absolute URLs are not allowed in the endpoint'); }); +test('join allows absolute endpoint when allowBaseUrlOverride is true', function () { + expect(URLHelper::join('https://api.example.com', 'https://auth.provider.com/token', true)) + ->toBe('https://auth.provider.com/token'); +}); + +test('join with empty base and absolute endpoint ignores allowBaseUrlOverride flag', function () { + expect(URLHelper::join('', 'https://solo.example.com/hook', false)) + ->toBe('https://solo.example.com/hook'); +}); + test('creating a pending request with a request that returns absolute URL from resolveEndpoint throws', function () { $connector = new TestConnector('https://api.trusted.com'); $request = new AbsoluteEndpointRequest('https://attacker.example.com/callback'); @@ -33,6 +43,26 @@ ->toThrow(InvalidArgumentException::class, 'Absolute URLs are not allowed in the endpoint'); }); +test('connector allowBaseUrlOverride allows pending request with absolute endpoint', function () { + $connector = new TestConnector('https://api.trusted.com'); + $connector->allowBaseUrlOverride = true; + $request = new AbsoluteEndpointRequest('https://other-api.example.com/v1'); + + $pending = $connector->createPendingRequest($request); + + expect($pending->getUrl())->toBe('https://other-api.example.com/v1'); +}); + +test('request allowBaseUrlOverride false denies absolute endpoint even when connector allows', function () { + $connector = new TestConnector('https://api.trusted.com'); + $connector->allowBaseUrlOverride = true; + $request = new AbsoluteEndpointRequest('https://other.example.com/x'); + $request->allowBaseUrlOverride = false; + + expect(fn () => $connector->createPendingRequest($request)) + ->toThrow(InvalidArgumentException::class); +}); + test('the URL helper can parse a variety of query parameters', function (string $query, array $expected) { expect(URLHelper::parseQueryString($query))->toBe($expected); })->with([