Merge pull request #336 from OpenConext/feature/use-entiltlement-saml-attributes-for-activation-flow

pablothedude · web-flow · commit 6eca10cc52f5 · 2024-11-11T10:15:54.000+01:00
Use saml entitlement attribute for activation flow
diff --git a/ci/qa/phpstan-baseline.php b/ci/qa/phpstan-baseline.php
@@ -741,21 +741,6 @@
 	'count' => 1,
 	'path' => __DIR__ . '/../../src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Factory/SamlFactory.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Method Surfnet\\\\StepupSelfService\\\\SelfServiceBundle\\\\Service\\\\ActivationFlowService\\:\\:__construct\\(\\) has parameter \\$options with no value type specified in iterable type array\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/../../src/Surfnet/StepupSelfService/SelfServiceBundle/Service/ActivationFlowService.php',
-];
-$ignoreErrors[] = [
-	'message' => '#^Method Surfnet\\\\StepupSelfService\\\\SelfServiceBundle\\\\Service\\\\ActivationFlowService\\:\\:getPreference\\(\\) should return Surfnet\\\\StepupSelfService\\\\SelfServiceBundle\\\\Value\\\\ActivationFlowPreferenceInterface but returns mixed\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/../../src/Surfnet/StepupSelfService/SelfServiceBundle/Service/ActivationFlowService.php',
-];
-$ignoreErrors[] = [
-	'message' => '#^Parameter \\#1 \\$preference of class Surfnet\\\\StepupSelfService\\\\SelfServiceBundle\\\\Value\\\\ActivationFlowPreference constructor expects string, array\\|string given\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/../../src/Surfnet/StepupSelfService/SelfServiceBundle/Service/ActivationFlowService.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Parameter \\#2 \\$array of function array_key_exists expects array, array\\<string, int\\|string\\>\\|false given\\.$#',
 	'count' => 1,
@@ -1126,11 +1111,6 @@
 	'count' => 1,
 	'path' => __DIR__ . '/../../src/Surfnet/StepupSelfService/SelfServiceBundle/Twig/Extensions/Extension/SecondFactorType.php',
 ];
-$ignoreErrors[] = [
-	'message' => '#^Property Surfnet\\\\StepupSelfService\\\\SelfServiceBundle\\\\Value\\\\ActivationFlowPreference\\:\\:\\$allowedPreferences type has no value type specified in iterable type array\\.$#',
-	'count' => 1,
-	'path' => __DIR__ . '/../../src/Surfnet/StepupSelfService/SelfServiceBundle/Value/ActivationFlowPreference.php',
-];
 $ignoreErrors[] = [
 	'message' => '#^Cannot cast mixed to string\\.$#',
 	'count' => 2,
diff --git a/config/openconext/parameters.yaml.dist b/config/openconext/parameters.yaml.dist
@@ -75,6 +75,10 @@ parameters:
 
     preferred_activation_flow_name: activate
     preferred_activation_flow_options: [ra, self]
+    activation_flow_attribute_name: urn:mace:dir:attribute-def:eduPersonEntitlement
+    activation_flow_attributes:
+        ra: urn:mace:surf.nl:surfsecureid:activation:ra
+        self: urn:mace:surf.nl:surfsecureid:activation:self
 
     # Self-asserted tokens: enable/disable recovery methods
     #
diff --git a/config/packages/security.yaml b/config/packages/security.yaml
@@ -17,7 +17,7 @@ security:
 
         saml_based:
             custom_authenticators:
-                - Surfnet\SamlBundle\Security\Authentication\SamlAuthenticator
+                - Surfnet\StepupSelfService\SelfServiceBundle\Security\Authentication\SamlAuthenticator
             logout:
                 path: /logout
 
diff --git a/config/packages/surfnet_stepup_self_service_self_service.yaml b/config/packages/surfnet_stepup_self_service_self_service.yaml
@@ -11,3 +11,5 @@ surfnet_stepup_self_service_self_service:
   preferred_activation_flow:
     query_string_field_name: "%preferred_activation_flow_name%"
     options: "%preferred_activation_flow_options%"
+    saml_attribute_field_name: "%activation_flow_attribute_name%"
+    saml_attributes: "%activation_flow_attributes%"
diff --git a/config/services.yaml b/config/services.yaml
@@ -57,6 +57,7 @@ services:
       - '@security.helper'
       - "%logout_redirect_url%"
 
+  Surfnet\StepupSelfService\SelfServiceBundle\Security\Authentication\SamlAuthenticator:
 
   Surfnet\StepupSelfService\SelfServiceBundle\EventListener\AuthenticatedUserListener:
   Surfnet\StepupSelfService\SelfServiceBundle\EventListener\ExplicitSessionTimeoutListener:
diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/EntryPointController.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Controller/EntryPointController.php
@@ -20,12 +20,12 @@
 
 namespace Surfnet\StepupSelfService\SelfServiceBundle\Controller;
 
-use Surfnet\StepupSelfService\SelfServiceBundle\Security\Authentication\AuthenticatedSessionStateHandler;
 use Surfnet\StepupSelfService\SelfServiceBundle\Service\ActivationFlowService;
 use Surfnet\StepupSelfService\SelfServiceBundle\Service\SecondFactorService;
 use Surfnet\StepupSelfService\SelfServiceBundle\Service\SelfAssertedTokens\RecoveryTokenService;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\HttpFoundation\RedirectResponse;
+use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\Routing\Attribute\Route;
 
 class EntryPointController extends AbstractController
@@ -34,17 +34,17 @@ public function __construct(
         private readonly SecondFactorService $secondFactorService,
         private readonly RecoveryTokenService $recoveryTokenService,
         private readonly ActivationFlowService $activationFlowService,
-        private readonly AuthenticatedSessionStateHandler $authStateHandler
     ) {
     }
     #[Route(path: '/', name: 'ss_entry_point', methods:['GET'])]
-    public function decideSecondFactorFlow() : RedirectResponse
+    public function decideSecondFactorFlow(Request $request) : RedirectResponse
     {
         $identity = $this->getUser()->getIdentity();
         $hasSecondFactor = $this->secondFactorService->doSecondFactorsExistForIdentity($identity->id);
         $hasRecoveryToken = $this->recoveryTokenService->hasRecoveryToken($identity);
         // Check if we need to do a registration flow nudge
-        $this->activationFlowService->process($this->authStateHandler->getCurrentRequestUri());
+        // This is only used when already logged in
+        $this->activationFlowService->processPreferenceFromUri($request->getUri());
 
         return $hasSecondFactor || $hasRecoveryToken
             ? $this->redirectToRoute('ss_second_factor_list')
diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/DependencyInjection/Configuration.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/DependencyInjection/Configuration.php
@@ -132,10 +132,29 @@ private function appendActivationFlow(NodeBuilder $childNodes): void
                     ->end()
                     ->arrayNode('options')
                         ->prototype('scalar')
+                            ->isRequired()
+                            ->info('The options describing the preferred activation flow. Example: ra, self')
+                        ->end()
+                    ->end()
+                    ->scalarNode('saml_attribute_field_name')
                         ->isRequired()
-                        ->info('The options describing the preferred activation flow. Example: ra, self')
+                        ->info('The name of the entitlement attribute (in SAML assertion) that is read to determine the preferred activation flow')
                     ->end()
-                ->end()
-            ->end();
+                    ->arrayNode('saml_attributes')
+                        ->isRequired()
+                        ->children()
+                            ->scalarNode('ra')
+                                ->isRequired()
+                                ->info('The entitlement attribute name for the ra vetting flow')
+                            ->end()
+                            ->scalarNode('self')
+                                ->isRequired()
+                                ->info('The entitlement attribute name for the self vetting flow')
+                            ->end()
+                        ->end()
+                    ->end()
+
+            ->end()
+        ;
     }
 }
diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/DependencyInjection/SurfnetStepupSelfServiceSelfServiceExtension.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/DependencyInjection/SurfnetStepupSelfServiceSelfServiceExtension.php
@@ -109,7 +109,9 @@ private function parseActivationFlowPreferenceConfiguration(
         ContainerBuilder $container
     ): void {
         $container->getDefinition(ActivationFlowService::class)
-            ->replaceArgument(2, $preferenceConfig['query_string_field_name'])
-            ->replaceArgument(3, $preferenceConfig['options']);
+            ->replaceArgument(3, $preferenceConfig['query_string_field_name'])
+            ->replaceArgument(4, $preferenceConfig['options'])
+            ->replaceArgument(5, $preferenceConfig['saml_attribute_field_name'])
+            ->replaceArgument(6, $preferenceConfig['saml_attributes']);
     }
 }
diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/EventListener/AuthenticatedUserListener.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/EventListener/AuthenticatedUserListener.php
@@ -56,7 +56,7 @@ public function updateLastInteractionMoment(RequestEvent $event): void
         }
         $this->logger->notice('Logged in user with a session within time limits detected, updating session state');
 
-        // see ExplicitSessionTimeoutHandler for the rationale
+        // see ExplicitSessionTimeoutListener for the rationale
         if ($event->getRequest()->getMethod() === 'GET') {
             $this->sessionStateHandler->setCurrentRequestUri($event->getRequest()->getRequestUri());
         }
diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Resources/config/services.yaml b/src/Surfnet/StepupSelfService/SelfServiceBundle/Resources/config/services.yaml
@@ -192,10 +192,13 @@ services:
 
     Surfnet\StepupSelfService\SelfServiceBundle\Service\ActivationFlowService:
         arguments:
-            - '@request_stack'
+            - '@Surfnet\StepupSelfService\SelfServiceBundle\Security\Authentication\AuthenticatedSessionStateHandler'
+            - "@security.token_storage"
             - "@logger"
             - '' # See extension
             - [] # See extension
+            - '' # See extension
+            - [] # See extension
 
 
     Surfnet\StepupSelfService\SelfServiceBundle\Service\IdentityService:
diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Authentication/AuthenticatedSessionStateHandler.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Authentication/AuthenticatedSessionStateHandler.php
@@ -21,6 +21,7 @@
 namespace Surfnet\StepupSelfService\SelfServiceBundle\Security\Authentication;
 
 use Surfnet\StepupSelfService\SelfServiceBundle\Exception\LogicException;
+use Surfnet\StepupSelfService\SelfServiceBundle\Value\ActivationFlowPreferenceInterface;
 use Surfnet\StepupSelfService\SelfServiceBundle\Value\DateTime;
 
 interface AuthenticatedSessionStateHandler
@@ -57,6 +58,10 @@ public function setCurrentRequestUri(string $uri): void;
 
     public function getCurrentRequestUri(): string;
 
+    public function setRequestedActivationFlowPreference(ActivationFlowPreferenceInterface $preference): void;
+
+    public function getRequestedActivationFlowPreference(): ActivationFlowPreferenceInterface;
+
     /**
      * Migrates the current session to a new session id while maintaining all
      * session attributes.
diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Authentication/SamlAuthenticator.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Authentication/SamlAuthenticator.php
@@ -0,0 +1,78 @@
+<?php
+
+declare(strict_types = 1);
+
+/**
+ * Copyright 2024 SURFnet bv
+ *
+ * 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 Surfnet\StepupSelfService\SelfServiceBundle\Security\Authentication;
+
+use Surfnet\SamlBundle\Security\Authentication\SamlAuthenticator as StepupSamlAuthenticator;
+use Surfnet\StepupSelfService\SelfServiceBundle\Service\ActivationFlowService;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\Security\Core\Authentication\Token\TokenInterface;
+use Symfony\Component\Security\Core\Exception\AuthenticationException;
+use Symfony\Component\Security\Http\Authenticator\InteractiveAuthenticatorInterface;
+use Symfony\Component\Security\Http\Authenticator\Passport\Passport;
+use Symfony\Component\Security\Http\EntryPoint\AuthenticationEntryPointInterface;
+
+class SamlAuthenticator implements InteractiveAuthenticatorInterface, AuthenticationEntryPointInterface
+{
+    public function __construct(
+        readonly private StepupSamlAuthenticator $authenticator,
+        private readonly ActivationFlowService $activationFlowService,
+    ) {
+    }
+
+    public function start(Request $request, ?AuthenticationException $authException = null): Response
+    {
+        // Check if we need to do a registration flow nudge
+        // This is used for when we are not logged in yet because the authentication is done in the Stepup-SAML-bundle
+        $this->activationFlowService->processPreferenceFromUri($request->getUri());
+        return $this->authenticator->start($request, $authException);
+    }
+
+    public function supports(Request $request): ?bool
+    {
+        return $this->authenticator->supports($request);
+    }
+
+    public function authenticate(Request $request): Passport
+    {
+        return $this->authenticator->authenticate($request);
+    }
+
+    public function createToken(Passport $passport, string $firewallName): TokenInterface
+    {
+        return $this->authenticator->createToken($passport, $firewallName);
+    }
+
+    public function onAuthenticationSuccess(Request $request, TokenInterface $token, string $firewallName): ?Response
+    {
+        return $this->authenticator->onAuthenticationSuccess($request, $token, $firewallName);
+    }
+
+    public function onAuthenticationFailure(Request $request, AuthenticationException $exception): ?Response
+    {
+        return $this->authenticator->onAuthenticationFailure($request, $exception);
+    }
+
+    public function isInteractive(): bool
+    {
+        return $this->authenticator->isInteractive();
+    }
+}
diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Authentication/SamlInteractionProvider.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Authentication/SamlInteractionProvider.php
diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Authentication/Session/SessionStorage.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Security/Authentication/Session/SessionStorage.php
@@ -23,6 +23,9 @@
 use Surfnet\StepupSelfService\SelfServiceBundle\Exception\LogicException;
 use Surfnet\StepupSelfService\SelfServiceBundle\Security\Authentication\SamlAuthenticationStateHandler;
 use Surfnet\StepupSelfService\SelfServiceBundle\Security\Authentication\AuthenticatedSessionStateHandler;
+use Surfnet\StepupSelfService\SelfServiceBundle\Value\ActivationFlowPreference;
+use Surfnet\StepupSelfService\SelfServiceBundle\Value\ActivationFlowPreferenceInterface;
+use Surfnet\StepupSelfService\SelfServiceBundle\Value\ActivationFlowPreferenceNotExpressed;
 use Surfnet\StepupSelfService\SelfServiceBundle\Value\DateTime;
 use Symfony\Component\HttpFoundation\RequestStack;
 
@@ -116,6 +119,21 @@ public function clearRequestId(): void
         $this->requestStack->getSession()->remove(self::SAML_SESSION_KEY . 'request_id');
     }
 
+    public function setRequestedActivationFlowPreference(ActivationFlowPreferenceInterface $preference): void
+    {
+        $this->requestStack->getSession()->set(self::SAML_SESSION_KEY . 'activation_preference', $preference->__toString());
+    }
+
+    public function getRequestedActivationFlowPreference(): ActivationFlowPreferenceInterface
+    {
+        if ($this->requestStack->getSession()->has(self::SAML_SESSION_KEY . 'activation_preference')) {
+            /** @var string $preference */
+            $preference = $this->requestStack->getSession()->get(self::SAML_SESSION_KEY . 'activation_preference');
+            return ActivationFlowPreference::fromString($preference);
+        }
+        return new ActivationFlowPreferenceNotExpressed();
+    }
+
     public function invalidate(): void
     {
         $this->requestStack->getSession()->invalidate();
diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Service/ActivationFlowService.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Service/ActivationFlowService.php
diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Tests/Service/ActivationFlowServiceTest.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Tests/Service/ActivationFlowServiceTest.php
diff --git a/src/Surfnet/StepupSelfService/SelfServiceBundle/Value/ActivationFlowPreference.php b/src/Surfnet/StepupSelfService/SelfServiceBundle/Value/ActivationFlowPreference.php

Original file line number	Diff line number	Diff line change
`@@ -75,6 +75,10 @@ parameters:`
`75`	`75`
`76`	`76`	`preferred_activation_flow_name: activate`
`77`	`77`	`preferred_activation_flow_options: [ra, self]`
	`78`	`+ activation_flow_attribute_name: urn:mace:dir:attribute-def:eduPersonEntitlement`
	`79`	`+ activation_flow_attributes:`
	`80`	`+ ra: urn:mace:surf.nl:surfsecureid:activation:ra`
	`81`	`+ self: urn:mace:surf.nl:surfsecureid:activation:self`
`78`	`82`
`79`	`83`	`# Self-asserted tokens: enable/disable recovery methods`
`80`	`84`	`#`
Original file line number	Diff line number	Diff line change
`@@ -109,7 +109,9 @@ private function parseActivationFlowPreferenceConfiguration(`
`109`	`109`	`ContainerBuilder $container`
`110`	`110`	`): void {`
`111`	`111`	`$container->getDefinition(ActivationFlowService::class)`
`112`		`- ->replaceArgument(2, $preferenceConfig['query_string_field_name'])`
`113`		`- ->replaceArgument(3, $preferenceConfig['options']);`
	`112`	`+ ->replaceArgument(3, $preferenceConfig['query_string_field_name'])`
	`113`	`+ ->replaceArgument(4, $preferenceConfig['options'])`
	`114`	`+ ->replaceArgument(5, $preferenceConfig['saml_attribute_field_name'])`
	`115`	`+ ->replaceArgument(6, $preferenceConfig['saml_attributes']);`
`114`	`116`	`}`
`115`	`117`	`}`
Original file line number	Diff line number	Diff line change
`@@ -56,7 +56,7 @@ public function updateLastInteractionMoment(RequestEvent $event): void`
`56`	`56`	`}`
`57`	`57`	`$this->logger->notice('Logged in user with a session within time limits detected, updating session state');`
`58`	`58`
`59`		`- // see ExplicitSessionTimeoutHandler for the rationale`
	`59`	`+ // see ExplicitSessionTimeoutListener for the rationale`
`60`	`60`	`if ($event->getRequest()->getMethod() === 'GET') {`
`61`	`61`	`$this->sessionStateHandler->setCurrentRequestUri($event->getRequest()->getRequestUri());`
`62`	`62`	`}`