From 7b56a623af1912d16e8d0d563f9f16b0b7c56a8f Mon Sep 17 00:00:00 2001
From: Andjelko Horvat <comel@vingd.com>
Date: Fri, 6 May 2011 11:50:38 +0000
Subject: [PATCH] modules/openid: new exceptions, separate linkback endpoint
 and some cleaning (issue 398).

git-svn-id: https://simplesamlphp.googlecode.com/svn/trunk@2831 44740490-163a-0410-bde0-09ae8108e29a
---
 .../openid/lib/Auth/Source/OpenIDConsumer.php | 23 +++++++++-------
 modules/openid/lib/StateStore.php             |  2 +-
 modules/openid/www/consumer.php               | 17 +++++-------
 modules/openid/www/linkback.php               | 26 +++++++++++++++++++
 4 files changed, 47 insertions(+), 21 deletions(-)
 create mode 100644 modules/openid/www/linkback.php

diff --git a/modules/openid/lib/Auth/Source/OpenIDConsumer.php b/modules/openid/lib/Auth/Source/OpenIDConsumer.php
index 09b0ad914..b983260a0 100644
--- a/modules/openid/lib/Auth/Source/OpenIDConsumer.php
+++ b/modules/openid/lib/Auth/Source/OpenIDConsumer.php
@@ -102,10 +102,14 @@ class sspmod_openid_Auth_Source_OpenIDConsumer extends SimpleSAML_Auth_Source {
 		$state['openid:AuthId'] = $this->authId;
 
 		if ($this->target !== NULL) {
+			/* We know our OpenID target URL. Skip the page where we ask for it. */
 			$this->doAuth($state, $this->target);
+
+			/* doAuth() never returns. */
+			assert('FALSE');
 		}
 
-		$id = SimpleSAML_Auth_State::saveState($state, 'openid:state');
+		$id = SimpleSAML_Auth_State::saveState($state, 'openid:init');
 
 		$url = SimpleSAML_Module::getModuleURL('openid/consumer.php');
 		SimpleSAML_Utilities::redirect($url, array('AuthState' => $id));
@@ -133,8 +137,7 @@ class sspmod_openid_Auth_Source_OpenIDConsumer extends SimpleSAML_Auth_Source {
 	private function getReturnTo($stateId) {
 		assert('is_string($stateId)');
 
-		return SimpleSAML_Module::getModuleURL('openid/consumer.php', array(
-			'returned' => 1,
+		return SimpleSAML_Module::getModuleURL('openid/linkback.php', array(
 			'AuthState' => $stateId,
 		));
 	}
@@ -163,7 +166,7 @@ class sspmod_openid_Auth_Source_OpenIDConsumer extends SimpleSAML_Auth_Source {
 	public function doAuth(array &$state, $openid) {
 		assert('is_string($openid)');
 
-		$stateId = SimpleSAML_Auth_State::saveState($state, 'openid:state');
+		$stateId = SimpleSAML_Auth_State::saveState($state, 'openid:auth');
 
 		$consumer = $this->getConsumer($state);
 
@@ -172,7 +175,7 @@ class sspmod_openid_Auth_Source_OpenIDConsumer extends SimpleSAML_Auth_Source {
 
 		// No auth request means we can't begin OpenID.
 		if (!$auth_request) {
-			throw new Exception("Authentication error; not a valid OpenID.");
+			throw new SimpleSAML_Error_BadRequest('Not a valid OpenID: ' . var_export($openid, TRUE));
 		}
 
 		$sreg_request = Auth_OpenID_SRegRequest::build(
@@ -229,7 +232,7 @@ class sspmod_openid_Auth_Source_OpenIDConsumer extends SimpleSAML_Auth_Source {
 
 			// If the redirect URL can't be built, display an error message.
 			if (Auth_OpenID::isFailure($redirect_url)) {
-				throw new Exception("Could not redirect to server: " . $redirect_url->message);
+				throw new SimpleSAML_Error_AuthSource($this->authId, 'Could not redirect to server: ' . var_export($redirect_url->message, TRUE));
 			}
 
 			SimpleSAML_Utilities::redirect($redirect_url);
@@ -240,7 +243,7 @@ class sspmod_openid_Auth_Source_OpenIDConsumer extends SimpleSAML_Auth_Source {
 
 			// Display an error if the form markup couldn't be generated; otherwise, render the HTML.
 			if (Auth_OpenID::isFailure($form_html)) {
-				throw new Exception("Could not redirect to server: " . $form_html->message);
+				throw new SimpleSAML_Error_AuthSource($this->authId, 'Could not redirect to server: ' . var_export($form_html->message, TRUE));
 			} else {
 				echo '<html><head><title>OpenID transaction in progress</title></head>
 					<body onload=\'document.getElementById("' . $form_id . '").submit()\'>' .
@@ -269,12 +272,12 @@ class sspmod_openid_Auth_Source_OpenIDConsumer extends SimpleSAML_Auth_Source {
 		// Check the response status.
 		if ($response->status == Auth_OpenID_CANCEL) {
 			// This means the authentication was cancelled.
-			throw new Exception('Verification cancelled.');
+			throw new SimpleSAML_Error_UserAborted();
 		} else if ($response->status == Auth_OpenID_FAILURE) {
 			// Authentication failed; display the error message.
-			throw new Exception("OpenID authentication failed: " . $response->message);
+			throw new SimpleSAML_Error_AuthSource($this->authId, 'Authentication failed: ' . var_export($response->message, TRUE));
 		} else if ($response->status != Auth_OpenID_SUCCESS) {
-			throw new Exceptioon('General error. Try again.');
+			throw new SimpleSAML_Error_AuthSource($this->authId, 'General error. Try again.');
 		}
 
 		// This means the authentication succeeded; extract the
diff --git a/modules/openid/lib/StateStore.php b/modules/openid/lib/StateStore.php
index e11ede26d..3e653ce91 100644
--- a/modules/openid/lib/StateStore.php
+++ b/modules/openid/lib/StateStore.php
@@ -177,7 +177,7 @@ class sspmod_openid_StateStore extends Auth_OpenID_OpenIDStore{
 		$this->associations[$server_url][$handle] = $association->serialize();
 
 		/* We rely on saveState saving with the same id as before. */
-		SimpleSAML_Auth_State::saveState($this->state, 'openid:state');
+		SimpleSAML_Auth_State::saveState($this->state, 'openid:auth');
 
 		return TRUE;
 	}
diff --git a/modules/openid/www/consumer.php b/modules/openid/www/consumer.php
index 0fcc46f5d..0ea166ac6 100644
--- a/modules/openid/www/consumer.php
+++ b/modules/openid/www/consumer.php
@@ -1,23 +1,20 @@
 <?php
 
-$config = SimpleSAML_Configuration::getInstance();
-
 /* Find the authentication state. */
-if (!array_key_exists('AuthState', $_REQUEST)) {
+if (!array_key_exists('AuthState', $_REQUEST) || empty($_REQUEST['AuthState'])) {
 	throw new SimpleSAML_Error_BadRequest('Missing mandatory parameter: AuthState');
 }
-$state = SimpleSAML_Auth_State::loadState($_REQUEST['AuthState'], 'openid:state');
+
 $authState = $_REQUEST['AuthState'];
-$authSource = SimpleSAML_Auth_Source::getById($state['openid:AuthId']);
+$state = SimpleSAML_Auth_State::loadState($authState, 'openid:init');
+$sourceId = $state['openid:AuthId'];
+$authSource = SimpleSAML_Auth_Source::getById($sourceId);
 if ($authSource === NULL) {
-	throw new SimpleSAML_Error_BadRequest('Invalid AuthId \'' . $state['openid:AuthId'] . '\' - not found.');
+	throw new SimpleSAML_Error_BadRequest('Invalid AuthId \'' . $sourceId . '\' - not found.');
 }
 
-
 try {
-	if (array_key_exists('returned', $_GET)) {
-		$authSource->postAuth($state);
-	} elseif (!empty($_GET['openid_url'])) {
+	if (!empty($_GET['openid_url'])) {
 		$authSource->doAuth($state, (string)$_GET['openid_url']);
 	}
 } catch (Exception $e) {
diff --git a/modules/openid/www/linkback.php b/modules/openid/www/linkback.php
new file mode 100644
index 000000000..d63286b18
--- /dev/null
+++ b/modules/openid/www/linkback.php
@@ -0,0 +1,26 @@
+<?php
+
+$config = SimpleSAML_Configuration::getInstance();
+
+/* Find the authentication state. */
+if (!array_key_exists('AuthState', $_REQUEST) || empty($_REQUEST['AuthState'])) {
+	throw new SimpleSAML_Error_BadRequest('Missing mandatory parameter: AuthState');
+}
+
+$authState = $_REQUEST['AuthState'];
+$state = SimpleSAML_Auth_State::loadState($authState, 'openid:auth');
+$sourceId = $state['openid:AuthId'];
+$authSource = SimpleSAML_Auth_Source::getById($sourceId);
+if ($authSource === NULL) {
+	throw new SimpleSAML_Error_BadRequest('Invalid AuthId \'' . $sourceId . '\' - not found.');
+}
+
+try {
+	$authSource->postAuth($state);
+	/* postAuth() should never return. */
+	assert('FALSE');
+} catch (SimpleSAML_Error_Exception $e) {
+	SimpleSAML_Auth_State::throwException($state, $e);
+} catch (Exception $e) {
+	SimpleSAML_Auth_State::throwException($state, new SimpleSAML_Error_AuthSource($sourceId, 'Error on OpenID linkback endpoint.', $e));
+}
-- 
GitLab