Merge pull request #858 from mschwager/master

Fix #828, infinite loop in AttributeMap behavior

modules/core/lib/Auth/Process/AttributeMap.php

@@ -115,24 +115,28 @@ class sspmod_core_Auth_Process_AttributeMap extends SimpleSAML_Auth_ProcessingFi
         assert(is_array($request));
         assert(array_key_exists('Attributes', $request));
 
-        $attributes =& $request['Attributes'];
+        $mapped_attributes = array();
 
-        foreach ($attributes as $name => $values) {
+        foreach ($request['Attributes'] as $name => $values) {
             if (array_key_exists($name, $this->map)) {
                 if (!is_array($this->map[$name])) {
-                    if (!$this->duplicate) {
-                        unset($attributes[$name]);
+                    if ($this->duplicate) {
+                        $mapped_attributes[$name] = $values;
                     }
-                    $attributes[$this->map[$name]] = $values;
+                    $mapped_attributes[$this->map[$name]] = $values;
                 } else {
                     foreach ($this->map[$name] as $to_map) {
-                        $attributes[$to_map] = $values;
+                        $mapped_attributes[$to_map] = $values;
                     }
-                    if (!$this->duplicate && !in_array($name, $this->map[$name], true)) {
-                        unset($attributes[$name]);
+                    if ($this->duplicate && !in_array($name, $this->map[$name], true)) {
+                        $mapped_attributes[$name] = $values;
                     }
                 }
+            } else {
+                $mapped_attributes[$name] = $values;
             }
         }
+
+        $request['Attributes'] = $mapped_attributes;
     }
 }

tests/modules/core/lib/Auth/Process/AttributeMapTest.php

@@ -108,6 +108,51 @@ class Test_Core_Auth_Process_AttributeMap extends TestCase
         $this->assertEquals($expected, $result);
     }
 
+    public function testCircular()
+    {
+        $config = [
+            'attribute1' => 'attribute1',
+            'attribute2' => 'attribute2',
+        ];
+        $request = [
+            'Attributes' => [
+                'attribute1' => ['value'],
+                'attribute2' => ['value'],
+            ],
+        ];
+
+        $processed = self::processFilter($config, $request);
+        $result = $processed['Attributes'];
+        $expected = [
+            'attribute1' => ['value'],
+            'attribute2' => ['value'],
+        ];
+
+        $this->assertEquals($expected, $result);
+    }
+
+    public function testMissingMap()
+    {
+        $config = [
+            'attribute1' => 'attribute3',
+        ];
+        $request = [
+            'Attributes' => [
+                'attribute1' => ['value'],
+                'attribute2' => ['value'],
+            ],
+        ];
+
+        $processed = self::processFilter($config, $request);
+        $result = $processed['Attributes'];
+        $expected = [
+            'attribute2' => ['value'],
+            'attribute3' => ['value'],
+        ];
+
+        $this->assertEquals($expected, $result);
+    }
+
     public function testInvalidOriginalAttributeType()
     {
         $config = [