Fix Successful login being logged before login was successful

config/packages/ci/parameters.yml

@@ -2,6 +2,8 @@ parameters:
     api.users.nameidlookup.username: nameid
     api.users.nameidlookup.password: secret
     feature_api_users_nameid_lookup: true
+    auth.log.attributes:
+        uid: 'urn:mace:dir:attribute-def:uid'
     encryption_keys:
         default:
             publicFile: /config/engine/engineblock.crt

config/services/bridge.yml

@@ -14,6 +14,11 @@ services:
         arguments:
             - '@OpenConext\EngineBlock\Logger\AuthenticationLogger'
 
+    OpenConext\EngineBlockBridge\Logger\LoginLogger:
+        arguments:
+            - '@OpenConext\EngineBlockBridge\Logger\AuthenticationLoggerAdapter'
+            - '%auth.log.attributes%'
+
     OpenConext\EngineBlockBridge\Authentication\Repository\UserDirectoryAdapter:
         arguments:
             - '@OpenConext\EngineBlockBundle\Authentication\Service\UserService'

library/EngineBlock/Corto/Filter/Input.php

@@ -16,6 +16,8 @@
  * limitations under the License.
  */
 
+use OpenConext\EngineBlockBundle\Configuration\FeatureConfigurationInterface;
+
 /**
  * Called by Corto before consent, right after it receives an Assertion with attributes from an Identity Provider.
  */
@@ -33,7 +35,7 @@ class EngineBlock_Corto_Filter_Input extends EngineBlock_Corto_Filter_Abstract
     public function getCommands()
     {
         $diContainer          = EngineBlock_ApplicationSingleton::getInstance()->getDiContainer();
-        $featureConfiguration = $diContainer->getFeatureConfiguration();
+        $featureConfiguration = $this->resolveFeatureConfiguration();
         $logger               = EngineBlock_ApplicationSingleton::getLog();
 
         $blockUsersOnViolation = $featureConfiguration->isEnabled('eb.block_user_on_violation');
@@ -114,4 +116,11 @@ public function getCommands()
 
         return array_merge($commands, $outputFilter->getCommands());
     }
+
+    protected function resolveFeatureConfiguration(): FeatureConfigurationInterface
+    {
+        return EngineBlock_ApplicationSingleton::getInstance()
+            ->getDiContainer()
+            ->getFeatureConfiguration();
+    }
 }

library/EngineBlock/Corto/Filter/Output.php

@@ -18,10 +18,11 @@
 
 use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider;
 use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
+use OpenConext\EngineBlockBundle\Configuration\FeatureConfigurationInterface;
 
 /**
- * Commands are run before consent if the feature run_all_manipulations_prior_to_consent is turned on
- * and after consent if the feature is turned off
+ * Output filter commands run after consent (when called via filter()) or before
+ * consent (when the Input filter merges them via getCommands()).
  */
 class EngineBlock_Corto_Filter_Output extends EngineBlock_Corto_Filter_Abstract
 {
@@ -33,28 +34,41 @@ public function filter(
         ServiceProvider $serviceProvider,
         IdentityProvider $identityProvider
     ) {
-        $featureConfiguration = EngineBlock_ApplicationSingleton::getInstance()
-            ->getDiContainer()
-            ->getFeatureConfiguration();
+        if (!$this->resolveFeatureConfiguration()->isEnabled('eb.run_all_manipulations_prior_to_consent')) {
+            parent::filter(
+                $response,
+                $responseAttributes,
+                $request,
+                $serviceProvider,
+                $identityProvider
+            );
+        }
 
-        if ($featureConfiguration->isEnabled('eb.run_all_manipulations_prior_to_consent')) {
-            return;
+        $sessionKey = $serviceProvider->entityId . '>' . $request->getId();
+        $collabPersonId = $_SESSION[$sessionKey]['collabPersonId']
+            ?? $response->getCollabPersonId();
+
+        if (!$collabPersonId) {
+            throw new EngineBlock_Corto_Filter_Command_Exception_PreconditionFailed('Missing collabPersonId');
         }
 
-        parent::filter(
+        $diContainerRuntime = EngineBlock_ApplicationSingleton::getInstance()->getDiContainerRuntime();
+        $diContainerRuntime->loginLogger->logLogin(
             $response,
-            $responseAttributes,
             $request,
             $serviceProvider,
-            $identityProvider
+            $identityProvider,
+            $this->_server->getRepository(),
+            $collabPersonId,
+            $responseAttributes,
         );
     }
 
     /**
      * These commands will be evaluated in order.
      *
      * A command can throw an exception and halt SSO,
-     * it can manipulate the response or it's attributes or it can communicate with external systems.
+     * it can manipulate the response or its attributes or it can communicate with external systems.
      * One thing it can't do is communicate with the user.
      *
      * @return array
@@ -88,9 +102,13 @@ public function getCommands()
 
             // Convert all attributes to their OID format (if known) and add these.
             new EngineBlock_Corto_Filter_Command_DenormalizeAttributes(),
-
-            // Log the login
-            new EngineBlock_Corto_Filter_Command_LogLogin($diContainer->getAuthenticationLogger(), $diContainer->getAuthLogAttributes()),
         );
     }
+
+    protected function resolveFeatureConfiguration(): FeatureConfigurationInterface
+    {
+        return EngineBlock_ApplicationSingleton::getInstance()
+            ->getDiContainer()
+            ->getFeatureConfiguration();
+    }
 }

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

@@ -378,7 +378,7 @@ protected function _createUnsolicitedRequest()
             $request->setKeyId($keyid);
         }
         if ($destination) {
-            // Set for logging purposes (LogLogin Command) note that only the REQUEST_URI (no hostname + protocol)
+            // Set for logging purposes, note that only the REQUEST_URI (no hostname + protocol)
             $sspRequest->setDestination($destination);
         }
 

library/EngineBlock/Saml2/AuthnRequestSessionRepository.php

@@ -30,8 +30,8 @@
  */
 class EngineBlock_Saml2_AuthnRequestSessionRepository
 {
-    private const SESSION_KEY_REQUESTS = 'SAMLRequest';
-    private const SESSION_KEY_LINKS    = 'SAMLRequestLinks';
+    private const string SESSION_KEY_REQUESTS = 'SAMLRequest';
+    private const string SESSION_KEY_LINKS    = 'SAMLRequestLinks';
 
     /**
      * @var RequestStack

src/OpenConext/EngineBlock/Request/CorrelationIdRepository.php

@@ -23,7 +23,7 @@
 
 final class CorrelationIdRepository
 {
-    private const SESSION_KEY = 'CorrelationIds';
+    private const string SESSION_KEY = 'CorrelationIds';
 
     public function __construct(private readonly RequestStack $requestStack)
     {

src/OpenConext/EngineBlockBridge/Logger/LoginLogger.php

@@ -0,0 +1,87 @@
+<?php
+
+/**
+ * Copyright 2026 SURFnet B.V.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+namespace OpenConext\EngineBlockBridge\Logger;
+
+use EngineBlock_Saml2_AuthnRequestAnnotationDecorator;
+use EngineBlock_Saml2_ResponseAnnotationDecorator;
+use EngineBlock_SamlHelper;
+use OpenConext\EngineBlock\Metadata\Entity\IdentityProvider;
+use OpenConext\EngineBlock\Metadata\Entity\ServiceProvider;
+use OpenConext\EngineBlock\Metadata\MetadataRepository\MetadataRepositoryInterface;
+
+class LoginLogger
+{
+    private AuthenticationLoggerAdapter $authenticationLogger;
+    private array $configuredLogAttributes;
+
+    public function __construct(AuthenticationLoggerAdapter $authenticationLogger, array $configuredLogAttributes)
+    {
+        $this->authenticationLogger = $authenticationLogger;
+        $this->configuredLogAttributes = $configuredLogAttributes;
+    }
+
+    /**
+     * Log a successful login.
+     *
+     * @param string $collabPersonId Resolved collabPersonId (from response or session)
+     * @param array $responseAttributes Final response attributes after all filter commands
+     */
+    public function logLogin(
+        EngineBlock_Saml2_ResponseAnnotationDecorator $response,
+        EngineBlock_Saml2_AuthnRequestAnnotationDecorator $request,
+        ServiceProvider $serviceProvider,
+        IdentityProvider $identityProvider,
+        MetadataRepositoryInterface $repository,
+        string $collabPersonId,
+        array $responseAttributes,
+    ): void {
+        // Get the Requester chain, which starts at the oldest (farthest away from us SP) and ends with our next hop.
+        $requesterChain = EngineBlock_SamlHelper::getSpRequesterChain(
+            $serviceProvider,
+            $request,
+            $repository
+        );
+
+        // Remove the SP that is our next hop
+        array_pop($requesterChain);
+
+        $logAttributes = [];
+        if (!empty($this->configuredLogAttributes)) {
+            foreach ($this->configuredLogAttributes as $attributeLabel => $responseAttributeKey) {
+                if (array_key_exists((string) $responseAttributeKey, $responseAttributes)) {
+                    $attributeValues = implode(',', $responseAttributes[$responseAttributeKey]);
+                    $logAttributes[$attributeLabel] = $attributeValues;
+                }
+            }
+        }
+
+        $this->authenticationLogger->logLogin(
+            $serviceProvider,
+            $identityProvider,
+            $collabPersonId,
+            $request->getKeyId(),
+            $requesterChain,
+            $response->getNameIdValue(),
+            $response->getAssertion()->getAuthnContextClassRef(),
+            $request->getDestination(),
+            $request->getIDPList(),
+            $logAttributes
+        );
+    }
+}

src/OpenConext/EngineBlockBundle/Bridge/DiContainerRuntime.php

@@ -22,6 +22,7 @@
 use OpenConext\EngineBlock\Request\CurrentCorrelationId;
 use OpenConext\EngineBlock\Service\FeedbackInfoCollectorInterface;
 use OpenConext\EngineBlock\Service\FeedbackStateHelperInterface;
+use OpenConext\EngineBlockBridge\Logger\LoginLogger;
 use OpenConext\EngineBlockBundle\Service\WayfRenderer;
 use Twig\Environment;
 
@@ -41,6 +42,7 @@ public function __construct(
         public CurrentCorrelationId $currentCorrelationId,
         public FeedbackStateHelperInterface $feedbackStateHelper,
         public FeedbackInfoCollectorInterface $feedbackInfoCollector,
+        public LoginLogger $loginLogger,
         private array $preferredIdpEntityIds = [],
     ) {
     }

src/OpenConext/EngineBlockBundle/Bridge/EngineBlockBootstrapper.php

@@ -23,6 +23,7 @@
 use OpenConext\EngineBlock\Request\CurrentCorrelationId;
 use OpenConext\EngineBlock\Service\FeedbackInfoCollectorInterface;
 use OpenConext\EngineBlock\Service\FeedbackStateHelperInterface;
+use OpenConext\EngineBlockBridge\Logger\LoginLogger;
 use OpenConext\EngineBlockBundle\Service\WayfRenderer;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
 use Symfony\Component\HttpKernel\KernelEvents;
@@ -39,6 +40,7 @@ public function __construct(
         CurrentCorrelationId $currentCorrelationId,
         FeedbackStateHelperInterface $feedbackStateHelper,
         FeedbackInfoCollectorInterface $feedbackInfoCollector,
+        LoginLogger $loginLogger,
         array $preferredIdpEntityIds = [],
     ) {
         $this->diContainerRuntime = new DiContainerRuntime(
@@ -48,6 +50,7 @@ public function __construct(
             $currentCorrelationId,
             $feedbackStateHelper,
             $feedbackInfoCollector,
+            $loginLogger,
             $preferredIdpEntityIds,
         );
     }

src/OpenConext/EngineBlockFunctionalTestingBundle/Features/AttributeManipulationWithAllManipulationsBeforeConsent.feature

@@ -156,5 +156,19 @@ Feature:
     And the response should not match xpath '/samlp:Response/saml:Assertion/saml:Subject/saml:NameID[@Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent" and text()="NOOT"]'
     And the response should match xpath '/samlp:Response/saml:Assertion/saml:Subject/saml:NameID[@Format="urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"]'
 
+  Scenario: The login audit log records the post-manipulation attribute value
+    Given SP "SP-with-Attribute-Manipulations" has the following Attribute Manipulation:
+      """
+      $attributes['urn:mace:dir:attribute-def:uid'] = array("manipulated-uid-value");
+      """
+    When I log in at "SP-with-Attribute-Manipulations"
+    And I select "Dummy-IdP" on the WAYF
+    And I pass through EngineBlock
+    And I pass through the IdP
+    And I give my consent
+    And I pass through EngineBlock
+    Then the url should match "functional-testing/SP-with-Attribute-Manipulations/acs"
+    And the login grant log should contain response attribute "uid" with value "manipulated-uid-value"
+
 #
 #  Scenario: Sp and IdP attribute manipulations