Skip to content

Commit 2d5fa88

Browse files
SpomkySpomky
committed
feat: make the algorithm, checker and serializer managers immutable
A manager expresses a policy ("these algorithms and no others"), so it must not be possible for a consumer to widen it. AlgorithmManager::add() mutated a service that is usually shared: it is deprecated in favour of with(), which returns a new manager and leaves the current one untouched. The state of the checker and serializer managers is now readonly, and both serializer managers, already final, are readonly classes.
1 parent 9d8a44e commit 2d5fa88

8 files changed

Lines changed: 264 additions & 43 deletions

File tree

‎src/Library/Checker/ClaimCheckerManager.php‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,9 @@
1212
/**
1313
* This class manages claim checkers and performs claim checks.
1414
*
15+
* The set of checkers is fixed at construction time: a manager expresses a policy that no consumer is allowed to widen
16+
* afterwards.
17+
*
1518
* @final The class will be final in 5.0.0: implement ClaimCheckerManagerInterface and decorate the service instead of
1619
* extending it.
1720
*
@@ -22,17 +25,19 @@ class ClaimCheckerManager implements ClaimCheckerManagerInterface
2225
/**
2326
* @var ClaimChecker[]
2427
*/
25-
private array $checkers = [];
28+
private readonly array $checkers;
2629

2730
/**
2831
* @param ClaimChecker[] $checkers
2932
*/
3033
public function __construct(iterable $checkers)
3134
{
3235
InheritanceChecker::warnIfExtended(static::class, self::class, ClaimCheckerManagerInterface::class);
36+
$indexedCheckers = [];
3337
foreach ($checkers as $checker) {
34-
$this->add($checker);
38+
$indexedCheckers[$checker->supportedClaim()] = $checker;
3539
}
40+
$this->checkers = $indexedCheckers;
3641
}
3742

3843
/**
@@ -68,12 +73,6 @@ public function check(array $claims, array $mandatoryClaims = []): array
6873
return $checkedClaims;
6974
}
7075

71-
private function add(ClaimChecker $checker): void
72-
{
73-
$claim = $checker->supportedClaim();
74-
$this->checkers[$claim] = $checker;
75-
}
76-
7776
/**
7877
* @param string[] $mandatoryClaims
7978
*/

‎src/Library/Checker/HeaderCheckerManager.php‎

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020
* It allows to add header parameter checkers and token type supports.
2121
* The factory is responsible to create a Header Checker Manager with the header parameter checkers found based
2222
*
23+
* The checkers and the token type supports are fixed at construction time: a manager expresses a policy that no
24+
* consumer is allowed to widen afterwards.
25+
*
2326
* @final The class will be final in 5.0.0: implement HeaderCheckerManagerInterface and decorate the service instead
2427
* of extending it.
2528
*/
@@ -28,12 +31,12 @@ class HeaderCheckerManager implements HeaderCheckerManagerInterface
2831
/**
2932
* @var array<string, HeaderChecker>
3033
*/
31-
private array $checkers = [];
34+
private readonly array $checkers;
3235

3336
/**
3437
* @var TokenTypeSupport[]
3538
*/
36-
private array $tokenTypes = [];
39+
private readonly array $tokenTypes;
3740

3841
/**
3942
* @param HeaderChecker[] $checkers
@@ -42,12 +45,17 @@ class HeaderCheckerManager implements HeaderCheckerManagerInterface
4245
public function __construct(iterable $checkers, iterable $tokenTypes)
4346
{
4447
InheritanceChecker::warnIfExtended(static::class, self::class, HeaderCheckerManagerInterface::class);
48+
$indexedCheckers = [];
4549
foreach ($checkers as $checker) {
46-
$this->add($checker);
50+
$indexedCheckers[$checker->supportedHeader()] = $checker;
4751
}
52+
$this->checkers = $indexedCheckers;
53+
54+
$supportedTokenTypes = [];
4855
foreach ($tokenTypes as $tokenType) {
49-
$this->addTokenTypeSupport($tokenType);
56+
$supportedTokenTypes[] = $tokenType;
5057
}
58+
$this->tokenTypes = $supportedTokenTypes;
5159
}
5260

5361
/**
@@ -84,17 +92,6 @@ public function check(JWT $jwt, int $index, array $mandatoryHeaderParameters = [
8492
throw new InvalidArgumentException('Unsupported token type.');
8593
}
8694

87-
private function addTokenTypeSupport(TokenTypeSupport $tokenType): void
88-
{
89-
$this->tokenTypes[] = $tokenType;
90-
}
91-
92-
private function add(HeaderChecker $checker): void
93-
{
94-
$header = $checker->supportedHeader();
95-
$this->checkers[$header] = $checker;
96-
}
97-
9895
private function checkDuplicatedHeaderParameters(array $header1, array $header2): void
9996
{
10097
$inter = array_intersect_key($header1, $header2);

‎src/Library/Core/AlgorithmManager.php‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use Jose\Component\Core\Exception\UnsupportedAlgorithmException;
88
use function array_key_exists;
99
use function sprintf;
10+
use function trigger_deprecation;
1011

1112
final class AlgorithmManager
1213
{
@@ -21,7 +22,7 @@ final class AlgorithmManager
2122
public function __construct(iterable $algorithms)
2223
{
2324
foreach ($algorithms as $algorithm) {
24-
$this->add($algorithm);
25+
$this->register($algorithm);
2526
}
2627
}
2728

@@ -67,10 +68,42 @@ public function get(string $algorithm): Algorithm
6768
return $this->algorithms[$algorithm];
6869
}
6970

71+
/**
72+
* Returns a new manager that supports the algorithms of the current one plus the given ones.
73+
*
74+
* This method is immutable: the current manager is left untouched, so that a manager shared as a service keeps
75+
* expressing the policy it was built with. An algorithm whose name is already supported replaces the previous one
76+
* in the returned manager.
77+
*/
78+
public function with(Algorithm ...$algorithms): self
79+
{
80+
$clone = clone $this;
81+
foreach ($algorithms as $algorithm) {
82+
$clone->register($algorithm);
83+
}
84+
85+
return $clone;
86+
}
87+
7088
/**
7189
* Adds an algorithm to the manager.
90+
*
91+
* @deprecated since 4.3.0, will be removed in 5.0.0. Use {@see self::with()} instead.
7292
*/
7393
public function add(Algorithm $algorithm): void
94+
{
95+
trigger_deprecation(
96+
'web-token/jwt-framework',
97+
'4.3.0',
98+
'The method "%s::add()" is deprecated and will be removed in 5.0.0. It widens the policy of a manager that is usually a shared service: use "%s::with()" instead, which returns a new manager and leaves the current one untouched.',
99+
self::class,
100+
self::class
101+
);
102+
103+
$this->register($algorithm);
104+
}
105+
106+
private function register(Algorithm $algorithm): void
74107
{
75108
$name = $algorithm->name();
76109
$this->algorithms[$name] = $algorithm;

‎src/Library/Encryption/Serializer/JWESerializerManager.php‎

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,27 @@
1010
use Jose\Component\Encryption\JWE;
1111
use function sprintf;
1212

13-
final class JWESerializerManager
13+
/**
14+
* The set of serializers is fixed at construction time: a manager shared as a service cannot be silently extended by
15+
* one of its consumers.
16+
*/
17+
final readonly class JWESerializerManager
1418
{
1519
/**
1620
* @var JWESerializer[]
1721
*/
18-
private array $serializers = [];
22+
private array $serializers;
1923

2024
/**
2125
* @param JWESerializer[] $serializers
2226
*/
2327
public function __construct(iterable $serializers)
2428
{
29+
$indexedSerializers = [];
2530
foreach ($serializers as $serializer) {
26-
$this->add($serializer);
31+
$indexedSerializers[$serializer->name()] = $serializer;
2732
}
33+
$this->serializers = $indexedSerializers;
2834
}
2935

3036
/**
@@ -76,12 +82,4 @@ public function unserialize(string $input, ?string &$name = null): JWE
7682

7783
throw new InvalidSerializationException('Unsupported input.', 0, $lastError);
7884
}
79-
80-
/**
81-
* Adds a serializer to the manager.
82-
*/
83-
private function add(JWESerializer $serializer): void
84-
{
85-
$this->serializers[$serializer->name()] = $serializer;
86-
}
8785
}

‎src/Library/Signature/Serializer/JWSSerializerManager.php‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,27 @@
1010
use Jose\Component\Signature\JWS;
1111
use function sprintf;
1212

13-
final class JWSSerializerManager
13+
/**
14+
* The set of serializers is fixed at construction time: a manager shared as a service cannot be silently extended by
15+
* one of its consumers.
16+
*/
17+
final readonly class JWSSerializerManager
1418
{
1519
/**
1620
* @var JWSSerializer[]
1721
*/
18-
private array $serializers = [];
22+
private array $serializers;
1923

2024
/**
2125
* @param JWSSerializer[] $serializers
2226
*/
2327
public function __construct(iterable $serializers)
2428
{
29+
$indexedSerializers = [];
2530
foreach ($serializers as $serializer) {
26-
$this->add($serializer);
31+
$indexedSerializers[$serializer->name()] = $serializer;
2732
}
33+
$this->serializers = $indexedSerializers;
2834
}
2935

3036
/**
@@ -74,9 +80,4 @@ public function unserialize(string $input, ?string &$name = null): JWS
7480

7581
throw new InvalidSerializationException('Unsupported input.', 0, $lastError);
7682
}
77-
78-
private function add(JWSSerializer $serializer): void
79-
{
80-
$this->serializers[$serializer->name()] = $serializer;
81-
}
8283
}

‎tests/Component/Core/AlgorithmManagerTest.php‎

Lines changed: 128 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Jose\Tests\Component\Core;
6+
7+
use Jose\Component\Core\AlgorithmManager;
8+
use PHPUnit\Framework\Attributes\Test;
9+
use PHPUnit\Framework\TestCase;
10+
use const E_USER_DEPRECATED;
11+
12+
/**
13+
* @internal
14+
*/
15+
final class AlgorithmManagerTest extends TestCase
16+
{
17+
#[Test]
18+
public function theManagerIsBuiltFromTheAlgorithmsPassedToTheConstructor(): void
19+
{
20+
$algorithm = new FooAlgorithm();
21+
22+
$sut = new AlgorithmManager([$algorithm]);
23+
24+
static::assertTrue($sut->has('foo'));
25+
static::assertSame(['foo'], $sut->list());
26+
static::assertSame([
27+
'foo' => $algorithm,
28+
], $sut->all());
29+
static::assertSame($algorithm, $sut->get('foo'));
30+
}
31+
32+
#[Test]
33+
public function theConstructorDoesNotTriggerTheDeprecationOfTheAddMethod(): void
34+
{
35+
$deprecations = $this->collectDeprecations(static function (): void {
36+
new AlgorithmManager([new FooAlgorithm()]);
37+
});
38+
39+
static::assertSame([], $deprecations);
40+
}
41+
42+
#[Test]
43+
public function withReturnsANewManagerAndLeavesTheCurrentOneUntouched(): void
44+
{
45+
$sut = new AlgorithmManager([new FooAlgorithm()]);
46+
47+
$new = $sut->with(new BarAlgorithm());
48+
49+
static::assertNotSame($sut, $new);
50+
static::assertSame(['foo'], $sut->list());
51+
static::assertSame(['foo', 'bar'], $new->list());
52+
}
53+
54+
#[Test]
55+
public function withAcceptsSeveralAlgorithmsAtOnce(): void
56+
{
57+
$sut = new AlgorithmManager([]);
58+
59+
$new = $sut->with(new FooAlgorithm(), new BarAlgorithm());
60+
61+
static::assertSame(['foo', 'bar'], $new->list());
62+
}
63+
64+
#[Test]
65+
public function withReplacesAnAlgorithmThatHasTheSameName(): void
66+
{
67+
$replacement = new FooAlgorithm();
68+
$sut = new AlgorithmManager([new FooAlgorithm()]);
69+
70+
$new = $sut->with($replacement);
71+
72+
static::assertSame(['foo'], $new->list());
73+
static::assertSame($replacement, $new->get('foo'));
74+
}
75+
76+
#[Test]
77+
public function withDoesNotTriggerAnyDeprecation(): void
78+
{
79+
$sut = new AlgorithmManager([new FooAlgorithm()]);
80+
81+
$deprecations = $this->collectDeprecations(static function () use ($sut): void {
82+
$sut->with(new BarAlgorithm());
83+
});
84+
85+
static::assertSame([], $deprecations);
86+
}
87+
88+
#[Test]
89+
public function theAddMethodIsDeprecatedButStillMutatesTheManager(): void
90+
{
91+
$sut = new AlgorithmManager([new FooAlgorithm()]);
92+
$algorithm = new BarAlgorithm();
93+
94+
$deprecations = $this->collectDeprecations(static function () use ($sut, $algorithm): void {
95+
$sut->add($algorithm);
96+
});
97+
98+
static::assertCount(1, $deprecations);
99+
static::assertStringContainsString(
100+
'The method "Jose\Component\Core\AlgorithmManager::add()" is deprecated and will be removed in 5.0.0.',
101+
$deprecations[0]
102+
);
103+
static::assertSame($algorithm, $sut->get('bar'));
104+
}
105+
106+
/**
107+
* @param callable(): void $callback
108+
*
109+
* @return list<string>
110+
*/
111+
private function collectDeprecations(callable $callback): array
112+
{
113+
$deprecations = [];
114+
set_error_handler(static function (int $errno, string $errstr) use (&$deprecations): bool {
115+
$deprecations[] = $errstr;
116+
117+
return true;
118+
}, E_USER_DEPRECATED);
119+
120+
try {
121+
$callback();
122+
} finally {
123+
restore_error_handler();
124+
}
125+
126+
return $deprecations;
127+
}
128+
}

0 commit comments

Comments
 (0)