From 84149d69b0a80098497d5856c26679df17318259 Mon Sep 17 00:00:00 2001 From: Edward Sammut Alessi Date: Wed, 25 Mar 2026 14:07:20 +0100 Subject: [PATCH] fix(frontend): prevent invalid auth state - Fix some frontend login race conditions related to vue state not flushing before redirects happen, resulting in stale identity data being loaded into local storage - Extract local storage hooks out of the useIdentity hook so as to have a single shared ref for all of them, preventing accidental overwrites from race conditions when waiting for storage events to update other listeners - Update public key confirmation logic to cater for a situation where an auth0 login was required, but keys were still saved before being confirmed. Signed-off-by: Edward Sammut Alessi --- frontend/e2e/talemu/auth.spec.ts | 16 ++++ .../components/common/UserInfo/UserInfo.vue | 2 +- frontend/src/methods/auth.spec.ts | 3 +- frontend/src/methods/auth.ts | 24 +++-- frontend/src/methods/identity.spec.ts | 12 +-- frontend/src/methods/identity.ts | 21 +++-- frontend/src/methods/interceptor.spec.ts | 4 +- frontend/src/methods/interceptor.ts | 92 ++++++++----------- frontend/src/methods/key.spec.ts | 4 +- frontend/src/methods/key.ts | 21 +++-- frontend/src/views/omni/Auth/Authenticate.vue | 13 +-- 11 files changed, 117 insertions(+), 95 deletions(-) create mode 100644 frontend/e2e/talemu/auth.spec.ts diff --git a/frontend/e2e/talemu/auth.spec.ts b/frontend/e2e/talemu/auth.spec.ts new file mode 100644 index 00000000..a5254a2b --- /dev/null +++ b/frontend/e2e/talemu/auth.spec.ts @@ -0,0 +1,16 @@ +// Copyright (c) 2026 Sidero Labs, Inc. +// +// Use of this software is governed by the Business Source License +// included in the LICENSE file. +import { expect, test } from '../auth_fixtures' + +test.describe.configure({ mode: 'parallel' }) + +test('Logout', async ({ page }) => { + await page.goto('/') + + await page.getByRole('button', { name: 'user actions' }).click() + await page.getByRole('menuitem', { name: 'Log Out' }).click() + + await expect(page.getByText('Log in')).toBeVisible() +}) diff --git a/frontend/src/components/common/UserInfo/UserInfo.vue b/frontend/src/components/common/UserInfo/UserInfo.vue index 8c1d8198..7a5f871d 100644 --- a/frontend/src/components/common/UserInfo/UserInfo.vue +++ b/frontend/src/components/common/UserInfo/UserInfo.vue @@ -52,7 +52,7 @@ const name = computed(() => fullname || auth0?.user?.value?.name) {{ identity }}
- + Log Out
diff --git a/frontend/src/methods/auth.spec.ts b/frontend/src/methods/auth.spec.ts index 4cb66c45..f4e983f8 100644 --- a/frontend/src/methods/auth.spec.ts +++ b/frontend/src/methods/auth.spec.ts @@ -6,6 +6,7 @@ import { useAuth0 } from '@auth0/auth0-vue' import { afterEach, beforeEach, describe, expect, test, vi } from 'vitest' import { ref } from 'vue' +import { RequestError } from '@/api/fetch.pb' import { Code } from '@/api/google/rpc/code.pb' import { AuthService } from '@/api/omni/auth/auth.pb' import { AuthType, authType } from '@/methods' @@ -117,7 +118,7 @@ describe('useLogout', () => { }) test('should not throw when RevokePublicKey fails with UNAUTHENTICATED error', async () => { - const error = new Error('Unauthenticated') as Error & { code: Code } + const error = new RequestError('Unauthenticated') error.code = Code.UNAUTHENTICATED vi.mocked(AuthService.RevokePublicKey).mockRejectedValue(error) diff --git a/frontend/src/methods/auth.ts b/frontend/src/methods/auth.ts index 19ec8e59..a834f2ec 100644 --- a/frontend/src/methods/auth.ts +++ b/frontend/src/methods/auth.ts @@ -5,9 +5,10 @@ import { useAuth0 } from '@auth0/auth0-vue' import type { ComputedRef, Ref } from 'vue' -import { computed, onBeforeMount, ref, watch } from 'vue' +import { computed, nextTick, onBeforeMount, ref, watch } from 'vue' import { Runtime } from '@/api/common/omni.pb' +import { RequestError } from '@/api/fetch.pb' import { Code } from '@/api/google/rpc/code.pb' import type { Resource } from '@/api/grpc' import { ResourceService } from '@/api/grpc' @@ -256,24 +257,27 @@ export function useLogout() { if (keys.publicKeyID.value) { try { await AuthService.RevokePublicKey({ public_key_id: keys.publicKeyID.value }) - } catch (error) { + } catch (e) { // During a log out action being unauthenticated is fine - if (error.code !== Code.UNAUTHENTICATED) throw error + if (!(e instanceof RequestError) || e.code !== Code.UNAUTHENTICATED) throw e } } - await auth0?.logout({ - logoutParams: { - returnTo: window.location.origin, - }, - }) - keys.clear() identity.clear() currentUser.value = undefined - if (authType.value !== AuthType.Auth0) { + // Wait for storages to be set + await nextTick() + + if (auth0) { + await auth0.logout({ + logoutParams: { + returnTo: window.location.origin, + }, + }) + } else { redirectToURL(`/logout?${AuthFlowQueryParam}=${FrontendAuthFlow}`) } } diff --git a/frontend/src/methods/identity.spec.ts b/frontend/src/methods/identity.spec.ts index 1aa62d47..ff343d59 100644 --- a/frontend/src/methods/identity.spec.ts +++ b/frontend/src/methods/identity.spec.ts @@ -11,9 +11,9 @@ describe('useIdentity', () => { test('defaults to null', async () => { const { identity, avatar, fullname } = useIdentity() - expect(identity.value).toBeNull() - expect(avatar.value).toBeNull() - expect(fullname.value).toBeNull() + expect(identity.value).toBe('') + expect(avatar.value).toBe('') + expect(fullname.value).toBe('') }) test('sets values', async () => { @@ -37,9 +37,9 @@ describe('useIdentity', () => { clear() - expect(identity.value).toBeNull() - expect(avatar.value).toBeNull() - expect(fullname.value).toBeNull() + expect(identity.value).toBe('') + expect(avatar.value).toBe('') + expect(fullname.value).toBe('') }) test('persists values', async () => { diff --git a/frontend/src/methods/identity.ts b/frontend/src/methods/identity.ts index 0db9bd3f..ab156d85 100644 --- a/frontend/src/methods/identity.ts +++ b/frontend/src/methods/identity.ts @@ -2,21 +2,26 @@ // // Use of this software is governed by the Business Source License // included in the LICENSE file. -import { useLocalStorage } from '@vueuse/core' +import { StorageSerializers, useLocalStorage, type UseStorageOptions } from '@vueuse/core' + +const storageOptions: UseStorageOptions = { + serializer: StorageSerializers.string, + writeDefaults: false, +} + +const identityRef = useLocalStorage('identity', '', storageOptions) +const fullnameRef = useLocalStorage('fullname', '', storageOptions) +const avatarRef = useLocalStorage('avatar', '', storageOptions) export function useIdentity() { - const identityRef = useLocalStorage('identity', null) - const fullnameRef = useLocalStorage('fullname', null) - const avatarRef = useLocalStorage('avatar', null) - return { identity: identityRef, fullname: fullnameRef, avatar: avatarRef, clear() { - identityRef.value = null - fullnameRef.value = null - avatarRef.value = null + identityRef.value = '' + fullnameRef.value = '' + avatarRef.value = '' }, } } diff --git a/frontend/src/methods/interceptor.spec.ts b/frontend/src/methods/interceptor.spec.ts index 240cdc42..f20f9874 100644 --- a/frontend/src/methods/interceptor.spec.ts +++ b/frontend/src/methods/interceptor.spec.ts @@ -23,7 +23,7 @@ vi.mock(import('@/methods/key'), async (importOriginal) => { useKeys: vi.fn(() => ({ keyPair: ref(mockKey), publicKeyID: ref('public_key_id'), - keyExpirationTime: ref(null), + keyExpirationTime: ref(new Date(0)), clear: vi.fn(), })), signDetached: vi.fn().mockResolvedValue(new ArrayBuffer(10)), @@ -193,7 +193,7 @@ describe('useRegisterAPIInterceptor', () => { vi.mocked(useKeys).mockReturnValue({ keyPair: keyPairRef, publicKeyID: ref('public_key_id'), - keyExpirationTime: ref(null), + keyExpirationTime: ref(new Date(0)), clear: vi.fn(), }) diff --git a/frontend/src/methods/interceptor.ts b/frontend/src/methods/interceptor.ts index 76aa88ac..21e9f86b 100644 --- a/frontend/src/methods/interceptor.ts +++ b/frontend/src/methods/interceptor.ts @@ -2,9 +2,10 @@ // // Use of this software is governed by the Business Source License // included in the LICENSE file. +import { until } from '@vueuse/core' import { getUnixTime } from 'date-fns' import fetchIntercept from 'fetch-intercept' -import { onBeforeMount, onUnmounted, ref, watch } from 'vue' +import { onScopeDispose } from 'vue' import { b64Encode } from '@/api/fetch.pb' import { @@ -25,63 +26,50 @@ export function useRegisterAPIInterceptor() { const { keyPair, publicKeyID } = useKeys() const { identity } = useIdentity() - const unregisterInterceptor = ref<() => void>() - - onBeforeMount(() => { - unregisterInterceptor.value = fetchIntercept.register({ - async request(url, config?: { headers?: Headers; method?: string }) { - url = encodeURI(url) - - if ( - !/^\/(api|image)/.test(url) || - (url.startsWith('/api/auth.') && !url.startsWith('/api/auth.AuthService/RevokePublicKey')) - ) { - return [url, config] - } - - config ||= {} - config.headers ||= new Headers() - - const ts = getUnixTime(Date.now()).toString() - - if (url.startsWith('/api')) { - config.headers.set(`Grpc-Metadata-${TimestampHeaderKey}`, ts) - - const payload = JSON.stringify(buildPayload(url, config)) - const signature = await generateSignatureHeader(payload) - - config.headers.set(`Grpc-Metadata-${PayloadHeaderKey}`, payload) - config.headers.set(`Grpc-Metadata-${SignatureHeaderKey}`, signature) - } else if (url.startsWith('/image')) { - config.headers.set(TimestampHeaderKey, ts) - - const sha256 = 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' // empty string sha256 - const payload = [config.method ?? 'GET', url, ts, sha256].join('\n') - const signature = await generateSignatureHeader(payload) - - config.headers.set(SignatureHeaderKey, signature) - } + const unregister = fetchIntercept.register({ + async request(url, config?: { headers?: Headers; method?: string }) { + url = encodeURI(url) + if ( + !/^\/(api|image)/.test(url) || + (url.startsWith('/api/auth.') && !url.startsWith('/api/auth.AuthService/RevokePublicKey')) + ) { return [url, config] - }, - }) + } + + config ||= {} + config.headers ||= new Headers() + + const ts = getUnixTime(Date.now()).toString() + + if (url.startsWith('/api')) { + config.headers.set(`Grpc-Metadata-${TimestampHeaderKey}`, ts) + + const payload = JSON.stringify(buildPayload(url, config)) + const signature = await generateSignatureHeader(payload) + + config.headers.set(`Grpc-Metadata-${PayloadHeaderKey}`, payload) + config.headers.set(`Grpc-Metadata-${SignatureHeaderKey}`, signature) + } else if (url.startsWith('/image')) { + config.headers.set(TimestampHeaderKey, ts) + + const sha256 = 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855' // empty string sha256 + const payload = [config.method ?? 'GET', url, ts, sha256].join('\n') + const signature = await generateSignatureHeader(payload) + + config.headers.set(SignatureHeaderKey, signature) + } + + return [url, config] + }, }) + onScopeDispose(unregister) + async function generateSignatureHeader(payload: string) { if (!keyPair.value) { // Wait for keys to be created. - await new Promise((resolve) => { - const handle = watch( - keyPair, - (keyPair) => { - if (!keyPair) return - - handle.stop() - resolve() - }, - { immediate: true }, - ) - }) + await until(keyPair).toBeTruthy() } const array = new Uint8Array(await signDetached(payload, keyPair.value!)) @@ -90,8 +78,6 @@ export function useRegisterAPIInterceptor() { return `${SignatureVersionV1} ${identity.value} ${fingerprint} ${signature}` } - - onUnmounted(() => unregisterInterceptor.value?.()) } const includedHeaders = [ diff --git a/frontend/src/methods/key.spec.ts b/frontend/src/methods/key.spec.ts index 190cae55..f9051e2a 100644 --- a/frontend/src/methods/key.spec.ts +++ b/frontend/src/methods/key.spec.ts @@ -68,7 +68,7 @@ describe('useKeys', () => { const { keyPair, keyExpirationTime, publicKeyID } = useKeys() expect(keyPair.value).toBeFalsy() - expect(keyExpirationTime.value).toBeFalsy() + expect(keyExpirationTime.value).toEqual(new Date(0)) expect(publicKeyID.value).toBeFalsy() }) @@ -82,7 +82,7 @@ describe('useKeys', () => { clear() expect(keyPair.value).toBeFalsy() - expect(keyExpirationTime.value).toBeFalsy() + expect(keyExpirationTime.value).toEqual(new Date(0)) expect(publicKeyID.value).toBeFalsy() }) diff --git a/frontend/src/methods/key.ts b/frontend/src/methods/key.ts index 024560a5..388e013f 100644 --- a/frontend/src/methods/key.ts +++ b/frontend/src/methods/key.ts @@ -2,7 +2,7 @@ // // Use of this software is governed by the Business Source License // included in the LICENSE file. -import { useLocalStorage } from '@vueuse/core' +import { StorageSerializers, until, useLocalStorage } from '@vueuse/core' import { useIDBKeyval } from '@vueuse/integrations/useIDBKeyval' import { add, differenceInMilliseconds, formatRFC3339, isAfter } from 'date-fns' import { watchEffect } from 'vue' @@ -15,9 +15,18 @@ import { AuthFlowQueryParam, FrontendAuthFlow, RedirectQueryParam } from '@/api/ const { data: keyPair, isFinished: keyPairLoaded } = useIDBKeyval( 'keyPair', null, + { writeDefaults: false }, ) -const keyExpirationTime = useLocalStorage('keyExpirationTime', null) -const publicKeyID = useLocalStorage('publicKeyID', null) + +const keyExpirationTime = useLocalStorage('keyExpirationTime', new Date(0), { + serializer: StorageSerializers.date, + writeDefaults: false, +}) + +const publicKeyID = useLocalStorage('publicKeyID', '', { + serializer: StorageSerializers.string, + writeDefaults: false, +}) export function useKeys() { return { @@ -26,8 +35,8 @@ export function useKeys() { publicKeyID, clear() { keyPair.value = null - keyExpirationTime.value = null - publicKeyID.value = null + keyExpirationTime.value = new Date(0) + publicKeyID.value = '' }, } } @@ -96,7 +105,7 @@ export async function signDetached(data: string, keyPair: CryptoKeyPair) { export async function hasValidKeys() { // IndexedDB is async storage, and might not yet have been initialised - if (!keyPairLoaded.value) return new Promise((r) => setTimeout(() => r(hasValidKeys()), 20)) + if (!keyPairLoaded.value) await until(keyPairLoaded).toBe(true) if (!keyPair.value || !keyExpirationTime.value) return false diff --git a/frontend/src/views/omni/Auth/Authenticate.vue b/frontend/src/views/omni/Auth/Authenticate.vue index d434d0fb..b4426a83 100644 --- a/frontend/src/views/omni/Auth/Authenticate.vue +++ b/frontend/src/views/omni/Auth/Authenticate.vue @@ -9,7 +9,7 @@ import type { User } from '@auth0/auth0-spa-js' import type { Auth0VueClient } from '@auth0/auth0-vue' import { useAuth0 } from '@auth0/auth0-vue' import { jwtDecode } from 'jwt-decode' -import { computed, onMounted, ref, watch } from 'vue' +import { computed, nextTick, onMounted, ref, watch } from 'vue' import { useRoute, useRouter } from 'vue-router' import { b64Encode, type fetchOption, RequestError } from '@/api/fetch.pb' @@ -153,9 +153,9 @@ const generatePublicKey = async (identity: string) => { return } - try { - await confirmPublicKey(res.publicKeyId, res.keyPair) - } catch { + await confirmPublicKey(res.publicKeyId, res.keyPair) + + if (!confirmed.value) { keysGenerating.value = false return } @@ -168,6 +168,9 @@ const generatePublicKey = async (identity: string) => { identityStorage.fullname.value = name.value ?? '' identityStorage.avatar.value = picture.value ?? '' + // Wait for storages to be set + await nextTick() + const redirect = route.query[RedirectQueryParam]?.toString() if (!redirect) { @@ -225,8 +228,6 @@ const confirmPublicKey = async (publicKeyId: string, keyPair?: CryptoKeyPair) => } showError('Failed to confirm public key', e.message) - - throw e } }