From 91ccfcb151836d8b94c52a1a63e91abefff2a559 Mon Sep 17 00:00:00 2001 From: ale <ale@incal.net> Date: Thu, 26 Sep 2019 17:22:23 +0100 Subject: [PATCH] Try to improve handling of missing session cookies in mod_sso Redirect the user to the (unauthenticated) destination, so that it might try again the request. This introduces potential for a redirect loop unfortunately, so let's see how well it works. --- src/mod_sso/mod_sso.c | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/mod_sso/mod_sso.c b/src/mod_sso/mod_sso.c index 7fef0a6..1a35fae 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; -- GitLab