fix(client): clear the native session on logout #385

Merged

james merged 1 commit from fix/native-logout-clears-session into main 2026-06-29 18:32:30 +00:00

Conversation 1 Commits 1 Files changed 4 +148 -18

james commented 2026-06-29 18:23:41 +00:00

Owner

Copy link

The bug

On Android (any native build), tapping Log out sends you to the login screen — but close the app and reopen it and you're back on the chat screen, still signed in. Logging out never actually clears your credentials.

Cause

useLogout() (@carol/api-client) POSTs /api/auth/logout and clears the query cache. That endpoint only revokes the web carol_session cookie — native auth is the bearer access + refresh tokens in SecureStore (apps/client/lib/auth/storage.ts). Nothing ever called clearTokens(), so on the next launch the proactive refresh (apps/client/lib/auth/refresh.ts → ensureFreshAccessToken) reads the surviving refresh token, mints a fresh pair, and the protected layout drops you straight back into the app. The useLogout comment even claimed "native consumers just discard their tokens" — but nothing did.

Fix

New apps/client/lib/auth/logout.ts → performLogout: fires the server logout and, on settle (success or failure — so an offline logout still ends the session), discards the stored bearer tokens and routes to /login. clearTokens() is a no-op on web, so the one path is correct for both targets. A failing keystore clear can't surface as an unhandled rejection (navigates regardless). Both Sidebar logout buttons (collapsed + expanded) now go through it, and the misleading useLogout comment is corrected to point here.

Tests

• New apps/client/tests/logout.test.ts pins the orchestration (mirrors refresh.test.ts's injected-deps style, no renderer): fires the mutation once + runs the drawer-close side effect; clears tokens then navigates on settle; still navigates if clearing throws.
• Full client suite green (194 passed); lint + typecheck clean.

Follow-up (not in this PR)

This clears the session locally, which fixes the reported symptom completely. It does not revoke the refresh-token family server-side, so a token extracted from the device before logout stays valid until it expires (≤30 days). Worth a separate hardening pass — apps/api/db/repositories/refresh-tokens.ts already has revokeFamily(); /api/auth/logout would need to accept the bearer/refresh token and revoke. Happy to do it next if you want it.

🤖 Generated with Claude Code

## The bug On Android (any native build), tapping **Log out** sends you to the login screen — but **close the app and reopen it and you're back on the chat screen**, still signed in. Logging out never actually clears your credentials. ## Cause `useLogout()` (`@carol/api-client`) POSTs `/api/auth/logout` and clears the query cache. That endpoint only revokes the **web** `carol_session` cookie — native auth is the bearer access + refresh tokens in SecureStore (`apps/client/lib/auth/storage.ts`). Nothing ever called `clearTokens()`, so on the next launch the proactive refresh (`apps/client/lib/auth/refresh.ts` → `ensureFreshAccessToken`) reads the surviving refresh token, mints a fresh pair, and the protected layout drops you straight back into the app. The `useLogout` comment even claimed "native consumers just discard their tokens" — but nothing did. ## Fix New `apps/client/lib/auth/logout.ts` → `performLogout`: fires the server logout and, **on settle** (success *or* failure — so an offline logout still ends the session), discards the stored bearer tokens and routes to `/login`. `clearTokens()` is a no-op on web, so the one path is correct for both targets. A failing keystore clear can't surface as an unhandled rejection (navigates regardless). Both `Sidebar` logout buttons (collapsed + expanded) now go through it, and the misleading `useLogout` comment is corrected to point here. ## Tests - New `apps/client/tests/logout.test.ts` pins the orchestration (mirrors `refresh.test.ts`'s injected-deps style, no renderer): fires the mutation once + runs the drawer-close side effect; clears tokens **then** navigates on settle; still navigates if clearing throws. - Full client suite green (194 passed); lint + typecheck clean. ## Follow-up (not in this PR) This clears the session **locally**, which fixes the reported symptom completely. It does **not** revoke the refresh-token *family* server-side, so a token extracted from the device before logout stays valid until it expires (≤30 days). Worth a separate hardening pass — `apps/api/db/repositories/refresh-tokens.ts` already has `revokeFamily()`; `/api/auth/logout` would need to accept the bearer/refresh token and revoke. Happy to do it next if you want it. 🤖 Generated with [Claude Code](https://claude.com/claude-code)

james added 1 commit 2026-06-29 18:23:41 +00:00

fix(client): clear native session on logout so it doesn't auto-sign-in ... 8aebae4a47

On native, the logout button POSTed /api/auth/logout (which only clears
the web session cookie) and navigated to /login, but never discarded the
access + refresh tokens in SecureStore. The proactive refresh
(lib/auth/refresh.ts) then read the surviving refresh token on the next
launch and silently signed the user back in — close the app, reopen, and
you land back on the chat screen, session never actually cleared.

Add performLogout (lib/auth/logout.ts): fire the server logout and, on
settle (success OR failure, so offline logout still works), clear the
stored bearer tokens and route to /login. clearTokens is a no-op on web,
so the single path is correct for both targets. Wire both Sidebar logout
buttons through it; unit-test the orchestration.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

forgejo-actions commented 2026-06-29 18:28:59 +00:00

Copy link

📊 Test coverage

Patch coverage: no testable lines changed.

Overall (app/, lib/, db/, excluding UI per ADR-0019):

Metric	Value	Soft target
Lines	79.6% ✅	≥ 50%
Branches	71.6% ⚠️	≥ 75%
Functions	81.2%	informational

Soft thresholds per ADR-0019. Coverage is informational and does not block merge.

<!-- coverage-comment --> ## 📊 Test coverage **Patch coverage:** no testable lines changed. **Overall** (`app/`, `lib/`, `db/`, excluding UI per ADR-0019): | Metric | Value | Soft target | |---|---|---| | Lines | 79.6% ✅ | ≥ 50% | | Branches | 71.6% ⚠️ | ≥ 75% | | Functions | 81.2% | informational | Soft thresholds per [ADR-0019](docs/adr/0019-coverage-soft-targets.md). Coverage is informational and does not block merge.

james merged commit b074afe2db into main 2026-06-29 18:32:30 +00:00

james deleted branch fix/native-logout-clears-session 2026-06-29 18:32:30 +00:00

james referenced this pull request from a commit 2026-06-29 18:32:31 +00:00

Merge pull request 'fix(client): clear the native session on logout' (#385) from fix/native-logout-clears-session into main

james referenced this pull request 2026-06-30 12:24:09 +00:00

fix(api): revoke the native refresh-token family on logout #390

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

area:auth

Authentication and authorization

  

area:ci

Forgejo Actions, build, test, security, release

  

area:db

Database abstraction, migrations, persistence

  

area:infra

Containerization, deployment, foundational tooling

  

area:native

React Native app (Android + Linux Flatpak)

  

area:pwa

PWA / web frontend

  

area:service

Backend service, API routes, persistence

  

epic

High-level tracker; links several feature tickets

  

feature

A user-facing feature

  

foundation

Foundational work that other tickets build on

No labels

area:auth

area:ci

area:db

area:infra

area:native

area:pwa

area:service

epic

feature

foundation

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

2 participants

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

james/carol!385

No description provided.