Skip to content

Fix SSO dataset error on site wide org - #1721

Open
vaimdevs wants to merge 15 commits into
masterfrom
fix/site-wide-org-dataset-error
Open

vaimdevs wants to merge 15 commits into
masterfrom
fix/site-wide-org-dataset-error

Conversation

@vaimdevs

@vaimdevs vaimdevs commented Jul 10, 2026 •

Copy link
Copy Markdown
Contributor

Changelog

# Fix
- SSO: the blocking `sso_login()` poll loop no longer spins without sleeping or timing out when IPython is not installed; the notebook countdown is display-only and can't skip the per-iteration wait (follow-up to #1721).
- `switch_org()`: the cached-token swap-in fast path is now bound to the Hub `user_id` claim, so a token cached for one user is never reinstated after `login()`/`api_token()` switched to another; opaque tokens never qualify (follow-up to #1721).
- `refresh()` no longer carries an org's verified-switch grant onto the reissued token: the server resets the active org on every reissue, so the carried grant let `plot()` skip the required `/switch/` call and silently upload to the default org while `session.org_name` still showed the requested one. Once the token actually rotates, the stale cache entry for that org is dropped first so `switch_org()`'s same-user fast path can't swap the superseded pre-refresh token back in as active either (follow-up to #1721).
- **Organization switch validation is no longer skipped by memoization**: `ArrowUploader._switch_org` now validates t
he HTTP response and checks for SSO challenges before recording the switch in its memo. Previously, failed switches (
including SSO challenges) were unconditionally memoized, causing `_maybe_switch_org` to skip the validating `Graphist
ryClient.switch_org` call on subsequent attempts. A failed switch no longer poisons the memo, allowing the full valid
ation path to run and raise `OrgSwitchRequiresSsoException` when needed. This fixes a critical issue where SSO org sw
itches could silently fail on the automatic fallback-to-personal-org path without triggering proper error handling (#1721).



Summary

After SSO login on a site-wide SSO server, the session could end up bound to the reserved SITE org (or to no org at all), and subsequent dataset uploads failed because nothing can be created under SITE. This PR fixes that, and along the way hardens the surrounding org-switch and SSO-polling code that the fix depends on.

Changes

1. SSO org resolution (ArrowUploader.sso_get_token)

Four-layer fallback for deciding which org the session is bound to after SSO:

  • Layer 1 — server returned active_organization: use it, but treat the reserved SITE sentinel as unbound (with a warning that the IdP org claim is likely mis-mapped server-side). Caller-supplied org_name must match or be absent.
  • Layer 2 — caller passed org_name, server silent: preserve caller intent; membership validates lazily on later requests.
  • Layer 3 — neither: new _personal_org_from_api() looks up the user's personal org via GET /api/v2/my/organizations/ filtered on is_personal. Replaces the earlier JWT-username-slugify reconstruction, which couldn't reproduce server-side slug-collision suffixing.
  • Layer 4 — nothing found: log and complete with no org binding.

2. Unified org-switch request (graphistry/utils/requests.py)

GraphistryClient.switch_org and ArrowUploader._switch_org had duplicate POST-and-validate code. New switch_org_request() is the single source of truth and raises:

  • OrgSwitchError — non-2xx HTTP status or non-OK body status.
  • OrgSwitchIdpChallenge — server quirk: for a plain member (not owner/admin) of an org with its own IdP, the switch endpoint returns status: OK without actually switching and instead returns an SSO challenge in data['idp']. Previously this was silently treated as success, leaving the caller on their previous org.

3. Per-org verified-token cache (ClientSession)

Replaces the single _last_switched_org_token tuple with _verified_org_tokens: Dict[org_name -> (token, exp)] plus is_org_verified() / mark_org_verified() / get_verified_token(). Entries self-expire based on the JWT exp claim (read without signature verification — the token is already trusted, having come from the server via the auth flow just completed).

  • A token just SSO-minted for org X is marked verified immediately so the follow-up _maybe_switch_org never re-triggers an IdP challenge.
  • switch_org() gains two fast paths: already verified under the current token → skip the round trip; a still-unexpired older token was verified for that org → swap it back in as active.
  • Failed or IdP-challenged switches are no longer memoized (the old code recorded success before checking the response).
  • refresh() carries the org grant forward to the newly issued token.

4. SSO polling robustness (GraphistryClient.sso_login)

New SsoPendingException distinguishes "browser login not finished yet" from real failures. The poll loop now:

  • keeps waiting on pending until sso_timeout;
  • retries transient requests.RequestException / JSON ValueError up to SSO_POLL_MAX_CONSECUTIVE_ERRORS (3) consecutive times before giving up;
  • propagates SsoStateInvalidException and other permanent errors immediately.

The per-poll log is demoted from error to debug so it stops printing once a second during a normal login.

5. Mid-switch org tracking

New session.sso_requested_org_name records the org an in-flight SSO login was initiated for, since session.org_name still holds the previous org until the switch is confirmed. _sso_get_token() passes it explicitly to ArrowUploader.

6. Cleanup

Removed dead commented-out code (sso_state/sso_auth_url stubs, api_token_refresh_ms, old polling loop), return-type annotations and mypy fixes in utils/requests.py / switch_org.

Tests

  • tests/utils/test_requests.py (new) — switch_org_request success, HTTP error, non-OK body, and IdP-challenge branches.
  • test_arrow_uploader.py — _switch_org memoization/no-memoization on failure, SITE sentinel handling, _personal_org_from_api fallback, SsoPendingException classification.
  • test_client_session.py — verified-token cache and JWT exp expiry.
  • test_pygraphistry.py — switch_org fast paths, poll-loop failure classification, refresh carrying the org grant.

🤖 Generated with Claude Code

https://claude.ai/code/session_01TyjxnzaqeB3SeirofUQhSj

@vaimdevs vaimdevs changed the title Fix SSO dataset on site wide org Fix SSO dataset error on site wide org Jul 10, 2026
vaimdev added 2 commits July 10, 2026 21:26
…false success

switch_org() treated any status=='OK' response as a completed switch, but
the server overloads that status for two different outcomes: a real switch
(data has pk/org_name/organization_slug) and an SSO re-authentication
challenge (data['idp'] populated) for members of SSO-protected orgs who
aren't the owner/admin. The challenge case never actually updates the
server-side active org, so treating it as success silently left callers on
their previous org while locally claiming to have switched.

Now raises with the idp names and a pointer to sso_login(org_name=...) as
the alternative, instead of logging a false "Switched to organization" and
setting self.session.org_name to a value the server never accepted.
@vaimdev
vaimdev requested a review from aucahuasi July 17, 2026 15:36
vaimdev and others added 2 commits July 22, 2026 17:54
_switch_org wrote _last_switched_org_token unconditionally after the
POST, so a 403/500 or an SSO-challenge response (200 + data['idp'])
got recorded as a completed switch. That poisoned memo made
_maybe_switch_org short-circuit later, skipping switch_org() -- the
only implementation that validates the response and raises on an SSO
challenge. Now _switch_org returns without writing the memo on a
non-2xx status or an idp challenge.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
vaimdev and others added 2 commits August 28, 2026 11:32
…ogic

ArrowUploader._switch_org and GraphistryClient.switch_org each POSTed to
the same org-switch endpoint with their own copy of the HTTP-status,
body-status, and SSO-idp-challenge checks. Extract the shared logic into
switch_org_request() in graphistry/utils/requests.py, raising
OrgSwitchError / OrgSwitchIdpChallenge. Each caller keeps its own
success/failure policy (ArrowUploader logs+swallows, GraphistryClient
raises) since that divergence is intentional API contract, not
duplication.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TNGYwRNwAFQdYDucbwgtUm
…quests.py

Type-hygiene guard flagged missing __init__ return annotations and a bare
dict for OrgSwitchIdpChallenge's idp param.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GgDZr3AgU9uKz5EWcVZZVY
@vaimdev
vaimdev force-pushed the fix/site-wide-org-dataset-error branch from 75c3b4f to f043c72 Compare August 28, 2026 08:21
Guard baseline already sat at max cast() count; cast() would've pushed it
over. Use a localized type: ignore instead, per DEVELOP.md guidance.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GgDZr3AgU9uKz5EWcVZZVY
@vaimdev
vaimdev force-pushed the fix/site-wide-org-dataset-error branch from f043c72 to 4a9d63e Compare August 28, 2026 08:50
vaimdev and others added 2 commits August 28, 2026 17:55
Adds direct unit tests for graphistry/utils/requests.py's switch_org_request
(success, HTTP error, non-OK body w/ detail, unparseable JSON, SSO idp
challenge) and ArrowUploader._switch_org (no-op guards, dedup cache,
OrgSwitchError/OrgSwitchIdpChallenge/generic-exception swallow paths),
raising changed-line coverage on the switch-org refactor.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GgDZr3AgU9uKz5EWcVZZVY
mock.patch("graphistry.utils.requests.requests.post") failed under the
full suite (AttributeError: module 'graphistry' has no attribute 'utils')
because some other test's import juggling leaves the parent package's
'utils' attribute unset. mock.patch.object against the imported module
sidesteps the string-resolution path entirely.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GgDZr3AgU9uKz5EWcVZZVY
@vaimdev
vaimdev force-pushed the fix/site-wide-org-dataset-error branch from 95a9c9a to 510625a Compare September 2, 2026 09:02
…ent-only)

Cache per-org verified JWTs (exp-aware) in ClientSession so switch_org()
can no-op or swap in a cached token instead of re-triggering SSO, without
any server-side changes. Multi-org: each org's token cached independently,
so switching back to an org verified under an earlier (now-inactive)
token still skips re-challenge as long as that token hasn't expired.

- client_session.py: _verified_org_tokens dict (org -> (token, exp)),
  is_org_verified / mark_org_verified / get_verified_token, JWT exp
  read (no signature verification, token already trusted).
- pygraphistry.py: switch_org() fast paths (same-token skip, cached-token
  swap-in); sso_login()/_handle_auth_url() record verification immediately
  on token issuance instead of waiting on a follow-up switch call.
- arrow_uploader.py: _switch_org() uses the same cache API.
- Tests added/migrated in test_pygraphistry.py, test_client_session.py,
  tests/utils/test_requests.py.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UnsBpuxHHTvC2bAUNaCfED
@vaimdev
vaimdev force-pushed the fix/site-wide-org-dataset-error branch from 510625a to f24fb45 Compare September 2, 2026 09:06
vaimdev and others added 2 commits September 3, 2026 17:21
The SSO token endpoint answers with a non-OK body status ("State is
invalid") on every poll until the browser login completes, so the poll
loop's blanket `except Exception` was load-bearing. But it also swallowed
permanent failures -- an org mismatch burned the full sso_timeout and then
reported a misleading "run sso_get_token()" message.

Type the retryable cases instead:

- New SsoPendingException, raised by ArrowUploader.sso_get_token for a
  non-OK body status and for an unrecognized body with no 'status' key
  (proxy error page, DRF detail). Nothing before the token is parsed can
  be classified as permanent.
- The poll loop waits on SsoPendingException uncapped until sso_timeout,
  retries RequestException/ValueError up to SSO_POLL_MAX_CONSECUTIVE_ERRORS
  (ValueError covers both json and simplejson decode errors), and lets
  everything else -- org mismatch, missing JWT in an OK response --
  propagate on the first poll.

Also fix switch_org's cached-token fast path clearing _is_authenticated:
it went through the api_token() setter, whose side effect forced a
needless refresh() on the next authenticate(); that refresh minted a token
not verified for the org, reinstating the round trip the fast path exists
to skip. Assign the session field directly, and carry the org grant onto
the refreshed token in refresh() so the fast path stays hit.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AvLMs6S5UosfKf2YB2sCRS
vaimdev added a commit that referenced this pull request Sep 15, 2026
B1 follow-up to #1721: refresh() carried the org grant from the
pre-refresh token onto the newly reissued one, but the server resets
the active org on every reissue -- so the carried grant let
_maybe_switch_org() skip the required /switch/ POST, and plot()
silently uploaded to the default org while session.org_name still
showed the requested one.

Deleting the carry-grant lines alone regresses further: switch_org()'s
same-user fast path (added by 5cdd1d6 for B3) would then find the
stale cache entry left over from before the rotation and swap that
superseded, pre-refresh token back in as active -- no real switch
request, and refresh()'s own return value reverted to the old token.

Fix: ClientSession.forget_verified_org() drops the cache entry for the
active org once refresh() confirms the token actually rotated (skipped
on a no-op refresh, so it doesn't discard a still-valid verification),
forcing a real switch_org() call with the new token.

Tests: test_refresh_rotated_token_does_not_skip_switch pins the
mocked-switch_org_request behavior; test_refresh_rotated_token_hits_real_switch_endpoint
is the HTTP-level version (only requests.post mocked, real
ArrowUploader.refresh()/switch_org_request() code paths run) -- both
fail against the pre-fix code.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B1Pvaw1aZQvtraEWkEvdyt

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants