UserPass(Org)Base: Fix error propagation in UserPass(Org)Base authentication sources.

Thansk to Thijs Kinkhorst for indentifying the problem!

git-svn-id: https://simplesamlphp.googlecode.com/svn/trunk@3179 44740490-163a-0410-bde0-09ae8108e29a

5 files changed, 30 insertions, 29 deletions

diff --git a/modules/core/lib/Auth/UserPassBase.php b/modules/core/lib/Auth/UserPassBase.php
index b7ee4dc..80eae48 100644
--- a/modules/core/lib/Auth/UserPassBase.php
+++ b/modules/core/lib/Auth/UserPassBase.php
@@ -185,13 +185,12 @@ abstract class sspmod_core_Auth_UserPassBase extends SimpleSAML_Auth_Source {
 	 * Handle login request.
 	 *
 	 * This function is used by the login form (core/www/loginuserpass.php) when the user
-	 * enters a username and password. On success, it will not return. If an error occurs,
-	 * it will return the error code.
+	 * enters a username and password. On success, it will not return. On wrong
+	 * username/password failure, and other errors, it will throw an exception.
 	 *
 	 * @param string $authStateId  The identifier of the authentication state.
 	 * @param string $username  The username the user wrote.
 	 * @param string $password  The password the user wrote.
-	 * @return string Error code in the case of an error.
 	 */
 	public static function handleLogin($authStateId, $username, $password) {
 		assert('is_string($authStateId)');
@@ -213,16 +212,8 @@ abstract class sspmod_core_Auth_UserPassBase extends SimpleSAML_Auth_Source {
 		 * was called. We should call login() on the same authentication source.
 		 */
 
-		try {
-			/* Attempt to log in. */
-			$attributes = $source->login($username, $password);
-		} catch (SimpleSAML_Error_Error $e) {
-			/*
-			 * Login failed. Return the error code to the login form, so that it
-			 * can display an error message to the user.
-			 */
-			return $e->getErrorCode();
-		}
+		/* Attempt to log in. */
+		$attributes = $source->login($username, $password);
 
 		/* Save the attributes we received from the login-function in the $state-array. */
 		assert('is_array($attributes)');
diff --git a/modules/core/lib/Auth/UserPassOrgBase.php b/modules/core/lib/Auth/UserPassOrgBase.php
index a2c7308..dc3a92e 100644
--- a/modules/core/lib/Auth/UserPassOrgBase.php
+++ b/modules/core/lib/Auth/UserPassOrgBase.php
@@ -196,14 +196,12 @@ abstract class sspmod_core_Auth_UserPassOrgBase extends SimpleSAML_Auth_Source {
 	 *
 	 * This function is used by the login form (core/www/loginuserpassorg.php) when the user
 	 * enters a username and password. On success, it will not return. On wrong
-	 * username/password failure, it will return the error code. Other failures will throw an
-	 * exception.
+	 * username/password failure, and other errors, it will throw an exception.
 	 *
 	 * @param string $authStateId  The identifier of the authentication state.
 	 * @param string $username  The username the user wrote.
 	 * @param string $password  The password the user wrote.
 	 * @param string $organization  The id of the organization the user chose.
-	 * @return string Error code in the case of an error.
 	 */
 	public static function handleLogin($authStateId, $username, $password, $organization) {
 		assert('is_string($authStateId)');
@@ -230,17 +228,13 @@ abstract class sspmod_core_Auth_UserPassOrgBase extends SimpleSAML_Auth_Source {
 			} else {
 				if ($orgMethod === 'force') {
 					/* The organization should be a part of the username, but isn't. */
-					return 'WRONGUSERPASS';
+					throw new SimpleSAML_Error_Error('WRONGUSERPASS');
 				}
 			}
 		}
 
-		try {
-			/* Attempt to log in. */
-			$attributes = $source->login($username, $password, $organization);
-		} catch (SimpleSAML_Error_Error $e) {
-			return $e->getErrorCode();
-		}
+		/* Attempt to log in. */
+		$attributes = $source->login($username, $password, $organization);
 
 		// Add the selected Org to the state
 		$state[self::ORGID] = $organization;
diff --git a/modules/core/templates/loginuserpass.php b/modules/core/templates/loginuserpass.php
index 76e6d4c..35272a2 100644
--- a/modules/core/templates/loginuserpass.php
+++ b/modules/core/templates/loginuserpass.php
@@ -16,8 +16,8 @@ if ($this->data['errorcode'] !== NULL) {
 	<div style="border-left: 1px solid #e8e8e8; border-bottom: 1px solid #e8e8e8; background: #f5f5f5">
 		<img src="/<?php echo $this->data['baseurlpath']; ?>resources/icons/experience/gtk-dialog-error.48x48.png" class="float-l" style="margin: 15px " />
 		<h2><?php echo $this->t('{login:error_header}'); ?></h2>
-		<p><b><?php echo $this->t('{errors:title_' . $this->data['errorcode'] . '}'); ?></b></p>
-		<p><?php echo $this->t('{errors:descr_' . $this->data['errorcode'] . '}'); ?></p>
+		<p><b><?php echo htmlspecialchars($this->t('{errors:title_' . $this->data['errorcode'] . '}', $this->data['errorparams'])); ?></b></p>
+		<p><?php echo htmlspecialchars($this->t('{errors:descr_' . $this->data['errorcode'] . '}', $this->data['errorparams'])); ?></p>
 	</div>
 <?php
 }
diff --git a/modules/core/www/loginuserpass.php b/modules/core/www/loginuserpass.php
index 77f1b91..1a4e3f5 100644
--- a/modules/core/www/loginuserpass.php
+++ b/modules/core/www/loginuserpass.php
@@ -41,6 +41,9 @@ if (array_key_exists('password', $_REQUEST)) {
 	$password = '';
 }
 
+$errorCode = NULL;
+$errorParams = NULL;
+
 if (!empty($_REQUEST['username']) || !empty($password)) {
 	/* Either username or password set - attempt to log in. */
 
@@ -56,9 +59,13 @@ if (!empty($_REQUEST['username']) || !empty($password)) {
 		setcookie($source->getAuthId() . '-username', $username, $params['expire'], $params['path'], $params['domain'], $params['secure'], $params['httponly']);
 	}
 
-	$errorCode = sspmod_core_Auth_UserPassBase::handleLogin($authStateId, $username, $password);
-} else {
-	$errorCode = NULL;
+	try {
+		sspmod_core_Auth_UserPassBase::handleLogin($authStateId, $username, $password);
+	} catch (SimpleSAML_Error_Error $e) {
+		/* Login failed. Extract error code and parameters, to display the error. */
+		$errorCode = $e->getErrorCode();
+		$errorParams = $e->getParameters();
+	}
 }
 
 $globalConfig = SimpleSAML_Configuration::getInstance();
@@ -78,6 +85,7 @@ if (array_key_exists('forcedUsername', $state)) {
 }
 $t->data['links'] = $source->getLoginLinks();
 $t->data['errorcode'] = $errorCode;
+$t->data['errorparams'] = $errorParams;
 
 if (isset($state['SPMetadata'])) {
 	$t->data['SPMetadata'] = $state['SPMetadata'];
diff --git a/modules/core/www/loginuserpassorg.php b/modules/core/www/loginuserpassorg.php
index b1d1a08..ba43f9b 100644
--- a/modules/core/www/loginuserpassorg.php
+++ b/modules/core/www/loginuserpassorg.php
@@ -50,6 +50,7 @@ if (array_key_exists('organization', $_REQUEST)) {
 }
 
 $errorCode = NULL;
+$errorParams = NULL;
 if ($organizations === NULL || !empty($organization)) {
 	if (!empty($username) && !empty($password)) {
 
@@ -61,7 +62,13 @@ if ($organizations === NULL || !empty($organization)) {
 			setcookie($source->getAuthId() . '-username', $username, $params['expire'], $params['path'], $params['domain'], $params['secure'], $params['httponly']);
 		}
 
-		$errorCode = sspmod_core_Auth_UserPassOrgBase::handleLogin($authStateId, $username, $password, $organization);
+		try {
+			sspmod_core_Auth_UserPassOrgBase::handleLogin($authStateId, $username, $password, $organization);
+		} catch (SimpleSAML_Error_Error $e) {
+			/* Login failed. Extract error code and parameters, to display the error. */
+			$errorCode = $e->getErrorCode();
+			$errorParams = $e->getParameters();
+		}
 	}
 }
 
@@ -74,6 +81,7 @@ $t->data['rememberUsernameEnabled'] = $source->getRememberUsernameEnabled();
 $t->data['rememberUsernameChecked'] = $source->getRememberUsernameChecked();
 if (isset($_COOKIE[$source->getAuthId() . '-username'])) $t->data['rememberUsernameChecked'] = TRUE;
 $t->data['errorcode'] = $errorCode;
+$t->data['errorparams'] = $errorParams;
 
 if ($organizations !== NULL) {
 	$t->data['selectedOrg'] = $organization;