Move checks to functions

Author: phavekes

src/Surfnet/AzureMfa/Application/Service/AzureMfaService.php

@@ -39,6 +39,8 @@
 use Symfony\Component\HttpFoundation\RequestStack;
 use Symfony\Component\HttpFoundation\Session\SessionInterface;
 use Exception;
+use Surfnet\AzureMfa\Domain\Institution\ValueObject\Institution;
+use Surfnet\AzureMfa\Domain\Institution\ValueObject\IdentityProviderInterface;
 
 /**
  * @SuppressWarnings("PHPMD.CouplingBetweenObjects")
@@ -182,36 +184,61 @@ public function handleResponse(Request $request): User
         $user = $this->session->get('user');
         assert($user instanceof User);
 
-        // Retrieve its institution and identity provider
-        $this->logger->info('Match the user email address '.$user->getEmailAddress().'to one of the registered institutions');
+        $this->logger->info('Match the user email address '.$user->getEmailAddress().' to one of the registered institutions');
         $institution = $this->matchingService->findInstitutionByEmail($user->getEmailAddress());
         if (is_null($institution)) {
             throw new AzureADException('The Institution could not be found using the users mail address');
         }
+
         $azureMfaIdentityProvider = $this->identityProviderCache->build($institution->getName());
+        $assertion = $this->processResponseWithOptionalMetadataRefresh(
+            $request,
+            $azureMfaIdentityProvider,
+            $institution
+        );
+
+        $this->validateResponseAttributes($assertion->getAttributes(), $user, $institution, $azureMfaIdentityProvider);
+
+        $this->logger->info(sprintf(
+            'The mail attribute in the response %s matched the email address of the registering/authenticating user: %s',
+            $assertion->getAttributes()[self::SAML_EMAIL_ATTRIBUTE],
+            $user->getEmailAddress()->getEmailAddress()
+        ));
 
+        return $user;
+    }
+
+    private function processResponseWithOptionalMetadataRefresh(
+        Request $request,
+        IdentityProviderInterface $azureMfaIdentityProvider,
+        Institution $institution
+    ): mixed {
         try {
             $this->logger->info('Process the SAML Response');
-            $assertion = $this->postBinding->processResponse(
+
+            return $this->postBinding->processResponse(
                 $request,
                 $azureMfaIdentityProvider,
                 $this->serviceProvider
             );
         } catch (Exception $e) {
-            // If the signature validation fails, we try to rebuild the identity provider
-            // This will effectively refresh the metadata for the institution
-            // Todo: refactor this to be more robust, e.g. by checking the exception type
             if (!$institution->supportsMetadataUpdate() || $e->getMessage() !== 'Unable to validate Signature') {
                 throw $e;
             }
 
-            $this->logger->info(sprintf('Unable to validate signature refreshing metadata for %s', $institution->getName()->getInstitutionName()));
+            $this->logger->info(sprintf(
+                'Unable to validate signature refreshing metadata for %s',
+                $institution->getName()->getInstitutionName()
+            ));
 
             $azureMfaIdentityProvider = $this->identityProviderCache->rebuild($institution->getName());
 
-            $this->logger->info('Reprocess the SAML Response from '.$institution->getName()->getInstitutionName().' after updating the metadata');
+            $this->logger->info(
+                'Reprocess the SAML Response from '.$institution->getName()->getInstitutionName().' after updating the metadata'
+            );
+
             try {
-                $assertion = $this->postBinding->processResponse(
+                return $this->postBinding->processResponse(
                     $request,
                     $azureMfaIdentityProvider,
                     $this->serviceProvider
@@ -224,12 +251,14 @@ public function handleResponse(Request $request): User
                 throw $retryException;
             }
         }
+    }
 
-        $attributes = $assertion->getAttributes();
-
-        // If the IDP was an AzureAD endpoint (the entityID or Issuer starts with https://login.microsoftonline.com/, or preferably an config parameter in institutions.yaml)
-        // the SAML response attribute 'http://schemas.microsoft.com/claims/authnmethodsreferences'
-        // should contain 'http://schemas.microsoft.com/claims/multipleauthn'
+    private function validateResponseAttributes(
+        array $attributes,
+        User $user,
+        Institution $institution,
+        IdentityProviderInterface $azureMfaIdentityProvider
+    ): void {
         if ($azureMfaIdentityProvider->isAzureAD()) {
             $this->logger->info($institution->getName()->getInstitutionName().' is an AzureAD IdP. Validating authnmethodsreferences in the response.');
             if (!isset($attributes['http://schemas.microsoft.com/claims/authnmethodsreferences']) || !in_array('http://schemas.microsoft.com/claims/multipleauthn', $attributes['http://schemas.microsoft.com/claims/authnmethodsreferences'])) {
@@ -241,7 +270,7 @@ public function handleResponse(Request $request): User
 
         if (!isset($attributes[self::SAML_EMAIL_ATTRIBUTE])) {
             throw new MissingMailAttributeException(
-                'The mail attribute in the Azure MFA assertion from '.$institution->getName()->getInstitutionName(). 'was missing'
+                'The mail attribute in the Azure MFA assertion from '.$institution->getName()->getInstitutionName().' was missing'
             );
         }
 
@@ -253,12 +282,5 @@ public function handleResponse(Request $request): User
                 strtolower($user->getEmailAddress()->getEmailAddress())
             ));
         }
-
-            $this->logger->info(sprintf(
-                'The mail attribute in the response %s matched the email address of the registering/authenticating user: %s',
-                $attributes[self::SAML_EMAIL_ATTRIBUTE],
-                $user->getEmailAddress()->getEmailAddress()
-            ));
-            return $user;
     }
 }