diff --git a/nodes/src/nodes/tool_gmail/gmail_client.py b/nodes/src/nodes/tool_gmail/gmail_client.py index c2254c4a1..81dca5c58 100644 --- a/nodes/src/nodes/tool_gmail/gmail_client.py +++ b/nodes/src/nodes/tool_gmail/gmail_client.py @@ -73,7 +73,11 @@ def resolve_refresh_url(token_url: object) -> str | None: allowed = set(_BROKER_HOSTS) env_broker = os.environ.get('RR_OAUTH_BROKER_URL', '') if env_broker: - env_host = urlparse(env_broker).hostname + # A schemeless value (e.g. "broker.example.com") parses with + # hostname=None and would be silently dropped from the allowlist, so + # add a scheme before parsing when one is missing. + to_parse = env_broker if '://' in env_broker else f'https://{env_broker}' + env_host = urlparse(to_parse).hostname if env_host: allowed.add(env_host.lower()) @@ -87,9 +91,55 @@ def resolve_refresh_url(token_url: object) -> str | None: return token_url +# Google's canonical OAuth token endpoint, used when a token omits token_uri. +_GOOGLE_TOKEN_URI = 'https://oauth2.googleapis.com/token' + +# Hosts the google-auth library may POST the refresh_token to when minting a +# new access token. Like the broker URL, token_uri rides in the untrusted token +# payload, so a token-supplied value is honored only if it is https and its host +# is one of Google's official token endpoints — otherwise a tampered token could +# redirect the refresh_token to an attacker host. +_GOOGLE_TOKEN_HOSTS = frozenset({'oauth2.googleapis.com', 'accounts.google.com'}) + + +def resolve_token_uri(token_uri: object) -> str: + """Validate a token-supplied OAuth token endpoint against Google's hosts. + + ``token_uri`` is where the google-auth library POSTs the ``refresh_token`` + to obtain a fresh access token. The token payload is untrusted (it rides in + saved pipes and broker responses), so a tampered token could point this at + an attacker host and exfiltrate the refresh_token. + + Returns the URL when it is https and its host is one of Google's official + token endpoints. Falls back to the canonical endpoint when the token omits + the field. Raises ValueError for any other value so a tampered token fails + loud instead of silently posting credentials elsewhere. + """ + # Only an absent value (None or empty string) falls back; other falsy + # non-strings (0, False, [], {}) are tampering and must fail loud. + if token_uri is None or token_uri == '': + return _GOOGLE_TOKEN_URI + if not isinstance(token_uri, str): + raise ValueError('Gmail token token_uri must be a string') + + parsed = urlparse(token_uri) + if parsed.scheme != 'https' or not parsed.hostname or parsed.hostname.lower() not in _GOOGLE_TOKEN_HOSTS: + raise ValueError( + f'Gmail token token_uri {token_uri!r} is not a trusted Google OAuth token endpoint ' + '(expected https and one of: ' + ', '.join(sorted(_GOOGLE_TOKEN_HOSTS)) + '). ' + 'Reconnect your Google account.' + ) + return token_uri + + # HTTP status codes worth retrying (rate-limit + transient server errors). _RETRY_STATUSES = {429, 500, 502, 503, 504} +# Fallback access-token lifetime when the broker response omits expiry_date. +# Matches Google's standard access-token lifetime; without it the credential +# keeps a stale past expiry and google-auth refreshes on every API call. +_DEFAULT_ACCESS_TOKEN_LIFETIME_S = 3600 + # Google's full-access Gmail scope — a superset of all granular Gmail scopes. _GMAIL_FULL_SCOPE = 'https://mail.google.com/' @@ -118,6 +168,78 @@ def _decode_blob(value: str) -> str: return value +def _refresh_access_token_via_broker(broker_url: str, refresh_token: str): + """POST the refresh_token to the OAuth broker and return (access_token, expiry). + + Every failure is surfaced as a google-auth ``RefreshError`` with a clear + reconnect message rather than leaking a raw transport error or silently + keeping a stale token: + + - An HTTP error status means the broker was reached but rejected the + request — it must not be misreported as "broker unreachable" (an + ``HTTPError`` is a subclass of ``URLError``, so it is matched first). + - A connection failure or a socket timeout during ``read()`` (an + ``OSError``, not a ``URLError``) is reported as unreachable. + - A non-JSON or malformed body is reported as an invalid response. + - A 200 without ``access_token`` raises instead of keeping the stale token. + - A 200 without ``expiry_date`` falls back to a default lifetime instead of + leaving a stale past expiry that forces a refresh on every call. + + Returns: + tuple[str, datetime.datetime]: the new access token and its expiry. + """ + import datetime as _dt + import urllib.error as _uerr + import urllib.request as _req + + import google.auth.exceptions as _gae + + body = json.dumps({'refresh_token': refresh_token}).encode() + req = _req.Request(broker_url, data=body, headers={'Content-Type': 'application/json'}, method='POST') + try: + with _req.urlopen(req, timeout=10) as resp: + raw = resp.read() + data = json.loads(raw.decode()) + except _uerr.HTTPError as exc: + raise _gae.RefreshError( + f'Gmail token refresh was rejected by the broker (HTTP {exc.code}). Please reconnect your Google account.' + ) from exc + except (_uerr.URLError, OSError) as exc: + raise _gae.RefreshError( + f'Gmail token refresh failed (broker unreachable: {exc}). Please reconnect your Google account.' + ) from exc + except ValueError as exc: + raise _gae.RefreshError( + 'Gmail token refresh returned an invalid response from the broker. Please reconnect your Google account.' + ) from exc + + # Valid JSON that is not an object (null, a number, a list) would crash + # data.get(...) with AttributeError — treat it as an invalid response. + if not isinstance(data, dict): + raise _gae.RefreshError( + 'Gmail token refresh returned an invalid response from the broker. Please reconnect your Google account.' + ) + + access_token = data.get('access_token') + if not isinstance(access_token, str) or not access_token: + raise _gae.RefreshError( + 'Gmail token refresh returned no access token from the broker. Please reconnect your Google account.' + ) + + ms = data.get('expiry_date') + if ms is None: + expiry = _dt.datetime.utcnow() + _dt.timedelta(seconds=_DEFAULT_ACCESS_TOKEN_LIFETIME_S) + elif isinstance(ms, bool) or not isinstance(ms, (int, float)): + # A non-numeric expiry_date would raise a raw TypeError during + # timestamp conversion — surface it as an invalid response instead. + raise _gae.RefreshError( + 'Gmail token refresh returned an invalid response from the broker. Please reconnect your Google account.' + ) + else: + expiry = _dt.datetime.utcfromtimestamp(ms / 1000) + return access_token, expiry + + def build_service(auth_type: str, cfg: dict, scopes: list[str]) -> Any: """Build a Gmail v1 service from node config, scoped to ``scopes``. @@ -136,7 +258,9 @@ def build_service(auth_type: str, cfg: dict, scopes: list[str]) -> Any: refresh_token = info.get('refresh_token') client_id = info.get('client_id') client_secret = info.get('client_secret') - token_uri = info.get('token_uri', 'https://oauth2.googleapis.com/token') + # Untrusted input: reject any token endpoint not owned by Google so a + # tampered token can never redirect the refresh_token POST. + token_uri = resolve_token_uri(info.get('token_uri')) # Untrusted input: reject any refresh URL not pointing at a known broker. oauth_server_url = resolve_refresh_url(info.get('oauth_server_url')) expiry_date_ms = info.get('expiry_date') @@ -182,34 +306,13 @@ def build_service(auth_type: str, cfg: dict, scopes: list[str]) -> Any: class _BrokerCredentials(Credentials): def refresh(self, request: object) -> None: # type: ignore[override] - import urllib.error as _uerr - import urllib.request as _req - import google.auth.exceptions as _gae if not _broker_url or not self.refresh_token: raise _gae.RefreshError( 'Gmail access token has expired. Please reconnect your Google account in the node settings.' ) - body = json.dumps({'refresh_token': self.refresh_token}).encode() - req = _req.Request( - _broker_url, - data=body, - headers={'Content-Type': 'application/json'}, - method='POST', - ) - try: - with _req.urlopen(req, timeout=10) as resp: - data = json.loads(resp.read().decode()) - except _uerr.URLError as exc: - raise _gae.RefreshError( - f'Gmail token refresh failed (broker unreachable: {exc}). ' - 'Please reconnect your Google account.' - ) from exc - self.token = data.get('access_token') or self.token - ms = data.get('expiry_date') - if ms: - self.expiry = _dt.datetime.utcfromtimestamp(ms / 1000) + self.token, self.expiry = _refresh_access_token_via_broker(_broker_url, self.refresh_token) creds = _BrokerCredentials( token=access_token, diff --git a/nodes/test/mocks/google/auth/__init__.py b/nodes/test/mocks/google/auth/__init__.py new file mode 100644 index 000000000..543e36ff0 --- /dev/null +++ b/nodes/test/mocks/google/auth/__init__.py @@ -0,0 +1,6 @@ +# ============================================================================= +# MIT License +# Copyright (c) 2026 Aparavi Software AG +# ============================================================================= + +"""Mock google.auth package (exceptions) for tool_gmail broker-refresh tests.""" diff --git a/nodes/test/mocks/google/auth/exceptions.py b/nodes/test/mocks/google/auth/exceptions.py new file mode 100644 index 000000000..77f3d7592 --- /dev/null +++ b/nodes/test/mocks/google/auth/exceptions.py @@ -0,0 +1,19 @@ +# ============================================================================= +# MIT License +# Copyright (c) 2026 Aparavi Software AG +# ============================================================================= + +"""Minimal stand-in for google.auth.exceptions used in tests. + +Only ``RefreshError`` is needed: the Gmail broker-refresh path raises it to +signal a failed token refresh, and the real ``google-auth`` package is not +installed in the node test environment. +""" + + +class GoogleAuthError(Exception): + """Base class mirroring google.auth.exceptions.GoogleAuthError.""" + + +class RefreshError(GoogleAuthError): + """Raised when a credential refresh fails (mirrors the real RefreshError).""" diff --git a/nodes/test/tool_gmail/test_gmail.py b/nodes/test/tool_gmail/test_gmail.py index 814add131..0bfb8f5ce 100644 --- a/nodes/test/tool_gmail/test_gmail.py +++ b/nodes/test/tool_gmail/test_gmail.py @@ -213,6 +213,180 @@ def test_refresh_url_env_override_adds_host(monkeypatch): assert gmail_client.resolve_refresh_url('https://oauth2.rocketride.ai/refresh') +def test_refresh_url_env_override_accepts_schemeless_host(monkeypatch): + # A schemeless RR_OAUTH_BROKER_URL must still register its host (urlparse + # otherwise yields hostname=None and the host is silently dropped). + monkeypatch.setenv('RR_OAUTH_BROKER_URL', 'broker.internal.example') + url = 'https://broker.internal.example/refresh' + assert gmail_client.resolve_refresh_url(url) == url + + +def test_token_uri_accepts_google_endpoints(): + for url in ( + 'https://oauth2.googleapis.com/token', + 'https://accounts.google.com/o/oauth2/token', + ): + assert gmail_client.resolve_token_uri(url) == url + + +def test_token_uri_absent_falls_back_to_canonical(): + assert gmail_client.resolve_token_uri(None) == 'https://oauth2.googleapis.com/token' + assert gmail_client.resolve_token_uri('') == 'https://oauth2.googleapis.com/token' + + +@pytest.mark.parametrize( + 'bad', + [ + 'http://oauth2.googleapis.com/token', # scheme downgrade + 'https://evil.example.com/token', # attacker host — would exfiltrate refresh_token + 'https://oauth2.googleapis.com.evil.com/token', # suffix confusion + 'ftp://oauth2.googleapis.com/token', + ], +) +def test_token_uri_rejects_untrusted(bad): + with pytest.raises(ValueError): + gmail_client.resolve_token_uri(bad) + + +def test_token_uri_rejects_non_string(): + with pytest.raises(ValueError): + gmail_client.resolve_token_uri({'url': 'https://oauth2.googleapis.com/token'}) + + +@pytest.mark.parametrize('bad', [0, False, [], {}]) +def test_token_uri_rejects_falsy_non_string(bad): + # Only None/'' fall back to the canonical endpoint; other falsy non-strings + # are tampering and must raise, not silently return the default. + with pytest.raises(ValueError): + gmail_client.resolve_token_uri(bad) + + +# --------------------------------------------------------------------------- +# _refresh_access_token_via_broker — broker refresh reliability (#1560) +# --------------------------------------------------------------------------- + +import datetime as _dt # noqa: E402 +import urllib.error as _uerr # noqa: E402 + + +class _FakeResp: + """Minimal urlopen() response context manager for tests.""" + + def __init__(self, *, body=b'', read_raises=None): + self._body = body + self._read_raises = read_raises + + def read(self): + if self._read_raises is not None: + raise self._read_raises + return self._body + + def __enter__(self): + return self + + def __exit__(self, *exc): + return False + + +def _make_urlopen(*, raises=None, body=b'', read_raises=None): + def _open(req, timeout=None): + if raises is not None: + raise raises + return _FakeResp(body=body, read_raises=read_raises) + + return _open + + +@pytest.fixture +def refresh_error(monkeypatch): + """Prepend the mock Google SDK so the broker helper's ``import + google.auth.exceptions`` resolves (the real SDK is absent in the node test + env), and hand the test the RefreshError type it raises. + """ + mocks = Path(__file__).resolve().parents[1] / 'mocks' + monkeypatch.syspath_prepend(str(mocks)) + import google.auth.exceptions as _gae + + return _gae.RefreshError + + +def test_broker_refresh_success_uses_broker_expiry(monkeypatch, refresh_error): + payload = json.dumps({'access_token': 'AT', 'expiry_date': 1_000_000}).encode() + monkeypatch.setattr('urllib.request.urlopen', _make_urlopen(body=payload)) + tok, exp = gmail_client._refresh_access_token_via_broker('https://oauth2.rocketride.ai/refresh', 'RT') + assert tok == 'AT' + assert exp == _dt.datetime.utcfromtimestamp(1000) + + +def test_broker_refresh_defaults_expiry_when_missing(monkeypatch, refresh_error): + # A 200 without expiry_date must not leave a stale past expiry (which would + # force a refresh on every API call); fall back to a sane future lifetime. + payload = json.dumps({'access_token': 'AT'}).encode() + monkeypatch.setattr('urllib.request.urlopen', _make_urlopen(body=payload)) + before = _dt.datetime.utcnow() + tok, exp = gmail_client._refresh_access_token_via_broker('https://broker/refresh', 'RT') + assert tok == 'AT' + assert exp > before + + +def test_broker_refresh_missing_access_token_raises(monkeypatch, refresh_error): + # A 200 without access_token must fail loud, not silently keep the stale token. + payload = json.dumps({'expiry_date': 1_000_000}).encode() + monkeypatch.setattr('urllib.request.urlopen', _make_urlopen(body=payload)) + with pytest.raises(refresh_error, match='no access token'): + gmail_client._refresh_access_token_via_broker('https://broker/refresh', 'RT') + + +def test_broker_refresh_http_error_reports_rejected(monkeypatch, refresh_error): + # HTTPError is a subclass of URLError: the broker was reached but rejected + # the request, so it must not be misreported as "unreachable". + err = _uerr.HTTPError('https://broker/refresh', 500, 'boom', {}, None) + monkeypatch.setattr('urllib.request.urlopen', _make_urlopen(raises=err)) + with pytest.raises(refresh_error, match='rejected by the broker'): + gmail_client._refresh_access_token_via_broker('https://broker/refresh', 'RT') + + +def test_broker_refresh_urlerror_reports_unreachable(monkeypatch, refresh_error): + monkeypatch.setattr('urllib.request.urlopen', _make_urlopen(raises=_uerr.URLError('down'))) + with pytest.raises(refresh_error, match='unreachable'): + gmail_client._refresh_access_token_via_broker('https://broker/refresh', 'RT') + + +def test_broker_refresh_read_timeout_reports_unreachable(monkeypatch, refresh_error): + # A timeout during resp.read() raises OSError (TimeoutError), not URLError. + monkeypatch.setattr('urllib.request.urlopen', _make_urlopen(read_raises=TimeoutError('read timed out'))) + with pytest.raises(refresh_error, match='unreachable'): + gmail_client._refresh_access_token_via_broker('https://broker/refresh', 'RT') + + +def test_broker_refresh_invalid_json_reports_invalid(monkeypatch, refresh_error): + monkeypatch.setattr('urllib.request.urlopen', _make_urlopen(body=b'not json')) + with pytest.raises(refresh_error, match='invalid response'): + gmail_client._refresh_access_token_via_broker('https://broker/refresh', 'RT') + + +def test_broker_refresh_non_dict_body_reports_invalid(monkeypatch, refresh_error): + # Valid JSON that is not an object (list/number/null) must not crash with + # AttributeError — treat it as an invalid response. + monkeypatch.setattr('urllib.request.urlopen', _make_urlopen(body=b'[1, 2, 3]')) + with pytest.raises(refresh_error, match='invalid response'): + gmail_client._refresh_access_token_via_broker('https://broker/refresh', 'RT') + + +def test_broker_refresh_non_string_access_token_raises(monkeypatch, refresh_error): + payload = json.dumps({'access_token': 123, 'expiry_date': 1_000_000}).encode() + monkeypatch.setattr('urllib.request.urlopen', _make_urlopen(body=payload)) + with pytest.raises(refresh_error, match='no access token'): + gmail_client._refresh_access_token_via_broker('https://broker/refresh', 'RT') + + +def test_broker_refresh_non_numeric_expiry_reports_invalid(monkeypatch, refresh_error): + payload = json.dumps({'access_token': 'AT', 'expiry_date': 'soon'}).encode() + monkeypatch.setattr('urllib.request.urlopen', _make_urlopen(body=payload)) + with pytest.raises(refresh_error, match='invalid response'): + gmail_client._refresh_access_token_via_broker('https://broker/refresh', 'RT') + + def test_message_get_cleans_headers(): raw = { 'id': 'm1',