Skip to content

Commit 758845b

Browse files
turegjorupturegjorup
committed
Merge remote-tracking branch 'origin/develop' into ci/mutation-tests-php-version
# Conflicts: # CHANGELOG.md
2 parents 12669d8 + 5e74d38 commit 758845b

8 files changed

Lines changed: 75 additions & 23 deletions

‎CHANGELOG.md‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2222
via a single-entry matrix (`Mutation tests (8.3, prefer-stable)`), so
2323
the job name makes explicit what mutation testing runs on. No effect on
2424
the published package.
25+
- Dev: test fixtures use RFC 2606 reserved domains only —
26+
`provider.example.org` for IdP-side URLs (metadata, authorization) and
27+
`app.example.org` for application-side URLs (redirect/callback, CLI
28+
login), replacing real registrable domains (`app.com`, `provider.com`,
29+
`other.com`, `test.com`). No effect on the published package.
30+
- Dev: strengthened Security tests based on mutation testing findings —
31+
the redirect-route parameters are asserted to reach the router when
32+
building a provider redirect URI, `validateClaims` is asserted to look
33+
up the exact provider key from the session and to merge
34+
`open_id_connect_provider` into the returned claims, and a request
35+
without any `loginToken` parameter is asserted to be rejected as
36+
unauthorized. No effect on the published package.
2537
- Dev: strengthened CLI login flow tests based on mutation testing
2638
findings — redeeming an unknown token is asserted to throw
2739
`TokenNotFoundException` specifically, both cache entries (token and

‎tests/Controller/LoginControllerTest.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public function testLogin(): void
3333
->expects($this->exactly(1))
3434
->method('getAuthorizationUrl')
3535
->with(['state' => 'abcd', 'nonce' => '1234', 'response_type' => 'code', 'scope' => 'openid email profile'])
36-
->willReturn('https://test.com');
36+
->willReturn('https://provider.example.org/authorize');
3737

3838
$controller = $this->createController($mockProvider);
3939

@@ -58,7 +58,7 @@ public function testLogin(): void
5858
});
5959

6060
$response = $controller->login($request, $mockSession, 'test');
61-
$this->assertSame('https://test.com', $response->getTargetUrl());
61+
$this->assertSame('https://provider.example.org/authorize', $response->getTargetUrl());
6262
}
6363

6464
public function testUnknownProviderKeyMapsTo404(): void

‎tests/DependencyInjection/ConfigurationTest.php‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ public function testFullConfig(): void
6666
$input['user_provider'] = 'my_user_provider';
6767
$input['openid_providers']['provider1']['options']['leeway'] = 30;
6868
$input['openid_providers']['provider1']['options']['cache_duration'] = 3600;
69-
$input['openid_providers']['provider1']['options']['redirect_uri'] = 'https://app.com/callback';
69+
$input['openid_providers']['provider1']['options']['redirect_uri'] = 'https://app.example.org/callback';
7070
$input['openid_providers']['provider1']['options']['allow_http'] = true;
7171

7272
$config = $this->processor->processConfiguration(
@@ -79,7 +79,7 @@ public function testFullConfig(): void
7979
$provider = $config['openid_providers']['provider1']['options'];
8080
$this->assertSame(30, $provider['leeway']);
8181
$this->assertSame(3600, $provider['cache_duration']);
82-
$this->assertSame('https://app.com/callback', $provider['redirect_uri']);
82+
$this->assertSame('https://app.example.org/callback', $provider['redirect_uri']);
8383
$this->assertTrue($provider['allow_http']);
8484
}
8585

@@ -100,7 +100,7 @@ public function testRedirectRouteConfig(): void
100100
public function testBothRedirectUriAndRouteThrows(): void
101101
{
102102
$input = $this->getMinimalConfig();
103-
$input['openid_providers']['provider1']['options']['redirect_uri'] = 'https://app.com/callback';
103+
$input['openid_providers']['provider1']['options']['redirect_uri'] = 'https://app.example.org/callback';
104104
$input['openid_providers']['provider1']['options']['redirect_route'] = 'my_route';
105105

106106
$this->expectException(InvalidConfigurationException::class);
@@ -186,7 +186,7 @@ public function testMultipleProviders(): void
186186
$input = $this->getMinimalConfig();
187187
$input['openid_providers']['provider2'] = [
188188
'options' => [
189-
'metadata_url' => 'https://other.com/.well-known/openid-configuration',
189+
'metadata_url' => 'https://other-provider.example.org/.well-known/openid-configuration',
190190
'client_id' => 'other_id',
191191
'client_secret' => 'other_secret',
192192
],

‎tests/Security/CliLoginTokenAuthenticatorTest.php‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,19 @@ public function testAuthenticateWithEmptyToken(): void
6161
$this->authenticator->authenticate($request);
6262
}
6363

64+
public function testAuthenticateWithMissingToken(): void
65+
{
66+
// No loginToken query parameter at all: the null from query->get()
67+
// must be coerced to '' and rejected as "no token provided", not
68+
// passed on to the login helper.
69+
$request = new Request();
70+
71+
$this->expectException(CustomUserMessageAuthenticationException::class);
72+
$this->expectExceptionMessage('No login token provided');
73+
74+
$this->authenticator->authenticate($request);
75+
}
76+
6477
public function testAuthenticateWithInvalidToken(): void
6578
{
6679
$this->stubCliLoginHelper

‎tests/Security/OpenIdConfigurationProviderManagerTest.php‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use PHPUnit\Framework\MockObject\Stub;
1010
use PHPUnit\Framework\TestCase;
1111
use Symfony\Component\Cache\Adapter\ArrayAdapter;
12+
use Symfony\Component\Routing\Generator\UrlGeneratorInterface;
1213
use Symfony\Component\Routing\RouterInterface;
1314

1415
class OpenIdConfigurationProviderManagerTest extends TestCase
@@ -77,9 +78,14 @@ public function testGetProviderThrowsOnInvalidKey(): void
7778

7879
public function testGetProviderWithRedirectRoute(): void
7980
{
80-
$this->stubRouter
81+
// Expect the exact arguments so dropping the route parameters (or the
82+
// route itself) when building the redirect URI fails the test.
83+
$mockRouter = $this->createMock(RouterInterface::class);
84+
$mockRouter->expects($this->once())
8185
->method('generate')
82-
->willReturn('https://app.com/callback');
86+
->with('my_route', ['param' => 'value'], UrlGeneratorInterface::ABSOLUTE_URL)
87+
->willReturn('https://app.example.org/callback');
88+
$this->stubRouter = $mockRouter;
8389

8490
$manager = $this->createManager([
8591
'test' => $this->getBaseProviderConfig() + [
@@ -96,7 +102,7 @@ public function testGetProviderWithRedirectRouteNoParameters(): void
96102
{
97103
$this->stubRouter
98104
->method('generate')
99-
->willReturn('https://app.com/callback');
105+
->willReturn('https://app.example.org/callback');
100106

101107
$manager = $this->createManager([
102108
'test' => $this->getBaseProviderConfig() + [
@@ -112,7 +118,7 @@ public function testGetProviderWithLeeway(): void
112118
{
113119
$manager = $this->createManager([
114120
'test' => $this->getBaseProviderConfig() + [
115-
'redirect_uri' => 'https://app.com/callback',
121+
'redirect_uri' => 'https://app.example.org/callback',
116122
'leeway' => 30,
117123
],
118124
]);
@@ -125,7 +131,7 @@ public function testGetProviderWithCacheDuration(): void
125131
{
126132
$manager = $this->createManager([
127133
'test' => $this->getBaseProviderConfig() + [
128-
'redirect_uri' => 'https://app.com/callback',
134+
'redirect_uri' => 'https://app.example.org/callback',
129135
'cache_duration' => 3600,
130136
],
131137
]);
@@ -138,7 +144,7 @@ public function testGetProviderWithAllowHttp(): void
138144
{
139145
$manager = $this->createManager([
140146
'test' => $this->getBaseProviderConfig() + [
141-
'redirect_uri' => 'https://app.com/callback',
147+
'redirect_uri' => 'https://app.example.org/callback',
142148
'allow_http' => true,
143149
],
144150
]);
@@ -166,7 +172,7 @@ public function testGetProviderForwardsHttpClientOptions(): void
166172
{
167173
$manager = $this->createManager([
168174
'test' => $this->getBaseProviderConfig() + [
169-
'redirect_uri' => 'https://app.com/callback',
175+
'redirect_uri' => 'https://app.example.org/callback',
170176
'http_client_options' => [
171177
'timeout' => 1.5,
172178
'proxy' => 'http://proxy:8080',
@@ -189,7 +195,7 @@ public function testGetProviderWithoutHttpClientOptionsLeavesGuzzleDefaults(): v
189195
{
190196
$manager = $this->createManager([
191197
'test' => $this->getBaseProviderConfig() + [
192-
'redirect_uri' => 'https://app.com/callback',
198+
'redirect_uri' => 'https://app.example.org/callback',
193199
],
194200
]);
195201

@@ -205,7 +211,7 @@ public function testGetProviderCachesInstance(): void
205211
{
206212
$manager = $this->createManager([
207213
'test' => $this->getBaseProviderConfig() + [
208-
'redirect_uri' => 'https://app.com/callback',
214+
'redirect_uri' => 'https://app.example.org/callback',
209215
],
210216
]);
211217

‎tests/Security/OpenIdLoginAuthenticatorTest.php‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -110,18 +110,30 @@ public function testValidateClaimsSuccess(): void
110110
$stubProvider = $this->createStub(OpenIdConfigurationProvider::class);
111111

112112
$claims = new \stdClass();
113-
$claims->email = 'test@test.com';
113+
$claims->email = 'test@example.org';
114114
$claims->name = 'Test Tester';
115115
$stubProvider->method('validateIdToken')->willReturn($claims);
116116

117-
$this->stubProviderManager->method('getProvider')->willReturn($stubProvider);
117+
// Expect the exact provider key from the session, so a lookup with a
118+
// mangled key fails the test instead of silently matching any key.
119+
$mockProviderManager = $this->createMock(OpenIdConfigurationProviderManager::class);
120+
$mockProviderManager->expects($this->once())
121+
->method('getProvider')
122+
->with('test_provider_1')
123+
->willReturn($stubProvider);
124+
$authenticator = new TestAuthenticator($mockProviderManager);
118125

119126
$request = new Request(query: ['state' => 'test_state', 'code' => 'test_code']);
120127
$this->setSessionOnRequest($request);
121128

122-
$passport = $this->authenticator->authenticate($request);
129+
$passport = $authenticator->authenticate($request);
130+
131+
$this->assertSame('test@example.org', $passport->getUser()->getUserIdentifier());
123132

124-
$this->assertSame('test@test.com', $passport->getUser()->getUserIdentifier());
133+
// The claims contract: the IdP claims plus the provider key that
134+
// authenticated the user.
135+
$this->assertSame('Test Tester', $authenticator->lastClaims['name'] ?? null);
136+
$this->assertSame('test_provider_1', $authenticator->lastClaims['open_id_connect_provider'] ?? null);
125137
}
126138

127139
private function setSessionOnRequest(Request $request, ?string $nonce = 'test_nonce'): void

‎tests/Security/TestAuthenticator.php‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,18 @@
1313

1414
class TestAuthenticator extends OpenIdLoginAuthenticator
1515
{
16+
/**
17+
* Claims returned by the last validateClaims() call, exposed so tests
18+
* can assert on the full claims array (validateClaims is protected).
19+
*
20+
* @var array<string, string>
21+
*/
22+
public array $lastClaims = [];
23+
1624
public function authenticate(Request $request): Passport
1725
{
1826
$claims = $this->validateClaims($request);
27+
$this->lastClaims = $claims;
1928

2029
return new SelfValidatingPassport(
2130
new UserBadge(

‎tests/config/itkdev_openid_connect.yml‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ itkdev_openid_connect:
77
openid_providers:
88
test_provider_1:
99
options:
10-
metadata_url: "https://provider.com/openid-configuration"
10+
metadata_url: "https://provider.example.org/openid-configuration"
1111
client_id: "test_id"
1212
client_secret: "test_secret"
13-
redirect_uri: "https://app.com/callback_uri"
13+
redirect_uri: "https://app.example.org/callback_uri"
1414
test_provider_2:
1515
options:
16-
metadata_url: "https://provider.com/openid-configuration"
16+
metadata_url: "https://provider.example.org/openid-configuration"
1717
client_id: "test_id"
1818
leeway: 5
1919
client_secret: "test_secret"
20-
redirect_uri: "https://app.com/callback_uri"
20+
redirect_uri: "https://app.example.org/callback_uri"

0 commit comments

Comments
 (0)