Skip to content

Commit 1307b1d

Browse files
JonPurvisJonPurvis
authored
Feature | v4 - Add opt in to base url overrides (#542)
* add opt in to base url overrides Made-with: Cursor * add dedicated fixture classes * add dedicated fixture classes * action feedback * chain properly * run cs fixer
1 parent d418356 commit 1307b1d

14 files changed

Lines changed: 305 additions & 7 deletions

‎src/Helpers/OAuth2/OAuthConfig.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,12 @@ class OAuthConfig
6060
*/
6161
protected array $defaultScopes = [];
6262

63+
/**
64+
* When true, OAuth endpoints (authorize, token, user) may be full URLs that differ from the connector base.
65+
* Do not enable with user-controlled endpoint values (SSRF / credential leakage).
66+
*/
67+
protected bool $allowBaseUrlOverride = false;
68+
6369
/**
6470
* Get the Client ID
6571
*/
@@ -120,6 +126,27 @@ public function setRedirectUri(string $redirectUri): static
120126
return $this;
121127
}
122128

129+
/**
130+
* Whether absolute URLs are allowed for OAuth authorize, token, and user endpoints.
131+
*/
132+
public function getAllowBaseUrlOverride(): bool
133+
{
134+
return $this->allowBaseUrlOverride;
135+
}
136+
137+
/**
138+
* Allow OAuth endpoints to be absolute URLs (different host than the connector base).
139+
* Do not enable when endpoint values are user-controlled.
140+
*
141+
* @return $this
142+
*/
143+
public function setAllowBaseUrlOverride(bool $allow = true): static
144+
{
145+
$this->allowBaseUrlOverride = $allow;
146+
147+
return $this;
148+
}
149+
123150
/**
124151
* Get the authorization endpoint.
125152
*/

‎src/Helpers/URLHelper.php‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -22,18 +22,23 @@ public static function matches(string $pattern, string $value): bool
2222
/**
2323
* Join a base url and an endpoint together.
2424
*
25-
* When the connector has a base URL, the endpoint must be a relative path (e.g. "/users" or "users").
26-
* Absolute URLs in the endpoint are not allowed in that case (SSRF / credential leakage).
25+
* When the connector has a base URL, the endpoint must be a relative path (e.g. "/users" or "users"),
26+
* unless $allowBaseUrlOverride is true (e.g. OAuth provider URLs). Allowing override with user-controlled
27+
* endpoints reintroduces SSRF and credential leakage.
2728
* When the base URL is empty (e.g. Solo Request), the endpoint may be an absolute URL.
2829
*
29-
* @throws InvalidArgumentException When the endpoint is an absolute URL and the base URL is not empty
30+
* @throws InvalidArgumentException When the endpoint is an absolute URL, the base URL is not empty, and override is not allowed
3031
*/
31-
public static function join(string $baseUrl, string $endpoint): string
32+
public static function join(string $baseUrl, string $endpoint, bool $allowBaseUrlOverride = false): string
3233
{
3334
$baseTrimmed = trim($baseUrl, '/ ');
3435
if ($baseTrimmed !== '' && static::isValidUrl($endpoint)) {
36+
if ($allowBaseUrlOverride) {
37+
return $endpoint;
38+
}
39+
3540
throw new InvalidArgumentException(
36-
'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.'
41+
'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.'
3742
);
3843
}
3944

‎src/Http/Connector.php‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,13 @@ abstract class Connector
3636
use Makeable;
3737
use HasTries;
3838

39+
/**
40+
* When true, resolveEndpoint() may return an absolute URL (different host than base).
41+
* Set on the connector instance or declare e.g. `public bool $allowBaseUrlOverride = true` on your subclass.
42+
* Enabling with user-controlled endpoints risks SSRF and credential leakage.
43+
*/
44+
public bool $allowBaseUrlOverride = false;
45+
3946
/**
4047
* Define the base URL of the API.
4148
*/

‎src/Http/PendingRequest.php‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,21 +14,27 @@
1414
use Saloon\Contracts\FakeResponse;
1515
use Saloon\Http\Faking\MockClient;
1616
use Saloon\Contracts\Authenticator;
17+
use Saloon\Helpers\OAuth2\OAuthConfig;
18+
use Saloon\Http\OAuth2\GetUserRequest;
1719
use Saloon\Contracts\Body\BodyRepository;
1820
use Saloon\Http\PendingRequest\MergeBody;
1921
use Saloon\Http\PendingRequest\MergeDelay;
2022
use Saloon\Http\Middleware\DelayMiddleware;
2123
use Saloon\Http\PendingRequest\BootPlugins;
24+
use Saloon\Http\OAuth2\GetAccessTokenRequest;
2225
use Saloon\Traits\Auth\AuthenticatesRequests;
2326
use Saloon\Http\Middleware\ValidateProperties;
27+
use Saloon\Http\OAuth2\GetRefreshTokenRequest;
2428
use Saloon\Http\Middleware\DetermineMockResponse;
2529
use Saloon\Exceptions\InvalidResponseClassException;
2630
use Saloon\Exceptions\Request\FatalRequestException;
2731
use Saloon\Traits\PendingRequest\ManagesPsrRequests;
2832
use Saloon\Http\PendingRequest\MergeRequestProperties;
2933
use Saloon\Http\PendingRequest\BootConnectorAndRequest;
34+
use Saloon\Http\OAuth2\GetClientCredentialsTokenRequest;
3035
use Saloon\Traits\RequestProperties\HasRequestProperties;
3136
use Saloon\Http\PendingRequest\AuthenticatePendingRequest;
37+
use Saloon\Http\OAuth2\GetClientCredentialsTokenBasicAuthRequest;
3238

3339
class PendingRequest
3440
{
@@ -89,7 +95,11 @@ public function __construct(Connector $connector, Request $request, ?MockClient
8995
$this->connector = $connector;
9096
$this->request = $request;
9197
$this->method = $request->getMethod();
92-
$this->url = URLHelper::join($this->connector->resolveBaseUrl(), $this->request->resolveEndpoint());
98+
$this->url = URLHelper::join(
99+
$this->connector->resolveBaseUrl(),
100+
$this->request->resolveEndpoint(),
101+
$this->resolveAllowBaseUrlOverrideForUrl(),
102+
);
93103
$this->authenticator = $request->getAuthenticator() ?? $connector->getAuthenticator();
94104
$this->mockClient = $mockClient ?? $request->getMockClient() ?? $connector->getMockClient() ?? MockClient::getGlobal();
95105

@@ -316,4 +326,35 @@ protected function tap(callable $callable): static
316326

317327
return $this;
318328
}
329+
330+
/**
331+
* Resolve whether an absolute URL may be used when joining the connector base with the request endpoint.
332+
* Uses the request flag when set, else OAuth config for token/user internal requests, else the connector flag.
333+
*/
334+
protected function resolveAllowBaseUrlOverrideForUrl(): bool
335+
{
336+
if ($this->request->allowBaseUrlOverride !== null) {
337+
return $this->request->allowBaseUrlOverride;
338+
}
339+
340+
if ($this->usesOAuthConfigTokenOrUserEndpoint() && method_exists($this->connector, 'oauthConfig') && $this->connector->oauthConfig() instanceof OAuthConfig) {
341+
return $this->connector->oauthConfig()->getAllowBaseUrlOverride();
342+
}
343+
344+
return $this->connector->allowBaseUrlOverride;
345+
}
346+
347+
/**
348+
* True for internal OAuth2 requests (token exchange, refresh, client credentials, or user info).
349+
*/
350+
protected function usesOAuthConfigTokenOrUserEndpoint(): bool
351+
{
352+
$request = $this->request;
353+
354+
return $request instanceof GetAccessTokenRequest
355+
|| $request instanceof GetRefreshTokenRequest
356+
|| $request instanceof GetUserRequest
357+
|| $request instanceof GetClientCredentialsTokenRequest
358+
|| $request instanceof GetClientCredentialsTokenBasicAuthRequest;
359+
}
319360
}

‎src/Http/Request.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,12 @@ abstract class Request
3636
use Bootable;
3737
use Makeable;
3838

39+
/**
40+
* When non-null, overrides connector / OAuth resolution for absolute endpoints in resolveEndpoint().
41+
* null = inherit. Set true/false on the instance or declare on your request subclass.
42+
*/
43+
public ?bool $allowBaseUrlOverride = null;
44+
3945
/**
4046
* Define the HTTP method.
4147
*/

‎src/Http/SoloRequest.php‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,12 @@ abstract class SoloRequest extends Request
1111
{
1212
use HasConnector;
1313

14+
/**
15+
* When true, OAuth endpoints (authorize, token, user) may be full URLs that differ from the connector base.
16+
* Do not enable with user-controlled endpoint values (SSRF / credential leakage).
17+
*/
18+
public ?bool $allowBaseUrlOverride = true;
19+
1420
/**
1521
* Create a new connector instance.
1622
*/

‎src/Traits/OAuth2/AuthorizationCodeGrant.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,8 @@ public function getAuthorizationUrl(array $scopes = [], ?string $state = null, s
6060
$query = http_build_query($queryParameters, '', '&', PHP_QUERY_RFC3986);
6161
$query = trim($query, '?&');
6262

63-
$url = URLHelper::join($this->resolveBaseUrl(), $config->getAuthorizeEndpoint());
63+
$allows = $config->getAllowBaseUrlOverride() || $this->allowBaseUrlOverride;
64+
$url = URLHelper::join($this->resolveBaseUrl(), $config->getAuthorizeEndpoint(), $allows);
6465

6566
$glue = str_contains($url, '?') ? '&' : '?';
6667

‎tests/Feature/Oauth2/AbsoluteOAuthEndpointTest.php‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Saloon\Http\OAuth2\GetClientCredentialsTokenRequest;
6+
use Saloon\Tests\Fixtures\Connectors\AuthorizeUrlOAuthConnector;
7+
use Saloon\Tests\Fixtures\Connectors\AbsoluteTokenOAuthConnector;
8+
use Saloon\Tests\Fixtures\Connectors\AbsoluteTokenOAuthConnectorWithoutAllow;
9+
use Saloon\Tests\Fixtures\Connectors\AuthorizeUrlOAuthConnectorWithConnectorAllowOnly;
10+
11+
test('OAuth client credentials pending request uses absolute token URL when OAuthConfig allows base override', function () {
12+
$connector = new AbsoluteTokenOAuthConnector;
13+
$request = new GetClientCredentialsTokenRequest($connector->oauthConfig());
14+
15+
$pending = $connector->createPendingRequest($request);
16+
17+
expect($pending->getUrl())->toBe('https://auth.external.com/oauth/token');
18+
});
19+
20+
test('OAuth client credentials throws when token endpoint is absolute and OAuthConfig does not allow override', function () {
21+
$connector = new AbsoluteTokenOAuthConnectorWithoutAllow;
22+
$request = new GetClientCredentialsTokenRequest($connector->oauthConfig());
23+
24+
expect(fn () => $connector->createPendingRequest($request))
25+
->toThrow(InvalidArgumentException::class);
26+
});
27+
28+
test('request allowBaseUrlOverride false overrides OAuthConfig allow for token URL', function () {
29+
$connector = new AbsoluteTokenOAuthConnector;
30+
$request = new GetClientCredentialsTokenRequest($connector->oauthConfig());
31+
$request->allowBaseUrlOverride = false;
32+
33+
expect(fn () => $connector->createPendingRequest($request))
34+
->toThrow(InvalidArgumentException::class);
35+
});
36+
37+
test('getAuthorizationUrl works with absolute authorize endpoint when OAuthConfig allows override', function () {
38+
$connector = new AuthorizeUrlOAuthConnector;
39+
$url = $connector->getAuthorizationUrl();
40+
41+
expect($url)->toStartWith('https://login.provider.com/authorize');
42+
});
43+
44+
test('getAuthorizationUrl works with absolute authorize when connector allows override only', function () {
45+
$connector = new AuthorizeUrlOAuthConnectorWithConnectorAllowOnly;
46+
$url = $connector->getAuthorizationUrl();
47+
48+
expect($url)->toStartWith('https://login.provider.com/authorize');
49+
});

‎tests/Fixtures/Connectors/AbsoluteTokenOAuthConnector.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Saloon\Tests\Fixtures\Connectors;
6+
7+
use Saloon\Http\Connector;
8+
use Saloon\Helpers\OAuth2\OAuthConfig;
9+
use Saloon\Traits\OAuth2\ClientCredentialsGrant;
10+
11+
class AbsoluteTokenOAuthConnector extends Connector
12+
{
13+
use ClientCredentialsGrant;
14+
15+
public function resolveBaseUrl(): string
16+
{
17+
return 'https://api.service.com';
18+
}
19+
20+
protected function defaultOauthConfig(): OAuthConfig
21+
{
22+
return OAuthConfig::make()
23+
->setClientId('id')
24+
->setClientSecret('secret')
25+
->setTokenEndpoint('https://auth.external.com/oauth/token')
26+
->setAllowBaseUrlOverride();
27+
}
28+
}

‎tests/Fixtures/Connectors/AbsoluteTokenOAuthConnectorWithoutAllow.php‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Saloon\Tests\Fixtures\Connectors;
6+
7+
use Saloon\Http\Connector;
8+
use Saloon\Helpers\OAuth2\OAuthConfig;
9+
use Saloon\Traits\OAuth2\ClientCredentialsGrant;
10+
11+
class AbsoluteTokenOAuthConnectorWithoutAllow extends Connector
12+
{
13+
use ClientCredentialsGrant;
14+
15+
public function resolveBaseUrl(): string
16+
{
17+
return 'https://api.service.com';
18+
}
19+
20+
protected function defaultOauthConfig(): OAuthConfig
21+
{
22+
return OAuthConfig::make()
23+
->setClientId('id')
24+
->setClientSecret('secret')
25+
->setTokenEndpoint('https://auth.external.com/oauth/token');
26+
}
27+
}

0 commit comments

Comments
 (0)