Skip to content

Commit 1cc4102

Browse files
mulldugmulldug
committed
fix(oauth2): use exact matching for JS client origin validation
Replace str_contains substring check with in_array exact match to prevent crafted short origins from satisfying the allowed-origins guard on JS_CLIENT tokens. Splits the space-delimited allowed_origins string from the introspection response into individual entries before comparing.
1 parent 12a29e2 commit 1cc4102

3 files changed

Lines changed: 190 additions & 9 deletions

File tree

‎.gitignore‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ phpunit.xml
3232
.phpunit.cache/
3333
docker-compose/mysql/config/*.sql
3434
docker-compose/mysql/model/*.sql
35+
docker-compose.override.yml
3536
package.xml
3637
.env.dev
3738
rector.php

‎app/Http/Middleware/OAuth2BearerAccessTokenRequestValidator.php‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -171,16 +171,22 @@ public function handle($request, Closure $next)
171171
);
172172
throw new InvalidGrantTypeException(OAuth2Protocol::OAuth2Protocol_Error_InvalidToken);
173173
}
174-
if (
175-
$token_info->getApplicationType() === 'JS_CLIENT'
176-
&& (is_null($origin) || empty($origin)|| str_contains($token_info->getAllowedOrigins(), $origin) === false )
177-
) {
174+
if ($token_info->getApplicationType() === 'JS_CLIENT') {
178175
//check origins
179-
throw new OAuth2ResourceServerException(
180-
403,
181-
OAuth2Protocol::OAuth2Protocol_Error_UnauthorizedClient,
182-
sprintf('invalid origin %s - allowed ones (%s)', $origin, $token_info->getAllowedOrigins())
183-
);
176+
$allowedOrigins = array_filter(array_map(function ($o) {
177+
$o = trim($o);
178+
if ($o === '') return '';
179+
try { $o = (new Normalizer($o))->normalize(); } catch (\Throwable $e) {}
180+
return rtrim($o, '/');
181+
}, explode(' ', $token_info->getAllowedOrigins() ?? '')));
182+
183+
if (is_null($origin) || empty($origin) || !in_array(rtrim($origin, '/'), $allowedOrigins, true)) {
184+
throw new OAuth2ResourceServerException(
185+
403,
186+
OAuth2Protocol::OAuth2Protocol_Error_UnauthorizedClient,
187+
sprintf('invalid origin %s - allowed ones (%s)', $origin, $token_info->getAllowedOrigins())
188+
);
189+
}
184190
}
185191
//check scopes
186192
Log::debug('OAuth2BearerAccessTokenRequestValidator::handle checking token scopes ...');

‎tests/Unit/OAuth2BearerAccessTokenRequestValidatorTest.php‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
<?php namespace Tests\Unit;
2+
/**
3+
* Copyright 2026 OpenStack Foundation
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
* http://www.apache.org/licenses/LICENSE-2.0
8+
* Unless required by applicable law or agreed to in writing, software
9+
* distributed under the License is distributed on an "AS IS" BASIS,
10+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
11+
* See the License for the specific language governing permissions and
12+
* limitations under the License.
13+
**/
14+
15+
use App\Http\Middleware\OAuth2BearerAccessTokenRequestValidator;
16+
use App\Models\ResourceServer\IAccessTokenService;
17+
use App\Models\ResourceServer\IApiEndpoint;
18+
use App\Models\ResourceServer\IApiEndpointRepository;
19+
use Illuminate\Http\JsonResponse;
20+
use Illuminate\Http\Request;
21+
use Illuminate\Support\Facades\Log;
22+
use Illuminate\Support\Facades\Route;
23+
use models\oauth2\IResourceServerContext;
24+
use Tests\TestCase;
25+
26+
/**
27+
* Subclass that bypasses getallheaders()/SERVER parsing by returning a fixed header map.
28+
* $fixedHeaders must be assigned before parent::__construct() because the parent ctor
29+
* calls $this->getHeaders() immediately.
30+
*/
31+
class TestableBearerValidator extends OAuth2BearerAccessTokenRequestValidator
32+
{
33+
private array $fixedHeaders;
34+
35+
public function __construct(
36+
array $fixedHeaders,
37+
IResourceServerContext $context,
38+
IApiEndpointRepository $endpoint_repository,
39+
IAccessTokenService $token_service
40+
) {
41+
$this->fixedHeaders = $fixedHeaders;
42+
parent::__construct($context, $endpoint_repository, $token_service);
43+
}
44+
45+
protected function getHeaders(): array
46+
{
47+
return $this->fixedHeaders;
48+
}
49+
}
50+
51+
/**
52+
* Class OAuth2BearerAccessTokenRequestValidatorTest
53+
*
54+
* Verifies that the JS_CLIENT origin check correctly accepts normalized URLs
55+
* and rejects bare hostnames or requests with no Origin header.
56+
*/
57+
final class OAuth2BearerAccessTokenRequestValidatorTest extends TestCase
58+
{
59+
private const TOKEN = 'test-bearer-token';
60+
private const HOST = 'example.com';
61+
private const ALLOWED_ORIGINS = 'https://example.com https://foo.bar';
62+
63+
private IResourceServerContext $context;
64+
private IApiEndpointRepository $endpointRepo;
65+
private IAccessTokenService $tokenService;
66+
67+
protected function setUp(): void
68+
{
69+
parent::setUp();
70+
71+
Route::get('/api/test', fn() => 'ok');
72+
73+
$this->context = $this->createMock(IResourceServerContext::class);
74+
75+
$endpoint = $this->createMock(IApiEndpoint::class);
76+
$endpoint->method('isActive')->willReturn(true);
77+
$endpoint->method('getScopesNames')->willReturn(['openid']);
78+
79+
$this->endpointRepo = $this->createMock(IApiEndpointRepository::class);
80+
$this->endpointRepo->method('getApiEndpointByUrlAndMethod')->willReturn($endpoint);
81+
82+
// AccessToken is final, so use an anonymous stub instead of createMock().
83+
$tokenStub = new class {
84+
public function getLifetime() { return 3600; }
85+
public function getAudience() { return 'example.com'; }
86+
public function getApplicationType() { return 'JS_CLIENT'; }
87+
public function getAllowedOrigins(): ?string { return 'https://example.com https://foo.bar'; }
88+
public function getScope() { return 'openid'; }
89+
public function getClientId() { return 'test-client'; }
90+
public function getUserId(): ?int { return null; }
91+
public function getAllowedReturnUris() { return ''; }
92+
};
93+
94+
$this->tokenService = $this->createMock(IAccessTokenService::class);
95+
$this->tokenService->method('get')->willReturn($tokenStub);
96+
97+
Log::shouldReceive('debug')->zeroOrMoreTimes();
98+
Log::shouldReceive('warning')->zeroOrMoreTimes();
99+
Log::shouldReceive('info')->zeroOrMoreTimes();
100+
Log::shouldReceive('error')->zeroOrMoreTimes();
101+
}
102+
103+
// -------------------------------------------------------------------------
104+
105+
private function buildValidator(string $originHeader = ''): TestableBearerValidator
106+
{
107+
$headers = ['authorization' => 'Bearer ' . self::TOKEN];
108+
if ($originHeader !== '') {
109+
$headers['origin'] = $originHeader;
110+
}
111+
return new TestableBearerValidator(
112+
$headers,
113+
$this->context,
114+
$this->endpointRepo,
115+
$this->tokenService
116+
);
117+
}
118+
119+
private function buildRequest(string $originHeader = ''): Request
120+
{
121+
$server = ['HTTP_HOST' => self::HOST];
122+
if ($originHeader !== '') {
123+
$server['HTTP_ORIGIN'] = $originHeader;
124+
}
125+
return Request::create('/api/test', 'GET', [], [], [], $server);
126+
}
127+
128+
private function next(): \Closure
129+
{
130+
return fn($req) => new JsonResponse(['ok' => true], 200);
131+
}
132+
133+
// -------------------------------------------------------------------------
134+
135+
public function test_exact_origin_url_is_accepted(): void
136+
{
137+
$this->context->expects($this->once())->method('setAuthorizationContext');
138+
139+
$response = $this->buildValidator('https://example.com')
140+
->handle($this->buildRequest('https://example.com'), $this->next());
141+
142+
$this->assertEquals(200, $response->getStatusCode());
143+
}
144+
145+
public function test_trailing_slash_origin_is_accepted(): void
146+
{
147+
$this->context->expects($this->once())->method('setAuthorizationContext');
148+
149+
$response = $this->buildValidator('https://example.com/')
150+
->handle($this->buildRequest('https://example.com/'), $this->next());
151+
152+
$this->assertEquals(200, $response->getStatusCode());
153+
}
154+
155+
public function test_bare_hostname_without_scheme_is_rejected(): void
156+
{
157+
$this->context->expects($this->never())->method('setAuthorizationContext');
158+
159+
$response = $this->buildValidator('example')
160+
->handle($this->buildRequest('example'), $this->next());
161+
162+
$this->assertEquals(403, $response->getStatusCode());
163+
}
164+
165+
public function test_missing_origin_is_rejected(): void
166+
{
167+
$this->context->expects($this->never())->method('setAuthorizationContext');
168+
169+
$response = $this->buildValidator()
170+
->handle($this->buildRequest(), $this->next());
171+
172+
$this->assertEquals(403, $response->getStatusCode());
173+
}
174+
}

0 commit comments

Comments
 (0)