From 671b6f8fd409e4c1fb44b4f495c2c33a937bf202 Mon Sep 17 00:00:00 2001
From: Tim van Dijen <tvdijen@gmail.com>
Date: Tue, 11 May 2021 10:13:20 +0200
Subject: [PATCH] Fix SubjectID/PairwiseID authprocs; do not fail on non-fatal
 errors. Just silently continue

---
 modules/core/lib/Auth/Process/PairwiseID.php |  5 ++
 modules/core/lib/Auth/Process/SubjectID.php  | 52 +++++++++++---------
 2 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/modules/core/lib/Auth/Process/PairwiseID.php b/modules/core/lib/Auth/Process/PairwiseID.php
index b404491f6..edfe0deb0 100644
--- a/modules/core/lib/Auth/Process/PairwiseID.php
+++ b/modules/core/lib/Auth/Process/PairwiseID.php
@@ -73,6 +73,11 @@ class PairwiseID extends SubjectID
         $userID = $this->getIdentifyingAttribute($state);
         $scope = $this->getScopeAttribute($state);
 
+        if ($scope === null || $userID === null) {
+            // Attributes missing, precondition not met
+            return;
+        }
+
         if (!empty($state['saml:RequesterID'])) {
             // Proxied request - use actual SP entity ID
             $sp_entityid = $state['saml:RequesterID'][0];
diff --git a/modules/core/lib/Auth/Process/SubjectID.php b/modules/core/lib/Auth/Process/SubjectID.php
index 5bd3b1cca..25e303994 100644
--- a/modules/core/lib/Auth/Process/SubjectID.php
+++ b/modules/core/lib/Auth/Process/SubjectID.php
@@ -116,6 +116,11 @@ class SubjectID extends Auth\ProcessingFilter
         $userID = $this->getIdentifyingAttribute($state);
         $scope = $this->getScopeAttribute($state);
 
+        if ($scope === null || $userID === null) {
+            // Attributes missing, precondition not met
+            return;
+        }
+
         $value = strtolower($userID . '@' . $scope);
         $this->validateGeneratedIdentifier($value);
 
@@ -127,20 +132,21 @@ class SubjectID extends Auth\ProcessingFilter
      * Retrieve the identifying attribute from the state and test it for erroneous conditions
      *
      * @param array $state
-     * @return string
+     * @return string|null
      * @throws \SimpleSAML\Assert\AssertionFailedException if the pre-conditions are not met
      */
-    protected function getIdentifyingAttribute(array $state): string
+    protected function getIdentifyingAttribute(array $state): ?string
     {
-        Assert::keyExists($state, 'Attributes');
-        Assert::keyExists(
-            $state['Attributes'],
-            $this->identifyingAttribute,
-            sprintf(
-                "core:" . static::NAME . ": Missing attribute '%s', which is needed to generate the ID.",
-                $this->identifyingAttribute
-            )
-        );
+        if (!array_key_exists('Attributes', $state) || !array_key_exists($this->identifyingAttribute, $state['Attributes'])) {
+            $this->logger::warning(
+                sprintf(
+                    "core:" . static::NAME . ": Missing attribute '%s', which is needed to generate the ID.",
+                    $this->identifyingAttribute
+                )
+            );
+
+            return null;
+        }
 
         $userID = $state['Attributes'][$this->identifyingAttribute][0];
         Assert::stringNotEmpty($userID, 'core' . static::NAME . ': \'identifyingAttribute\' cannot be an empty string.');
@@ -153,20 +159,21 @@ class SubjectID extends Auth\ProcessingFilter
      * Retrieve the scope attribute from the state and test it for erroneous conditions
      *
      * @param array $state
-     * @return string
+     * @return string|null
      * @throws \SimpleSAML\Assert\AssertionFailedException if the pre-conditions are not met
      */
-    protected function getScopeAttribute(array $state): string
+    protected function getScopeAttribute(array $state): ?string
     {
-        Assert::keyExists($state, 'Attributes');
-        Assert::keyExists(
-            $state['Attributes'],
-            $this->scopeAttribute,
-            sprintf(
-                "core:" . static::NAME . ": Missing attribute '%s', which is needed to generate the ID.",
-                $this->scopeAttribute
-            )
-        );
+        if (!array_key_exists('Attributes', $state) || !array_key_exists($this->scopeAttribute, $state['Attributes'])) {
+            $this->logger::warning(
+                sprintf(
+                    "core:" . static::NAME . ": Missing attribute '%s', which is needed to generate the ID.",
+                    $this->scopeAttribute
+                )
+            );
+
+            return null;
+        }
 
         $scope = $state['Attributes'][$this->scopeAttribute][0];
         Assert::stringNotEmpty($scope, 'core' . static::NAME . ': \'scopeAttribute\' cannot be an empty string.');
@@ -183,7 +190,6 @@ class SubjectID extends Auth\ProcessingFilter
             'core:' . static::NAME . ': \'scopeAttribute\' contains illegal characters.'
 //            ProtocolViolationException::class
         );
-
         return $scope;
     }
 
-- 
GitLab