refactor: replace CurrentCorrelationId getter/setter with public property

Author: kayjoosten

library/EngineBlock/Application/DiContainer.php

@@ -20,6 +20,7 @@
 use OpenConext\EngineBlock\Metadata\Factory\Factory\ServiceProviderFactory;
 use OpenConext\EngineBlock\Metadata\LoaRepository;
 use OpenConext\EngineBlock\Metadata\MetadataRepository\MetadataRepositoryInterface;
+use OpenConext\EngineBlock\Request\CorrelationIdService;
 use OpenConext\EngineBlock\Service\MfaHelperInterface;
 use OpenConext\EngineBlock\Service\ReleaseAsEnforcer;
 use OpenConext\EngineBlock\Service\TimeProvider\TimeProviderInterface;
@@ -615,11 +616,11 @@ public function getNameIdSubstituteResolver()
     }
 
     /**
-     * @return \OpenConext\EngineBlock\Request\CorrelationIdService
+     * @return CorrelationIdService
      */
-    public function getCorrelationIdService(): \OpenConext\EngineBlock\Request\CorrelationIdService
+    public function getCorrelationIdService(): CorrelationIdService
     {
-        return $this->container->get(\OpenConext\EngineBlock\Request\CorrelationIdService::class);
+        return $this->container->get(CorrelationIdService::class);
     }
 
     /**

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

@@ -85,9 +85,9 @@ public function serve($serviceName, Request $httpRequest)
             );
         }
 
-        $authnRequestRepository = EngineBlock_ApplicationSingleton::getInstance()
-            ->getDiContainer()
-            ->getAuthnRequestSessionRepository();
+        $container = EngineBlock_ApplicationSingleton::getInstance()->getDiContainer();
+
+        $authnRequestRepository = $container->getAuthnRequestSessionRepository();
         $request = $authnRequestRepository->findRequestById($id);
 
         if (!$request) {
@@ -96,13 +96,7 @@ public function serve($serviceName, Request $httpRequest)
             );
         }
 
-        // Resolve here so log entries emitted during this leg carry the correlation ID.
-        // ProxyServer::sendAuthenticationRequest will also call mint()+link()+resolve()
-        // on the SP request ID — that is idempotent and sets the same value.
-        // The IdP request ID is only resolvable in AssertionConsumer (Leg 3).
-        $correlationIdService = EngineBlock_ApplicationSingleton::getInstance()
-            ->getDiContainer()
-            ->getCorrelationIdService();
+        $correlationIdService = $container->getCorrelationIdService();
         $correlationIdService->resolve($id);
 
         // Flush log if SP or IdP has additional logging enabled

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

@@ -73,6 +73,7 @@ public function __construct(
     public function serve($serviceName, Request $httpRequest)
     {
         $application = EngineBlock_ApplicationSingleton::getInstance();
+        $container = $application->getDiContainer();
 
         $log = $this->_server->getLogger();
 
@@ -202,9 +203,9 @@ public function serve($serviceName, Request $httpRequest)
         // Multiple IdPs found...
 
         // Auto-select IdP when 'feature_enable_sso_notification' is enabled and send AuthenticationRequest on success
-        if ($application->getDiContainer()->getFeatureConfiguration()->isEnabled("eb.enable_sso_notification")) {
-            $idpEntityId = $application->getDiContainer()->getSsoNotificationService()->
-                handleSsoNotification($application->getDiContainer()->getSymfonyRequest()->cookies, $this->_server);
+        if ($container->getFeatureConfiguration()->isEnabled("eb.enable_sso_notification")) {
+            $idpEntityId = $container->getSsoNotificationService()->
+                handleSsoNotification($container->getSymfonyRequest()->cookies, $this->_server);
 
             if (!empty($idpEntityId)) {
                 try {
@@ -220,8 +221,8 @@ public function serve($serviceName, Request $httpRequest)
         }
 
         // Auto-select IdP when 'wayf.rememberChoice' feature is enabled and is allowed for the current request
-        if (($application->getDiContainer()->getRememberChoice() === true) && !($request->getForceAuthn() || $request->isDebugRequest())) {
-            $cookies = $application->getDiContainer()->getSymfonyRequest()->cookies->all();
+        if (($container->getRememberChoice() === true) && !($request->getForceAuthn() || $request->isDebugRequest())) {
+            $cookies = $container->getSymfonyRequest()->cookies->all();
             if (array_key_exists('rememberchoice', $cookies)) {
                 $remembered = json_decode($cookies['rememberchoice']);
                 if (array_search($remembered, $candidateIDPs) !== false) {
@@ -240,10 +241,10 @@ public function serve($serviceName, Request $httpRequest)
             return;
         }
 
-        $authnRequestRepository = $application->getDiContainer()->getAuthnRequestSessionRepository();
+        $authnRequestRepository = $container->getAuthnRequestSessionRepository();
         $authnRequestRepository->store($request);
 
-        $correlationIdService = $application->getDiContainer()->getCorrelationIdService();
+        $correlationIdService = $container->getCorrelationIdService();
         $correlationIdService->mint($request->getId());
         $correlationIdService->resolve($request->getId());
 

library/EngineBlock/Corto/ProxyServer.php

@@ -461,21 +461,17 @@ public function sendAuthenticationRequest(
             }
         }
 
-        $authenticationState = EngineBlock_ApplicationSingleton::getInstance()->getDiContainer()
-            ->getAuthenticationStateHelper()
-            ->getAuthenticationState();
+        $container =  EngineBlock_ApplicationSingleton::getInstance()->getDiContainer();
+
+        $authenticationState = $container->getAuthenticationStateHelper()->getAuthenticationState();
         $authenticationState->startAuthenticationOnBehalfOf($ebRequest->getId(), $serviceProvider);
 
         // Store the original Request
-        $authnRequestRepository = EngineBlock_ApplicationSingleton::getInstance()
-            ->getDiContainer()
-            ->getAuthnRequestSessionRepository();
+        $authnRequestRepository = $container->getAuthnRequestSessionRepository();
         $authnRequestRepository->store($spRequest);
         $authnRequestRepository->link($ebRequest, $spRequest);
 
-        $correlationIdService = EngineBlock_ApplicationSingleton::getInstance()
-            ->getDiContainer()
-            ->getCorrelationIdService();
+        $correlationIdService = $container->getCorrelationIdService();
         $correlationIdService->mint($spRequest->getId());
         $correlationIdService->link($ebRequest->getId(), $spRequest->getId());
         $correlationIdService->resolve($spRequest->getId());

src/OpenConext/EngineBlock/Logger/Processor/CorrelationIdProcessor.php

@@ -30,7 +30,7 @@ public function __construct(private readonly CurrentCorrelationId $correlationId
 
     public function __invoke(LogRecord $record): LogRecord
     {
-        $record->extra['correlation_id'] = $this->correlationId->get();
+        $record->extra['correlation_id'] = $this->correlationId->correlationId;
 
         return $record;
     }

src/OpenConext/EngineBlock/Request/CorrelationId.php

@@ -26,6 +26,6 @@ public function __construct(public readonly string $correlationId)
 
     public static function mint(): self
     {
-        return new self(bin2hex(random_bytes(16)));
+        return new self(bin2hex(random_bytes(8)));
     }
 }

src/OpenConext/EngineBlock/Request/CorrelationIdService.php

@@ -47,7 +47,7 @@ public function resolve(?string $requestId): void
         $cid = $this->repository->find($requestId);
 
         if ($cid !== null) {
-            $this->current->set($cid->correlationId);
+            $this->current->correlationId = $cid->correlationId;
         }
     }
 }

src/OpenConext/EngineBlock/Request/CurrentCorrelationId.php

@@ -20,15 +20,5 @@
 
 final class CurrentCorrelationId
 {
-    private ?string $correlationId = null;
-
-    public function set(string $correlationId): void
-    {
-        $this->correlationId = $correlationId;
-    }
-
-    public function get(): ?string
-    {
-        return $this->correlationId;
-    }
+    public ?string $correlationId = null;
 }

tests/unit/OpenConext/EngineBlock/Logger/Processor/CorrelationIdProcessorTest.php

@@ -34,7 +34,7 @@ class CorrelationIdProcessorTest extends TestCase
     public function correlation_id_is_added_to_the_record(): void
     {
         $correlationId = new CurrentCorrelationId();
-        $correlationId->set('test-correlation-id');
+        $correlationId->correlationId = 'test-correlation-id';
 
         $processor = new CorrelationIdProcessor($correlationId);
         $record = new LogRecord(

tests/unit/OpenConext/EngineBlock/Request/CorrelationIdFlowTest.php

@@ -77,26 +77,26 @@ public function test_wayf_flow_all_four_legs_share_the_same_correlation_id(): vo
         // Leg 1 — SSO: mint the correlation ID.
         $this->service->mint($spRequestId);
         $this->service->resolve($spRequestId);
-        $mintedCx = $this->current->get();
+        $mintedCx = $this->current->correlationId;
         $this->assertNotNull($mintedCx, 'SSO must mint a correlation ID');
 
         // Leg 2 — ContinueToIdp: resolves SP request ID A.
         $leg2Current = new CurrentCorrelationId();
         $this->newServiceWithCurrent($leg2Current)->resolve($spRequestId);
-        $this->assertSame($mintedCx, $leg2Current->get(), 'ContinueToIdp must see the same correlation ID');
+        $this->assertSame($mintedCx, $leg2Current->correlationId, 'ContinueToIdp must see the same correlation ID');
 
         // ProxyServer links the IdP request ID to the SP request ID.
         $this->service->link($idpRequestId, $spRequestId);
 
         // Leg 3 — ACS: IdP response InResponseTo=B, resolves via B.
         $leg3Current = new CurrentCorrelationId();
         $this->newServiceWithCurrent($leg3Current)->resolve($idpRequestId);
-        $this->assertSame($mintedCx, $leg3Current->get(), 'ACS must see the same correlation ID');
+        $this->assertSame($mintedCx, $leg3Current->correlationId, 'ACS must see the same correlation ID');
 
         // Leg 4 — Consent: resolves SP request ID A again.
         $leg4Current = new CurrentCorrelationId();
         $this->newServiceWithCurrent($leg4Current)->resolve($spRequestId);
-        $this->assertSame($mintedCx, $leg4Current->get(), 'Consent must see the same correlation ID');
+        $this->assertSame($mintedCx, $leg4Current->correlationId, 'Consent must see the same correlation ID');
     }
 
     // ── Direct path (no WAYF) ─────────────────────────────────────────────────
@@ -109,7 +109,7 @@ public function test_direct_flow_acs_and_consent_share_the_correlation_id_minted
         $this->service->mint($spRequestId);
         $this->service->link($idpRequestId, $spRequestId);
         $this->service->resolve($spRequestId);
-        $mintedCx = $this->current->get();
+        $mintedCx = $this->current->correlationId;
         $this->assertNotNull($mintedCx);
 
         $ids = $this->session->get('CorrelationIds');
@@ -157,6 +157,6 @@ public function test_replaying_an_sso_request_does_not_change_the_correlation_id
     public function test_unknown_request_id_does_not_set_correlation_id(): void
     {
         $this->service->resolve('_unknown-id');
-        $this->assertNull($this->current->get(), 'Correlation ID must remain null for unknown request IDs');
+        $this->assertNull($this->current->correlationId, 'Correlation ID must remain null for unknown request IDs');
     }
 }