Skip to content

Commit bf1e8e1

Browse files
committed
feat: wire correlation ID through all four SAML flow legs
Each HTTP leg resolves the correlation ID at the top of its handler so every log entry emitted during that leg carries the correct ID: Leg 1 SSO — mint() + resolve() in SingleSignOn (WAYF path) mint() + link() + resolve() in ProxyServer (direct path) Leg 2 ContinueToIdp — resolve() so debug log lines carry the ID; ProxyServer also calls link() to associate the IdP request ID with the SP request ID Leg 3 ACS — resolve() via InResponseTo (IdP request ID) Leg 4 Consent — resolve() via SP request ID in ProvideConsent and ProcessConsent DiContainer exposes getCorrelationIdRepository() as the bridge from legacy Corto code to the Symfony service. Includes a Behat feature covering the WAYF path, the direct (no-WAYF) path, and concurrent flows; and a unit test for AuthnRequestSessionRepository.
1 parent 723df89 commit bf1e8e1

10 files changed

Lines changed: 192 additions & 0 deletions

File tree

library/EngineBlock/Application/DiContainer.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,4 +613,12 @@ public function getNameIdSubstituteResolver()
613613
{
614614
return new EngineBlock_Arp_NameIdSubstituteResolver($this->container->get('engineblock.compat.logger'));
615615
}
616+
617+
/**
618+
* @return \OpenConext\EngineBlock\Request\CorrelationIdRepository
619+
*/
620+
public function getCorrelationIdRepository(): \OpenConext\EngineBlock\Request\CorrelationIdRepository
621+
{
622+
return $this->container->get(\OpenConext\EngineBlock\Request\CorrelationIdRepository::class);
623+
}
616624
}

library/EngineBlock/Corto/Module/Service/AssertionConsumer.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,9 @@ public function serve($serviceName, Request $httpRequest)
8989
$receivedRequest = $this->_server->getReceivedRequestFromResponse($receivedResponse);
9090

9191
$application = EngineBlock_ApplicationSingleton::getInstance();
92+
93+
$correlationIdRepository = $application->getDiContainer()->getCorrelationIdRepository();
94+
$correlationIdRepository->resolve($receivedResponse->getInResponseTo());
9295
$log = $application->getLogInstance();
9396

9497
if(!$receivedRequest instanceof EngineBlock_Saml2_AuthnRequestAnnotationDecorator){

library/EngineBlock/Corto/Module/Service/ContinueToIdp.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,15 @@ public function serve($serviceName, Request $httpRequest)
9494
);
9595
}
9696

97+
// Resolve here so log entries emitted during this leg (e.g. the debug
98+
// block below) carry the correlation ID. ProxyServer::sendAuthenticationRequest
99+
// will also call resolve() when it links the IdP request ID — that is
100+
// idempotent and sets the same value.
101+
$correlationIdRepository = EngineBlock_ApplicationSingleton::getInstance()
102+
->getDiContainer()
103+
->getCorrelationIdRepository();
104+
$correlationIdRepository->resolve($id);
105+
97106
// Flush log if SP or IdP has additional logging enabled
98107
if ($request->isDebugRequest()) {
99108
$sp = $this->getEngineSpRole($this->_server);

library/EngineBlock/Corto/Module/Service/ProcessConsent.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,12 @@ public function serve($serviceName, Request $httpRequest)
8585
$response = $processStep->getResponse();
8686

8787
$request = $this->_server->getReceivedRequestFromResponse($response);
88+
89+
$correlationIdRepository = EngineBlock_ApplicationSingleton::getInstance()
90+
->getDiContainer()
91+
->getCorrelationIdRepository();
92+
$correlationIdRepository->resolve($request->getId());
93+
8894
$serviceProvider = $this->_server->getRepository()->fetchServiceProviderByEntityId($request->getIssuer()->getValue());
8995

9096
$destinationMetadata = EngineBlock_SamlHelper::getDestinationSpMetadata(

library/EngineBlock/Corto/Module/Service/ProvideConsent.php

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,11 @@ public function serve($serviceName, Request $httpRequest)
100100

101101
$receivedRequest = $this->_server->getReceivedRequestFromResponse($response);
102102

103+
$correlationIdRepository = EngineBlock_ApplicationSingleton::getInstance()
104+
->getDiContainer()
105+
->getCorrelationIdRepository();
106+
$correlationIdRepository->resolve($receivedRequest->getId());
107+
103108
// update previous response with current response
104109
$this->_processingStateHelper->updateStepResponseByRequestId(
105110
$receivedRequest->getId(),

library/EngineBlock/Corto/Module/Service/SingleSignOn.php

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,10 @@ public function serve($serviceName, Request $httpRequest)
237237
$authnRequestRepository = new EngineBlock_Saml2_AuthnRequestSessionRepository($log);
238238
$authnRequestRepository->store($request);
239239

240+
$correlationIdRepository = $application->getDiContainer()->getCorrelationIdRepository();
241+
$correlationIdRepository->mint($request->getId());
242+
$correlationIdRepository->resolve($request->getId());
243+
240244
// Show WAYF
241245
$log->info("Multiple candidate IdPs: redirecting to WAYF");
242246
$this->_showWayf($request, $candidateIDPs);

library/EngineBlock/Corto/ProxyServer.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,12 @@ public function sendAuthenticationRequest(
471471
$authnRequestRepository->store($spRequest);
472472
$authnRequestRepository->link($ebRequest, $spRequest);
473473

474+
$correlationIdRepository = EngineBlock_ApplicationSingleton::getInstance()
475+
->getDiContainer()
476+
->getCorrelationIdRepository();
477+
$correlationIdRepository->mint($spRequest->getId());
478+
$correlationIdRepository->link($ebRequest->getId(), $spRequest->getId());
479+
$correlationIdRepository->resolve($spRequest->getId());
474480

475481
$this->getBindingsModule()->send($ebRequest, $identityProvider);
476482
}
@@ -556,6 +562,13 @@ public function sendStepupAuthenticationRequest(
556562
$authnRequestRepository->store($spRequest);
557563
$authnRequestRepository->link($ebRequest, $spRequest);
558564

565+
$correlationIdRepository = EngineBlock_ApplicationSingleton::getInstance()
566+
->getDiContainer()
567+
->getCorrelationIdRepository();
568+
$correlationIdRepository->mint($spRequest->getId());
569+
$correlationIdRepository->link($ebRequest->getId(), $spRequest->getId());
570+
$correlationIdRepository->resolve($spRequest->getId());
571+
559572
$this->getBindingsModule()->send($ebRequest, $identityProvider, true);
560573
}
561574

library/EngineBlock/Saml2/AuthnRequestSessionRepository.php

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,7 @@ public function store(
9191
) {
9292
// Store the original Request
9393
$this->requestStorage[$spRequest->getId()] = $spRequest;
94+
9495
return $this;
9596
}
9697

@@ -105,6 +106,8 @@ public function link(
105106
) {
106107
// Store the mapping from the new request ID to the original request ID
107108
$this->linkStorage[$fromRequest->getId()] = $toRequest->getId();
109+
108110
return $this;
109111
}
112+
110113
}
Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
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+
30+
# ── Direct path (no WAYF) ───────────────────────────────────────────────────
31+
# When only one IdP is available the WAYF is skipped; the correlation ID is
32+
# minted inside ProxyServer.sendAuthenticationRequest() and linked to the IdP
33+
# request. AssertionConsumer and consent legs must resolve it from the IdP
34+
# request ID stored in InResponseTo.
35+
Scenario: A user authenticating without the WAYF completes the full flow
36+
Given an Identity Provider named "CorrId-IdP-Only"
37+
When I log in at "CorrId-SP"
38+
And I pass through EngineBlock
39+
And I pass through the IdP
40+
And I give my consent
41+
And I pass through EngineBlock
42+
Then the url should match "functional-testing/CorrId-SP/acs"
43+
44+
# ── Concurrent flows ────────────────────────────────────────────────────────
45+
# Two simultaneous authentications in separate browser tabs share the same PHP
46+
# session. Each flow must mint its own correlation ID and the two IDs must
47+
# not bleed into each other. Both flows must complete successfully and land
48+
# on the correct SP ACS URL.
49+
# Requires the @functional tag to use the Chrome driver (browser tabs need JS).
50+
@functional
51+
Scenario: Two concurrent authentication flows each complete independently
52+
Given an Identity Provider named "CorrId-IdP-A"
53+
And an Identity Provider named "CorrId-IdP-B"
54+
When I open 2 browser tabs identified by "Tab-A, Tab-B"
55+
And I switch to "Tab-A"
56+
And I log in at "CorrId-SP"
57+
And I select "CorrId-IdP-A" on the WAYF
58+
And I switch to "Tab-B"
59+
And I log in at "CorrId-SP"
60+
And I select "CorrId-IdP-B" on the WAYF
61+
And I pass through the IdP
62+
And I give my consent
63+
Then the url should match "functional-testing/CorrId-SP/acs"
64+
And I switch to "Tab-A"
65+
And I pass through the IdP
66+
And I give my consent
67+
Then the url should match "functional-testing/CorrId-SP/acs"
Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
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+
use Mockery as m;
20+
use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
21+
use PHPUnit\Framework\TestCase;
22+
use SAML2\AuthnRequest;
23+
24+
class EngineBlock_Test_Saml2_AuthnRequestSessionRepositoryTest extends TestCase
25+
{
26+
use MockeryPHPUnitIntegration;
27+
28+
protected function setUp(): void
29+
{
30+
$_SESSION = [];
31+
}
32+
33+
protected function tearDown(): void
34+
{
35+
$_SESSION = [];
36+
}
37+
38+
private function makeRequest(string $id): EngineBlock_Saml2_AuthnRequestAnnotationDecorator
39+
{
40+
$authnRequest = new AuthnRequest();
41+
$authnRequest->setId($id);
42+
return new EngineBlock_Saml2_AuthnRequestAnnotationDecorator($authnRequest);
43+
}
44+
45+
private function makeRepository(): EngineBlock_Saml2_AuthnRequestSessionRepository
46+
{
47+
$logger = m::mock(Psr\Log\LoggerInterface::class);
48+
return new EngineBlock_Saml2_AuthnRequestSessionRepository($logger);
49+
}
50+
51+
public function test_store_saves_request()
52+
{
53+
$repository = $this->makeRepository();
54+
$request = $this->makeRequest('_sp-request-A');
55+
56+
$repository->store($request);
57+
58+
$storedRequest = $repository->findRequestById('_sp-request-A');
59+
$this->assertSame($request, $storedRequest);
60+
}
61+
62+
public function test_link_stores_request_mapping()
63+
{
64+
$repository = $this->makeRepository();
65+
$spRequest = $this->makeRequest('_sp-request-A');
66+
$idpRequest = $this->makeRequest('_idp-request-B');
67+
68+
$repository->store($spRequest);
69+
$repository->link($idpRequest, $spRequest);
70+
71+
$linkedRequestId = $repository->findLinkedRequestId('_idp-request-B');
72+
$this->assertSame('_sp-request-A', $linkedRequestId);
73+
}
74+
}

0 commit comments

Comments
 (0)