diff --git a/CHANGELOG.md b/CHANGELOG.md index fc19263c98..d7a9383e4e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,15 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **GFQL Cypher relationship uniqueness across `MATCH` clauses**: sequential connected `MATCH` clauses can reuse the same relationship with a new relationship variable, as in `MATCH (x)-[e]->(y) MATCH (y)<-[f]-(z)`, while uniqueness remains path-global within each clause (#2235). ## [0.61.2] - 2026-10-23 +### Added + +- GFQL `searchAny` / `search_nodes` / `search_edges` now search **datetime** columns when the term is numeric, matching what the viz inspector displays rather than `repr`: a timestamp renders as the inspector's moment format (`Jan 5 2024, 3:04:05 am UTC`), so `"2024"` finds rows in that year. Datetimes previously declined. The zone is a `temporal_tz` parameter (default UTC) because the inspector renders in the viewer's zone, which a server cannot know — and the zone is not cosmetic, since it decides which day a timestamp lands on and therefore which rows match. Verified byte-identical to real `moment-timezone` on 3000 timestamps across 12 zones, including both DST directions, half- and quarter-hour offsets, and zones whose abbreviation changes seasonally. + - Searching a datetime column means rendering it, which a numeric-term search now pays for on every datetime column present: on a 300k-row frame one datetime column takes a `searchAny` from 77ms to 629ms, about three times what a float column added when #2089 gated floats in the same way. A term that is not numeric does not gate datetime columns in and pays nothing, which is pinned by spying on the renderer rather than by timing. + - pandas and polars render every zone and answer identical rows; polars matches pandas on 2968/3000 fuzz cases, the residue being `Africa/Casablanca`, where polars' bundled timezone database and Python's disagree about its future DST. cuDF matches at UTC (0/5000 mismatches on a GB10) and declines any other zone rather than mislabel it: cudf 26.02.01's `strftime` ignores `tz_convert`, answering `UTC` for `%Z` whatever the zone and reading the underlying UTC instant for `%p`, so an `Asia/Kolkata` column would render the right hour beside the wrong meridiem. + +- GFQL `searchAny` / `search_nodes` / `search_edges` now search **float** columns when the term is numeric, matching what the viz inspector displays rather than `repr`: fractional values render to a fixed number of decimals (so `0.1 + 0.2` is found by `"0.3"` and never by `"0.30000000000000004"`), whole values render without a trailing `.0`, and NaN plus the Int32 sentinel `2147483647` display nothing and never match. Precision is a `float_precision` parameter (default 4). This removes the previous cuDF float decline and the polars float exclusion (#1695). + - Only pandas can reproduce the inspector's half-away-from-zero rounding; polars and cuDF round half-to-even, so a value whose `float_precision+1`-th decimal is exactly 5 renders one unit lower there. Measured on 26,008 realistic column values: pandas 100%, cuDF 99.996%, polars 99.931% agreement with the UI. Pinned in `test_known_cross_engine_divergences.py`. + ### Fixed - **igraph calls on nodes with a `name` column**: a node attribute named `name`, other than the node ID column, no longer makes `to_igraph`, `compute_igraph`, and `CALL graphistry.igraph.*` fail with `Vertex attribute conflict`. It moves to a generated name for igraph, which reserves `name` for node IDs, and is restored by `from_igraph` (#2217). diff --git a/graphistry/client_session.py b/graphistry/client_session.py index d44b5d7adb..fc742bda22 100644 --- a/graphistry/client_session.py +++ b/graphistry/client_session.py @@ -35,21 +35,40 @@ ] -def _jwt_exp(token: str) -> Optional[float]: - """Best-effort read of a JWT's "exp" claim, without verifying its signature -- +def _jwt_payload(token: str) -> Optional[Dict[str, Any]]: # hygiene-ok: explicit-any -- decoded JWT claims are arbitrary server-provided JSON + """Best-effort decode of a JWT's payload, without verifying its signature -- the token is already trusted (this client received it from the server over - the auth flow it just completed); this only reads a claim out of a token + the auth flow it just completed); this only reads claims out of a token it already holds, never validates one from an untrusted source.""" try: payload_b64 = token.split(".")[1] padded = payload_b64 + "=" * (-len(payload_b64) % 4) payload = json.loads(base64.urlsafe_b64decode(padded)) + return payload if isinstance(payload, dict) else None + except Exception: + return None + + +def _jwt_exp(token: str) -> Optional[float]: + payload = _jwt_payload(token) + if payload is None: + return None + try: exp = payload.get("exp") return float(exp) if exp is not None else None except Exception: return None +def _jwt_user_id(token: str) -> Optional[str]: + """The Hub's "user_id" claim as a string, or None when the token is opaque / has no such claim.""" + payload = _jwt_payload(token) + if payload is None: + return None + user_id = payload.get("user_id") + return str(user_id) if user_id is not None and user_id != "" else None + + def _jwt_expired(token: str) -> bool: exp = _jwt_exp(token) # No readable exp claim -> not expired by this check; let the server reject it if it's actually stale. @@ -137,12 +156,31 @@ def mark_org_verified(self, token: Optional[str], org_name: str) -> None: return self._verified_org_tokens[org_name] = (token, _jwt_exp(token)) - def get_verified_token(self, org_name: str) -> Optional[str]: + def forget_verified_org(self, org_name: str) -> None: + """Drop any cached verification for org_name. + + Callers that just rotated the active token (e.g. refresh()) must call + this for the current org before re-checking verification: the server + resets the active org on every token reissue, so a cache entry from + before the rotation is stale, and get_verified_token()'s same_user_as + guard would otherwise happily swap the old (superseded) token back in + as active since it still belongs to the same user. + """ + self._verified_org_tokens.pop(org_name, None) + + def get_verified_token(self, org_name: str, same_user_as: Optional[str] = None) -> Optional[str]: """Return a still-unexpired token previously verified for org_name, if any. May differ from the currently-active token (e.g. a later SSO login for a different org minted a new one) -- callers that use this to skip a switch should swap it in as the active token. + + When ``same_user_as`` is given, the cached token is only returned if + both it and ``same_user_as`` carry a readable Hub ``user_id`` claim and + the two match. The cache is keyed by org only, and login()/api_token() + can change the active principal without resetting the session, so + without this guard a switch could silently reinstate another user's + token. Opaque tokens (no claim) never qualify. """ entry = self._verified_org_tokens.get(org_name) if entry is None: @@ -151,6 +189,10 @@ def get_verified_token(self, org_name: str) -> Optional[str]: if _jwt_expired(token): del self._verified_org_tokens[org_name] return None + if same_user_as is not None: + cached_user = _jwt_user_id(token) + if cached_user is None or cached_user != _jwt_user_id(same_user_as): + return None return token def copy(self) -> "ClientSession": diff --git a/graphistry/exceptions.py b/graphistry/exceptions.py index 9697600286..81b336780b 100644 --- a/graphistry/exceptions.py +++ b/graphistry/exceptions.py @@ -5,12 +5,6 @@ class SsoException(Exception): pass -class SsoRetrieveTokenTimeoutException(SsoException): - """ - Koa, 15 Sep 2022 Custom Exception to Sso retrieve token time out exception scenario - """ - pass - class SsoStateInvalidException(SsoException): """ 30 Jun 2025 Raised when the SSO state is missing, invalid, or not initialized during token retrieval. diff --git a/graphistry/pygraphistry.py b/graphistry/pygraphistry.py index bebfedf6cc..ee458369c5 100644 --- a/graphistry/pygraphistry.py +++ b/graphistry/pygraphistry.py @@ -33,7 +33,7 @@ from . import bolt_util from .plotter import Plotter from .util import in_databricks, setup_logger, in_ipython -from .exceptions import SsoRetrieveTokenTimeoutException, TokenExpireException, SsoStateInvalidException, SsoPendingException +from .exceptions import TokenExpireException, SsoStateInvalidException, SsoPendingException from .messages import ( MSG_REGISTER_MISSING_PASSWORD, @@ -340,25 +340,24 @@ def _handle_auth_url(self, auth_url: str, sso_timeout: Optional[int], sso_opt_in logger.debug("Transient error polling SSO token, will retry: %s", exc) token = None org_name = None - try: - if not token: - if elapsed_time % 10 == 1: - count_down = "Waiting for token : {} seconds ...".format(sso_timeout - elapsed_time + 1) - print(count_down) + if token: + break + if elapsed_time % 10 == 1: + count_down = "Waiting for token : {} seconds ...".format(sso_timeout - elapsed_time + 1) + print(count_down) + # Display-only nicety: must never affect the sleep/timeout below (IPython is an optional dependency). + if in_ipython(): + try: from IPython.core.display import HTML from IPython.display import display display(HTML(f'{count_down}')) - time.sleep(1) - elapsed_time = elapsed_time + 1 - if elapsed_time > sso_timeout: - raise SsoRetrieveTokenTimeoutException("[SSO] Get token timeout") - else: - break - except SsoRetrieveTokenTimeoutException as toe: - logger.debug(toe, exc_info=1) + except Exception as exc: + logger.debug("Could not render SSO countdown in notebook: %s", exc) + time.sleep(1) + elapsed_time = elapsed_time + 1 + if elapsed_time > sso_timeout: + logger.debug("[SSO] Get token timeout after %s seconds", sso_timeout) break - except Exception: - token = None if token: # set org_name to sso org self.session.org_name = org_name @@ -432,23 +431,23 @@ def refresh(self, token: Optional[str] = None, fail_silent: bool = False) -> Opt logger.debug("JWT refresh via token") if using_self_token: self.session._is_authenticated = False + prior_token = self.api_token() if using_self_token else token token = ( ArrowUploader( client_session=self.session, server_base_path=self.protocol() - + "://" + + "://" + self.server(), certificate_validation=self.certificate_validation(), ) - .refresh(self.api_token() if using_self_token else token) + .refresh(prior_token) .token ) self.api_token(token) self.session._is_authenticated = True refreshed_org = self.session.org_name - if token and refreshed_org and self.session.get_verified_token(refreshed_org): - # Refresh reissues under the same org context; carry the grant to the new token. - self.session.mark_org_verified(token, refreshed_org) + if refreshed_org and token != prior_token: + self.session.forget_verified_org(refreshed_org) self._maybe_switch_org(refreshed_org) return self.api_token() except Exception as e: @@ -2725,8 +2724,8 @@ def switch_org(self, value: str): logger.debug("Already switched to organization %s with current token; skipping request.", org_name) return - # Fast path 2: an earlier, still-unexpired token already verified org_name (now superseded as active by a later SSO login elsewhere) -- swap it back in. - cached_token = self.session.get_verified_token(org_name) + # Fast path 2: swap back in an earlier, still-unexpired token verified for org_name -- same Hub user only, since the cache is per-org and login()/api_token() don't reset it. + cached_token = self.session.get_verified_token(org_name, same_user_as=token) if token else None if cached_token: # Direct assign: api_token()'s setter would clear _is_authenticated and force a needless refresh(). self.session.api_token = cached_token diff --git a/graphistry/tests/test_client_session.py b/graphistry/tests/test_client_session.py index 28725d0664..5f1b7b75d5 100644 --- a/graphistry/tests/test_client_session.py +++ b/graphistry/tests/test_client_session.py @@ -11,9 +11,13 @@ from unittest import mock -def _fake_jwt(exp=None) -> str: +def _fake_jwt(exp=None, user_id=None) -> str: header = base64.urlsafe_b64encode(b'{"alg":"none"}').rstrip(b"=").decode() - payload = {} if exp is None else {"exp": exp} + payload = {} + if exp is not None: + payload["exp"] = exp + if user_id is not None: + payload["user_id"] = user_id body = base64.urlsafe_b64encode(json.dumps(payload).encode()).rstrip(b"=").decode() return f"{header}.{body}.sig" @@ -56,6 +60,32 @@ def test_unknown_org_returns_none(self): assert session.get_verified_token("never-switched") is None assert session.is_org_verified("tok", "never-switched") is False + def test_same_user_guard_matches_on_user_id_claim(self): + session = ClientSession() + cached = _fake_jwt(exp=time.time() + 3600, user_id=42) + session.mark_org_verified(cached, "org-a") + + same_user = _fake_jwt(exp=time.time() + 1800, user_id=42) + other_user = _fake_jwt(exp=time.time() + 1800, user_id=43) + + assert session.get_verified_token("org-a", same_user_as=same_user) == cached + assert session.get_verified_token("org-a", same_user_as=other_user) is None + # user_id is compared as a string so int/str encodings of the same id agree. + assert session.get_verified_token("org-a", same_user_as=_fake_jwt(exp=time.time() + 1800, user_id="42")) == cached + + def test_same_user_guard_rejects_tokens_without_user_claim(self): + session = ClientSession() + claimless_cached = _fake_jwt(exp=time.time() + 3600) + session.mark_org_verified(claimless_cached, "org-a") + assert session.get_verified_token("org-a", same_user_as=_fake_jwt(user_id=42)) is None + + session.mark_org_verified("opaque-pkey-token", "org-b") + assert session.get_verified_token("org-b", same_user_as="another-opaque-token") is None + assert session.get_verified_token("org-b", same_user_as=_fake_jwt(user_id=42)) is None + + # The guard is opt-in: unguarded lookups keep their previous behavior. + assert session.get_verified_token("org-a") == claimless_cached + class TestClientSession: """ diff --git a/graphistry/tests/test_pygraphistry.py b/graphistry/tests/test_pygraphistry.py index fcc8ca0e58..f7007bda10 100644 --- a/graphistry/tests/test_pygraphistry.py +++ b/graphistry/tests/test_pygraphistry.py @@ -116,6 +116,69 @@ def test_refresh_switches_when_org_changes(mock_refresh): mock_switch.assert_called_once_with("new-org") +@patch("graphistry.pygraphistry.ArrowUploader.refresh") +def test_refresh_rotated_token_does_not_skip_switch(mock_refresh): + """The server resets the active org on every reissue (that's what refresh() + calling switch_org() guards against -- see test_refresh_switches_org). A + rotated token must not be waved through by a stale cache entry either: + regression test for the token cached under the old, pre-refresh token + getting reinstated instead of the switch actually running.""" + mock_arrow = unittest.mock.MagicMock() + mock_arrow.token = "tok-new" + mock_refresh.return_value = mock_arrow + + client = graphistry.client() + client.session.org_name = "acme" + client.api_token("tok-old") + client.session.mark_org_verified("tok-old", "acme") + + with patch("graphistry.pygraphistry.switch_org_request") as mock_request: + result = client.refresh() + + mock_request.assert_called_once() + assert result == "tok-new" + assert client.api_token() == "tok-new" + + +def test_refresh_rotated_token_hits_real_switch_endpoint(): + """HTTP-level version of test_refresh_rotated_token_does_not_skip_switch: + only requests.post is mocked, so ArrowUploader.refresh() and the real + switch_org_request() run unmocked. Confirms the actual outgoing /switch/ + POST carries the new (rotated) token as Bearer auth, not the stale + pre-refresh one, and that a non-OK-shaped /switch/ body would have + surfaced as OrgSwitchError rather than being silently accepted.""" + client = graphistry.client() + client.session.org_name = "acme" + client.api_token("tok-old") + client.session.mark_org_verified("tok-old", "acme") + + def fake_post(url, **kwargs): + response = unittest.mock.MagicMock() + response.status_code = 200 + if url.endswith("/api/v2/auth/token/refresh"): + response.json.return_value = {"token": "tok-new"} + elif url.endswith("/api/v2/o/acme/switch/"): + # Real server behavior for a token that was just rotated: it must + # be presented as Bearer auth for the switch to succeed. + auth = kwargs.get("headers", {}).get("Authorization") + assert auth == "Bearer tok-new", f"switch request used wrong token: {auth}" + response.json.return_value = {"status": "OK", "data": {}} + else: + raise AssertionError(f"unexpected POST to {url}") + return response + + with patch("requests.post", side_effect=fake_post) as mock_post: + result = client.refresh() + + assert result == "tok-new" + assert client.api_token() == "tok-new" + urls_called = [c.args[0] for c in mock_post.call_args_list] + assert any(u.endswith("/api/v2/o/acme/switch/") for u in urls_called), ( + "refresh() must trigger a real org switch for the rotated token, not reuse a stale cache entry" + ) + assert client.session.is_org_verified("tok-new", "acme") + + def test_maybe_switch_org_cached_pair_skips(): client = graphistry.client() client.api_token("tok123") @@ -149,22 +212,29 @@ def test_maybe_switch_org_new_org_switches(): mock_switch.assert_called_once_with("mock-org") -def _fake_jwt(exp: Optional[float] = None) -> str: - """Minimal unsigned-looking JWT with an optional exp claim, for exercising - ClientSession's exp-aware verified-org cache without a real server.""" +def _fake_jwt(exp: Optional[float] = None, user_id: Optional[int] = None, jti: Optional[str] = None) -> str: + """Minimal unsigned-looking JWT with optional exp / user_id claims, for exercising + ClientSession's exp-aware, user-bound verified-org cache without a real server. + ``jti`` only makes two otherwise-identical tokens distinct.""" header = base64.urlsafe_b64encode(b'{"alg":"none"}').rstrip(b"=").decode() - payload = {} if exp is None else {"exp": exp} + payload: dict = {} + if exp is not None: + payload["exp"] = exp + if user_id is not None: + payload["user_id"] = user_id + if jti is not None: + payload["jti"] = jti body = base64.urlsafe_b64encode(json.dumps(payload).encode()).rstrip(b"=").decode() return f"{header}.{body}.sig" def test_switch_org_reuses_earlier_verified_token_for_different_org(): """Org A was SSO-verified under token1; a later SSO login to org B mints - token2 (now active). Switching back to org A should swap token1 back in - locally instead of hitting the server, since token1 hasn't expired.""" + token2 (now active) for the same user. Switching back to org A should swap + token1 back in locally instead of hitting the server, since token1 hasn't expired.""" client = graphistry.client() - token1 = _fake_jwt(exp=time.time() + 3600) - token2 = _fake_jwt(exp=time.time() + 3600) + token1 = _fake_jwt(exp=time.time() + 3600, user_id=7, jti="t1") + token2 = _fake_jwt(exp=time.time() + 3600, user_id=7, jti="t2") client.api_token(token1) client.session.mark_org_verified(token1, "org-a") @@ -184,10 +254,47 @@ def test_switch_org_reuses_earlier_verified_token_for_different_org(): assert client.session._is_authenticated is True +def test_switch_org_does_not_reuse_cached_token_of_another_user(): + """Cache is keyed by org only, and login()/api_token() can change the active + principal without resetting the session. A cached token belonging to a + different Hub user must never be swapped back in.""" + client = graphistry.client() + user_a_token = _fake_jwt(exp=time.time() + 3600, user_id=1) + user_b_token = _fake_jwt(exp=time.time() + 3600, user_id=2) + + client.api_token(user_a_token) + client.session.mark_org_verified(user_a_token, "shared-org") + client.api_token(user_b_token) + + with patch("graphistry.pygraphistry.switch_org_request") as mock_req: + client.switch_org("shared-org") + + mock_req.assert_called_once() + assert client.api_token() == user_b_token + + +def test_switch_org_does_not_reuse_cached_token_without_user_claim(): + """Opaque tokens (no readable user_id) can't prove they belong to the current + user, so they never qualify for the swap-in fast path.""" + client = graphistry.client() + opaque_cached = "opaque-token-a" + opaque_active = "opaque-token-b" + + client.api_token(opaque_cached) + client.session.mark_org_verified(opaque_cached, "org-a") + client.api_token(opaque_active) + + with patch("graphistry.pygraphistry.switch_org_request") as mock_req: + client.switch_org("org-a") + + mock_req.assert_called_once() + assert client.api_token() == opaque_active + + def test_switch_org_does_not_reuse_expired_cached_token(): client = graphistry.client() - expired_token = _fake_jwt(exp=time.time() - 10) - active_token = _fake_jwt(exp=time.time() + 3600) + expired_token = _fake_jwt(exp=time.time() - 10, user_id=7, jti="old") + active_token = _fake_jwt(exp=time.time() + 3600, user_id=7, jti="new") client.api_token(expired_token) client.session.mark_org_verified(expired_token, "org-a") @@ -248,6 +355,25 @@ def test_pending_state_stops_at_timeout(self): assert out is None + def test_pending_state_stops_at_timeout_without_ipython(self): + # IPython is an optional dependency; its absence must not skip the + # per-iteration sleep/timeout, or the loop spins forever at full speed. + client = self._client() + blocked = {"IPython": None, "IPython.core": None, "IPython.core.display": None, "IPython.display": None} + with patch.dict("sys.modules", blocked): + with patch.object( + client, "_sso_get_token", + side_effect=SsoPendingException("State is invalid") + ) as mock_poll: + with patch("graphistry.pygraphistry.time.sleep") as mock_sleep: + out = client._handle_auth_url("http://auth", 3, None) + + assert out is None + # elapsed_time starts at 1; polls at 1, 2, 3 then elapsed hits 4 > 3 and exits. + assert mock_poll.call_count == 3 + # One initial sleep before the loop plus one per poll. + assert mock_sleep.call_count == 4 + def test_status_less_body_is_treated_as_pending(self): # A proxy/DRF error body with no 'status' key must not abort the login. outcomes = [