In reference to Issue #20: On Thu 2011-12-08 11:53:44 -0500, Christian Boos <reply+i-2489113-97f3eb1c5c6ef559403b66ade9d4d94c192f4bfa-66...@reply.github.com> wrote:
> Short summary: in a custom login page > (https://members.mayfirst.org/openid/login/), the form adds a hidden > referer parameter. This query parameter gets preserved as expected, > but the problem is that it is also **duplicated** (see > http://trac.edgewall.org/ticket/10491#comment:11 for a reproduction > recipe). The targeted application (Trac) doesn't like this duplicated > parameter and fails. I can give more details about the situation in which this happens. My relevant apache configuration for foo.example.org is: <Location "/login"> AuthType OpenID require valid-user AuthOpenIDDBLocation /srv/mod_auth_openid/db/openid.db AuthOpenIDUseCookie Off AuthOpenIDLoginPage http://bar.example.org/openid/login/ </Location> I start with an empty database. The login sequence is: 0) GET http://foo.example.org/ (user clicks for:) 1) GET http://foo.example.org/login (HTTP 302 redirection to:) 2) GET http://bar.example.org/openid/login/? modauthopenid.referrer=http%3A%2F%2Ffoo.example.org%2Flogin%3F (user chooses an ID, and clicks submit:) 3) GET http://foo.example.org/login? referer=http%3A%2F%2Ffoo.example.org%2F& openid_identifier=https%3A%2F%2Fid.mayfirst.org%2Fdkg (HTTP 302 redirection to:) 4) GET https://id.mayfirst.org/openid/provider? openid.assoc_handle=2012-11-26T18%3A44%3A42Zc4ffb4be939d1f3af09b1f810928cb35c0836d0102c950f839b19622df577ceb& openid.claimed_id=https%3A%2F%2Fid.mayfirst.org%2Fdkg& openid.identity=https%3A%2F%2Fid.mayfirst.org%2Fdkg& openid.mode=checkid_setup& openid.ns=http%3A%2F%2Fspecs.openid.net%2Fauth%2F2.0& openid.realm=http%3A%2F%2Ffoo.example.org%2F& openid.return_to=http%3A%2F%2Ffoo.example.org%2Flogin%3Freferer%3Dhttp%253A%252F%252Ffoo.example.org%252F%26modauthopenid.nonce%3DLh1AO9ngBY%26referer%3Dhttp%253A%252F%252Ffoo.example.org%252F& openid.trust_root=http%3A%2F%2Ffoo.example.org%2F note that openid.return_to here is (decoded and re-wrapped): http://foo.example.org/login? referer=http%3A%2F%2Ffoo.example.org%2F& modauthopenid.nonce=Lh1AO9ngBY& referer=http%3A%2F%2Ffoo.example.org%2F So the HTTP 302 redirection that leads from GET 3 to GET 4 is generated by mod_auth_openid, based on the parameters which come in from the AuthOpenIDLoginPage. It duplicates the "referer" GET parameter, which was passed by the AuthOpenIDLoginPage. So i think what is happening is: a) mod_authopenid_method_handler() extracts the current request's GET parameters into an opkele::params_t object named params(), and (based on the presence of the openid_identifier parameter), decides to start_authentication_session() with that params object. b) in start_authentication_session(), we extract the full URL (including all the non-openid GET parameters) of the current request into return_to via full_uri(), and then append the params to it. This results in a doubling of every non-openid GET parameter into the return_to variable. I don't understand why it's done this way, or what is gained from it. The attached patch avoids keeping the params in the return_to extraction in (b), and seems to resolve the issue for me under this configuration. --dkg
diff --git a/src/mod_auth_openid.cpp b/src/mod_auth_openid.cpp
index 8196506..777e779 100644
--- a/src/mod_auth_openid.cpp
+++ b/src/mod_auth_openid.cpp
@@ -217,9 +217,11 @@ static const command_rec mod_authopenid_cmds[] = {
{NULL}
};
+enum keep_params_t { kp_all, kp_clean, kp_none };
+
// Get the full URI of the request_rec's request location
-// clean_params specifies whether or not all openid.* and modauthopenid.* params should be cleared
-static void full_uri(request_rec *r, std::string& result, modauthopenid_config *s_cfg, bool clean_params=false) {
+// keep_params specifies whether whether we keep all parameters, clean out the openid.* and modauthopenid.* params, or keep none of them.
+static void full_uri(request_rec *r, std::string& result, modauthopenid_config *s_cfg, keep_params_t keep_params=kp_all) {
std::string hostname(r->hostname);
std::string uri(r->uri);
apr_port_t i_port = ap_get_server_port(r);
@@ -230,12 +232,19 @@ static void full_uri(request_rec *r, std::string& result, modauthopenid_config *
std::string s_port = (i_port == 80 || i_port == 443) ? "" : ":" + std::string(port);
std::string args;
- if(clean_params) {
- opkele::params_t params;
- if(r->args != NULL) params = modauthopenid::parse_query_string(std::string(r->args));
- modauthopenid::remove_openid_vars(params);
- args = params.append_query("", "");
- } else {
+ switch (keep_params) {
+ case kp_clean:
+ {
+ opkele::params_t params;
+ if(r->args != NULL) params = modauthopenid::parse_query_string(std::string(r->args));
+ modauthopenid::remove_openid_vars(params);
+ args = params.append_query("", "");
+ }
+ break;
+ case kp_none:
+ args = "";
+ break;
+ default:
args = (r->args == NULL) ? "" : "?" + std::string(r->args);
}
@@ -256,7 +266,7 @@ static int show_input(request_rec *r, modauthopenid_config *s_cfg, modauthopenid
modauthopenid::remove_openid_vars(params);
std::string uri_location;
- full_uri(r, uri_location, s_cfg, true);
+ full_uri(r, uri_location, s_cfg, kp_clean);
params["modauthopenid.referrer"] = uri_location;
params["modauthopenid.error"] = modauthopenid::error_to_string(e, true);
@@ -271,7 +281,7 @@ static int show_input(request_rec *r, modauthopenid_config *s_cfg) {
params = modauthopenid::parse_query_string(std::string(r->args));
modauthopenid::remove_openid_vars(params);
std::string uri_location;
- full_uri(r, uri_location, s_cfg, true);
+ full_uri(r, uri_location, s_cfg, kp_clean);
params["modauthopenid.referrer"] = uri_location;
return modauthopenid::http_redirect(r, params.append_query(s_cfg->login_page, ""));
}
@@ -390,7 +400,7 @@ static int start_authentication_session(request_rec *r, modauthopenid_config *s_
modauthopenid::make_rstring(10, nonce);
modauthopenid::MoidConsumer consumer(std::string(s_cfg->db_location), nonce, return_to);
params["modauthopenid.nonce"] = nonce;
- full_uri(r, return_to, s_cfg, true);
+ full_uri(r, return_to, s_cfg, kp_none);
return_to = params.append_query(return_to, "");
// get identity provider and redirect
pgpUYazk3JM1u.pgp
Description: PGP signature
