Skip to content

fix(jwt): _sessionDestroyJwt must invoke completion callback on all paths#33

Merged
kennethphough merged 1 commit into
masterfrom
fix/jwt-logout-callback
May 22, 2026
Merged

fix(jwt): _sessionDestroyJwt must invoke completion callback on all paths#33
kennethphough merged 1 commit into
masterfrom
fix/jwt-logout-callback

Conversation

@kennethphough

Copy link
Copy Markdown
Member

Summary

  • JWT logout in Shipyard 2.0.0 looks broken — click logout, you stay on the post-logout page
  • Root cause: _sessionDestroyJwt forgot to invoke the user-supplied completion callback on success + early-return paths. addLogoutHandler puts the redirect in that callback.
  • HMAC sessionDestroy already invokes the callback on both success and error. JWT was inconsistent.

Fix

_sessionDestroyJwt now invokes the callback on all three exit paths (success, error, no-refresh-token early return) — matches HMAC contract.

Test plan

  • 3 new regression tests in tests/kyte.test.js, one per exit path
  • Each test mocks $.ajax, asserts callback fires + tokens cleared
  • All 14 tests pass (11 existing + 3 new)
  • Bug reproduced + fix verified on dev Shipyard against ETOM Kyte Dev backend

Patch bump to v2.0.1.

🤖 Generated with Claude Code

…aths

`addLogoutHandler` passes a `location.href = '/'` callback to
`sessionDestroy` expecting it to fire after logout completes.
HMAC sessionDestroy invokes the callback on both success and error
paths of its delete('Session', ...) call. JWT mode's equivalent
_sessionDestroyJwt was missing the invocation in two places:

  - success handler of POST /jwt/logout
  - early return when !refreshToken (already-logged-out case)

Symptom: in JWT mode the redirect never fired. User clicks logout,
tokens get cleared and revoked server-side, but they stay on the
post-logout page — looking "still logged in."

Fix: invoke the completion callback on all three exit paths,
matching the HMAC contract. Renamed nothing (the misleading
`error` parameter name is preserved for API compatibility, with
a comment clarifying it's actually a completion callback).

Regression tests: three new tests in tests/kyte.test.js cover
each exit path with mocked $.ajax, asserting both the callback
invocation and the token-clear side effect. Bumps to 2.0.1
(patch).

All 14 tests pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@kennethphough
kennethphough merged commit e9213a6 into master May 22, 2026
3 checks passed
@kennethphough
kennethphough deleted the fix/jwt-logout-callback branch May 22, 2026 16:16
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.

1 participant