diff --git a/README.md b/README.md index 68b995f..334902c 100644 --- a/README.md +++ b/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. diff --git a/authwave/authenticator.py b/authwave/authenticator.py index 310cc06..6ef77e4 100644 --- a/authwave/authenticator.py +++ b/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)) diff --git a/tests/fixtures/php-oracle.php b/tests/fixtures/php-oracle.php index 21486f0..88fa555 100644 --- a/tests/fixtures/php-oracle.php +++ b/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); diff --git a/tests/test_callback_cleanup.py b/tests/test_callback_cleanup.py new file mode 100644 index 0000000..fbb3056 --- /dev/null +++ b/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"