mirror of
https://github.com/hanzoai/id.git
synced 2026-10-16 18:38:15 +00:00
sign out has to clear this browser, not just the IdP
Reported as "sign out is not working", and it half was. Portal navigated
straight to client.logout(), which builds the RP-initiated logout URL and does
nothing else. The server end is genuinely correct -- measured against prod, the
leftover token comes back 401 "the access token is invalid or revoked" -- so
this is not an access leak. What survived was the token STRING and its
siblings: every hanzo_iam_* key stayed in storage after signing out.
That is enough to read as broken, because the presence of that key is how other
surfaces decide you are signed in. hanzoai/playground's AuthGuard branches on
sessionStorage.getItem('hanzo_iam_access_token') before anything else. And a
token that outlives the session it names is worth deleting wherever it sits.
signOut() is now ONE call: clear, then hand back the IdP URL. A URL builder
that also mutates storage would be a trap, so logout() keeps its single job and
the complete operation gets its own name -- the call site asks for the outcome,
not the two halves.
Three details that are the bug in miniature:
- Sweep the hanzo_iam_ PREFIX, never a list of literals. A list here is a
second copy of the SDK's key set, and the copy is what goes stale when the
SDK adds one. There are nine today.
- Collect keys, THEN delete. Removing while iterating by index re-indexes the
store and skips every other key.
- Both storages: the token is in sessionStorage, but the PKCE verifier is in
localStorage by design (it has to survive the redirect to the IdP).
Tested with a neighbour key that must SURVIVE, so "it cleared everything"
cannot pass; and the guard is known to bite -- neutering the prefix check fails
it by name, restoring passes. 172/172, typechecks clean.
Co-authored-by: Hanzo Dev <[email protected]>
This commit is contained in:
@@ -79,7 +79,9 @@ export function Portal({
|
||||
// Signed in: the apps launcher.
|
||||
const apps = appsFor(org.orgId)
|
||||
const billingUrl = billingFor(org.orgId)
|
||||
const logoutUrl = client.logout(undefined, `${org.publicOrigin}/login`)
|
||||
// signOut, not logout: the latter only builds the IdP URL and leaves this
|
||||
// browser's `hanzo_iam_*` keys in place, so the token string outlived the
|
||||
// session it named. Called on click, not at render — it clears storage.
|
||||
|
||||
return (
|
||||
<div className="hanzo-id-page hanzo-id-portal">
|
||||
@@ -111,7 +113,7 @@ export function Portal({
|
||||
isAuthenticated
|
||||
usageUrl={billingUrl}
|
||||
usageLabel="Billing"
|
||||
onSignOut={() => { window.location.href = logoutUrl }}
|
||||
onSignOut={() => { window.location.href = client.signOut(`${org.publicOrigin}/login`) }}
|
||||
/>
|
||||
</div>
|
||||
</main>
|
||||
|
||||
@@ -683,3 +683,55 @@ test('getAppLogin sends the passed redirect_uri, and defaults to the portal call
|
||||
const u2 = new URL(urls[1]!)
|
||||
assert.equal(u2.searchParams.get('redirectUri'), 'https://hanzo.id/callback')
|
||||
})
|
||||
|
||||
// Sign-out has to clear THIS BROWSER, not just end the session at the IdP.
|
||||
//
|
||||
// The bug: Portal navigated straight to client.logout(), which builds the
|
||||
// RP-initiated logout URL and nothing else. The server really did revoke the
|
||||
// token (measured against prod: the leftover token 401s "invalid or revoked"),
|
||||
// but every `hanzo_iam_*` key survived — so the token STRING outlived the
|
||||
// session it named, and anything treating that key's presence as "signed in"
|
||||
// still believed you were.
|
||||
test('signOut clears every hanzo_iam_* key, in BOTH storages, and returns the IdP URL', () => {
|
||||
const store = () => {
|
||||
const m = new Map<string, string>()
|
||||
return {
|
||||
get length() { return m.size },
|
||||
key: (i: number) => [...m.keys()][i] ?? null,
|
||||
getItem: (k: string) => m.get(k) ?? null,
|
||||
setItem: (k: string, v: string) => void m.set(k, v),
|
||||
removeItem: (k: string) => void m.delete(k),
|
||||
clear: () => m.clear(),
|
||||
_keys: () => [...m.keys()],
|
||||
}
|
||||
}
|
||||
const ss = store(), ls = store()
|
||||
const g = globalThis as Record<string, unknown>
|
||||
const [ps, pl] = [g.sessionStorage, g.localStorage]
|
||||
g.sessionStorage = ss
|
||||
g.localStorage = ls
|
||||
try {
|
||||
// Three of the SDK's keys, plus a neighbour that must SURVIVE — otherwise
|
||||
// "it cleared everything" would pass this test just as well.
|
||||
for (const s of [ss, ls]) {
|
||||
s.setItem('hanzo_iam_access_token', 'x')
|
||||
s.setItem('hanzo_iam_expires_at', 'x')
|
||||
s.setItem('hanzo_iam_code_verifier:abc', 'x')
|
||||
s.setItem('theme', 'dark')
|
||||
}
|
||||
const client = createAuthClient({
|
||||
org: { orgId: 'hanzo', iamUrl: 'https://hanzo.id', iamIssuer: 'https://hanzo.id',
|
||||
clientId: 'hanzo-console', appName: 'hanzo-console',
|
||||
publicOrigin: 'https://hanzo.id', brandPackage: '@hanzo/brand' } as never,
|
||||
})
|
||||
const url = client.signOut('https://hanzo.id/login')
|
||||
|
||||
assert.deepEqual(ss._keys(), ['theme'], 'sessionStorage')
|
||||
assert.deepEqual(ls._keys(), ['theme'], 'localStorage')
|
||||
assert.match(url, /\/v1\/iam\/oauth\/logout\?/)
|
||||
assert.match(url, /post_logout_redirect_uri=https%3A%2F%2Fhanzo\.id%2Flogin/)
|
||||
} finally {
|
||||
g.sessionStorage = ps
|
||||
g.localStorage = pl
|
||||
}
|
||||
})
|
||||
|
||||
@@ -91,6 +91,24 @@ export interface AuthClient {
|
||||
authorize(req: OAuthAuthorizeRequest): string
|
||||
exchange(code: string, codeVerifier?: string): Promise<TokenResponse>
|
||||
logout(idTokenHint?: string, postLogoutRedirectUri?: string): string
|
||||
/**
|
||||
* Sign out COMPLETELY: drop this browser's own tokens, then hand back the
|
||||
* IdP's RP-initiated logout URL for the caller to navigate to.
|
||||
*
|
||||
* `logout()` alone is only half of it, and the missing half is the half a
|
||||
* person notices. It builds the IdP URL, which ends the session at the
|
||||
* server — measured, the access token really is revoked there — but it
|
||||
* touches nothing this browser stored, so `hanzo_iam_access_token` and its
|
||||
* siblings survive a sign-out. Anything that treats the presence of that key
|
||||
* as "signed in" then still believes you are (hanzoai/playground's AuthGuard
|
||||
* reads exactly that key), and a token string that outlives its session is a
|
||||
* thing to delete on principle even where nothing reads it.
|
||||
*
|
||||
* So sign-out is ONE call, not a URL plus a cleanup every caller has to
|
||||
* remember. Local first, then the redirect: a navigation ends this
|
||||
* document, and anything left after `location.href` is a coin flip.
|
||||
*/
|
||||
signOut(postLogoutRedirectUri?: string): string
|
||||
/**
|
||||
* Read the live enabled-auth-methods view for an application from
|
||||
* `/v1/iam/get-app-login` — the canonical source of truth for which
|
||||
@@ -467,6 +485,33 @@ export function createAuthClient(opts: AuthClientOptions): AuthClient {
|
||||
return url.toString()
|
||||
}
|
||||
|
||||
function signOut(postLogoutRedirectUri?: string): string {
|
||||
// Every key the SDK owns is namespaced `hanzo_iam_*` — access_token,
|
||||
// expires_at, state, code_verifier, current_org, current_project,
|
||||
// post_login_redirect. Sweep the PREFIX rather than naming them: a list of
|
||||
// literals here is a second copy of the SDK's key set, and the copy is what
|
||||
// goes stale when the SDK adds one. The prefix is the contract.
|
||||
//
|
||||
// Both storages. The token lives in sessionStorage, but the PKCE verifier
|
||||
// is deliberately in localStorage (it has to survive the full-page redirect
|
||||
// to the IdP), and apps cache the token there too.
|
||||
for (const store of [
|
||||
typeof sessionStorage !== 'undefined' ? sessionStorage : null,
|
||||
typeof localStorage !== 'undefined' ? localStorage : null,
|
||||
]) {
|
||||
if (!store) continue
|
||||
// Collect first, then delete: removing while iterating by index
|
||||
// re-indexes the store and skips every other key.
|
||||
const doomed: string[] = []
|
||||
for (let i = 0; i < store.length; i++) {
|
||||
const k = store.key(i)
|
||||
if (k && k.startsWith('hanzo_iam_')) doomed.push(k)
|
||||
}
|
||||
for (const k of doomed) store.removeItem(k)
|
||||
}
|
||||
return logout(undefined, postLogoutRedirectUri)
|
||||
}
|
||||
|
||||
async function getAppLogin(clientId?: string, redirectUri?: string): Promise<AppLogin | null> {
|
||||
const id = clientId ?? org.clientId
|
||||
const url = new URL('/v1/iam/get-app-login', org.iamUrl)
|
||||
@@ -609,6 +654,7 @@ export function createAuthClient(opts: AuthClientOptions): AuthClient {
|
||||
authorize,
|
||||
exchange,
|
||||
logout,
|
||||
signOut,
|
||||
getAppLogin,
|
||||
getAccount,
|
||||
mfaInitiate,
|
||||
|
||||
Reference in New Issue
Block a user