From 4cb55ef43d7abca18f260081aac2a2fb8439efea Mon Sep 17 00:00:00 2001
From: briskt <3172830+briskt@users.noreply.github.com>
Date: Tue, 11 Jun 2024 12:25:29 +0800
Subject: [PATCH] Revert "share the use of static functions in Mfa authproc"

This reverts commit 9d6bd38d15b51cbfb0787836bd4bb2c80346c802.
---
 modules/mfa/lib/Auth/Process/Mfa.php          |  8 +--
 .../lib/Auth/Process/ProfileReview.php        | 66 +++++++++++++++++--
 2 files changed, 65 insertions(+), 9 deletions(-)

diff --git a/modules/mfa/lib/Auth/Process/Mfa.php b/modules/mfa/lib/Auth/Process/Mfa.php
index 0f1d681a..4e629d1a 100644
--- a/modules/mfa/lib/Auth/Process/Mfa.php
+++ b/modules/mfa/lib/Auth/Process/Mfa.php
@@ -320,7 +320,7 @@ public static function getTemplateFor(string $mfaType): string
      * @param array $state
      * @return string
      */
-    public static function getRelayStateUrl(array $state): string
+    protected static function getRelayStateUrl($state)
     {
         if (array_key_exists('saml:RelayState', $state)) {
             $samlRelayState = $state['saml:RelayState'];
@@ -407,12 +407,12 @@ protected function initComposerAutoloader(): void
         }
     }
     
-    public static function isHeadedToUrl(array $state, string $url): bool
+    protected static function isHeadedToMfaSetupUrl($state, $mfaSetupUrl)
     {
         if (array_key_exists('saml:RelayState', $state)) {
             $currentDestination = self::getRelayStateUrl($state);
             if (! empty($currentDestination)) {
-                return (strpos($currentDestination, $url) === 0);
+                return (strpos($currentDestination, $mfaSetupUrl) === 0);
             }
         }
         return false;
@@ -579,7 +579,7 @@ public function process(&$state): void
         // Get the necessary info from the state data.
         $employeeId = $this->getAttribute($this->employeeIdAttr, $state);
         $mfa = $this->getAttributeAllValues('mfa', $state);
-        $isHeadedToMfaSetupUrl = self::isHeadedToUrl(
+        $isHeadedToMfaSetupUrl = self::isHeadedToMfaSetupUrl(
             $state,
             $this->mfaSetupUrl
         );
diff --git a/modules/profilereview/lib/Auth/Process/ProfileReview.php b/modules/profilereview/lib/Auth/Process/ProfileReview.php
index b34e0567..789f6838 100644
--- a/modules/profilereview/lib/Auth/Process/ProfileReview.php
+++ b/modules/profilereview/lib/Auth/Process/ProfileReview.php
@@ -7,7 +7,6 @@
 use SimpleSAML\Auth\ProcessingFilter;
 use SimpleSAML\Auth\State;
 use SimpleSAML\Module;
-use SimpleSAML\Module\mfa\Auth\Process\Mfa;
 use SimpleSAML\Module\profilereview\LoggerFactory;
 use SimpleSAML\Session;
 use SimpleSAML\Utils\HTTP;
@@ -68,8 +67,8 @@ protected function loadValuesFromConfig(array $config, array $attributes): void
     {
         foreach ($attributes as $attribute) {
             $this->$attribute = $config[$attribute] ?? null;
-
-            Mfa::validateConfigValue(
+            
+            self::validateConfigValue(
                 $attribute,
                 $this->$attribute,
                 $this->logger
@@ -77,6 +76,28 @@ protected function loadValuesFromConfig(array $config, array $attributes): void
         }
     }
     
+    /**
+     * Validate the given config value
+     *
+     * @param string $attribute The name of the attribute.
+     * @param mixed $value The value to check.
+     * @param LoggerInterface $logger The logger.
+     * @throws \Exception
+     */
+    public static function validateConfigValue($attribute, $value, $logger)
+    {
+        if (empty($value) || !is_string($value)) {
+            $exception = new \Exception(sprintf(
+                'The value we have for %s (%s) is empty or is not a string',
+                $attribute,
+                var_export($value, true)
+            ), 1507146042);
+
+            $logger->critical($exception->getMessage());
+            throw $exception;
+        }
+    }
+    
     /**
      * Get the specified attribute from the given state data.
      *
@@ -118,6 +139,30 @@ protected function getAttributeAllValues($attributeName, $state)
         return is_null($attributeData) ? null : (array)$attributeData;
     }
 
+    /**
+     * Return the saml:RelayState if it begins with "http" or "https". Otherwise
+     * return an empty string.
+     *
+     * @param array $state
+     * @returns string
+     * @return mixed|string
+     */
+    protected static function getRelayStateUrl($state)
+    {
+        if (array_key_exists('saml:RelayState', $state)) {
+            $samlRelayState = $state['saml:RelayState'];
+            
+            if (strpos($samlRelayState, "http://") === 0) {
+                return $samlRelayState;
+            }
+
+            if (strpos($samlRelayState, "https://") === 0) {
+                return $samlRelayState;
+            }
+        }
+        return '';
+    }
+
     protected function initComposerAutoloader()
     {
         $path = __DIR__ . '/../../../vendor/autoload.php';
@@ -125,6 +170,17 @@ protected function initComposerAutoloader()
             require_once $path;
         }
     }
+    
+    protected static function isHeadedToProfileUrl($state, $ProfileUrl)
+    {
+        if (array_key_exists('saml:RelayState', $state)) {
+            $currentDestination = self::getRelayStateUrl($state);
+            if (! empty($currentDestination)) {
+                return (strpos($currentDestination, $ProfileUrl) === 0);
+            }
+        }
+        return false;
+    }
 
     /**
      * Redirect the user to set up profile.
@@ -135,7 +191,7 @@ public static function redirectToProfile(&$state)
     {
         $profileUrl = $state['ProfileUrl'];
         // Tell the profile-setup URL where the user is ultimately trying to go (if known).
-        $currentDestination = Mfa::getRelayStateUrl($state);
+        $currentDestination = self::getRelayStateUrl($state);
         if (! empty($currentDestination)) {
             $profileUrl = HTTP::addURLParameters(
                 $profileUrl,
@@ -165,7 +221,7 @@ public function process(&$state)
     {
         // Get the necessary info from the state data.
         $employeeId = $this->getAttribute($this->employeeIdAttr, $state);
-        $isHeadedToProfileUrl = Mfa::isHeadedToUrl($state, $this->profileUrl);
+        $isHeadedToProfileUrl = self::isHeadedToProfileUrl($state, $this->profileUrl);
 
         $mfa = $this->getAttributeAllValues('mfa', $state);
         $method = $this->getAttributeAllValues('method', $state);