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

4 files changed, 163 insertions, 86 deletions

diff --git a/configure.in b/configure.in
index bcbc312..56de6db 100644
--- a/configure.in
+++ b/configure.in
@@ -121,7 +121,8 @@ AC_ARG_ENABLE(debug,
 		[Compile binaries in debug mode]))
 
 if test "$enable_debug" = "yes"; then
-	CFLAGS="$CFLAGS -g -O0 -Wall -Werror"
+	CFLAGS="-Wall -Werror $CFLAGS -g -O0"
+	CXXFLAGS="-Wall -Werror $CXXFLAGS -g -O0"
 	AC_DEFINE_UNQUOTED(_DEBUG, 1, [In debug mode])
 	echo "enabling debug compile mode"
 fi
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);
+	}
 }
diff --git a/module/mod_auth_singleid.c b/module/mod_auth_singleid.c
index 3b6304a..9c38434 100644
--- a/module/mod_auth_singleid.c
+++ b/module/mod_auth_singleid.c
@@ -92,8 +92,8 @@ static int
 shared_initialize (apr_pool_t *p, server_rec *s)
 {
 	apr_file_t *file = NULL;
+	char *lock_name = NULL;
 	const char *tmpdir;
-	char *lock_name;
 	int rc;
 
 	/* This may be called more than once */
@@ -324,7 +324,7 @@ session_cookie_value (request_rec *r, const char *name)
 		return NULL;
 
 	while (*cookies) {
-		pair = ap_get_token (r->pool, &cookies, 0);
+		pair = ap_get_token (r->pool, &cookies, 1);
 		if (!pair)
 			break;
 		if (pair[0] == '$')
@@ -338,6 +338,7 @@ session_cookie_value (request_rec *r, const char *name)
 
 		if (*value != '=')
 			continue;
+		++value;
 		while (isspace (*value))
 			++value;
 
@@ -380,22 +381,22 @@ session_load_info (request_rec *r)
 	char *token, *sig, *end;
 	char *identifier;
 	long expiry;
+	size_t len;
 
 	value = session_cookie_value (r, "mod-auth-single-id");
 	if (!value)
 		return NULL;
 
-	sig = ap_get_token (r->pool, &value, 1);
+	sig = ap_get_token (r->pool, &value, 0);
+	if (!session_validate_sig (r->pool, sig, value))
+		return NULL;
 
 	/* The version of the session info, only 1 supported for now */
-	token = ap_get_token (r->pool, &value, 1);
+	token = ap_get_token (r->pool, &value, 0);
 	if (strcmp (token, "1") != 0)
 		return NULL;
 
-	if (!session_validate_sig (r->pool, sig, value))
-		return NULL;
-
-	token = ap_get_token (r->pool, &value, 1);
+	token = ap_get_token (r->pool, &value, 0);
 	expiry = strtol (token, &end, 10);
 	if (*end != '\0')
 		return NULL;
@@ -405,7 +406,13 @@ session_load_info (request_rec *r)
 		return NULL;
 
 	/* The identifier */
-	identifier = ap_get_token (r->pool, &value, 1);
+	identifier = ap_get_token (r->pool, &value, 0);
+	len = strlen (identifier);
+	if (identifier[0] == '"' && identifier[len - 1] == '"') {
+		identifier[len - 1] = 0;
+		++identifier;
+	}
+
 	if (!ap_is_url (identifier))
 		return NULL;
 
@@ -466,7 +473,7 @@ sid_request_qs (sid_request_t *req)
 }
 
 const char*
-sid_request_url (sid_request_t *req)
+sid_request_url (sid_request_t *req, int with_path)
 {
 	/* function to determine if a connection is using https */
 	static APR_OPTIONAL_FN_TYPE(ssl_is_https) *using_https = NULL;
@@ -487,7 +494,7 @@ sid_request_url (sid_request_t *req)
 	host = req->rec->hostname ? req->rec->hostname : ap_get_server_name (req->rec);
 	scheme = is_ssl ? "https" : "http";
 	port = ap_get_server_port (req->rec);
-	uri = req->rec->uri ? req->rec->uri : "";
+	uri = with_path && req->rec->uri ? req->rec->uri : "";
 
 	/* Default ports? */
 	if ((port == 80 && !is_ssl) || (port == 443 && is_ssl))
@@ -573,8 +580,8 @@ hook_authenticate (request_rec* r)
 	if (!(authtype = ap_auth_type (r)) || strcasecmp (SID_AUTHTYPE, authtype) != 0)
 		return DECLINED;
 
-	ctx = (sid_context_t*)ap_get_module_config(r->per_dir_config, &auth_singleid_module);
-	if(ctx->identifier == NULL)
+	ctx = (sid_context_t*)ap_get_module_config (r->per_dir_config, &auth_singleid_module);
+	if (ctx->identifier == NULL)
 		return DECLINED;
 
 	mainreq = r;
@@ -611,14 +618,70 @@ hook_authenticate (request_rec* r)
 	return req.result;
 }
 
+static int
+hook_access(request_rec *r)
+{
+	sid_context_t* ctx;
+	const char* authtype;
+	char *user = r->user;
+	int m = r->method_number;
+	int method_restricted = 0;
+	register int x;
+	const char *t, *w;
+	const apr_array_header_t *reqs_arr;
+	require_line *reqs;
+
+	/* Make sure it's for us */
+	if (!(authtype = ap_auth_type (r)) || strcasecmp (SID_AUTHTYPE, authtype) != 0)
+		return DECLINED;
+
+	ctx = (sid_context_t*)ap_get_module_config (r->per_dir_config, &auth_singleid_module);
+
+	reqs_arr = ap_requires (r);
+	if (!reqs_arr)
+		return OK;
+
+	reqs = (require_line *)reqs_arr->elts;
+	for (x = 0; x < reqs_arr->nelts; x++) {
+		if (!(reqs[x].method_mask & (AP_METHOD_BIT << m)))
+			continue;
+
+		method_restricted = 1;
+
+		t = reqs[x].requirement;
+		w = ap_getword_white (r->pool, &t);
+		if (!strcmp (w, "valid-user")) {
+			return OK;
+		} else if (!strcmp (w, "user")) {
+			while (t[0]) {
+				w = ap_getword_conf (r->pool, &t);
+				if (!strcmp (user, w)) {
+					return OK;
+				}
+			}
+		} else {
+			ap_log_rerror (APLOG_MARK, APLOG_ERR, 0, r,
+			               "access to %s failed, reason: unknown require "
+			               "directive:\"%s\"", r->uri, reqs[x].requirement);
+		}
+	}
+
+	if (!method_restricted)
+		return OK;
+
+	ap_log_rerror (APLOG_MARK, APLOG_ERR, 0, r,
+	               "access to %s failed, reason: user %s not allowed access",
+	               r->uri, user);
+	return HTTP_UNAUTHORIZED;
+}
+
 static void
 register_hooks(apr_pool_t *p)
 {
-	ap_log_perror (APLOG_MARK, APLOG_ERR, 0, p, "mod_auth_singleid registering hooks");
-
 	ap_hook_post_config (hook_initialize, NULL, NULL, APR_HOOK_MIDDLE);
 	ap_hook_child_init (hook_child, NULL, NULL, APR_HOOK_MIDDLE);
 	ap_hook_check_user_id (hook_authenticate, NULL, NULL, APR_HOOK_MIDDLE);
+	ap_hook_auth_checker (hook_access, NULL, NULL, APR_HOOK_MIDDLE);
 }
 
 module AP_MODULE_DECLARE_DATA auth_singleid_module = {
diff --git a/module/mod_auth_singleid.h b/module/mod_auth_singleid.h
index eb6d7f7..038e23e 100644
--- a/module/mod_auth_singleid.h
+++ b/module/mod_auth_singleid.h
@@ -19,7 +19,8 @@ void              sid_request_log_error          (sid_request_t *req,
 
 const char*       sid_request_qs                 (sid_request_t *req);
 
-const char*       sid_request_url                (sid_request_t *req);
+const char*       sid_request_url                (sid_request_t *req,
+                                                  int with_path);
 
 void              sid_request_respond            (sid_request_t *req,
                                                   int code,