diff --git a/src/mod_sso/mod_sso.c b/src/mod_sso/mod_sso.c index 7fef0a61f8622673d13719aaf5d63b655fe7f9fa..1a35fae7ced35e8abd06dc7f361ba893adf0da63 100644 --- a/src/mod_sso/mod_sso.c +++ b/src/mod_sso/mod_sso.c @@ -448,6 +448,14 @@ static int mod_sso_method_handler(request_rec *r) { return HTTP_BAD_REQUEST; } + // Check that the redirect destination is valid. + redir = modsso_url_decode(r->pool, params.d); + if (!is_valid_redir(r, redir, service)) { + ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, + "sso: invalid redirect to %s", redir); + return HTTP_BAD_REQUEST; + } + // Parse the SSO ticket and validate the nonce with the session. // Only do this if a session key is set (sessions are enabled). if (s_cfg->session_key != NULL) { @@ -455,7 +463,12 @@ static int mod_sso_method_handler(request_rec *r) { &unique_id, sso_login_path) < 0) { ap_log_error(APLOG_MARK, APLOG_INFO, 0, r->server, "sso: could not read session cookie"); - return HTTP_BAD_REQUEST; + // Instead of returning 400 here, we can try to improve the + // user experience by redirecting them to the 'redir' endpoint + // without setting the SSO cookie (hoping that whatever the + // reason for the lack of session cookie, it is a temporary + // issue). This creates the potential for a redirect loop. + return http_redirect(r, redir); } if ((err = sso_ticket_open(&t, params.t, s_cfg->public_key)) != SSO_OK) { ap_log_error(APLOG_MARK, APLOG_WARNING, 0, r->server, @@ -477,14 +490,7 @@ static int mod_sso_method_handler(request_rec *r) { modsso_set_cookie(r, sso_cookie_name, params.t, service_path); - redir = modsso_url_decode(r->pool, params.d); - if (!is_valid_redir(r, redir, service)) { - ap_log_error(APLOG_MARK, APLOG_ERR, 0, r->server, - "sso: invalid redirect to %s", redir); - return HTTP_BAD_REQUEST; - } else { - return http_redirect(r, redir); - } + return http_redirect(r, redir); } return DECLINED;