From 665995c7a86502048337e2608c2510adce54ed31 Mon Sep 17 00:00:00 2001 From: Kai Haase Date: Mon, 13 Jul 2026 16:02:34 +0200 Subject: [PATCH] 1.8.5: accept the nest-server `roles: string[]` shape in isAdmin so the admin UI stops silently disappearing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `isAdmin` was `user.value?.role === 'admin'` — Better-Auth's admin-plugin shape (a singular `role` string) only. But `@lenne.tech/nest-server` registers `roles` as a CORE Better-Auth additionalField (`type: 'string[]'`, `defaultValue: []`), so its users carry `roles: ['admin']` and NO singular `role` at all. Against a real nest-server backend `isAdmin` was therefore permanently `false`: every `v-if="isAdmin"` branch (admin nav, admin pages) vanished for actual admins, with no error anywhere — `undefined === 'admin'` is simply false. This was verified against a live nest-server deployment whose admin user returns `roles: ["admin"]` and no `role` field. Changes: - `LtUser` gains `roles?: string[]`, so the multi-role shape is a first-class, typed field rather than something consumers have to cast in. - `isAdmin` accepts EITHER shape (`role === 'admin'` OR `roles` includes 'admin'). `Array.isArray` guards a malformed `roles` in the client-writable auth-state cookie: junk yields `false` instead of throwing in the computed. - `roles` joins `AUTHZ_KEYS`, so the session merge treats it exactly like `role`. It is authorization-relevant now that `isAdmin` reads it: a stale cached `roles: ['admin']` would otherwise keep the admin UI alive after the backend revoked admin — the precise fail-open that list exists to prevent. Fail-closing costs nothing for backends that never send `roles`, because a key is only dropped when the CACHE has it and the session omits it — a `roles`-less backend never caches `roles`, so it is a no-op. And nest-server always returns `roles` from get-session (core additionalField), so its worst case is an honest `[]` (= not an admin), never a spurious drop. Tests: new `test/is-admin.test.ts` covers both shapes, multi-role arrays, empty arrays, absent user and malformed `roles`; `test/merge-session-user.test.ts` gains the `roles` fail-closed / downgrade / no-spurious-drop cases. All 4 new behavioural assertions were confirmed to fail against the pre-fix code. Co-Authored-By: Claude Opus 4.8 (1M context) --- package.json | 2 +- src/runtime/composables/auth/use-lt-auth.ts | 29 +++- src/runtime/types/auth.ts | 8 ++ test/is-admin.test.ts | 143 ++++++++++++++++++++ test/merge-session-user.test.ts | 64 +++++++++ 5 files changed, 243 insertions(+), 3 deletions(-) create mode 100644 test/is-admin.test.ts diff --git a/package.json b/package.json index 9772ff2..34e3f48 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@lenne.tech/nuxt-extensions", - "version": "1.8.4", + "version": "1.8.5", "description": "Reusable Nuxt 4 composables, components, and Better-Auth integration for lenne.tech projects", "repository": { "type": "git", diff --git a/src/runtime/composables/auth/use-lt-auth.ts b/src/runtime/composables/auth/use-lt-auth.ts index d6f5d84..ef5ffed 100644 --- a/src/runtime/composables/auth/use-lt-auth.ts +++ b/src/runtime/composables/auth/use-lt-auth.ts @@ -122,7 +122,22 @@ export function useLtAuth(): UseLtAuthReturn { // Computed properties based on stored state const user = computed(() => resolvedAuthState.value?.user ?? null); const isAuthenticated = computed(() => !!user.value); - const isAdmin = computed(() => user.value?.role === 'admin'); + // Admin detection accepts BOTH user shapes: + // - `role: 'admin'` — Better-Auth's admin plugin (single role). + // - `roles: ['admin']` — `@lenne.tech/nest-server`, which registers `roles` + // as a core Better-Auth additionalField (`string[]`). + // Its users carry NO singular `role`, so a `role`-only + // check is permanently false against a nest-server + // backend and the whole admin UI silently disappears. + // `Array.isArray` guards a malformed `roles` in the (client-writable) auth-state + // cookie: a non-array value must yield `false`, never throw inside the computed. + const isAdmin = computed(() => { + const current = user.value; + if (!current) { + return false; + } + return current.role === 'admin' || (Array.isArray(current.roles) && current.roles.includes('admin')); + }); const is2FAEnabled = computed(() => user.value?.twoFactorEnabled ?? false); // SSR-safe shared features state (useState is isolated per request on server, shared on client) @@ -300,8 +315,18 @@ export function useLtAuth(): UseLtAuthReturn { * field MUST either register it as a Better-Auth additionalField (so * get-session returns it) or add it here — otherwise it could persist stale in * the client cache. See the module CLAUDE.md "Authentication Cookie Rules". + * + * `roles` belongs here for the same reason `role` does: `isAdmin` reads it, so a + * stale cached `roles: ['admin']` would keep the admin UI alive after the backend + * revoked admin — the exact fail-open this list exists to prevent. Fail-closing it + * costs nothing for backends that never send `roles`: the key is only dropped when + * the CACHE has it and the session omits it, so a `roles`-less backend (whose cache + * never carries `roles`) is a no-op. And nest-server — the reason `roles` is + * supported at all — registers it as a CORE additionalField (`type: 'string[]'`, + * `defaultValue: []`), so its get-session always returns `roles`; the worst case is + * an honest `[]` (= not an admin), never a spurious drop. */ - const AUTHZ_KEYS = ['banExpires', 'banReason', 'banned', 'emailVerified', 'role', 'twoFactorEnabled'] as const satisfies readonly (keyof LtUser)[]; + const AUTHZ_KEYS = ['banExpires', 'banReason', 'banned', 'emailVerified', 'role', 'roles', 'twoFactorEnabled'] as const satisfies readonly (keyof LtUser)[]; /** * Merge a Better-Auth session user (partial) onto the cached user of the SAME diff --git a/src/runtime/types/auth.ts b/src/runtime/types/auth.ts index c49672d..3b9d09c 100644 --- a/src/runtime/types/auth.ts +++ b/src/runtime/types/auth.ts @@ -17,7 +17,15 @@ export interface LtUser { id: string; image?: string; name?: string; + /** Single-role shape (Better-Auth admin plugin). See also {@link LtUser.roles}. */ role?: string; + /** + * Multi-role shape. `@lenne.tech/nest-server` registers `roles` as a core + * Better-Auth additionalField (`type: 'string[]'`), so its users carry + * `roles: ['admin']` and usually NO singular `role`. Authorization-relevant: + * `isAdmin` accepts either shape, and `roles` is fail-closed on session merge. + */ + roles?: string[]; twoFactorEnabled?: boolean; } diff --git a/test/is-admin.test.ts b/test/is-admin.test.ts new file mode 100644 index 0000000..312f631 --- /dev/null +++ b/test/is-admin.test.ts @@ -0,0 +1,143 @@ +/** + * `isAdmin` must recognise BOTH user shapes. + * + * Bug this guards against — the admin UI silently disappearing: + * `isAdmin` used to be `user.value?.role === 'admin'`, i.e. Better-Auth's + * admin-plugin shape (a singular `role` string) only. `@lenne.tech/nest-server` + * registers `roles` as a CORE Better-Auth additionalField (`type: 'string[]'`), + * so its users carry `roles: ['admin']` and NO singular `role` at all. Against a + * real nest-server backend `isAdmin` was therefore permanently `false`, and every + * `v-if="isAdmin"` branch (admin nav, admin pages) vanished for actual admins — + * with no error anywhere, because `undefined === 'admin'` is simply false. + * + * Both shapes are now accepted, and neither may leak into the other's result. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +import { resetStubReactiveStores, resetStubRuntimeConfig } from './stubs/imports'; + +// Stub the Better-Auth client so `useLtAuth()` can be instantiated without the +// real client + its peer-deps. Only the surface the composable touches exists. +vi.mock('../src/runtime/composables/use-lt-auth-client', () => ({ + useLtAuthClient: () => ({ + changePassword: () => {}, + passkey: {}, + signIn: { email: async () => ({}) }, + signOut: async () => ({}), + signUp: { email: async () => ({}) }, + twoFactor: {}, + useSession: () => ({ value: { data: null, isPending: false } }), + }), +})); + +function clearAllCookies(): void { + for (const part of document.cookie.split(';')) { + const name = part.split('=')[0]?.trim(); + if (name) { + document.cookie = `${name}=; path=/; max-age=0`; + } + } +} + +beforeEach(() => { + resetStubRuntimeConfig(); + resetStubReactiveStores(); + clearAllCookies(); +}); + +afterEach(() => { + clearAllCookies(); + resetStubRuntimeConfig(); + resetStubReactiveStores(); +}); + +describe('isAdmin — multi-role shape (nest-server: roles: string[])', () => { + it('is true for `roles: ["admin"]` WITHOUT a singular `role` (the nest-server user)', async () => { + const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); + const auth = useLtAuth(); + + // Exactly what a real nest-server / Better-Auth get-session returns: no `role`. + auth.setUser({ id: 'u1', email: 'admin@example.com', roles: ['admin'] }); + + expect(auth.isAdmin.value).toBe(true); + }); + + it('is true when `admin` sits among several roles', async () => { + const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); + const auth = useLtAuth(); + + auth.setUser({ id: 'u1', email: 'a@example.com', roles: ['user', 'admin', 'editor'] }); + + expect(auth.isAdmin.value).toBe(true); + }); + + it('is false for a non-admin roles array', async () => { + const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); + const auth = useLtAuth(); + + auth.setUser({ id: 'u1', email: 'user@example.com', roles: ['user'] }); + + expect(auth.isAdmin.value).toBe(false); + }); + + it('is false for an empty roles array (nest-server defaultValue)', async () => { + const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); + const auth = useLtAuth(); + + auth.setUser({ id: 'u1', email: 'user@example.com', roles: [] }); + + expect(auth.isAdmin.value).toBe(false); + }); +}); + +describe('isAdmin — single-role shape (Better-Auth admin plugin: role: string)', () => { + it('is true for `role: "admin"` (unchanged upstream behaviour)', async () => { + const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); + const auth = useLtAuth(); + + auth.setUser({ id: 'u1', email: 'admin@example.com', role: 'admin' }); + + expect(auth.isAdmin.value).toBe(true); + }); + + it('is false for a non-admin `role`', async () => { + const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); + const auth = useLtAuth(); + + auth.setUser({ id: 'u1', email: 'user@example.com', role: 'user' }); + + expect(auth.isAdmin.value).toBe(false); + }); +}); + +describe('isAdmin — absent / malformed user', () => { + it('is false when no user is set', async () => { + const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); + const auth = useLtAuth(); + + expect(auth.user.value).toBeNull(); + expect(auth.isAdmin.value).toBe(false); + }); + + it('is false when the user carries neither `role` nor `roles`', async () => { + const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); + const auth = useLtAuth(); + + auth.setUser({ id: 'u1', email: 'plain@example.com' }); + + expect(auth.isAdmin.value).toBe(false); + }); + + it('is false (and does not throw) for a malformed non-array `roles`', async () => { + const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); + const auth = useLtAuth(); + + // The auth-state cookie is client-writable, so `roles` can arrive as junk. + // The computed must degrade to `false`, never throw (`.includes` on a number). + auth.setUser({ id: 'u1', email: 'evil@example.com', roles: 42 } as never); + + expect(() => auth.isAdmin.value).not.toThrow(); + expect(auth.isAdmin.value).toBe(false); + }); +}); diff --git a/test/merge-session-user.test.ts b/test/merge-session-user.test.ts index 3c0868a..5b81605 100644 --- a/test/merge-session-user.test.ts +++ b/test/merge-session-user.test.ts @@ -197,6 +197,70 @@ describe('mergeSessionUser keeps authorization keys fail-closed (AUTHZ_KEYS)', ( expect(user.leadTableColumns).toEqual(['x']); }); + it('drops a stale `roles` array the session omits (fail-closed → admin UI closes)', async () => { + const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); + const auth = useLtAuth(); + + // Cached nest-server user with admin rights + a nest-server-only field. + auth.setUser({ + id: 'u1', + email: 'a@example.com', + roles: ['admin'], + leadTableColumns: ['x'], + } as never); + + // Backend revoked admin and no longer returns `roles` at all. Keeping the + // cached value would be fail-open: the admin UI would stay visible. + mockSession = { data: { user: { id: 'u1', email: 'a@example.com' } }, isPending: false }; + + const ok = await auth.validateSession(); + expect(ok).toBe(true); + + const user = auth.user.value as unknown as Record; + expect(user.roles).toBeUndefined(); + expect(auth.isAdmin.value).toBe(false); + // … while a non-authz nest-server-only field still survives the merge. + expect(user.leadTableColumns).toEqual(['x']); + }); + + it('lets the session downgrade `roles` when it is present (admin → user)', async () => { + const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); + const auth = useLtAuth(); + + auth.setUser({ id: 'u1', email: 'a@example.com', roles: ['admin'] } as never); + // The realistic nest-server downgrade: get-session still returns `roles`, + // just without 'admin' in it (worst case an empty array — its defaultValue). + mockSession = { + data: { user: { id: 'u1', email: 'a@example.com', roles: ['user'] } }, + isPending: false, + }; + + const ok = await auth.validateSession(); + expect(ok).toBe(true); + + const user = auth.user.value as unknown as Record; + expect(user.roles).toEqual(['user']); + expect(auth.isAdmin.value).toBe(false); + }); + + it('keeps `roles` when the session still grants admin (no spurious drop on re-validation)', async () => { + const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); + const auth = useLtAuth(); + + auth.setUser({ id: 'u1', email: 'a@example.com', roles: ['admin'] } as never); + mockSession = { + data: { user: { id: 'u1', email: 'a@example.com', roles: ['admin'] } }, + isPending: false, + }; + + const ok = await auth.validateSession(); + expect(ok).toBe(true); + + // The reload path must NOT cost an admin their admin UI. + expect(auth.isAdmin.value).toBe(true); + expect((auth.user.value as unknown as Record).roles).toEqual(['admin']); + }); + it('lets the session overwrite an authorization field when it is present', async () => { const { useLtAuth } = await import('../src/runtime/composables/auth/use-lt-auth'); const auth = useLtAuth();