Reformat the PHP authproc filter. Add a new 'function' option that allows to...

Reformat the PHP authproc filter. Add a new 'function' option that allows to define an anonymous function. Deprecated the old 'code' function. Add tests.

modules/core/docs/authproc_php.txt

@@ -10,10 +10,13 @@ Parameters
 :   This is the name of the filter.
     It must be `'core:PHP'`.
 
+`function`
+:   The PHP function that should be run, defined as an anonymous function with one parameter called `$attributes`.
+    This is an associative array with the user's attributes, and can be modified to add or remove them.
+
 `code`
-:   The PHP code that should be run.
-    This PHP code will have one variable available - `$attributes`.
-    This is an associative array of attributes, and can be modified to add or remove attributes.
+:   **Deprecated**
+    If you are using this option, please migrate your code to an anonymous function defined in the `function` option.
 
 Examples
 --------
@@ -22,15 +25,15 @@ Add the `mail` attribute based on the user's `uid` attribute:
 
     10 => array(
         'class' => 'core:PHP',
-        'code' => '
-            if (empty($attributes["uid"])) {
-                throw new Exception("Missing uid attribute.");
+        'function' => function (&$attributes) {
+            if (empty($attributes['uid'])) {
+                throw new Exception('Missing uid attribute.');
             }
 
-            $uid = $attributes["uid"][0];
-            $mail = $uid . "@example.net";
-            $attributes["mail"] = array($mail);
-        ',
+            $uid = $attributes['uid'][0];
+            $mail = $uid.'@example.net';
+            $attributes['mail'] = array($mail);
+        },
     ),
 
 
@@ -38,9 +41,9 @@ Create a random number variable:
 
     10 => array(
         'class' => 'core:PHP',
-        'code' => '
-            $attributes["random"] = array(
+        'code' => function (&$attributes) {
+            $attributes['random'] = array(
                 (string)rand(),
             );
-        ',
+        },
     ),

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

 <?php
 
+
 /**
  * Attribute filter for running arbitrary PHP code.
  *
  * @package simpleSAMLphp
  */
-class sspmod_core_Auth_Process_PHP extends SimpleSAML_Auth_ProcessingFilter {
-
-	/**
-	 * The PHP code that should be run.
-	 *
-	 * @var string
-	 */
-	private $code;
-
-
-	/**
-	 * Initialize this filter, parse configuration
-	 *
-	 * @param array $config  Configuration information about this filter.
-	 * @param mixed $reserved  For future use.
-	 */
-	public function __construct($config, $reserved) {
-		parent::__construct($config, $reserved);
-
-		assert('is_array($config)');
-
-		if (!isset($config['code'])) {
-			throw new SimpleSAML_Error_Exception($this->authId . ': Missing required \'code\' option.');
-		}
-
-		$this->code = (string)$config['code'];
-	}
-
-
-	/**
-	 * Apply the PHP code to the attribtes.
-	 *
-	 * @param array &$request  The current request
-	 */
-	public function process(&$request) {
-		assert('is_array($request)');
-		assert('array_key_exists("Attributes", $request)');
-
-		$function = create_function('&$attributes', $this->code);
-		$function($request['Attributes']);
-	}
+class sspmod_core_Auth_Process_PHP extends SimpleSAML_Auth_ProcessingFilter
+{
+
+    /**
+     * The PHP code that should be run.
+     *
+     * @var string
+     */
+    private $code;
+
+    /**
+     * @var callable
+     */
+    private $function = null;
+
+
+    /**
+     * Initialize this filter, parse configuration
+     *
+     * @param array $config Configuration information about this filter.
+     * @param mixed $reserved For future use.
+     */
+    public function __construct($config, $reserved)
+    {
+        parent::__construct($config, $reserved);
+
+        assert('is_array($config)');
+
+        if (isset($config['function'])) {
+            $this->function = $config['function'];
+        } else { // TODO: remove this branch after removing the 'code' option.
+            if (!isset($config['code'])) {
+                throw new SimpleSAML_Error_Exception("core:PHP: Neither 'function' nor 'code' options defined.");
+            }
+            SimpleSAML_Logger::warning(
+                "Deprecated 'code' configuration option in PHP authentication processing filter."
+            );
+            $this->code = (string) $config['code'];
+        }
+    }
+
+
+    /**
+     * Apply the PHP code to the attributes.
+     *
+     * @param array &$request The current request
+     */
+    public function process(&$request)
+    {
+        assert('is_array($request)');
+        assert('array_key_exists("Attributes", $request)');
+
+        if ($this->function) {
+            $function = $this->function;
+            $function($request['Attributes']);
+        } else { // TODO: remove this branch after removing the 'code' option.
+            $function = create_function('&$attributes', $this->code);
+            $function($request['Attributes']);
+        }
+    }
 
 }

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

+<?php
+
+
+/**
+ * Test for the core:PHP filter.
+ */
+class Test_Core_Auth_Process_PHP extends PHPUnit_Framework_TestCase
+{
+
+    /**
+     * Helper function to run the filter with a given configuration.
+     *
+     * @param array $config The filter configuration.
+     * @param array $request The request state.
+     *
+     * @return array The state array after processing.
+     */
+    private static function processFilter(array $config, array $request)
+    {
+        $filter = new sspmod_core_Auth_Process_PHP($config, null);
+        @$filter->process($request);
+        return $request;
+    }
+
+
+    /**
+     * Test the configuration of the filter.
+     *
+     * @expectedException SimpleSAML_Error_Exception
+     */
+    public function testInvalidConfiguration()
+    {
+        $config = array();
+        new sspmod_core_Auth_Process_PHP($config, null);
+    }
+
+
+
+    /**
+     * Check that defining a function works as expected.
+     */
+    public function testFunctionDefined()
+    {
+        $config = array(
+            'function' => function (&$attributes) {
+                $attributes['key'] = 'value';
+            },
+        );
+        $request = array('Attributes' => array());
+        $expected = array(
+            'Attributes' => array(
+                'key' => 'value',
+            ),
+        );
+
+        $this->assertEquals($expected, $this->processFilter($config, $request));
+    }
+
+
+    /**
+     * Check that defining the code works as expected.
+     */
+    public function testCodeDefined()
+    {
+        $config = array(
+            'code' => '
+                $attributes["key"] = "value";
+            ',
+        );
+        $request = array('Attributes' => array());
+        $expected = array(
+            'Attributes' => array(
+                'key' => 'value',
+            ),
+        );
+
+        $this->assertEquals($expected, $this->processFilter($config, $request));
+    }
+
+
+    /**
+     * Check that when both the function and code are defined, only the function is executed.
+     */
+    public function testOptionsPrecedence()
+    {
+        $config = array(
+            'function' => function (&$attributes) {
+                $attributes['who'] = 'function';
+            },
+            'code'     => '
+                $attributes["who"] = "code";
+            ',
+        );
+        $request = array('Attributes' => array());
+        $expected = array(
+            'Attributes' => array(
+                'who' => 'function',
+            ),
+        );
+
+        $this->assertEquals($expected, $this->processFilter($config, $request));
+    }
+}