Repository navigation
UI auth: refine session token handling (backport 2.1 #36250) - #36436
Conversation
(cherry picked from commit ca5ccc3) Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
❌ PR checklist incompleteThis PR cannot be merged until the following are addressed on its linked issue:
The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically. Maintainers can bypass this check by adding the |
github-actions
Bot
added
the
safe to test
Code Review ✅ Approved🔴 High risk · Rejecting bot and personal-access tokens in the UI clears stored sessions; claim misclassification could sign out legitimate users and block UI access. Backport of the UI session token refinement to VerificationRules No rules evaluated Functional validation Not enabledAuto-approval Not enabledFor more verification details, comment OptionsDisplay: compact → Showing compact verification results. Comment with these commands to change the behavior for this request:
Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| it('signs out with a toast when a stored bot or personal access token is rejected', async () => { | ||
| await renderForLogout(); | ||
| const onRejected = mockSetApiOnlyTokenListener.mock.calls.at(-1)?.[0]; | ||
| await act(async () => { | ||
| onRejected?.(); | ||
| }); | ||
|
|
||
| expect(showErrorToast).toHaveBeenCalledWith( | ||
| 'server.api-only-token-ui-access-denied' | ||
| ); | ||
| expect(mockPause).toHaveBeenCalled(); | ||
| expect(markSignedOut).toHaveBeenCalled(); |
There was a problem hiding this comment.
The new test calls a captured listener and checks mocked calls to showErrorToast, pause, and markSignedOut. It never checks that the user becomes signed out, so it can pass even if the rest of sign-out fails.
The repository requires tests to check real outcomes rather than mock wiring. Before merging, drive a rejected stored token through the listener and check that the session is cleared and the user reaches sign-in.
Context Used: AGENTS.md (source)
Jest test CoverageUI tests summary
|
|
|
✅ SSO Login Nightly — PASSED View workflow run · Commit
Totals: 118 passed · 0 failed · 0 flaky · 0 skipped · duration 24m 55s |
Backport of #36250 to
2.1.Describe your changes:
Refines how the UI handles session tokens. Interactive UI sessions now use tokens from the login flow, and API tokens (bot and personal access tokens) are meant for API clients only. REST API behaviour is unchanged.
Cherry-picked from
ca5ccc3. The only conflict was the eslint suppression baseline incorpus.test.mjs, which is set to2.1's count (537).Test plan
components/Auth,utils/Auth,AuthProvider.util,SwTokenStorageUtils,useApplicationStore): 390 passDomainIsolationIT#test_ingestionBot_readsDomainAssetAndServicepasses locally against2.1check-i18n,corpus.test.mjs, decision-record check🤖 Generated with Claude Code
The code changes appear sound, but the sign-out test must check real outcomes to satisfy the repository’s testing requirement before merging.
Findings
Summary
The UI now rejects stored bot and personal access tokens while leaving REST API behavior unchanged.
Acknowledged and excluded:
chirag-madlanirecords in the decision that an already-running refresh can restore a session during sign-out. That existing race is deferred to #36319.Diagram
%%{init: {'theme': 'neutral'}}%% flowchart TD A["Read stored token"] --> B{"API-only claims?"} B -- No --> C["Return login token"] B -- Yes --> D["Clear app_state"] B -- Yes --> E["Notify AuthProvider once"] E --> F["Show translated toast"] F --> G["Pause renewal and sign out"] D --> H["Return empty token"]Reviews (1) · Last reviewed commit: "UI auth: refine session token handling (..." · Reviewed by Greptile