Move SimpleSAML_Auth_Default::extractPersistentAuthState() to...

Move SimpleSAML_Auth_Default::extractPersistentAuthState() to SimpleSAML_Auth_State::extractPersistentAuthState() and deprecate the former.

docs/simplesamlphp-upgrade-notes-1.14.txt

@@ -86,6 +86,7 @@ The following methods have changed their signature. Refer to the code for the up
 The following methods and classes have been deprecated. Refer to the code for alternatives:
 
     * `SimpleSAML_Auth_Default`
+    * `SimpleSAML_Auth_Default::extractPersistentAuthState()`
     * `SimpleSAML_Utilities`
     * `SimpleSAML_Utilities::addURLParameter()`
     * `SimpleSAML_Utilities::aesDecrypt()`

lib/SimpleSAML/Auth/Default.php

@@ -8,6 +8,8 @@
  *
  * @author Olav Morken, UNINETT AS.
  * @package simpleSAMLphp
+ *
+ * @deprecated This class will be removed in SSP 2.0.
  */
 class SimpleSAML_Auth_Default {
 
@@ -78,33 +80,14 @@ class SimpleSAML_Auth_Default {
 	 *
 	 * @param array $state  The state after the login.
 	 * @return array  The persistent authentication state.
+	 *
+	 * @deprecated This method will be removed in SSP 2.0. Please use
+	 * SimpleSAML_Auth_State::extractPersistentAuthState() instead.
 	 */
 	public static function extractPersistentAuthState(array &$state) {
 
-		/* Save persistent authentication data. */
-		$persistentAuthState = array();
-
-		if (isset($state['IdP'])) {
-			/* For backwards compatibility. */
-			$persistentAuthState['saml:sp:IdP'] = $state['IdP'];
-		}
-
-		if (isset($state['PersistentAuthData'])) {
-			foreach ($state['PersistentAuthData'] as $key) {
-				if (isset($state[$key])) {
-					$persistentAuthState[$key] = $state[$key];
-				}
-			}
-		}
-
-		/* Add those that should always be included. */
-		foreach (array('Attributes', 'Expire', 'LogoutState', 'AuthnInstant', 'RememberMe', 'saml:sp:NameID') as $a) {
-			if (isset($state[$a])) {
-				$persistentAuthState[$a] = $state[$a];
-			}
-		}
-
-		return $persistentAuthState;
+		$state = SimpleSAML_Auth_State::extractPersistentAuthState($state);
+		return $state;
 	}
 
 
@@ -124,7 +107,9 @@ class SimpleSAML_Auth_Default {
 
 		/* Save session state. */
 		$session = SimpleSAML_Session::getSessionFromRequest();
-		$session->doLogin($state['SimpleSAML_Auth_Default.id'], self::extractPersistentAuthState($state));
+		$authId = $state['SimpleSAML_Auth_Default.id'];
+		$state = SimpleSAML_Auth_State::extractPersistentAuthState($state);
+		$session->doLogin($authId, $state);
 
 		if (is_string($return)) {
 			/* Redirect... */
@@ -263,7 +248,7 @@ class SimpleSAML_Auth_Default {
 		assert('is_string($redirectTo)');
 
 		$session = SimpleSAML_Session::getSessionFromRequest();
-		$session->doLogin($authId, self::extractPersistentAuthState($state));
+		$session->doLogin($authId, SimpleSAML_Auth_State::extractPersistentAuthState($state));
 
 		\SimpleSAML\Utils\HTTP::redirectUntrustedURL($redirectTo);
 	}

lib/SimpleSAML/Auth/State.php

@@ -91,6 +91,44 @@ class SimpleSAML_Auth_State {
 	private static $stateTimeout = NULL;
 
 
+	/**
+	 * Extract the persistent authentication state from the state array.
+	 *
+	 * @param array $state The state array to analyze.
+	 * @return array The persistent authentication state.
+	 */
+	public static function extractPersistentAuthState(array $state)
+	{
+		// save persistent authentication data
+		$persistent = array();
+
+		if (array_key_exists('PersistentAuthData', $state)) {
+			foreach ($state['PersistentAuthData'] as $key) {
+				if (isset($state[$key])) {
+					$persistent[$key] = $state[$key];
+				}
+			}
+		}
+
+		// add those that should always be included
+		$mandatory = array(
+			'Attributes',
+			'Expire',
+			'LogoutState',
+			'AuthInstant',
+			'RememberMe',
+			'saml:sp:NameID'
+		);
+		foreach ($mandatory as $key) {
+			if (isset($state[$key])) {
+				$persistent[$key] = $state[$key];
+			}
+		}
+
+		return $persistent;
+	}
+
+
 	/**
 	 * Retrieve the ID of a state array.
 	 *

modules/saml/lib/Auth/Source/SP.php

@@ -439,7 +439,9 @@ class sspmod_saml_Auth_Source_SP extends SimpleSAML_Auth_Source {
 
 		// Update session state
 		$session = SimpleSAML_Session::getSessionFromRequest();
-		$session->doLogin($state['saml:sp:AuthId'], SimpleSAML_Auth_Default::extractPersistentAuthState($state));
+		$authId = $state['saml:sp:AuthId'];
+		$state = SimpleSAML_Auth_State::extractPersistentAuthState($state);
+		$session->doLogin($authId, $state);
 
 		// resume the login process
 		call_user_func($state['ReturnCallback'], $state);

tests/lib/SimpleSAML/Auth/StateTest.php

+<?php
+
+
+/**
+ * Tests for SimpleSAML_Auth_State
+ */
+class Auth_StateTest extends PHPUnit_Framework_TestCase
+{
+
+
+    /**
+     * Test the extractPersistentAuthState() function.
+     */
+    public function testExtractPersistentAuthState()
+    {
+
+        $mandatory = array(
+            'Attributes' => array(),
+            'Expire' => 1234,
+            'LogoutState' => 'logoutState',
+            'AuthInstant' => 123456,
+            'RememberMe' => true,
+            'saml:sp:NameID' => 'nameID',
+        );
+
+        // check just mandatory parameters
+        $state = $mandatory;
+        $expected = $mandatory;
+        $this->assertEquals(
+            $expected,
+            SimpleSAML_Auth_State::extractPersistentAuthState($state),
+            'Mandatory state attributes did not survive as expected'.print_r($expected, true)
+        );
+
+        // check missing mandatory parameters
+        unset($state['LogoutState']);
+        unset($state['RememberMe']);
+        $expected = $state;
+        $this->assertEquals(
+            $expected,
+            SimpleSAML_Auth_State::extractPersistentAuthState($state),
+            'Some error occurred with missing mandatory parameters'
+        );
+
+        // check additional non-persistent parameters
+        $additional = array(
+            'additional1' => 1,
+            'additional2' => 2,
+        );
+        $state = array_merge($mandatory, $additional);
+        $expected = $mandatory;
+        $this->assertEquals(
+            $expected,
+            SimpleSAML_Auth_State::extractPersistentAuthState($state),
+            'Additional parameters survived'
+        );
+
+        // check additional persistent parameters
+        $additional['PersistentAuthData'] = array('additional1');
+        $state = array_merge($mandatory, $additional);
+        $expected = $state;
+        unset($expected['additional2']);
+        unset($expected['PersistentAuthData']);
+        $this->assertEquals(
+            $expected,
+            SimpleSAML_Auth_State::extractPersistentAuthState($state),
+            'Some error occurred with additional, persistent parameters'
+        );
+
+        // check only additional persistent parameters
+        $state = $additional;
+        $expected = $state;
+        unset($expected['additional2']);
+        unset($expected['PersistentAuthData']);
+        $this->assertEquals(
+            $expected,
+            SimpleSAML_Auth_State::extractPersistentAuthState($state),
+            'Some error occurred with additional, persistent parameters, and no mandatory ones'
+        );
+    }
+}