Keep callback cleanup redirects local

README.md

@@ -21,6 +21,8 @@ def login(auth):
 
 Wrap the return route too. It can read `auth.is_logged_in()` and `auth.get_auth_user().id` / `.email`. The full [examples](examples) include login, logout and callback handling. See the [wiki](https://github.com/Authwave/python-client/wiki) for setup and the API reference.
 
+Callback cleanup redirects use a local path without a scheme or host. The client removes the authentication response parameter while preserving the other query values and fragment.
+
 The complete provider-issued key belongs on the server. The final underscore separates the deployment identifier from the raw 32-byte secret. `authwave_host` defaults to `login.authwave.com`.
 
 `MemorySessionStore` is for development. Production applications need a shared persistent `SessionStore` and HTTPS; cookies are Secure by default. Authentication data stays in server-side storage; never put serialized tokens in browser cookies. Authentication callbacks are synchronous and run at most once per exchange under sequential session processing.

authwave/authenticator.py

@@ -1,6 +1,6 @@
 import json
 import warnings
-from urllib.parse import parse_qs
+from urllib.parse import parse_qs, urlsplit, urlunsplit, quote
 from .cipher import InitVector, CipherText, decrypt
 from .errors import NotLoggedInException, RedirectHandler
 from .session import GlobalSessionContainer, SessionData
@@ -178,9 +178,10 @@ def scalar(value):
         self._tidy_response_data(return_query)
 
     def _tidy_response_data(self, return_query: str | None = None) -> None:
-        uri = (
-            self._current_uri
-            if return_query is None
-            else self._current_uri.with_query(return_query)
-        )
+        parts = urlsplit(self._current_uri)
+        path = "/" + parts.path.lstrip("/")
+        path = quote(path, safe="/%:@!$&'()*+,;=-._~")
+        uri = Uri(urlunsplit(("", "", path, parts.query, parts.fragment)))
+        if return_query is not None:
+            uri = uri.with_query(return_query)
         self._redirect.redirect(uri.without_query_value(self.RESPONSE_QUERY_PARAMETER))

tests/fixtures/php-oracle.php

@@ -9,6 +9,10 @@ class RecordingRedirect extends RedirectHandler {
  public array $redirects = [];
  public function redirect(UriInterface $uri, int $code = 303):void { $this->redirects[] = [(string)$uri, $code]; }
 }
+// These fixtures use in-memory state rather than a native browser session ID.
+class OracleSession extends \Authwave\GlobalSessionContainer {
+ public function regenerate():void {}
+}
 $cases = json_decode(stream_get_contents(STDIN), true);
 $out = [];
 foreach($cases as $index => $case) {
@@ -27,7 +31,7 @@ public function redirect(UriInterface $uri, int $code = 303):void { $this->redir
  }
  $error = null; $auth = null;
  try {
-  $auth = new Authenticator('deployment_with_underscores_'.$secret, $uri, $case['host'] ?? 'login.authwave.com', redirectHandler:$redirect, onAuthenticated:($case['callback'] ?? false) ? $callback : null);
+  $auth = new Authenticator('deployment_with_underscores_'.$secret, $uri, $case['host'] ?? 'login.authwave.com', redirectHandler:$redirect, session:new OracleSession(), onAuthenticated:($case['callback'] ?? false) ? $callback : null);
   if(isset($case['returnUri'])) $auth->setReturnUri($case['returnUri']);
   if(($case['action'] ?? '') === 'login') $auth->login($token);
   if(($case['action'] ?? '') === 'logout') $auth->logout($case['redirectTo'] ?? null, $token);

tests/test_callback_cleanup.py

@@ -0,0 +1,144 @@
+import asyncio
+from urllib.parse import quote_plus, urlsplit
+import pytest
+from authwave import (
+    Authenticator,
+    GlobalSessionContainer,
+    SessionData,
+    Token,
+    CipherText,
+    RedirectSignal,
+)
+from authwave.runtime import MemorySessionStore, SessionOptions
+from authwave.integrations.wsgi import AuthwaveWSGI
+from authwave.integrations.asgi import AuthwaveASGI
+
+KEY = "deployment_" + "s" * 32
+
+
+class MemorySession(GlobalSessionContainer):
+    def regenerate(self):
+        pass
+
+
+@pytest.mark.parametrize(
+    "input,expected",
+    [
+        ("//evil.example/path", "/path"),
+        ("//evil.example", "/"),
+        ("https://evil.example/path", "/path"),
+        ("https://user:pass@evil.example/path", "/path"),
+        ("https://app.example//evil.example/path", "/evil.example/path"),
+        ("/%2F%2Fevil.example/path", "/%2F%2Fevil.example/path"),
+        ("/members/a%2Fb", "/members/a%2Fb"),
+        ("/members/orders", "/members/orders"),
+    ],
+)
+def test_unsolicited_response_cleanup_stays_local(input, expected):
+    with pytest.raises(RedirectSignal) as result:
+        Authenticator(
+            KEY,
+            input + "?keep=%7e&flag&AUTHWAVE_RESPONSE_DATA=anything#details",
+            session=MemorySession({}),
+        )
+    uri = result.value.uri
+    assert result.value.code == 303
+    assert uri == expected + "?keep=%7e&flag#details"
+    assert urlsplit(uri).scheme == "" and urlsplit(uri).netloc == ""
+    assert uri.startswith("/") and not uri.startswith("//")
+
+
+@pytest.mark.parametrize("input", ["/\\evil.example/path", "x:/\\evil.example/path"])
+def test_backslash_cleanup_is_safe(input):
+    with pytest.raises(RedirectSignal) as result:
+        Authenticator(
+            KEY, input + "?AUTHWAVE_RESPONSE_DATA=anything", session=MemorySession({})
+        )
+    uri = result.value.uri
+    assert uri.startswith("/") and not uri.startswith("//")
+    assert "\\" not in uri
+    assert urlsplit(uri).netloc == ""
+
+
+def test_authenticated_cleanup_preserves_saved_query_and_encoded_path():
+    token = Token("s" * 32)
+    session = MemorySession({})
+    session.set(
+        SessionData.STORE_KEY,
+        SessionData(token, return_query="filter=original&flag&encoded=%7e"),
+    )
+    cipher = CipherText(
+        '{"id":"user-id","email":"person@example.com"}', token.get_secret_iv(), "s" * 32
+    )
+    with pytest.raises(RedirectSignal) as result:
+        Authenticator(
+            KEY,
+            "https://user:pass@evil.example//members/a%2Fb?AUTHWAVE_RESPONSE_DATA="
+            + quote_plus(str(cipher))
+            + "#details",
+            session=session,
+        )
+    assert result.value.uri == "/members/a%2Fb?filter=original&flag&encoded=%7e#details"
+    assert session.get(SessionData.STORE_KEY).get_data().id == "user-id"
+
+
+def test_duplicate_response_parameters_are_removed():
+    with pytest.raises(RedirectSignal) as result:
+        Authenticator(
+            KEY,
+            "//evil.example/path?AUTHWAVE_RESPONSE_DATA=one&flag&AUTHWAVE_RESPONSE_DATA=two&next=https://evil.example",
+            session=MemorySession({}),
+        )
+    assert result.value.uri == "/path?flag&next=https://evil.example"
+
+
+def test_wsgi_cleanup_header_cannot_redirect_to_another_host():
+    def app(environ, start_response):
+        pytest.fail("cleanup should redirect before the app")
+
+    wrapped = AuthwaveWSGI(app, SessionOptions(MemorySessionStore()), client_key=KEY)
+    recorded = {}
+
+    def start(status, headers, exc_info=None):
+        recorded.update(status=status, headers=dict(headers))
+
+    list(
+        wrapped(
+            {
+                "PATH_INFO": "//evil.example/path",
+                "QUERY_STRING": "keep=1&AUTHWAVE_RESPONSE_DATA=anything",
+            },
+            start,
+        )
+    )
+    assert recorded["status"] == "303 See Other"
+    assert recorded["headers"]["Location"] == "/path?keep=1"
+
+
+def test_asgi_cleanup_header_cannot_redirect_to_another_host():
+    async def app(scope, receive, send):
+        pytest.fail("cleanup should redirect before the app")
+
+    async def receive():
+        pytest.fail("receive is not needed")
+
+    messages = []
+
+    async def send(message):
+        messages.append(message)
+
+    wrapped = AuthwaveASGI(app, SessionOptions(MemorySessionStore()), client_key=KEY)
+    asyncio.run(
+        wrapped(
+            {
+                "type": "http",
+                "path": "//evil.example/path",
+                "query_string": b"keep=1&AUTHWAVE_RESPONSE_DATA=anything",
+                "headers": [],
+            },
+            receive,
+            send,
+        )
+    )
+    assert messages[0]["status"] == 303
+    assert dict(messages[0]["headers"])[b"location"] == b"/path?keep=1"