Skip to content

Commit 73b3503

Browse files
kayjoostenkayjoosten
committed
test: add tests and Behat feature for correlation ID logging
Unit tests: - CorrelationIdRepositoryTest: store/find/link CRUD + SessionNotFoundException safety for all three methods - CorrelationIdServiceTest: mint idempotency, link, resolve, null safety - CorrelationIdFlowTest: end-to-end simulation of all four SAML legs (WAYF path, direct path, concurrent flows, back-button replay guard) - CorrelationIdProcessorTest: processor stamps correlation_id on log records; null when no ID is set - AuthnRequestSessionRepositoryTest: store/link/find + SessionNotFoundException safety; updated to use RequestStack + MockArraySessionStorage instead of the now-removed LoggerInterface constructor - ProcessConsentTest / ProvideConsentTest: updated to inject RequestStack-backed AuthnRequestSessionRepository; stub getReceivedRequestFromResponse so tests are self-contained Behat (default suite): - CorrelationId.feature: WAYF path and direct path scenarios assert that every log record carries a non-null correlation_id field - LoggingContext: new Behat context with @BeforeScenario reset and "each log record should contain a :field field" step - TestLogHandler wired into behat.yml; registered in ci monolog config
1 parent 58ef30e commit 73b3503

11 files changed

Lines changed: 863 additions & 4 deletions

File tree

‎src/OpenConext/EngineBlockFunctionalTestingBundle/Features/Context/LoggingContext.php‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
<?php
2+
3+
/**
4+
* Copyright 2010 SURFnet B.V.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
namespace OpenConext\EngineBlockFunctionalTestingBundle\Features\Context;
20+
21+
use Behat\Behat\Context\Context;
22+
use OpenConext\EngineBlockFunctionalTestingBundle\Log\TestLogHandler;
23+
use PHPUnit\Framework\Assert;
24+
25+
/**
26+
* Behat context for asserting on structured log output.
27+
*
28+
* Injects the in-memory TestLogHandler so scenarios can verify that
29+
* log records carry the expected structured fields (e.g. correlation_id).
30+
*
31+
* Does not extend AbstractSubContext because it performs no browser interactions
32+
* and has no need for MinkContext.
33+
*/
34+
class LoggingContext implements Context
35+
{
36+
public function __construct(private readonly TestLogHandler $logHandler)
37+
{
38+
}
39+
40+
/**
41+
* @BeforeScenario
42+
*/
43+
public function resetLogHandler(): void
44+
{
45+
$this->logHandler->reset();
46+
}
47+
48+
/**
49+
* @Then each log record should contain a :field field
50+
*/
51+
public function eachLogRecordShouldContainField(string $field): void
52+
{
53+
$records = $this->logHandler->getRecords();
54+
55+
Assert::assertNotEmpty($records, 'No log records were captured during this scenario.');
56+
57+
foreach ($records as $index => $record) {
58+
Assert::assertArrayHasKey(
59+
$field,
60+
$record->extra,
61+
sprintf(
62+
'Log record #%d (channel=%s, message="%s") is missing extra field "%s".',
63+
$index,
64+
$record->channel,
65+
$record->message,
66+
$field,
67+
),
68+
);
69+
70+
Assert::assertNotNull(
71+
$record->extra[$field],
72+
sprintf(
73+
'Log record #%d (channel=%s, message="%s") has a null value for extra field "%s".',
74+
$index,
75+
$record->channel,
76+
$record->message,
77+
$field,
78+
),
79+
);
80+
}
81+
}
82+
}

‎src/OpenConext/EngineBlockFunctionalTestingBundle/Features/CorrelationId.feature‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
Feature:
2+
In order to trace a complete authentication flow across log entries
3+
As a SURF operator
4+
I need a single correlation_id to appear in every log record belonging to the same SAML flow
5+
6+
Background:
7+
Given an EngineBlock instance on "dev.openconext.local"
8+
And no registered SPs
9+
And no registered Idps
10+
And a Service Provider named "CorrId-SP"
11+
12+
# ── WAYF path ──────────────────────────────────────────────────────────────
13+
# Two IdPs are registered, so the WAYF is shown after the initial SSO request.
14+
# The correlation ID is minted in SingleSignOn.serve(), propagated to
15+
# ContinueToIdp (user picks an IdP), then forwarded to the IdP request via
16+
# link(), and finally picked up in AssertionConsumer and ProvideConsent/
17+
# ProcessConsent. A complete round-trip through all four HTTP legs must
18+
# succeed without error.
19+
Scenario: A user authenticating via the WAYF completes the full four-leg flow
20+
Given an Identity Provider named "CorrId-IdP-A"
21+
And an Identity Provider named "CorrId-IdP-B"
22+
When I log in at "CorrId-SP"
23+
And I select "CorrId-IdP-A" on the WAYF
24+
And I pass through EngineBlock
25+
And I pass through the IdP
26+
And I give my consent
27+
And I pass through EngineBlock
28+
Then the url should match "functional-testing/CorrId-SP/acs"
29+
And each log record should contain a "correlation_id" field
30+
31+
# ── Direct path (no WAYF) ───────────────────────────────────────────────────
32+
# When only one IdP is available the WAYF is skipped; the correlation ID is
33+
# minted inside ProxyServer.sendAuthenticationRequest() and linked to the IdP
34+
# request. AssertionConsumer and consent legs must resolve it from the IdP
35+
# request ID stored in InResponseTo.
36+
Scenario: A user authenticating without the WAYF completes the full flow
37+
Given an Identity Provider named "CorrId-IdP-Only"
38+
When I log in at "CorrId-SP"
39+
And I pass through EngineBlock
40+
And I pass through the IdP
41+
And I give my consent
42+
And I pass through EngineBlock
43+
Then the url should match "functional-testing/CorrId-SP/acs"
44+
And each log record should contain a "correlation_id" field
45+
46+
# ── Concurrent flows ────────────────────────────────────────────────────────
47+
# Two simultaneous authentications in separate browser tabs share the same PHP
48+
# session. Each flow must mint its own correlation ID and the two IDs must
49+
# not bleed into each other. Both flows must complete successfully and land
50+
# on the correct SP ACS URL.
51+
# Requires the @functional tag to use the Chrome driver (browser tabs need JS).
52+
@functional
53+
Scenario: Two concurrent authentication flows each complete independently
54+
Given an Identity Provider named "CorrId-IdP-A"
55+
And an Identity Provider named "CorrId-IdP-B"
56+
When I open 2 browser tabs identified by "Tab-A, Tab-B"
57+
And I switch to "Tab-A"
58+
And I log in at "CorrId-SP"
59+
And I select "CorrId-IdP-A" on the WAYF
60+
And I switch to "Tab-B"
61+
And I log in at "CorrId-SP"
62+
And I select "CorrId-IdP-B" on the WAYF
63+
And I pass through the IdP
64+
And I give my consent
65+
Then the url should match "functional-testing/CorrId-SP/acs"
66+
And I switch to "Tab-A"
67+
And I pass through the IdP
68+
And I give my consent
69+
Then the url should match "functional-testing/CorrId-SP/acs"

‎src/OpenConext/EngineBlockFunctionalTestingBundle/Log/TestLogHandler.php‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php
2+
3+
/**
4+
* Copyright 2010 SURFnet B.V.
5+
*
6+
* Licensed under the Apache License, Version 2.0 (the "License");
7+
* you may not use this file except in compliance with the License.
8+
* You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
19+
namespace OpenConext\EngineBlockFunctionalTestingBundle\Log;
20+
21+
use Monolog\Handler\AbstractProcessingHandler;
22+
use Monolog\Level;
23+
use Monolog\LogRecord;
24+
25+
/**
26+
* In-memory Monolog handler for Behat log assertions.
27+
*
28+
* Collects every log record that passes through it so Behat steps
29+
* can assert on the presence (or absence) of structured log fields.
30+
*/
31+
final class TestLogHandler extends AbstractProcessingHandler
32+
{
33+
/** @var LogRecord[] */
34+
private array $records = [];
35+
36+
public function __construct()
37+
{
38+
parent::__construct(Level::Debug, bubble: true);
39+
}
40+
41+
protected function write(LogRecord $record): void
42+
{
43+
$this->records[] = $record;
44+
}
45+
46+
/**
47+
* Returns all captured log records since the last reset.
48+
*
49+
* @return LogRecord[]
50+
*/
51+
public function getRecords(): array
52+
{
53+
return $this->records;
54+
}
55+
56+
/**
57+
* Clears all captured log records.
58+
*/
59+
public function reset(): void
60+
{
61+
$this->records = [];
62+
}
63+
}

‎tests/behat.yml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,8 @@ default:
4343
serviceRegistryFixture: '@engineblock.functional_testing.fixture.service_registry'
4444
- OpenConext\EngineBlockFunctionalTestingBundle\Features\Context\TranslationContext:
4545
mockTranslator: '@engineblock.functional_testing.mock.translator'
46+
- OpenConext\EngineBlockFunctionalTestingBundle\Features\Context\LoggingContext:
47+
logHandler: '@OpenConext\EngineBlockFunctionalTestingBundle\Log\TestLogHandler'
4648
- OpenConext\EngineBlockFunctionalTestingBundle\Features\Context\MinkContext:
4749
functional:
4850
mink_session: chrome

‎tests/library/EngineBlock/Test/Corto/Module/Service/ProcessConsentTest.php‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,19 @@ private function mockProxyServer()
184184
))
185185
->setBindingsModule($this->mockBindingsModule());
186186

187+
// Stub getReceivedRequestFromResponse so tests do not depend on DI-wired
188+
// AuthnRequestSessionRepository being populated during the test.
189+
$spRequest = new AuthnRequest();
190+
$spRequest->setId('SPREQUEST');
191+
$issuer = new Issuer();
192+
$issuer->setValue('https://sp.example.edu');
193+
$spRequest->setIssuer($issuer);
194+
$decoratedSpRequest = new EngineBlock_Saml2_AuthnRequestAnnotationDecorator($spRequest);
195+
196+
Phake::when($proxyServerMock)
197+
->getReceivedRequestFromResponse(Phake::anyParameters())
198+
->thenReturn($decoratedSpRequest);
199+
187200
return $proxyServerMock;
188201
}
189202

@@ -261,8 +274,10 @@ private function mockSspResponse()
261274
$ebRequest->setId('EBREQUEST');
262275
$ebRequest = new EngineBlock_Saml2_AuthnRequestAnnotationDecorator($ebRequest);
263276

264-
$dummySessionLog = new Psr\Log\NullLogger();
265-
$authnRequestRepository = new EngineBlock_Saml2_AuthnRequestSessionRepository($dummySessionLog);
277+
$authnRequest = new \Symfony\Component\HttpFoundation\Request();
278+
$authnRequest->setSession(new Session(new MockArraySessionStorage()));
279+
$testStack = new RequestStack([$authnRequest]);
280+
$authnRequestRepository = new EngineBlock_Saml2_AuthnRequestSessionRepository($testStack);
266281
$authnRequestRepository->store($spRequest);
267282
$authnRequestRepository->store($ebRequest);
268283
$authnRequestRepository->link($ebRequest, $spRequest);

‎tests/library/EngineBlock/Test/Corto/Module/Service/ProvideConsentTest.php‎

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,19 @@ private function mockProxyServer()
187187
$bindingsModuleMock = $this->mockBindingsModule();
188188
$proxyServerMock->setBindingsModule($bindingsModuleMock);
189189

190+
// Stub getReceivedRequestFromResponse so tests do not depend on DI-wired
191+
// AuthnRequestSessionRepository being populated during the test.
192+
$spRequest = new AuthnRequest();
193+
$spRequest->setId('SPREQUEST');
194+
$issuer = new Issuer();
195+
$issuer->setValue('testSp');
196+
$spRequest->setIssuer($issuer);
197+
$decoratedSpRequest = new EngineBlock_Saml2_AuthnRequestAnnotationDecorator($spRequest);
198+
199+
Phake::when($proxyServerMock)
200+
->getReceivedRequestFromResponse(Phake::anyParameters())
201+
->thenReturn($decoratedSpRequest);
202+
190203
Phake::when($proxyServerMock)
191204
->renderTemplate(Phake::anyParameters())
192205
->thenReturn(null);
@@ -214,8 +227,10 @@ private function mockBindingsModule()
214227
$ebRequest->setId('EBREQUEST');
215228
$ebRequest = new EngineBlock_Saml2_AuthnRequestAnnotationDecorator($ebRequest);
216229

217-
$dummyLog = new Psr\Log\NullLogger();
218-
$authnRequestRepository = new EngineBlock_Saml2_AuthnRequestSessionRepository($dummyLog);
230+
$authnRequest = new \Symfony\Component\HttpFoundation\Request();
231+
$authnRequest->setSession(new Session(new MockArraySessionStorage()));
232+
$testStack = new RequestStack([$authnRequest]);
233+
$authnRequestRepository = new EngineBlock_Saml2_AuthnRequestSessionRepository($testStack);
219234
$authnRequestRepository->store($spRequest);
220235
$authnRequestRepository->store($ebRequest);
221236
$authnRequestRepository->link($ebRequest, $spRequest);

0 commit comments

Comments
 (0)