Skip to content

Commit df556da

Browse files
cicnavicicnavi
committed
Add coverage
1 parent 4961aaa commit df556da

18 files changed

+840
-90
lines changed

‎UPGRADE.md‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,11 @@
1212
# Version 5 to 6
1313

1414
## New features
15-
15+
- Caching support for OIDC protocol artifacts like Access Tokens, Authorization Codes, Refresh Tokens, but also
16+
client and user data. The cache layer stands in front of the database store, so it can improve performance, especially
17+
in cases of sudden surge of users trying to authenticate. Implementation is based on Symfony Cache component, so any
18+
compatible Symfony cache adapter can be used. Check the module config file for more information on how to set the
19+
protocol cache.
1620
- OpenID capabilities
1721
- New federation endpoints:
1822
- endpoint for issuing configuration entity statement (statement about itself)
@@ -40,7 +44,7 @@ https://openid.net/specs/openid-connect-core-1_0.html#ClientAuthentication
4044

4145
- (optional) Issuer - you can now override the issuer (OP identifier). If not set, it falls back to current scheme, host
4246
and optionally a port (as in all previous module versions).
43-
- protocol caching adapter and its arguments
47+
- (optional) Protocol caching adapter and its arguments
4448
- (optional) OpenID Federation related options (needed if federation capabilities are to be used):
4549
- enabled or disabled federation capabilities
4650
- valid trust anchors

‎src/Helpers/Client.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ public function getFromRequest(
3030
$clientId = empty($params['client_id']) ? null : (string)$params['client_id'];
3131

3232
if (!is_string($clientId)) {
33-
throw new BadRequest('Client id is missing.');
33+
throw new BadRequest('Client ID is missing.');
3434
}
3535

3636
$client = $clientRepository->findById($clientId);

‎src/Helpers/Random.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,10 @@ public function getIdentifier(int $length = 40): string
2020

2121
try {
2222
return bin2hex(random_bytes($length));
23+
// @codeCoverageIgnoreStart
2324
} catch (Throwable $e) {
2425
throw OidcServerException::serverError('Could not generate a random string', $e);
2526
}
27+
// @codeCoverageIgnoreEnd
2628
}
2729
}

‎src/Repositories/ClientRepository.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,9 +119,11 @@ public function findById(string $clientIdentifier, ?string $owner = null): ?Clie
119119

120120
$row = current($rows);
121121

122+
// @codeCoverageIgnoreStart
122123
if (!is_array($row)) {
123124
return null;
124125
}
126+
// @codeCoverageIgnoreEnd
125127

126128
$clientEntity = $this->clientEntityFactory->fromState($row);
127129

@@ -171,9 +173,11 @@ public function findByEntityIdentifier(string $entityIdentifier, ?string $owner
171173

172174
$row = current($rows);
173175

176+
// @codeCoverageIgnoreStart
174177
if (!is_array($row)) {
175178
return null;
176179
}
180+
// @codeCoverageIgnoreEnd
177181

178182
$clientEntity = $this->clientEntityFactory->fromState($row);
179183

‎tests/config/module_oidc.php‎

Lines changed: 59 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@
2323
ModuleConfig::OPTION_TOKEN_REFRESH_TOKEN_TTL => 'P1M',
2424
ModuleConfig::OPTION_TOKEN_ACCESS_TOKEN_TTL => 'PT1H',
2525

26-
ModuleConfig::OPTION_CRON_TAG => 'hourly',
27-
2826
ModuleConfig::OPTION_TOKEN_SIGNER => Sha256::class,
2927

3028
ModuleConfig::OPTION_AUTH_SOURCE => 'default-sp',
@@ -44,15 +42,70 @@
4442

4543
ModuleConfig::OPTION_AUTH_FORCED_ACR_VALUE_FOR_COOKIE_AUTHENTICATION => null,
4644

47-
ModuleConfig::OPTION_FEDERATION_TOKEN_SIGNER => Sha256::class,
45+
ModuleConfig::OPTION_AUTH_PROCESSING_FILTERS => [
46+
],
47+
48+
ModuleConfig::OPTION_PROTOCOL_CACHE_ADAPTER => \Symfony\Component\Cache\Adapter\ArrayAdapter::class,
49+
ModuleConfig::OPTION_PROTOCOL_CACHE_ADAPTER_ARGUMENTS => [],
50+
ModuleConfig::OPTION_PROTOCOL_USER_ENTITY_CACHE_DURATION => null,
51+
ModuleConfig::OPTION_PROTOCOL_CLIENT_ENTITY_CACHE_DURATION => 'PT10M',
52+
53+
ModuleConfig::OPTION_CRON_TAG => 'hourly',
54+
55+
ModuleConfig::OPTION_ADMIN_UI_PERMISSIONS => [
56+
'attribute' => 'eduPersonEntitlement',
57+
'client' => ['urn:example:oidc:manage:client'],
58+
],
59+
60+
ModuleConfig::OPTION_ADMIN_UI_PAGINATION_ITEMS_PER_PAGE => 20,
61+
62+
ModuleConfig::OPTION_FEDERATION_ENABLED => false,
63+
64+
ModuleConfig::OPTION_FEDERATION_TRUST_ANCHORS => [
65+
// phpcs:ignore
66+
'https://ta.example.org/' => '{"keys":[{"kty": "RSA","alg": "RS256","use": "sig","kid": "Nzb...9Xs","e": "AQAB","n": "pnXB...ub9J"}]}',
67+
'https://ta2.example.org/' => null,
68+
],
69+
70+
ModuleConfig::OPTION_FEDERATION_AUTHORITY_HINTS => [
71+
'https://intermediate.example.org/',
72+
],
73+
74+
ModuleConfig::OPTION_FEDERATION_TRUST_MARK_TOKENS => [
75+
'eyJ...GHg',
76+
],
77+
78+
ModuleConfig::OPTION_FEDERATION_PARTICIPATION_LIMIT_BY_TRUST_MARKS => [
79+
// We are limiting federation participation using Trust Marks for 'https://ta.example.org/'.
80+
'https://ta.example.org/' => [
81+
// Entities must have (at least) one Trust Mark from the list below.
82+
\SimpleSAML\Module\oidc\Codebooks\LimitsEnum::OneOf->value => [
83+
'trust-mark-id',
84+
'trust-mark-id-2',
85+
],
86+
// Entities must have all Trust Marks from the list below.
87+
\SimpleSAML\Module\oidc\Codebooks\LimitsEnum::AllOf->value => [
88+
'trust-mark-id-3',
89+
'trust-mark-id-4',
90+
],
91+
],
92+
],
93+
94+
ModuleConfig::OPTION_FEDERATION_CACHE_ADAPTER => \Symfony\Component\Cache\Adapter\ArrayAdapter::class,
95+
ModuleConfig::OPTION_FEDERATION_CACHE_ADAPTER_ARGUMENTS => [],
96+
ModuleConfig::OPTION_FEDERATION_ENTITY_STATEMENT_DURATION => 'P1D',
97+
ModuleConfig::OPTION_FEDERATION_CACHE_DURATION_FOR_PRODUCED => 'PT2M',
98+
99+
ModuleConfig::OPTION_FEDERATION_CACHE_MAX_DURATION_FOR_FETCHED => 'PT6H',
100+
48101
ModuleConfig::OPTION_PKI_FEDERATION_PRIVATE_KEY_FILENAME =>
49102
ModuleConfig::DEFAULT_PKI_FEDERATION_PRIVATE_KEY_FILENAME,
50103
ModuleConfig::OPTION_PKI_FEDERATION_PRIVATE_KEY_PASSPHRASE => 'abc123',
51104
ModuleConfig::OPTION_PKI_FEDERATION_CERTIFICATE_FILENAME =>
52105
ModuleConfig::DEFAULT_PKI_FEDERATION_CERTIFICATE_FILENAME,
53-
ModuleConfig::OPTION_FEDERATION_AUTHORITY_HINTS => [
54-
'abc123',
55-
],
106+
107+
ModuleConfig::OPTION_FEDERATION_TOKEN_SIGNER => Sha256::class,
108+
56109
ModuleConfig::OPTION_ORGANIZATION_NAME => 'Foo corp',
57110
ModuleConfig::OPTION_CONTACTS => [
58111
'John Doe [email protected]',

‎tests/unit/src/Helpers/ArrTest.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,14 @@ protected function sut(): Arr
1616
return new Arr();
1717
}
1818

19+
public function testEnsureStringValues(): void
20+
{
21+
$this->assertSame(
22+
['1', '2'],
23+
$this->sut()->ensureStringValues([1, 2]),
24+
);
25+
}
26+
1927
public function testIsValueOneOf(): void
2028
{
2129
$this->assertTrue($this->sut()->isValueOneOf('a', ['a']));

‎tests/unit/src/Helpers/ClientTest.php‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace SimpleSAML\Test\Module\oidc\unit\Helpers;
6+
7+
use PHPUnit\Framework\Attributes\CoversClass;
8+
use PHPUnit\Framework\MockObject\MockObject;
9+
use PHPUnit\Framework\TestCase;
10+
use Psr\Http\Message\ServerRequestInterface;
11+
use SimpleSAML\Error\BadRequest;
12+
use SimpleSAML\Error\NotFound;
13+
use SimpleSAML\Module\oidc\Entities\ClientEntity;
14+
use SimpleSAML\Module\oidc\Helpers\Client;
15+
use SimpleSAML\Module\oidc\Helpers\Http;
16+
use SimpleSAML\Module\oidc\Repositories\ClientRepository;
17+
18+
#[CoversClass(Client::class)]
19+
class ClientTest extends TestCase
20+
{
21+
protected MockObject $httpMock;
22+
protected MockObject $requestMock;
23+
protected MockObject $clientRepositoryMock;
24+
protected MockObject $clientEntityMock;
25+
26+
protected function sut(
27+
?Http $http = null,
28+
): Client {
29+
$http ??= $this->httpMock;
30+
31+
return new Client($http);
32+
}
33+
34+
protected function setUp(): void
35+
{
36+
$this->httpMock = $this->createMock(Http::class);
37+
$this->requestMock = $this->createMock(ServerRequestInterface::class);
38+
$this->clientRepositoryMock = $this->createMock(ClientRepository::class);
39+
$this->clientEntityMock = $this->createMock(ClientEntity::class);
40+
}
41+
42+
public function testCanGetFromRequest(): void
43+
{
44+
$this->httpMock->expects($this->once())->method('getAllRequestParams')
45+
->willReturn(['client_id' => 'clientId']);
46+
47+
$this->clientRepositoryMock->expects($this->once())->method('findById')
48+
->with('clientId')
49+
->willReturn($this->clientEntityMock);
50+
51+
$this->assertInstanceOf(
52+
ClientEntity::class,
53+
$this->sut()->getFromRequest($this->requestMock, $this->clientRepositoryMock),
54+
);
55+
}
56+
57+
public function testGetFromRequestThrowsIfNoClientId(): void
58+
{
59+
$this->expectException(BadRequest::class);
60+
$this->expectExceptionMessage('Client ID');
61+
62+
$this->sut()->getFromRequest($this->requestMock, $this->clientRepositoryMock);
63+
}
64+
65+
public function testGetFromRequestThrowsIfClientNotFound(): void
66+
{
67+
$this->expectException(NotFound::class);
68+
$this->expectExceptionMessage('Client not found');
69+
70+
$this->httpMock->expects($this->once())->method('getAllRequestParams')
71+
->willReturn(['client_id' => 'clientId']);
72+
$this->clientRepositoryMock->expects($this->once())->method('findById')
73+
->with('clientId')
74+
->willReturn(null);
75+
76+
$this->sut()->getFromRequest($this->requestMock, $this->clientRepositoryMock);
77+
}
78+
}

‎tests/unit/src/Helpers/DateTimeTest.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace SimpleSAML\Test\Module\oidc\unit\Helpers;
6+
7+
use DateTimeImmutable;
8+
use PHPUnit\Framework\Attributes\CoversClass;
9+
use PHPUnit\Framework\TestCase;
10+
use SimpleSAML\Module\oidc\Helpers\DateTime;
11+
12+
#[CoversClass(DateTime::class)]
13+
class DateTimeTest extends TestCase
14+
{
15+
protected function sut(): DateTime
16+
{
17+
return new DateTime();
18+
}
19+
20+
public function testCanGetUtc(): void
21+
{
22+
$this->assertInstanceOf(\DateTimeImmutable::class, $this->sut()->getUtc());
23+
$this->assertSame(
24+
'UTC',
25+
$this->sut()->getUtc()->getTimezone()->getName(),
26+
);
27+
}
28+
29+
public function testCanGetFromTimestamp(): void
30+
{
31+
$timestamp = (new DateTimeImmutable())->getTimestamp();
32+
33+
$this->assertSame(
34+
$timestamp,
35+
$this->sut()->getFromTimestamp($timestamp)->getTimestamp(),
36+
);
37+
}
38+
39+
public function testCanGetSecondsToExpirationTime(): void
40+
{
41+
$expirationTime = (new DateTimeImmutable())->getTimestamp() + 60;
42+
43+
$this->assertSame(
44+
60,
45+
$this->sut()->getSecondsToExpirationTime($expirationTime),
46+
);
47+
}
48+
}

‎tests/unit/src/Helpers/HttpTest.php‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace SimpleSAML\Test\Module\oidc\unit\Helpers;
6+
7+
use PHPUnit\Framework\MockObject\MockObject;
8+
use PHPUnit\Framework\TestCase;
9+
use Psr\Http\Message\ServerRequestInterface;
10+
use SimpleSAML\Module\oidc\Helpers\Http;
11+
use SimpleSAML\OpenID\Codebooks\HttpMethodsEnum;
12+
13+
class HttpTest extends TestCase
14+
{
15+
protected MockObject $serverRequestMock;
16+
17+
protected function setUp(): void
18+
{
19+
$this->serverRequestMock = $this->createMock(ServerRequestInterface::class);
20+
}
21+
22+
protected function sut(): Http
23+
{
24+
return new Http();
25+
}
26+
27+
public function testCanGetAllRequestParams(): void
28+
{
29+
$this->serverRequestMock->expects($this->once())->method('getQueryParams')
30+
->willReturn(['a' => 'b']);
31+
32+
$this->serverRequestMock->expects($this->once())->method('getParsedBody')
33+
->willReturn(['c' => 'd']);
34+
35+
$this->assertSame(
36+
['a' => 'b', 'c' => 'd'],
37+
$this->sut()->getAllRequestParams($this->serverRequestMock),
38+
);
39+
}
40+
41+
public function testCanGetAllRequestParamsBasedOnAllowedMethodsForGet(): void
42+
{
43+
$this->serverRequestMock->expects($this->once())->method('getMethod')
44+
->willReturn(HttpMethodsEnum::GET->value);
45+
46+
$this->serverRequestMock->expects($this->once())->method('getQueryParams')
47+
->willReturn(['a' => 'b']);
48+
49+
$this->assertSame(
50+
['a' => 'b'],
51+
$this->sut()->getAllRequestParamsBasedOnAllowedMethods(
52+
$this->serverRequestMock,
53+
[HttpMethodsEnum::GET, HttpMethodsEnum::POST],
54+
),
55+
);
56+
}
57+
58+
public function testCanGetAllRequestParamsBasedOnAllowedMethodsForPost(): void
59+
{
60+
$this->serverRequestMock->expects($this->once())->method('getMethod')
61+
->willReturn(HttpMethodsEnum::POST->value);
62+
63+
$this->serverRequestMock->expects($this->once())->method('getParsedBody')
64+
->willReturn(['c' => 'd']);
65+
66+
$this->assertSame(
67+
['c' => 'd'],
68+
$this->sut()->getAllRequestParamsBasedOnAllowedMethods(
69+
$this->serverRequestMock,
70+
[HttpMethodsEnum::GET, HttpMethodsEnum::POST],
71+
),
72+
);
73+
}
74+
75+
public function testGerAllRequestParamsBasedOnAllowedMethodsReturnsNullForNonAllowedMethod(): void
76+
{
77+
$this->serverRequestMock->expects($this->once())->method('getMethod')
78+
->willReturn(HttpMethodsEnum::POST->value);
79+
80+
$this->assertNull(
81+
$this->sut()->getAllRequestParamsBasedOnAllowedMethods(
82+
$this->serverRequestMock,
83+
[HttpMethodsEnum::GET],
84+
),
85+
);
86+
}
87+
}

0 commit comments

Comments
 (0)