[cpp-sp] branch main updated: Clean up policing of redirects.

Codeberg noreply at shibboleth.net
Tue May 26 14:43:16 UTC 2026


This is an automated email from the git hooks/post-receive script.

codeberg pushed a commit to branch main
in repository cpp-sp.

View the commit online:
https://codeberg.org/Shibboleth/cpp-sp/commit/26d9de9990130c9b4479ed8d4cd144545613a840

The following commit(s) were added to refs/heads/main by this push:
     new 26d9de99 Clean up policing of redirects.
26d9de99 is described below

commit 26d9de9990130c9b4479ed8d4cd144545613a840
Author: Scott Cantor <scott at restingparrotsoftware.com>
AuthorDate: Tue May 26 10:43:02 2026 -0400

    Clean up policing of redirects.
---
 shibsp/handler/AbstractHandler.h        |  5 +++-
 shibsp/handler/impl/AbstractHandler.cpp |  5 +++-
 shibsp/handler/impl/LogoutConsumer.cpp  | 46 +++++++++++++++++++++++++++------
 shibsp/handler/impl/Passthrough.cpp     |  9 +------
 shibsp/handler/impl/TokenConsumer.cpp   |  2 +-
 5 files changed, 48 insertions(+), 19 deletions(-)

diff --git a/shibsp/handler/AbstractHandler.h b/shibsp/handler/AbstractHandler.h
index 0cbb2fb9..83dc3382 100644
--- a/shibsp/handler/AbstractHandler.h
+++ b/shibsp/handler/AbstractHandler.h
@@ -70,10 +70,13 @@ namespace shibsp {
          * 
          * @param request request to playback response into
          * @param wrappedResponse wrapped response data
+         * @param limitRedirect whether to invoke the redirect limiter in the case of a redirect
          * 
          * @return result of response playback to return from handler
          */
-        virtual std::pair<bool,long> unwrapResponse(SPRequest& request, DDF& wrappedResponse) const;
+        virtual std::pair<bool,long> unwrapResponse(
+            SPRequest& request, DDF& wrappedResponse, bool limitRedirect=false
+            ) const;
 
         /**
          * Bitmask of property sources to read from:
diff --git a/shibsp/handler/impl/AbstractHandler.cpp b/shibsp/handler/impl/AbstractHandler.cpp
index 5242517a..bff1fc6c 100644
--- a/shibsp/handler/impl/AbstractHandler.cpp
+++ b/shibsp/handler/impl/AbstractHandler.cpp
@@ -155,7 +155,7 @@ DDF AbstractHandler::wrapRequest(const SPRequest& request, const set<string>& he
     return in;
 }
 
-pair<bool,long> AbstractHandler::unwrapResponse(SPRequest& request, DDF& wrappedResponse) const
+pair<bool,long> AbstractHandler::unwrapResponse(SPRequest& request, DDF& wrappedResponse, bool limitRedirect) const
 {
     DDF http = wrappedResponse["http"];
     DDF h = http["headers"];
@@ -174,6 +174,9 @@ pair<bool,long> AbstractHandler::unwrapResponse(SPRequest& request, DDF& wrapped
     if (h.isstring()) {
         string dest(h.string());
         request.absolutize(dest);
+        if (limitRedirect) {
+            request.limitRedirect(dest.c_str());
+        }
         return make_pair(true, request.sendRedirect(dest.c_str()));
     }
 
diff --git a/shibsp/handler/impl/LogoutConsumer.cpp b/shibsp/handler/impl/LogoutConsumer.cpp
index e079087e..f09dcf5f 100644
--- a/shibsp/handler/impl/LogoutConsumer.cpp
+++ b/shibsp/handler/impl/LogoutConsumer.cpp
@@ -51,6 +51,7 @@ namespace shibsp {
         pair<bool,long> run(SPRequest& request, bool isHandler=true) const;
 
     private:
+        const char* getHomeURL(SPRequest& request) const;
         pair <bool,long> completeLogout(SPRequest& request, bool removeSession, const char* token) const;
 
         bool m_matchRequired;
@@ -113,6 +114,8 @@ pair<bool,long> LogoutConsumer::run(SPRequest& request, bool isHandler) const
     DDF wrapped = wrapRequest(request, emptyHeaderSet, false);
     input.add(wrapped);
 
+    input.addmember("home_url").unsafe_string(getHomeURL(request));
+
     // Call the Hub to process the message, suppressing any errors that occur.
 
     DDF output;
@@ -134,9 +137,16 @@ pair<bool,long> LogoutConsumer::run(SPRequest& request, bool isHandler) const
     }
     DDFJanitor outputJanitor(output);
 
-    // There are broadly two cases here, a logout request or a response being processed
+    // There are two cases here, a logout request or a response being processed
     // from an IdP, allowing that a dozen or more different errors can take place.
-    // The request case will potentially feed back a "token" member for use later.
+    // The request case will potentially feed back a "token" member for use later
+    // while the response case will provide a "status" and generally a wrapped response.
+    
+    if (output["status"].isint()) {
+        // Finish up a logout response. The session should be gone and if it wasn't
+        // this is a spurious logout message so we don't act on it.
+        return completeLogout(request, false, nullptr);
+    }
 
     // If we actually have a session in hand, we may need to initiate the notification loop.
     // Any token provided by the Hub call will be attached to that process.
@@ -211,14 +221,34 @@ pair <bool,long> LogoutConsumer::completeLogout(SPRequest& request, bool removeS
 
     DDF wrapped = output.getmember("http");
     if (wrapped.isstruct()) {
-        return unwrapResponse(request, wrapped);
+        return unwrapResponse(request, wrapped, true);
     }
 
-    const char* post_logout_url = request.getRequestSettings().first->getString(RequestMapper::LOGOUT_URL_PROP_NAME);
-    if (!post_logout_url) {
-        post_logout_url = request.getRequestSettings().first->getString(
-            RequestMapper::HOME_URL_PROP_NAME, RequestMapper::HOME_URL_PROP_DEFAULT);
+    const char* dest = output.getmember("target").string();
+    if (dest) {
+        // Relative URLs get promoted, absolutes get validated.
+        if (*dest == '/') {
+            string d(dest);
+            request.absolutize(d);
+            return make_pair(true, request.sendRedirect(d.c_str()));
+        } else {
+            request.limitRedirect(dest);
+            return make_pair(true, request.sendRedirect(dest));
+        }
+    }
+
+    // If no target from Hub we fall back to our own determination that favors
+    // the logoutURL setting over homeURL.
+    return make_pair(true, request.sendRedirect(getHomeURL(request)));
+}
+
+const char* LogoutConsumer::getHomeURL(SPRequest& request) const
+{
+    const char* dest = request.getRequestSettings().first->getString(RequestMapper::LOGOUT_URL_PROP_NAME);
+    if (dest) {
+        return dest;
     }
 
-    return make_pair(true, request.sendRedirect(post_logout_url));
+    return request.getRequestSettings().first->getString(RequestMapper::HOME_URL_PROP_NAME,
+        RequestMapper::HOME_URL_PROP_DEFAULT);
 }
diff --git a/shibsp/handler/impl/Passthrough.cpp b/shibsp/handler/impl/Passthrough.cpp
index b7d3fd4e..6526f968 100644
--- a/shibsp/handler/impl/Passthrough.cpp
+++ b/shibsp/handler/impl/Passthrough.cpp
@@ -96,14 +96,7 @@ pair<bool,long> Passthrough::run(SPRequest& request, bool isHandler) const
         DDF output = request.getAgent().getRemotingService()->send(input);
         DDFJanitor outputJanitor(output);
 
-        if (m_limitRedirects) {
-            const char* url = output.getmember("http.redirect").string();
-            if (url) {
-                request.limitRedirect(url);
-            }
-        }
-
-        return unwrapResponse(request, output);
+        return unwrapResponse(request, output, m_limitRedirects);
     }
     catch (exception& ex) {
         AgentException* agent_ex = dynamic_cast<AgentException*>(&ex);
diff --git a/shibsp/handler/impl/TokenConsumer.cpp b/shibsp/handler/impl/TokenConsumer.cpp
index 46071687..80937708 100644
--- a/shibsp/handler/impl/TokenConsumer.cpp
+++ b/shibsp/handler/impl/TokenConsumer.cpp
@@ -201,7 +201,7 @@ pair<bool,long> TokenConsumer::run(SPRequest& request, bool isHandler) const
         }
 
         // Handles all normal cases, including POST recovery.
-        return unwrapResponse(request, output);
+        return unwrapResponse(request, output, true);
     }
     catch (exception& ex) {
         AgentException* agent_ex = dynamic_cast<AgentException*>(&ex);

-- 
To stop receiving notification emails like this one, please contact
the administrator of this repository.


More information about the commits mailing list