Merge pull request #1606 from simplesamlphp/attributealter_merge

AttributeAlter:  Add option to merge target attribute values

modules/core/docs/authproc_attributealter.md

@@ -38,6 +38,11 @@ Parameters
 :   Indicates that the whole value of the attribute should be removed completely if there is a match.
     If no other values exist, the attribute will be removed completely.
     This parameter is OPTIONAL.
+
+`%merge`
+:   Indicates whether the altered values must be merged with the target attribute values. The default
+    behaviour is to overwrite the target attribute completely.
+    This parameter is OPTIONAL.
     
 Examples
 --------

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

@@ -53,6 +53,12 @@ class AttributeAlter extends Auth\ProcessingFilter
      */
     private string $target = '';
 
+    /**
+     * Should the altered value be merged with target values
+     * @var bool
+     */
+    private bool $merge = false;
+
 
     /**
      * Initialize this filter.
@@ -73,6 +79,8 @@ class AttributeAlter extends Auth\ProcessingFilter
                     $this->replace = true;
                 } elseif ($value === '%remove') {
                     $this->remove = true;
+                } elseif ($value === '%merge') {
+                    $this->merge = true;
                 } else {
                     throw new Error\Exception('Unknown flag : ' . var_export($value, true));
                 }
@@ -155,6 +163,11 @@ class AttributeAlter extends Auth\ProcessingFilter
 
                     if ($this->subject === $this->target) {
                         $value = $new_value;
+                    } else if ($this->merge === true) {
+                        $attributes[$this->target] = array_values(
+                            array_diff($attributes[$this->target], [$value])
+                        );
+                        $attributes[$this->target][] = $new_value;
                     } else {
                         $attributes[$this->target] = [$new_value];
                     }
@@ -183,8 +196,7 @@ class AttributeAlter extends Auth\ProcessingFilter
                     $attributes[$this->subject]
                 );
             } else {
-                /** @psalm-suppress InvalidArgument */
-                $attributes[$this->target] = array_diff(
+                $diff = array_diff(
                     preg_replace(
                         $this->pattern,
                         $this->replacement,
@@ -192,6 +204,14 @@ class AttributeAlter extends Auth\ProcessingFilter
                     ),
                     $attributes[$this->subject]
                 );
+
+                if ($this->merge === true) {
+                    /** @psalm-suppress InvalidArgument */
+                    $attributes[$this->target] = array_merge($diff, $attributes[$this->target] ?? []);
+                } else {
+                    /** @psalm-suppress InvalidArgument */
+                    $attributes[$this->target] = $diff;
+                }
             }
         }
     }

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

@@ -81,6 +81,34 @@ class AttributeAlterTest extends TestCase
     }
 
 
+    /**
+     * Test the most basic functionality with merging strategy.
+     */
+    public function testMergeWithTarget(): void
+    {
+        $config = [
+            'subject' => 'test',
+            'target' => 'test2',
+            'pattern' => '/wrong/',
+            'replacement' => 'right',
+            '%merge'
+        ];
+
+        $request = [
+            'Attributes' => [
+                 'test' => ['wrong'],
+                 'test2' => ['somethingelse'],
+             ],
+        ];
+
+        $result = self::processFilter($config, $request);
+        $attributes = $result['Attributes'];
+        $this->assertArrayHasKey('test2', $attributes);
+        $this->assertEquals($attributes['test'], ['wrong']);
+        $this->assertEquals($attributes['test2'], ['right', 'somethingelse']);
+    }
+
+
     /**
      * Module is a no op if subject attribute is not present.
      */
@@ -155,6 +183,31 @@ class AttributeAlterTest extends TestCase
     }
 
 
+    /**
+     * Test replacing attribute value with merging strategy.
+     */
+    public function testReplaceMergeMatchWithTarget(): void
+    {
+        $config = [
+            'subject' => 'source',
+            'pattern' => '/wrong/',
+            'replacement' => 'right',
+            'target' => 'test',
+            '%replace',
+            '%merge',
+        ];
+        $request = [
+            'Attributes' => [
+                'source' => ['wrong'],
+                'test'   => ['wrong', 'somethingelse'],
+            ],
+        ];
+        $result = self::processFilter($config, $request);
+        $attributes = $result['Attributes'];
+        $this->assertEquals($attributes['test'], ['somethingelse', 'right']);
+    }
+
+
     /**
      * Test replacing attribute values.
      */