From 3bea7a53eed8fe099064a9d109e576ae69da011c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o?= Date: Wed, 12 Aug 2026 01:04:32 +0200 Subject: [PATCH] fix(email): derive the Google OAuth redirect URI scheme from the request (#5995) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both the authorize and callback routes built the redirect URI with a hardcoded `http://` and the Host header. Behind any TLS terminator that produces `http://host:443/api/email/oauth/google/callback` — the wrong scheme and, on a split-port setup, a dead port. Google then refuses the authorize request or the token exchange, so OAuth email is unusable on every HTTPS deployment unless GOOGLE_OAUTH_REDIRECT_URI is pinned by hand. uvicorn's proxy-headers middleware already rewrites the scheme from X-Forwarded-Proto for trusted proxies (on by default, trusting 127.0.0.1), so request.url.scheme is correct both directly and behind a proxy. Google requires the callback's redirect_uri to match the authorize one exactly, so both sites change together. An explicit GOOGLE_OAUTH_REDIRECT_URI still wins, unchanged. --- routes/email_routes.py | 4 +- tests/test_email_oauth.py | 124 +++++++++++++++++++++++++++++++++++++- 2 files changed, 124 insertions(+), 4 deletions(-) diff --git a/routes/email_routes.py b/routes/email_routes.py index 81136e36e..03b73ebb5 100644 --- a/routes/email_routes.py +++ b/routes/email_routes.py @@ -5954,7 +5954,7 @@ def setup_email_routes(): raise HTTPException(400, "GOOGLE_OAUTH_CLIENT_ID not set — add it to .env") redirect_uri = ( os.environ.get("GOOGLE_OAUTH_REDIRECT_URI") - or f"http://{request.headers.get('host', 'localhost:7000')}/api/email/oauth/google/callback" + or f"{request.url.scheme}://{request.headers.get('host', 'localhost:7000')}/api/email/oauth/google/callback" ) state = make_oauth_state(account_id, owner) params = urllib.parse.urlencode({ @@ -5991,7 +5991,7 @@ def setup_email_routes(): client_secret = os.environ.get("GOOGLE_OAUTH_CLIENT_SECRET", "") redirect_uri = ( os.environ.get("GOOGLE_OAUTH_REDIRECT_URI") - or f"http://{request.headers.get('host', 'localhost:7000')}/api/email/oauth/google/callback" + or f"{request.url.scheme}://{request.headers.get('host', 'localhost:7000')}/api/email/oauth/google/callback" ) import httpx as _httpx try: diff --git a/tests/test_email_oauth.py b/tests/test_email_oauth.py index 4fad82739..ee34a9de7 100644 --- a/tests/test_email_oauth.py +++ b/tests/test_email_oauth.py @@ -29,6 +29,7 @@ import base64 import json import time import unittest.mock as mock +from types import SimpleNamespace import pytest @@ -272,8 +273,14 @@ def _callback_endpoint(): class _FakeRequest: - """Minimal stand-in for starlette Request — the callback only reads headers.""" - headers = {"host": "localhost:7000"} + """Minimal stand-in for starlette Request — the callback reads the Host header + and the request scheme. Behind a TLS terminator uvicorn's proxy-headers + middleware rewrites the scheme from `X-Forwarded-Proto`, so the route sees + `https` there and `http` on a plain origin.""" + + def __init__(self, scheme="http", host="localhost:7000"): + self.headers = {"host": host} + self.url = SimpleNamespace(scheme=scheme) def _location(resp): @@ -415,6 +422,119 @@ async def test_callback_valid_owner_writes_encrypted_tokens_to_intended_account( assert other.oauth_access_token is None, "tokens must only touch the intended account" +# ── Redirect URI scheme ─────────────────────────────────────────── +# +# Google rejects the token exchange unless the callback's `redirect_uri` is +# byte-identical to the one the authorize step sent, so both routes have to +# agree — including on the scheme. Deriving it from the request keeps HTTPS +# deployments working without pinning GOOGLE_OAUTH_REDIRECT_URI by hand; +# hardcoding `http://` produced an unusable redirect behind any TLS front. + +def _authorize_endpoint(): + """Return the live google_oauth_authorize endpoint from the email router.""" + from routes.email_routes import setup_email_routes + router = setup_email_routes() + for route in router.routes: + if route.path == "/api/email/oauth/google/authorize" and "GET" in getattr(route, "methods", set()): + return route.endpoint + raise AssertionError("google_oauth_authorize route not found") + + +def _posted_redirect_uri(mock_post): + """Pull `redirect_uri` out of the mocked Google token-exchange POST.""" + return mock_post.call_args.kwargs["data"]["redirect_uri"] + + +@pytest.mark.asyncio +@pytest.mark.parametrize("scheme", ("http", "https")) +async def test_callback_redirect_uri_follows_the_request_scheme(scheme, monkeypatch): + """The token exchange must echo the scheme the request actually arrived on — + `https` behind a TLS terminator, `http` on a plain origin.""" + from routes.email_helpers import make_oauth_state + + monkeypatch.delenv("GOOGLE_OAUTH_REDIRECT_URI", raising=False) + + db, Factory = _make_db() + _make_account(db, account_id="acct-s", owner="alice", imap_user="alice@example.com") + db.close() + + token_resp = mock.MagicMock() + token_resp.raise_for_status = mock.MagicMock() + token_resp.json.return_value = {"access_token": "ya29.t", "refresh_token": "1//r", "expires_in": 3600} + userinfo_resp = mock.MagicMock() + userinfo_resp.is_success = True + userinfo_resp.json.return_value = {"email": "alice@example.com", "name": "Alice"} + + state = make_oauth_state("acct-s", "alice") + + with mock.patch("httpx.post", return_value=token_resp) as mock_post, \ + mock.patch("httpx.get", return_value=userinfo_resp), \ + mock.patch("core.database.SessionLocal", Factory): + callback = _callback_endpoint() + await callback( + code="4/code", state=state, error=None, + request=_FakeRequest(scheme=scheme, host="odysseus.example.ts.net:7443"), + ) + + assert _posted_redirect_uri(mock_post) == ( + f"{scheme}://odysseus.example.ts.net:7443/api/email/oauth/google/callback" + ) + + +@pytest.mark.asyncio +async def test_callback_redirect_uri_env_override_still_wins(monkeypatch): + """An explicit GOOGLE_OAUTH_REDIRECT_URI is used verbatim — deriving the + scheme must not override a value the operator pinned by hand.""" + from routes.email_helpers import make_oauth_state + + pinned = "https://mail.example.com/api/email/oauth/google/callback" + monkeypatch.setenv("GOOGLE_OAUTH_REDIRECT_URI", pinned) + + db, Factory = _make_db() + _make_account(db, account_id="acct-p", owner="alice", imap_user="alice@example.com") + db.close() + + token_resp = mock.MagicMock() + token_resp.raise_for_status = mock.MagicMock() + token_resp.json.return_value = {"access_token": "ya29.t", "refresh_token": "1//r", "expires_in": 3600} + userinfo_resp = mock.MagicMock() + userinfo_resp.is_success = True + userinfo_resp.json.return_value = {"email": "alice@example.com", "name": "Alice"} + + state = make_oauth_state("acct-p", "alice") + + with mock.patch("httpx.post", return_value=token_resp) as mock_post, \ + mock.patch("httpx.get", return_value=userinfo_resp), \ + mock.patch("core.database.SessionLocal", Factory): + callback = _callback_endpoint() + await callback(code="4/code", state=state, error=None, request=_FakeRequest(scheme="http")) + + assert _posted_redirect_uri(mock_post) == pinned + + +@pytest.mark.asyncio +@pytest.mark.parametrize("scheme", ("http", "https")) +async def test_authorize_redirect_uri_follows_the_request_scheme(scheme, monkeypatch): + """The authorize step builds the same redirect_uri the callback will send. + `owner=""` is the unconfigured / single-user case, so no DB is touched.""" + import urllib.parse + + monkeypatch.delenv("GOOGLE_OAUTH_REDIRECT_URI", raising=False) + monkeypatch.setenv("GOOGLE_OAUTH_CLIENT_ID", "client-id.apps.googleusercontent.com") + + authorize = _authorize_endpoint() + resp = await authorize( + account_id="acct-a", + request=_FakeRequest(scheme=scheme, host="odysseus.example.ts.net:7443"), + owner="", + ) + + query = urllib.parse.parse_qs(urllib.parse.urlparse(resp.headers["location"]).query) + assert query["redirect_uri"] == [ + f"{scheme}://odysseus.example.ts.net:7443/api/email/oauth/google/callback" + ] + + @pytest.mark.asyncio async def test_callback_rejects_token_for_a_different_mailbox_identity(): """Reconnecting with another Google identity must not replace the token