Skip to content

Commit 4c40ede

Browse files
committed
WIP coverage
1 parent de52ba9 commit 4c40ede

File tree

9 files changed

+430
-33
lines changed

9 files changed

+430
-33
lines changed

src/Controllers/Admin/ClientController.php

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ public function resetSecret(Request $request): Response
121121
$this->logger->info($message, $client->getState());
122122
$this->sessionMessagesService->addMessage($message);
123123

124-
return $this->routes->getRedirectResponseToModuleUrl(
124+
return $this->routes->newRedirectResponseToModuleUrl(
125125
RoutesEnum::AdminClientsShow->value,
126126
[ParametersEnum::ClientId->value => $client->getIdentifier()],
127127
);
@@ -148,7 +148,7 @@ public function delete(Request $request): Response
148148
$this->logger->warning($message, $client->getState());
149149
$this->sessionMessagesService->addMessage($message);
150150

151-
return $this->routes->getRedirectResponseToModuleUrl(
151+
return $this->routes->newRedirectResponseToModuleUrl(
152152
RoutesEnum::AdminClients->value,
153153
);
154154
}
@@ -202,7 +202,7 @@ public function add(): Response
202202
$this->logger->info($message, $client->getState());
203203
$this->sessionMessagesService->addMessage($message);
204204

205-
return $this->routes->getRedirectResponseToModuleUrl(
205+
return $this->routes->newRedirectResponseToModuleUrl(
206206
RoutesEnum::AdminClientsShow->value,
207207
[ParametersEnum::ClientId->value => $client->getIdentifier()],
208208
);
@@ -276,7 +276,7 @@ public function edit(Request $request): Response
276276

277277
$this->sessionMessagesService->addMessage(Translate::noop('Client has been updated.'));
278278

279-
return $this->routes->getRedirectResponseToModuleUrl(
279+
return $this->routes->newRedirectResponseToModuleUrl(
280280
RoutesEnum::AdminClientsShow->value,
281281
[ParametersEnum::ClientId->value => $originalClient->getIdentifier()],
282282
);

src/Controllers/Admin/ConfigController.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,14 +45,14 @@ public function runMigrations(): Response
4545
if ($this->databaseMigration->isMigrated()) {
4646
$message = Translate::noop('Database is already migrated.');
4747
$this->sessionMessagesService->addMessage($message);
48-
return $this->routes->getRedirectResponseToModuleUrl(RoutesEnum::AdminMigrations->value);
48+
return $this->routes->newRedirectResponseToModuleUrl(RoutesEnum::AdminMigrations->value);
4949
}
5050

5151
$this->databaseMigration->migrate();
5252
$message = Translate::noop('Database migrated successfully.');
5353
$this->sessionMessagesService->addMessage($message);
5454

55-
return $this->routes->getRedirectResponseToModuleUrl(RoutesEnum::AdminMigrations->value);
55+
return $this->routes->newRedirectResponseToModuleUrl(RoutesEnum::AdminMigrations->value);
5656
}
5757

5858
public function protocolSettings(): Response

src/Controllers/Federation/EntityStatementController.php

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
use SimpleSAML\Module\oidc\Services\JsonWebTokenBuilderService;
1414
use SimpleSAML\Module\oidc\Services\OpMetadataService;
1515
use SimpleSAML\Module\oidc\Utils\FederationCache;
16+
use SimpleSAML\Module\oidc\Utils\Routes;
1617
use SimpleSAML\OpenID\Codebooks\ClaimsEnum;
1718
use SimpleSAML\OpenID\Codebooks\ClientRegistrationTypesEnum;
1819
use SimpleSAML\OpenID\Codebooks\ContentTypesEnum;
@@ -21,7 +22,6 @@
2122
use SimpleSAML\OpenID\Codebooks\HttpHeadersEnum;
2223
use SimpleSAML\OpenID\Codebooks\JwtTypesEnum;
2324
use SimpleSAML\OpenID\Federation;
24-
use Symfony\Component\HttpFoundation\JsonResponse;
2525
use Symfony\Component\HttpFoundation\Request;
2626
use Symfony\Component\HttpFoundation\Response;
2727

@@ -40,6 +40,7 @@ public function __construct(
4040
private readonly OpMetadataService $opMetadataService,
4141
private readonly ClientRepository $clientRepository,
4242
private readonly Helpers $helpers,
43+
private readonly Routes $routes,
4344
private readonly Federation $federation,
4445
private readonly ?FederationCache $federationCache,
4546
) {
@@ -171,7 +172,7 @@ public function fetch(Request $request): Response
171172
$subject = $request->query->get(ClaimsEnum::Sub->value);
172173

173174
if (empty($subject)) {
174-
return $this->prepareJsonErrorResponse(
175+
return $this->routes->newJsonErrorResponse(
175176
ErrorsEnum::InvalidRequest->value,
176177
sprintf('Missing parameter %s', ClaimsEnum::Sub->value),
177178
400,
@@ -193,7 +194,7 @@ public function fetch(Request $request): Response
193194

194195
$client = $this->clientRepository->findByEntityIdentifier($subject);
195196
if (empty($client)) {
196-
return $this->prepareJsonErrorResponse(
197+
return $this->routes->newJsonErrorResponse(
197198
ErrorsEnum::NotFound->value,
198199
sprintf('Subject not found (%s)', $subject),
199200
404,
@@ -202,7 +203,7 @@ public function fetch(Request $request): Response
202203

203204
$jwks = $client->getFederationJwks();
204205
if (empty($jwks)) {
205-
return $this->prepareJsonErrorResponse(
206+
return $this->routes->newJsonErrorResponse(
206207
ErrorsEnum::InvalidClient->value,
207208
sprintf('Subject does not contain JWKS claim (%s)', $subject),
208209
401,
@@ -263,21 +264,10 @@ public function fetch(Request $request): Response
263264

264265
protected function prepareEntityStatementResponse(string $entityStatementToken): Response
265266
{
266-
return new Response(
267+
return $this->routes->newResponse(
267268
$entityStatementToken,
268269
200,
269270
[HttpHeadersEnum::ContentType->value => ContentTypesEnum::ApplicationEntityStatementJwt->value,],
270271
);
271272
}
272-
273-
protected function prepareJsonErrorResponse(string $error, string $description, int $httpCode = 500): JsonResponse
274-
{
275-
return new JsonResponse(
276-
[
277-
'error' => $error,
278-
'error_description' => $description,
279-
],
280-
$httpCode,
281-
);
282-
}
283273
}

src/Utils/Routes.php

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88
use SimpleSAML\Module\oidc\Codebooks\ParametersEnum;
99
use SimpleSAML\Module\oidc\Codebooks\RoutesEnum;
1010
use SimpleSAML\Module\oidc\ModuleConfig;
11+
use Symfony\Component\HttpFoundation\JsonResponse;
1112
use Symfony\Component\HttpFoundation\RedirectResponse;
13+
use Symfony\Component\HttpFoundation\Response;
1214

1315
class Routes
1416
{
@@ -25,7 +27,11 @@ public function getModuleUrl(string $resource = '', array $parameters = []): str
2527
return $this->sspBridge->module()->getModuleUrl($resource, $parameters);
2628
}
2729

28-
public function getRedirectResponseToModuleUrl(
30+
/*****************************************************************************************************************
31+
* Response factory methods.
32+
****************************************************************************************************************/
33+
34+
public function newRedirectResponseToModuleUrl(
2935
string $resource = '',
3036
array $parameters = [],
3137
int $status = 302,
@@ -38,8 +44,38 @@ public function getRedirectResponseToModuleUrl(
3844
);
3945
}
4046

47+
public function newResponse(
48+
?string $content = '',
49+
int $status = 200,
50+
array $headers = [],
51+
): Response {
52+
return new Response($content, $status, $headers);
53+
}
54+
55+
public function newJsonResponse(
56+
mixed $data = null,
57+
int $status = 200,
58+
array $headers = [],
59+
bool $json = false,
60+
): JsonResponse {
61+
return new JsonResponse($data, $status, $headers, $json);
62+
}
63+
64+
public function newJsonErrorResponse(
65+
string $error,
66+
string $description,
67+
int $httpCode = 500,
68+
array $headers = [],
69+
): JsonResponse {
70+
return $this->newJsonResponse(
71+
['error' => $error, 'error_description' => $description],
72+
$httpCode,
73+
$headers,
74+
);
75+
}
76+
4177
/*****************************************************************************************************************
42-
* Admin area
78+
* Admin area URLs.
4379
****************************************************************************************************************/
4480

4581
public function urlAdminConfigProtocol(array $parameters = []): string
@@ -99,7 +135,7 @@ public function urlAdminClientsDelete(string $clientId, array $parameters = []):
99135
}
100136

101137
/*****************************************************************************************************************
102-
* OpenID Connect
138+
* OpenID Connect URLs.
103139
****************************************************************************************************************/
104140

105141
public function urlConfiguration(array $parameters = []): string
@@ -133,7 +169,7 @@ public function urlEndSession(array $parameters = []): string
133169
}
134170

135171
/*****************************************************************************************************************
136-
* OpenID Federation
172+
* OpenID Federation URLs.
137173
****************************************************************************************************************/
138174

139175
public function urlFederationConfiguration(array $parameters = []): string

tests/config/config.php

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@
8787
* qualified path to a file containing the certificate or key in PEM
8888
* format, such as 'cert.pem' or '/path/to/cert.pem'. If the path is
8989
* relative, it will be searched for in the directory defined by the
90-
* '' parameter below. When 'certdir' is specified as a relative
90+
* 'certdir' parameter below. When 'certdir' is specified as a relative
9191
* path, it will be interpreted as relative to the SimpleSAMLphp root
9292
* directory. Note that locations with no prefix included will be treated
9393
* as file locations.
@@ -564,6 +564,7 @@
564564
'core' => true,
565565
'admin' => true,
566566
'saml' => true,
567+
'oidc' => true,
567568
],
568569

569570

@@ -630,6 +631,8 @@
630631
* Set this to TRUE if the user only accesses your service
631632
* through https. If the user can access the service through
632633
* both http and https, this must be set to FALSE.
634+
*
635+
* If unset, SimpleSAMLphp will try to automatically determine the right value
633636
*/
634637
'session.cookie.secure' => true,
635638

@@ -994,12 +997,6 @@
994997
// Adopts language from attribute to use in UI
995998
30 => 'core:LanguageAdaptor',
996999

997-
45 => [
998-
'class' => 'core:StatisticsWithAttribute',
999-
'attributename' => 'realm',
1000-
'type' => 'saml20-idp-SSO',
1001-
],
1002-
10031000
/* When called without parameters, it will fallback to filter attributes 'the old way'
10041001
* by checking the 'attributes' parameter in metadata on IdP hosted and SP remote.
10051002
*/
Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace SimpleSAML\Test\Module\oidc\unit\Controllers\Admin;
6+
7+
use PHPUnit\Framework\Attributes\CoversClass;
8+
use PHPUnit\Framework\MockObject\MockObject;
9+
use PHPUnit\Framework\TestCase;
10+
use SimpleSAML\Module\oidc\Admin\Authorization;
11+
use SimpleSAML\Module\oidc\Controllers\Admin\ConfigController;
12+
use SimpleSAML\Module\oidc\Factories\TemplateFactory;
13+
use SimpleSAML\Module\oidc\ModuleConfig;
14+
use SimpleSAML\Module\oidc\Services\DatabaseMigration;
15+
use SimpleSAML\Module\oidc\Services\SessionMessagesService;
16+
use SimpleSAML\Module\oidc\Utils\Routes;
17+
use SimpleSAML\OpenID\Federation;
18+
use SimpleSAML\OpenID\Federation\Factories\TrustMarkFactory;
19+
20+
#[CoversClass(ConfigController::class)]
21+
class ConfigControllerTest extends TestCase
22+
{
23+
protected MockObject $moduleConfigMock;
24+
protected MockObject $templateFactoryMock;
25+
protected MockObject $authorizationMock;
26+
protected MockObject $databaseMigrationMock;
27+
protected MockObject $sessionMessagesServiceMock;
28+
protected MockObject $federationMock;
29+
protected MockObject $routesMock;
30+
protected MockObject $trustMarkFactoryMock;
31+
32+
protected function setUp(): void
33+
{
34+
$this->moduleConfigMock = $this->createMock(ModuleConfig::class);
35+
$this->templateFactoryMock = $this->createMock(TemplateFactory::class);
36+
$this->authorizationMock = $this->createMock(Authorization::class);
37+
$this->databaseMigrationMock = $this->createMock(DatabaseMigration::class);
38+
$this->sessionMessagesServiceMock = $this->createMock(SessionMessagesService::class);
39+
$this->federationMock = $this->createMock(Federation::class);
40+
$this->routesMock = $this->createMock(Routes::class);
41+
42+
$this->trustMarkFactoryMock = $this->createMock(TrustMarkFactory::class);
43+
$this->federationMock->method('trustMarkFactory')->willReturn($this->trustMarkFactoryMock);
44+
}
45+
46+
public function sut(
47+
?ModuleConfig $moduleConfig = null,
48+
?TemplateFactory $templateFactory = null,
49+
?Authorization $authorization = null,
50+
?DatabaseMigration $databaseMigration = null,
51+
?SessionMessagesService $sessionMessagesService = null,
52+
?Federation $federation = null,
53+
?Routes $routes = null,
54+
): ConfigController {
55+
$moduleConfig ??= $this->moduleConfigMock;
56+
$templateFactory ??= $this->templateFactoryMock;
57+
$authorization ??= $this->authorizationMock;
58+
$databaseMigration ??= $this->databaseMigrationMock;
59+
$sessionMessagesService ??= $this->sessionMessagesServiceMock;
60+
$federation ??= $this->federationMock;
61+
$routes ??= $this->routesMock;
62+
63+
return new ConfigController(
64+
$moduleConfig,
65+
$templateFactory,
66+
$authorization,
67+
$databaseMigration,
68+
$sessionMessagesService,
69+
$federation,
70+
$routes,
71+
);
72+
}
73+
74+
public function testCanCreateInstance(): void
75+
{
76+
$this->authorizationMock->expects($this->once())->method('requireAdmin');
77+
$this->assertInstanceOf(ConfigController::class, $this->sut());
78+
}
79+
80+
public function testCanShowMigrationsScreen(): void
81+
{
82+
$this->templateFactoryMock->expects($this->once())->method('build')
83+
->with('oidc:config/migrations.twig');
84+
85+
$this->sut()->migrations();
86+
}
87+
88+
public function testCanRunMigrations(): void
89+
{
90+
$this->databaseMigrationMock->expects($this->once())->method('migrate');
91+
$this->sessionMessagesServiceMock->expects($this->once())->method('addMessage')
92+
->with($this->stringContains('migrated'));
93+
94+
$this->sut()->runMigrations();
95+
}
96+
97+
public function testWontRunMigrationsIfAlreadyMigrated(): void
98+
{
99+
$this->databaseMigrationMock->expects($this->once())->method('isMigrated')->willReturn(true);
100+
$this->databaseMigrationMock->expects($this->never())->method('migrate');
101+
102+
$this->sut()->runMigrations();
103+
}
104+
105+
public function testCanShowProtocolSettingsScreen(): void
106+
{
107+
$this->templateFactoryMock->expects($this->once())->method('build')
108+
->with('oidc:config/protocol.twig');
109+
110+
$this->sut()->protocolSettings();
111+
}
112+
113+
public function testCanShowFederationSettingsScreen(): void
114+
{
115+
$this->templateFactoryMock->expects($this->once())->method('build')
116+
->with('oidc:config/federation.twig');
117+
118+
$this->sut()->federationSettings();
119+
}
120+
121+
public function testCanIncludeTrustMarksInFederationSettings(): void
122+
{
123+
$this->moduleConfigMock->method('getFederationTrustMarkTokens')->willReturn(['token']);
124+
$this->trustMarkFactoryMock->expects($this->once())->method('fromToken')
125+
->with($this->stringContains('token'));
126+
127+
$this->templateFactoryMock->expects($this->once())->method('build')
128+
->with('oidc:config/federation.twig');
129+
130+
$this->sut()->federationSettings();
131+
}
132+
}

0 commit comments

Comments
 (0)