Skip to content

Commit 04a4b39

Browse files
committed
Always include Access-Control-Allow-Origin header in authn related endpoint responses
1 parent 430996a commit 04a4b39

12 files changed

+250
-22
lines changed

src/Controllers/AccessTokenController.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,16 @@ public function token(Request $request): Response
6161
* @psalm-suppress DeprecatedMethod Until we drop support for old public/*.php routes, we need to bridge
6262
* between PSR and Symfony HTTP messages.
6363
*/
64-
return $this->psrHttpBridge->getHttpFoundationFactory()->createResponse(
64+
$response = $this->psrHttpBridge->getHttpFoundationFactory()->createResponse(
6565
$this->__invoke($this->psrHttpBridge->getPsrHttpFactory()->createRequest($request)),
6666
);
67+
68+
// If not already handled, allow CORS (for JS clients).
69+
if (!$response->headers->has('Access-Control-Allow-Origin')) {
70+
$response->headers->set('Access-Control-Allow-Origin', '*');
71+
}
72+
73+
return $response;
6774
} catch (OAuthServerException $exception) {
6875
return $this->errorResponder->forException($exception);
6976
}

src/Controllers/AuthorizationController.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,16 @@ public function authorization(Request $request): Response
106106
* @psalm-suppress DeprecatedMethod Until we drop support for old public/*.php routes, we need to bridge
107107
* between PSR and Symfony HTTP messages.
108108
*/
109-
return $this->psrHttpBridge->getHttpFoundationFactory()->createResponse(
109+
$response = $this->psrHttpBridge->getHttpFoundationFactory()->createResponse(
110110
$this->__invoke($this->psrHttpBridge->getPsrHttpFactory()->createRequest($request)),
111111
);
112+
113+
// If not already handled, allow CORS (for JS clients).
114+
if (!$response->headers->has('Access-Control-Allow-Origin')) {
115+
$response->headers->set('Access-Control-Allow-Origin', '*');
116+
}
117+
118+
return $response;
112119
} catch (OAuthServerException $exception) {
113120
return $this->errorResponder->forException($exception);
114121
}

src/Controllers/ConfigurationDiscoveryController.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@ public function __construct(private readonly OpMetadataService $opMetadataServic
2727

2828
public function __invoke(): JsonResponse
2929
{
30-
return new JsonResponse($this->opMetadataService->getMetadata());
30+
return new JsonResponse(
31+
$this->opMetadataService->getMetadata(),
32+
headers: ['Access-Control-Allow-Origin' => '*'],
33+
);
3134
}
3235
}

src/Controllers/Federation/EntityStatementController.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -310,7 +310,10 @@ protected function prepareEntityStatementResponse(string $entityStatementToken):
310310
return $this->routes->newResponse(
311311
$entityStatementToken,
312312
200,
313-
[HttpHeadersEnum::ContentType->value => ContentTypesEnum::ApplicationEntityStatementJwt->value,],
313+
[
314+
HttpHeadersEnum::ContentType->value => ContentTypesEnum::ApplicationEntityStatementJwt->value,
315+
'Access-Control-Allow-Origin' => '*',
316+
],
314317
);
315318
}
316319
}

src/Controllers/Federation/SubordinateListingsController.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ function (ClientEntityInterface $clientEntity): ?string {
6363

6464
return $this->routes->newJsonResponse(
6565
$subordinateEntityIdList,
66+
headers: ['Access-Control-Allow-Origin' => '*'],
6667
);
6768
}
6869
}

src/Controllers/JwksController.php

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ public function __invoke(): JsonResponse
3838

3939
public function jwks(): Response
4040
{
41-
return $this->psrHttpBridge->getHttpFoundationFactory()->createResponse($this->__invoke());
41+
$response = $this->psrHttpBridge->getHttpFoundationFactory()->createResponse($this->__invoke());
42+
$response->headers->set('Access-Control-Allow-Origin', '*');
43+
return $response;
4244
}
4345
}

src/Controllers/UserInfoController.php

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,16 @@ public function userInfo(Request $request): Response
9292
* @psalm-suppress DeprecatedMethod Until we drop support for old public/*.php routes, we need to bridge
9393
* between PSR and Symfony HTTP messages.
9494
*/
95-
return $this->psrHttpBridge->getHttpFoundationFactory()->createResponse(
95+
$response = $this->psrHttpBridge->getHttpFoundationFactory()->createResponse(
9696
$this->__invoke($this->psrHttpBridge->getPsrHttpFactory()->createRequest($request)),
9797
);
98+
99+
// If not already handled, allow CORS (for JS clients).
100+
if (!$response->headers->has('Access-Control-Allow-Origin')) {
101+
$response->headers->set('Access-Control-Allow-Origin', '*');
102+
}
103+
104+
return $response;
98105
} catch (OAuthServerException $exception) {
99106
return $this->errorResponder->forException($exception);
100107
}

tests/unit/src/Controllers/AccessTokenControllerTest.php

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,9 @@
1616
use SimpleSAML\Module\oidc\Repositories\AllowedOriginRepository;
1717
use SimpleSAML\Module\oidc\Server\AuthorizationServer;
1818
use SimpleSAML\Module\oidc\Services\ErrorResponder;
19+
use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory;
20+
use Symfony\Component\HttpFoundation\Request;
21+
use Symfony\Component\HttpFoundation\ResponseHeaderBag;
1922

2023
/**
2124
* @covers \SimpleSAML\Module\oidc\Controllers\AccessTokenController
@@ -31,6 +34,14 @@ class AccessTokenControllerTest extends TestCase
3134
protected MockObject $requestFactoryMock;
3235
protected MockObject $responseFactoryMock;
3336

37+
protected MockObject $symfonyRequestMock;
38+
39+
protected MockObject $symfonyResponseMock;
40+
41+
protected MockObject $httpFoundationFactoryMock;
42+
43+
protected MockObject $responseHeaderBagMock;
44+
3445

3546
/**
3647
* @throws \Exception
@@ -47,6 +58,15 @@ protected function setUp(): void
4758
$this->responseFactoryMock = $this->createMock(ResponseFactoryInterface::class);
4859
$this->responseFactoryMock->method('createResponse')->willReturn($this->responseMock);
4960
$this->psrHttpBridgeMock->method('getResponseFactory')->willReturn($this->responseFactoryMock);
61+
62+
$this->symfonyRequestMock = $this->createMock(Request::class);
63+
$this->symfonyResponseMock = $this->createMock(\Symfony\Component\HttpFoundation\Response::class);
64+
$this->responseHeaderBagMock = $this->createMock(ResponseHeaderBag::class);
65+
$this->symfonyResponseMock->headers = $this->responseHeaderBagMock;
66+
67+
$this->httpFoundationFactoryMock = $this->createMock(HttpFoundationFactory::class);
68+
$this->httpFoundationFactoryMock->method('createResponse')->willReturn($this->symfonyResponseMock);
69+
$this->psrHttpBridgeMock->method('getHttpFoundationFactory')->willReturn($this->httpFoundationFactoryMock);
5070
}
5171

5272
protected function mock(): AccessTokenController
@@ -100,6 +120,20 @@ public function testItHandlesCorsRequest(): void
100120
$this->mock()->__invoke($this->serverRequestMock);
101121
}
102122

123+
public function testItAlwaysReturnsAccessControlAllowOrigin(): void
124+
{
125+
$this->authorizationServerMock
126+
->expects($this->once())
127+
->method('respondToAccessTokenRequest')
128+
->willReturn($this->responseMock);
129+
130+
$this->responseHeaderBagMock->expects($this->once())
131+
->method('set')
132+
->with('Access-Control-Allow-Origin', '*');
133+
134+
$this->mock()->token($this->symfonyRequestMock);
135+
}
136+
103137
public function testItUsesRequestTrait(): void
104138
{
105139
$this->assertContains(RequestTrait::class, class_uses(AccessTokenController::class));

tests/unit/src/Controllers/AuthorizationControllerTest.php

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
use SimpleSAML\Module\oidc\Services\AuthenticationService;
2222
use SimpleSAML\Module\oidc\Services\ErrorResponder;
2323
use SimpleSAML\Module\oidc\Services\LoggerService;
24+
use Symfony\Bridge\PsrHttpMessage\Factory\HttpFoundationFactory;
25+
use Symfony\Component\HttpFoundation\Request;
26+
use Symfony\Component\HttpFoundation\ResponseHeaderBag;
2427

2528
/**
2629
* @covers \SimpleSAML\Module\oidc\Controllers\AuthorizationController
@@ -57,6 +60,11 @@ class AuthorizationControllerTest extends TestCase
5760

5861
protected static array $sampleRequestedAcrs = ['values' => ['1', '0'], 'essential' => false];
5962

63+
protected MockObject $symfonyRequestMock;
64+
protected MockObject $symfonyResponseMock;
65+
protected MockObject $responseHeaderBagMock;
66+
protected MockObject $httpFoundationFactoryMock;
67+
6068
/**
6169
* @throws \Exception
6270
*/
@@ -84,6 +92,15 @@ public function setUp(): void
8492
],
8593
'authorizationRequest' => $this->authorizationRequestMock,
8694
];
95+
96+
$this->symfonyRequestMock = $this->createMock(Request::class);
97+
$this->symfonyResponseMock = $this->createMock(\Symfony\Component\HttpFoundation\Response::class);
98+
$this->responseHeaderBagMock = $this->createMock(ResponseHeaderBag::class);
99+
$this->symfonyResponseMock->headers = $this->responseHeaderBagMock;
100+
101+
$this->httpFoundationFactoryMock = $this->createMock(HttpFoundationFactory::class);
102+
$this->httpFoundationFactoryMock->method('createResponse')->willReturn($this->symfonyResponseMock);
103+
$this->psrHttpBridgeMock->method('getHttpFoundationFactory')->willReturn($this->httpFoundationFactoryMock);
87104
}
88105

89106
public static function queryParameterValues(): array
@@ -98,6 +115,31 @@ public static function queryParameterValues(): array
98115
];
99116
}
100117

118+
protected function mock(
119+
?AuthenticationService $authenticationService = null,
120+
?AuthorizationServer $authorizationServer = null,
121+
?ModuleConfig $moduleConfig = null,
122+
?LoggerService $loggerService = null,
123+
?PsrHttpBridge $psrHttpBridge = null,
124+
?ErrorResponder $errorResponder = null,
125+
): AuthorizationController {
126+
$authenticationService ??= $this->authenticationServiceStub;
127+
$authorizationServer ??= $this->authorizationServerStub;
128+
$moduleConfig ??= $this->moduleConfigStub;
129+
$loggerService ??= $this->loggerServiceMock;
130+
$psrHttpBridge ??= $this->psrHttpBridgeMock;
131+
$errorResponder ??= $this->errorResponderMock;
132+
133+
return new AuthorizationController(
134+
$authenticationService,
135+
$authorizationServer,
136+
$moduleConfig,
137+
$loggerService,
138+
$psrHttpBridge,
139+
$errorResponder,
140+
);
141+
}
142+
101143
/**
102144
* @throws \SimpleSAML\Error\AuthSource
103145
* @throws \SimpleSAML\Error\BadRequest
@@ -128,6 +170,7 @@ public function testReturnsResponseWhenInvoked(array $queryParameters): void
128170
->method('getAuthorizationRequestFromState')
129171
->willReturn($this->authorizationRequestMock);
130172

173+
// TODO mivanci Move to mock() method.
131174
$controller = new AuthorizationController(
132175
$this->authenticationServiceStub,
133176
$this->authorizationServerStub,
@@ -486,4 +529,17 @@ public function testValidateAcrLogsWarningIfNoAcrsConfigured(): void
486529
$this->errorResponderMock,
487530
))($this->serverRequestStub);
488531
}
532+
533+
public function testItAlwaysReturnsAccessControlAllowOrigin(): void
534+
{
535+
$this->authorizationServerStub
536+
->method('completeAuthorizationRequest')
537+
->willReturn($this->responseStub);
538+
539+
$this->responseHeaderBagMock->expects($this->once())
540+
->method('set')
541+
->with('Access-Control-Allow-Origin', '*');
542+
543+
$this->mock()->authorization($this->symfonyRequestMock);
544+
}
489545
}

tests/unit/src/Controllers/ConfigurationDiscoveryControllerTest.php

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,38 +30,46 @@ class ConfigurationDiscoveryControllerTest extends TestCase
3030
'end_session_endpoint' => 'http://localhost/end-session',
3131
];
3232

33-
protected MockObject $oidcOpenIdProviderMetadataServiceMock;
33+
protected MockObject $opMetadataServiceMock;
3434
protected MockObject $serverRequestMock;
3535

3636
/**
3737
* @throws \Exception
3838
*/
3939
protected function setUp(): void
4040
{
41-
$this->oidcOpenIdProviderMetadataServiceMock = $this->createMock(OpMetadataService::class);
42-
$this->oidcOpenIdProviderMetadataServiceMock->method('getMetadata')->willReturn(self::OIDC_OP_METADATA);
41+
$this->opMetadataServiceMock = $this->createMock(OpMetadataService::class);
42+
$this->opMetadataServiceMock->method('getMetadata')->willReturn(self::OIDC_OP_METADATA);
4343

4444
$this->serverRequestMock = $this->createMock(ServerRequest::class);
4545
}
4646

47+
protected function mock(
48+
?OpMetadataService $opMetadataService = null,
49+
): ConfigurationDiscoveryController {
50+
$opMetadataService ??= $this->opMetadataServiceMock;
51+
52+
return new ConfigurationDiscoveryController($opMetadataService);
53+
}
54+
4755
public function testItIsInitializable(): void
4856
{
4957
$this->assertInstanceOf(
5058
ConfigurationDiscoveryController::class,
51-
new ConfigurationDiscoveryController($this->oidcOpenIdProviderMetadataServiceMock),
59+
$this->mock(),
5260
);
5361
}
5462

55-
protected function getStubbedInstance(): ConfigurationDiscoveryController
56-
{
57-
return new ConfigurationDiscoveryController($this->oidcOpenIdProviderMetadataServiceMock);
58-
}
59-
6063
public function testItReturnsOpenIdConnectConfiguration(): void
6164
{
6265
$this->assertSame(
63-
json_decode($this->getStubbedInstance()->__invoke()->getContent(), true),
66+
json_decode($this->mock()->__invoke()->getContent(), true),
6467
self::OIDC_OP_METADATA,
6568
);
6669
}
70+
71+
public function testItAlwaysReturnsAccessControlAllowOrigin(): void
72+
{
73+
$this->assertTrue($this->mock()->__invoke()->headers->has('Access-Control-Allow-Origin'),);
74+
}
6775
}

0 commit comments

Comments
 (0)