A bunch of fixes that make OpenID authentication work for the first time.

1 files changed, 81 insertions, 69 deletions

diff --git a/module/consumer.cc b/module/consumer.cc
index 91ab773..9933eeb 100644
--- a/module/consumer.cc
+++ b/module/consumer.cc
@@ -15,6 +15,7 @@ using opkele::exception;
 using opkele::failed_discovery;
 using opkele::failed_lookup;
 using opkele::failed_xri_resolution;
+using opkele::id_res_bad_nonce;
 using opkele::no_endpoint;
 using opkele::openid_endpoint_t;
 using opkele::openid_message_t;
@@ -34,21 +35,6 @@ public:
 		{ sid_shared_unlock (); }
 };
 
-class AdaptorFix :
-	public params_t
-{
-public:
-	AdaptorFix(params_t& params)
-		{ _params = params; };
-	virtual bool has_field(const string& n) const
-		{ return _params.has_param("openid." + n); }
-	virtual const string& get_field(const string& n) const
-		{ return _params.get_param("openid." + n); }
-private:
-    params_t _params;
-};
-
-
 class Consumer :
 	public prequeue_RP
 {
@@ -57,7 +43,7 @@ private: // types
 
 public: // interface
 	Consumer(const char *url, sid_storage_t *store)
-		: _url(url), _store(store), _index(0)
+		: _store(store), _url(url), _index(0)
 		{ }
 
 public: // overrides
@@ -202,11 +188,15 @@ Consumer::invalidate_assoc(const string& server, const string& handle)
 void
 Consumer::check_nonce(const string& server, const string& nonce)
 {
-	if (!_store)
-		throw dumb_RP("no storage initialized");
+	int res = 0;
 
-	LockShared lock;
-	sid_storage_check_nonce (_store, server.c_str(), nonce.c_str());
+	if (_store) {
+		LockShared lock;
+		res = sid_storage_check_nonce (_store, server.c_str(), nonce.c_str());
+	}
+
+	if (res == 1)
+		throw id_res_bad_nonce ("nonce is too old, or has already been used");
 }
 
 /* -----------------------------------------------------------------------
@@ -214,11 +204,13 @@ Consumer::check_nonce(const string& server, const string& nonce)
  */
 
 static void
-filter_openid_params (params_t &params)
+filter_openid_params (params_t &params, params_t &openid)
 {
 	for (params_t::iterator it = params.begin(); it != params.end(); ) {
 		const string& name = it->first;
 		if (name.find ("openid.") == 0) {
+			openid[name.substr(7)] = it->second;
+
 			/* We erase an increment together, must use post-increment operator */
 			params.erase (it++);
 		} else {
@@ -234,7 +226,10 @@ parse_query_string (const char *qs, params_t &params)
 	string pair, key, value;
 	const char *at;
 
-	while (qs && *qs) {
+	if (qs == NULL)
+		return;
+
+	while (*qs != 0) {
 		at = strchr (qs, '&');
 		if (at == NULL)
 			at = qs + strlen (qs);
@@ -249,7 +244,9 @@ parse_query_string (const char *qs, params_t &params)
 		}
 
 		params[opkele::util::url_decode(key)] = opkele::util::url_decode(value);
-		qs = at;
+		if (*at == 0)
+			break;
+		qs = at + 1;
 	}
 }
 
@@ -257,67 +254,75 @@ static void
 begin_auth (sid_request_t *req, Consumer &consumer, params_t &params,
             const string& trust_root, const string &identity)
 {
-	  /* Remove all openid params, and stash away extensions */
-	  filter_openid_params (params);
-	  string return_to = params.append_query (consumer.get_this_url(), "");
-
-	  params_t result;
-	  string redirect;
-
-	  try {
-		  openid_message_t cm;
-		  consumer.initiate (identity);
-		  consumer.checkid_ (cm, opkele::mode_checkid_setup, return_to, trust_root);
-		  redirect = cm.append_query (consumer.get_endpoint().uri);
-
-	  } catch (failed_xri_resolution &ex) {
-		  sid_request_respond (req, 503, "Invalid Identifier", NULL, NULL);
-		  sid_request_log_error (req, "failed xri resolution while while discovering identity provider",
-		                         ex.what ());
-		  return;
+	params_t openid;
 
-	  } catch (failed_discovery &ex) {
-		  sid_request_respond (req, 503, "Invalid Identifier", NULL, NULL);
-		  sid_request_log_error (req, "failed discovery while while discovering identity provider",
-		                         ex.what ());
-		  return;
+	/* Remove all openid params, and stash away extensions */
+	filter_openid_params (params, openid);
+	string return_to = consumer.get_this_url();
+	if (!params.empty())
+		return_to = params.append_query (return_to, "");
 
-	  } catch (bad_input &ex) {
-		  sid_request_respond (req, 503, "Invalid Identifier", NULL, NULL);
-		  sid_request_log_error (req, "bad input while while discovering identity provider",
-		                         ex.what());
-		  return;
+	params_t result;
+	string redirect;
 
-	  } catch (exception &ex) {
-		  sid_request_respond (req, 500, NULL, NULL, NULL);
-		  sid_request_log_error (req, "error while while discovering identity provider",
-		                         ex.what());
+	try {
+		openid_message_t cm;
+		consumer.initiate (identity);
+		consumer.checkid_ (cm, opkele::mode_checkid_setup, return_to, trust_root);
+		redirect = cm.append_query (consumer.get_endpoint().uri);
+
+	} catch (failed_xri_resolution &ex) {
+		sid_request_respond (req, 503, "Invalid Identifier", NULL);
+		sid_request_log_error (req, "failed xri resolution while while discovering identity provider",
+		                       ex.what ());
+		return;
+
+	} catch (failed_discovery &ex) {
+		sid_request_respond (req, 503, "Invalid Identifier", NULL);
+		sid_request_log_error (req, "failed discovery while while discovering identity provider",
+		                       ex.what ());
 		  return;
-	  }
 
-	  sid_request_respond (req, 307, "Moved Temporarily", NULL,
-	                       "Location", redirect.c_str(),
-	                       "Cache-Control", "no-cache",
-	                       NULL);
+	} catch (bad_input &ex) {
+		sid_request_respond (req, 503, "Invalid Identifier", NULL);
+		sid_request_log_error (req, "bad input while while discovering identity provider",
+		                       ex.what());
+		return;
+
+	} catch (exception &ex) {
+		sid_request_respond (req, 500, NULL, NULL);
+		sid_request_log_error (req, "error while while discovering identity provider",
+		                       ex.what());
+		return;
+	}
+
+	sid_request_respond (req, 307, "Moved Temporarily",
+	                     "Location", redirect.c_str(),
+	                     "Cache-Control", "no-cache",
+	                     NULL);
 }
 
 static void
 complete_auth (sid_request_t *req, Consumer &consumer, params_t &params)
 {
+	params_t openid;
+
+	filter_openid_params (params, openid);
+
 	try {
-		consumer.id_res(AdaptorFix(params));
+		consumer.id_res(openid);
 		string identity = consumer.get_claimed_id();
 		sid_request_authenticated (req, identity.c_str());
 	} catch (exception &ex) {
-		sid_request_respond (req, 500, NULL, NULL, NULL);
-		sid_request_log_error (req, "error while completing authentication: %s", ex.what());
+		sid_request_respond (req, 500, NULL, NULL);
+		sid_request_log_error (req, "error while completing authentication", ex.what());
 	}
 }
 
 static void
 cancelled_auth (sid_request_t *req, Consumer &consumer, params_t &params)
 {
-	sid_request_respond (req, 401, "Authentication Required", NULL, NULL);
+	sid_request_respond (req, 401, "Authentication Required", NULL);
 }
 
 void
@@ -327,18 +332,25 @@ sid_consumer_authenticate(sid_request_t *req, sid_storage_t *store,
 	params_t params;
 
 	assert (req);
-	assert (store);
 
 	const char *qs = sid_request_qs (req);
 	parse_query_string (qs, params);
 
-	const char *url = sid_request_url (req);
+	const char *url = sid_request_url (req, 1);
 	Consumer consumer(url, store);
 
-	if (params.has_param("openid.assoc_handle"))
+	/* Returning (hopefully successful) authentication */
+	if (params.has_param("openid.assoc_handle")) {
 		complete_auth (req, consumer, params);
-	else if (params.has_param("openid.mode") && params.get_param("openid.mode") == "cancel")
+
+	/* Returning cancelled authentication */
+	} else if (params.has_param("openid.mode") && params.get_param("openid.mode") == "cancel") {
 		cancelled_auth (req, consumer, params);
-	else
+
+	/* Begin a new authentication */
+	} else {
+		if (!trust_root)
+			trust_root = sid_request_url (req, 0);
 		begin_auth (req, consumer, params, trust_root, identity);
+	}
 }