diff --git a/api/pkg/validators/phone_handler_validator.go b/api/pkg/validators/phone_handler_validator.go index 9e21d26a9..9a539bd2f 100644 --- a/api/pkg/validators/phone_handler_validator.go +++ b/api/pkg/validators/phone_handler_validator.go @@ -74,7 +74,7 @@ func (validator *PhoneHandlerValidator) ValidateUpsert(ctx context.Context, user }, "messages_per_minute": []string{ "min:0", - "max:60", + "max:30", }, "max_send_attempts": []string{ "min:0", diff --git a/api/pkg/validators/phone_handler_validator_test.go b/api/pkg/validators/phone_handler_validator_test.go index c9dc72b63..cd28b9582 100644 --- a/api/pkg/validators/phone_handler_validator_test.go +++ b/api/pkg/validators/phone_handler_validator_test.go @@ -36,6 +36,37 @@ func TestPhoneHandlerValidatorAcceptsHTTPSNotificationURLOnUpsert(t *testing.T) assert.Empty(t, errors) } +func TestPhoneHandlerValidatorMessagesPerMinuteLimit(t *testing.T) { + tests := []struct { + name string + messagesPerMinute uint + valid bool + }{ + {name: "zero", messagesPerMinute: 0, valid: true}, + {name: "maximum", messagesPerMinute: 30, valid: true}, + {name: "above maximum", messagesPerMinute: 31, valid: false}, + {name: "previous maximum", messagesPerMinute: 60, valid: false}, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + validator := newPhoneHandlerValidator() + errors := validator.ValidateUpsert(context.Background(), "", requests.PhoneUpsert{ + PhoneNumber: "+18005550199", + SIM: entities.SIM1.String(), + MessageExpirationSeconds: 60, + MessagesPerMinute: test.messagesPerMinute, + }) + + if test.valid { + assert.Empty(t, errors) + } else { + assert.NotEmpty(t, errors["messages_per_minute"]) + } + }) + } +} + func TestPhoneHandlerValidatorAcceptsPrivateAndLoopbackNotificationHosts(t *testing.T) { validator := newPhoneHandlerValidator() diff --git a/tests/adapter_integration_test.go b/tests/adapter_integration_test.go index 70dbbe894..53e511b38 100644 --- a/tests/adapter_integration_test.go +++ b/tests/adapter_integration_test.go @@ -14,7 +14,7 @@ import ( func TestAdapterGatewayOutgoingMessage(t *testing.T) { ctx := context.Background() - phone := setupAdapterPhone(ctx, t, 60) + phone := setupAdapterPhone(ctx, t, 30) contact := randomPhoneNumber() content := "Adapter outgoing " + randomEncryptionKey() @@ -42,7 +42,7 @@ func TestAdapterGatewayOutgoingMessage(t *testing.T) { func TestAdapterGatewayIncomingMessage(t *testing.T) { ctx := context.Background() - phone := setupAdapterPhone(ctx, t, 60) + phone := setupAdapterPhone(ctx, t, 30) contact := randomPhoneNumber() content := "Adapter incoming " + randomEncryptionKey() @@ -57,7 +57,7 @@ func TestAdapterGatewayIncomingMessage(t *testing.T) { func TestAdapterGatewayHeartbeatWakeUp(t *testing.T) { ctx := context.Background() - phone := setupAdapterPhone(ctx, t, 60) + phone := setupAdapterPhone(ctx, t, 30) monitorID := uuid.NewString() dispatchInternalEvent(ctx, t, map[string]any{ diff --git a/tests/archive_new_outgoing_message_threads_integration_test.go b/tests/archive_new_outgoing_message_threads_integration_test.go index 850ee491e..dfd44553d 100644 --- a/tests/archive_new_outgoing_message_threads_integration_test.go +++ b/tests/archive_new_outgoing_message_threads_integration_test.go @@ -72,8 +72,8 @@ func TestArchiveNewOutgoingMessageThreadsOnFirstRegistration(t *testing.T) { func TestArchiveNewOutgoingMessageThreadsPerPhoneAndDirection(t *testing.T) { ctx := context.Background() - enabledPhone := setupPhone(ctx, t, 60) - defaultPhone := setupPhone(ctx, t, 60) + enabledPhone := setupPhone(ctx, t, 30) + defaultPhone := setupPhone(ctx, t, 30) require.NotEqual(t, enabledPhone.PhoneNumber, defaultPhone.PhoneNumber) existingContact := randomPhoneNumber() @@ -104,7 +104,7 @@ func TestArchiveNewOutgoingMessageThreadsPerPhoneAndDirection(t *testing.T) { func TestArchiveNewOutgoingMessageThreadsToggleAndPartialUpdates(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) require.True(t, updatePhoneThreadSettings(ctx, t, phone, map[string]bool{"archive_new_outgoing_message_threads": true}).ArchiveNewOutgoingMessageThreads) settings := updatePhoneThreadSettings(ctx, t, phone, map[string]bool{"unarchive_thread": true}) @@ -135,7 +135,7 @@ func TestArchiveNewOutgoingMessageThreadsReplyRespectsUnarchiveSetting(t *testin } { t.Run(test.name, func(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) updatePhoneThreadSettings(ctx, t, phone, map[string]bool{ "archive_new_outgoing_message_threads": true, "unarchive_thread": test.unarchive, diff --git a/tests/contacts_integration_test.go b/tests/contacts_integration_test.go index 2dcd4d5c0..a51863156 100644 --- a/tests/contacts_integration_test.go +++ b/tests/contacts_integration_test.go @@ -295,7 +295,7 @@ func TestContacts_CSVUpload(t *testing.T) { func TestContacts_MessageThreadsIncludeContactDetails(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) contactPhone := randomPhoneNumber() marker := strings.ToLower(uuid.New().String()) diff --git a/tests/integration_test.go b/tests/integration_test.go index 8b7436642..a5fdfa6dd 100644 --- a/tests/integration_test.go +++ b/tests/integration_test.go @@ -19,7 +19,7 @@ import ( func TestSendSMS_Encrypted(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) encryptionKey := randomEncryptionKey() signingKey, webhookPath := setupWebhook(ctx, t, phone.PhoneNumber, []string{ @@ -87,7 +87,7 @@ func TestSendSMS_Encrypted(t *testing.T) { func TestReceiveSMS_Encrypted(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) encryptionKey := randomEncryptionKey() signingKey, webhookPath := setupWebhook(ctx, t, phone.PhoneNumber, []string{ @@ -303,7 +303,7 @@ func TestRotateAPIKey_InvalidatesCache(t *testing.T) { func TestSendSMS_OutstandingFlow(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) signingKey, webhookPath := setupWebhook(ctx, t, phone.PhoneNumber, []string{ "message.phone.sent", @@ -352,7 +352,7 @@ func TestSendSMS_OutstandingFlow(t *testing.T) { func TestHeartbeat_StoreAndIndex(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) // Store a heartbeat via phone API key (retry to allow async phone-API-key association) storePayload := map[string]interface{}{ @@ -408,7 +408,7 @@ func TestHeartbeat_StoreAndIndex(t *testing.T) { func TestBulkSMS_CSV(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) // Build CSV content with 1 message contact := randomPhoneNumber() @@ -474,7 +474,7 @@ func TestBulkSMS_CSV(t *testing.T) { func TestBulkSMS_Excel(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) contact1 := randomPhoneNumber() contact2 := randomPhoneNumber() diff --git a/tests/read_receipts_test.go b/tests/read_receipts_test.go index 178c720de..2c7fac4b9 100644 --- a/tests/read_receipts_test.go +++ b/tests/read_receipts_test.go @@ -130,7 +130,7 @@ func clearMessageThreadUnreadCount(ctx context.Context, t *testing.T, threadID s func TestMessageThreadUnreadCount(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) contact := randomPhoneNumber() requestJSON( diff --git a/tests/unarchive_thread_integration_test.go b/tests/unarchive_thread_integration_test.go index c7fb3f55c..e85c0ad43 100644 --- a/tests/unarchive_thread_integration_test.go +++ b/tests/unarchive_thread_integration_test.go @@ -173,7 +173,7 @@ func archiveThread(ctx context.Context, t *testing.T, threadID string) { func TestUnarchiveThreadOnReceive_Enabled(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) setUnarchiveThread(ctx, t, phone.PhoneNumber, true) contact := randomPhoneNumber() @@ -205,7 +205,7 @@ func TestUnarchiveThreadOnReceive_Enabled(t *testing.T) { func TestUnarchiveThreadOnReceive_Disabled(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) // unarchive_thread defaults to false; do not enable it + phone := setupPhone(ctx, t, 30) // unarchive_thread defaults to false; do not enable it contact := randomPhoneNumber() content1 := "first inbound " + randomEncryptionKey() diff --git a/tests/unread_filter_integration_test.go b/tests/unread_filter_integration_test.go index 6366517b5..dce7042a1 100644 --- a/tests/unread_filter_integration_test.go +++ b/tests/unread_filter_integration_test.go @@ -31,8 +31,8 @@ func fetchFilteredThreads(ctx context.Context, t *testing.T, owner string, archi func TestMessageThreadUnreadFilter(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) - otherPhone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) + otherPhone := setupPhone(ctx, t, 30) require.NotEqual(t, phone.PhoneNumber, otherPhone.PhoneNumber) unreadContact := randomPhoneNumber() readContact := randomPhoneNumber() @@ -91,7 +91,7 @@ func TestMessageThreadUnreadFilterRejectsInvalidValue(t *testing.T) { func TestMessageThreadPaginationReturnsRequestedPages(t *testing.T) { ctx := context.Background() - phone := setupPhone(ctx, t, 60) + phone := setupPhone(ctx, t, 30) timestamp := time.Now().Add(-time.Minute).Truncate(time.Second) for index := 0; index < 20; index++ { receiveInbound(ctx, t, phone.PhoneAPIKey, fmt.Sprintf("+1800554%04d", index), phone.PhoneNumber, "Pagination thread", timestamp) diff --git a/web/app/pages/settings/index.vue b/web/app/pages/settings/index.vue index cd608ef1a..b4082691f 100644 --- a/web/app/pages/settings/index.vue +++ b/web/app/pages/settings/index.vue @@ -22,8 +22,8 @@ import { type User as FirebaseUser, } from 'firebase/auth' import QRCode from 'qrcode' -import { ErrorMessages } from '~/utils/errors' -import { getApiErrorMessage, toApiError } from '~/utils/api-error' +import { ErrorMessages, getErrorMessages } from '~/utils/errors' +import { getApiErrorMessage } from '~/utils/api-error' import type { EntitiesPhone, EntitiesWebhook, @@ -135,15 +135,6 @@ function resetErrors() { errorMessages.value = new ErrorMessages() } -function parseErrors(error: unknown): ErrorMessages { - const bag = new ErrorMessages() - const data = toApiError(error).data?.data - if (data && typeof data === 'object') { - Object.keys(data).forEach((key) => bag.addMany(key, data[key]!)) - } - return bag -} - // --------------------------------------------------------------------------- // API Key // --------------------------------------------------------------------------- @@ -295,7 +286,7 @@ async function saveWebhook() { showWebhookEdit.value = false await loadWebhooks() } catch (error: unknown) { - errorMessages.value = parseErrors(error) + errorMessages.value = getErrorMessages(error) if (errorMessages.value.size() === 0) { notificationsStore.addNotification({ message: getApiErrorMessage(error, 'Failed to save webhook'), @@ -403,7 +394,7 @@ async function saveDiscord() { showDiscordEdit.value = false await loadDiscordIntegrations() } catch (error: unknown) { - errorMessages.value = parseErrors(error) + errorMessages.value = getErrorMessages(error) if (errorMessages.value.size() === 0) { notificationsStore.addNotification({ message: getApiErrorMessage( @@ -446,6 +437,7 @@ async function deleteDiscord(id: string) { // --------------------------------------------------------------------------- const updatingPhone = ref(false) const showPhoneEdit = ref(false) +const showPhoneDelete = ref(false) const activePhone = ref(null) function showEditPhone(phoneId: string) { @@ -458,24 +450,29 @@ function showEditPhone(phoneId: string) { async function updatePhone() { if (!activePhone.value) return + resetErrors() updatingPhone.value = true try { await phonesStore.updatePhone(activePhone.value) showPhoneEdit.value = false activePhone.value = null + } catch (error: unknown) { + errorMessages.value = getErrorMessages(error) } finally { updatingPhone.value = false } } -async function deletePhone(phoneId: string) { +async function deletePhone() { + if (!activePhone.value || updatingPhone.value) return updatingPhone.value = true try { - await phonesStore.deletePhone(phoneId) + await phonesStore.deletePhone(activePhone.value.id) notificationsStore.addNotification({ message: 'Phone deleted successfully', type: 'success', }) + showPhoneDelete.value = false showPhoneEdit.value = false activePhone.value = null } catch (error: unknown) { @@ -646,9 +643,9 @@ function scheduleWindowError(index: number): string | null { ? errorMessages.value.get('windows') : [] if (messages.length === 0) return null - const message = messages.find((x) => x.includes(`day_of_week ${index}`)) + const message = messages.find((x) => x.includes(`Day of week ${index}`)) return message - ? message.replace(`day_of_week ${index}`, getWeekday(index)) + ? message.replace(`Day of week ${index}`, getWeekday(index)) : null } @@ -680,7 +677,7 @@ async function saveSchedule() { showScheduleEdit.value = false await loadSendSchedules() } catch (error: unknown) { - errorMessages.value = parseErrors(error) + errorMessages.value = getErrorMessages(error) if (errorMessages.value.size() === 0) { notificationsStore.addNotification({ type: 'error', @@ -1623,6 +1620,7 @@ onMounted(async () => { disabled label="ID" :model-value="activePhone.id" + :error-messages="errorMessages.get('id')" /> { density="compact" label="Phone Number" :model-value="activePhone.phone_number" + :error-messages="errorMessages.get('phone_number')" /> { density="compact" label="SIM" :model-value="activePhone.sim" + :error-messages="errorMessages.get('sim')" /> { density="compact" label="FCM Token" :model-value="activePhone.fcm_token" + :error-messages="errorMessages.get('fcm_token')" /> { density="compact" class="mt-6" label="Missed Call AutoReply" + :error-messages="errorMessages.get('missed_call_auto_reply')" persistent-placeholder persistent-hint placeholder="We are currently closed at the moment, please send us a text message from 09:00 to 17:00" @@ -1704,6 +1718,9 @@ onMounted(async () => { color="primary" density="compact" label="Archive New Outgoing Message Threads" + :error-messages=" + errorMessages.get('archive_new_outgoing_message_threads') + " hint="Automatically archive new conversations started by an outgoing message. Conversations started by an incoming message are not affected." persistent-hint /> @@ -1713,6 +1730,7 @@ onMounted(async () => { color="primary" density="compact" label="Unarchive Threads Automatically" + :error-messages="errorMessages.get('unarchive_thread')" hint="When a new message is received on an archived conversation, change it back to the unarchived state" persistent-hint /> @@ -1730,7 +1748,7 @@ onMounted(async () => { color="error" variant="text" :disabled="updatingPhone" - @click="deletePhone(activePhone?.id ?? '')" + @click="showPhoneDelete = true" > Delete @@ -1739,6 +1757,41 @@ onMounted(async () => { + + + Delete Phone + + Are you sure you want to delete the phone + {{ activePhone?.phone_number }}? This action cannot be undone. + + + + Delete + + + + Cancel + + + + + diff --git a/web/app/stores/auth.test.ts b/web/app/stores/auth.test.ts new file mode 100644 index 000000000..0016b41bb --- /dev/null +++ b/web/app/stores/auth.test.ts @@ -0,0 +1,197 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { createPinia, setActivePinia } from 'pinia' +import { ref } from 'vue' +import type { User as FirebaseUser } from 'firebase/auth' +import { useApi, setAuthHeader, setApiKey } from '../composables/useApi' +import { useAuthStore } from './auth' + +interface UserResponse { + data: { id: string; api_key: string } +} + +function deferred() { + let resolve!: (value: T) => void + const promise = new Promise((done) => { + resolve = done + }) + return { promise, resolve } +} + +function firebaseUser(id: string, token = Promise.resolve(`token-${id}`)) { + return { + uid: id, + email: null, + displayName: null, + getIdToken: () => token, + } as FirebaseUser +} + +function setupStore() { + const calls: Headers[] = [] + let response = Promise.resolve({ + data: { id: 'account-a', api_key: 'key-a' }, + }) + Object.assign(globalThis, { + ref, + useApi, + useRuntimeConfig: () => ({ + public: { apiBaseUrl: 'https://api.example.test' }, + }), + $fetch: { + create: + (config: { + headers: HeadersInit + onRequest: (context: { options: { headers: Headers } }) => void + }) => + async () => { + const options = { headers: new Headers(config.headers) } + config.onRequest({ options }) + calls.push(options.headers) + return response + }, + }, + usePhonesStore: () => ({ + activePhone: { id: 'phone-a' }, + loadPhones: async () => {}, + }), + }) + setAuthHeader(null) + setApiKey(null) + setActivePinia(createPinia()) + const store = useAuthStore() + return { + store, + calls, + respondWith: (next: Promise) => { + response = next + }, + } +} + +test('loading a user sends its API key alongside bearer auth on shared requests', async () => { + const { store, calls, respondWith } = setupStore() + const client = useApi().apiFetch + await store.onAuthStateChanged(firebaseUser('account-a')) + await store.loadUser() + await client('/v1/messages') + + assert.equal(calls.at(-1)?.get('Authorization'), 'Bearer token-account-a') + assert.equal(calls.at(-1)?.get('x-api-key'), 'key-a') + + respondWith( + Promise.resolve({ data: { id: 'account-a', api_key: 'new-key-a' } }), + ) + await store.loadUser() + await client('/v1/messages') + assert.equal(calls.at(-1)?.get('x-api-key'), 'new-key-a') +}) + +for (const logout of ['reset', 'auth callback', 'token callback'] as const) { + test(`a pending user load cannot restore credentials after ${logout}`, async () => { + const { store, calls, respondWith } = setupStore() + await store.onAuthStateChanged(firebaseUser('account-a')) + await store.loadUser() + const pending = deferred() + respondWith(pending.promise) + const load = store.loadUser() + + if (logout === 'reset') store.resetState() + if (logout === 'auth callback') await store.onAuthStateChanged(null) + if (logout === 'token callback') await store.onIdTokenChanged(null) + pending.resolve({ data: { id: 'account-a', api_key: 'old-key-a' } }) + await load + await useApi().apiFetch('/v1/messages') + + assert.equal(store.user, null) + assert.equal(store.authUser, null) + assert.equal(calls.at(-1)?.has('Authorization'), false) + assert.equal(calls.at(-1)?.has('x-api-key'), false) + }) +} + +for (const switchAccount of ['firebase', 'action'] as const) { + test(`switching accounts through ${switchAccount} clears old keys and discards pending loads`, async () => { + const { store, calls, respondWith } = setupStore() + await store.onAuthStateChanged(firebaseUser('account-a')) + await store.loadUser() + const pending = deferred() + respondWith(pending.promise) + const oldLoad = store.loadUser() + respondWith( + Promise.resolve({ data: { id: 'account-b', api_key: 'key-b' } }), + ) + + if (switchAccount === 'firebase') { + await store.onAuthStateChanged(firebaseUser('account-b')) + await store.loadUser() + } else { + await store.setAuthUserAction({ + id: 'account-b', + email: null, + displayName: null, + }) + } + assert.equal(calls.at(-1)?.has('x-api-key'), false) + pending.resolve({ data: { id: 'account-a', api_key: 'old-key-a' } }) + await oldLoad + await useApi().apiFetch('/v1/messages') + + assert.equal(store.user?.id, 'account-b') + assert.equal(calls.at(-1)?.get('x-api-key'), 'key-b') + }) +} + +test('logging back into the same account does not revive an earlier session response', async () => { + const { store, calls, respondWith } = setupStore() + await store.onAuthStateChanged(firebaseUser('account-a')) + const pending = deferred() + respondWith(pending.promise) + const oldLoad = store.loadUser() + store.resetState() + await store.onAuthStateChanged(firebaseUser('account-a')) + respondWith( + Promise.resolve({ data: { id: 'account-a', api_key: 'new-key-a' } }), + ) + await store.loadUser() + pending.resolve({ data: { id: 'account-a', api_key: 'old-key-a' } }) + await oldLoad + await useApi().apiFetch('/v1/messages') + + assert.equal(calls.at(-1)?.get('x-api-key'), 'new-key-a') +}) + +for (const operation of ['update', 'rotate'] as const) { + test(`a pending ${operation} cannot restore credentials after logout`, async () => { + const { store, calls, respondWith } = setupStore() + await store.onAuthStateChanged(firebaseUser('account-a')) + const pending = deferred() + respondWith(pending.promise) + const request = + operation === 'update' + ? store.updateUser({ timezone: 'UTC' }) + : store.rotateApiKey('account-a') + store.resetState() + pending.resolve({ data: { id: 'account-a', api_key: 'old-key-a' } }) + await request + await useApi().apiFetch('/v1/messages') + + assert.equal(store.user, null) + assert.equal(calls.at(-1)?.has('x-api-key'), false) + }) +} + +test('a pending Firebase token cannot restore credentials after logout', async () => { + const { store, calls } = setupStore() + const token = deferred() + const login = store.onAuthStateChanged( + firebaseUser('account-a', token.promise), + ) + store.resetState() + token.resolve('old-token-a') + await login + await useApi().apiFetch('/v1/messages') + + assert.equal(store.authUser, null) + assert.equal(calls.at(-1)?.has('Authorization'), false) +}) diff --git a/web/app/stores/auth.ts b/web/app/stores/auth.ts index 53c0546d0..9ae9ca7b7 100644 --- a/web/app/stores/auth.ts +++ b/web/app/stores/auth.ts @@ -1,6 +1,6 @@ import { defineStore } from 'pinia' import type { User as FirebaseUser } from 'firebase/auth' -import { setAuthHeader, setApiKey } from '~/composables/useApi' +import { setAuthHeader, setApiKey } from '../composables/useApi' import type { EntitiesUser } from '~~/shared/types/api' export interface AuthUser { @@ -14,45 +14,55 @@ export const useAuthStore = defineStore('auth', () => { const authUser = ref(null) const user = ref(null) const { apiFetch } = useApi() + let sessionVersion = 0 + + function clearCredentials() { + sessionVersion++ + user.value = null + setAuthHeader(null) + setApiKey(null) + } async function setAuthUserAction(newUser: AuthUser | null | undefined) { const userChanged = newUser?.id !== authUser.value?.id + if (userChanged) clearCredentials() authUser.value = newUser ?? null authStateChanged.value = true - if (userChanged && newUser !== null) { + if (userChanged && newUser) { await Promise.all([loadUser(), loadPhones()]) } } async function onAuthStateChanged(firebaseUser: FirebaseUser | null) { if (firebaseUser == null) { - authUser.value = null - user.value = null - authStateChanged.value = true - setApiKey('') + resetState() return } - setAuthHeader(await firebaseUser.getIdToken()) + if (authUser.value?.id !== firebaseUser.uid) clearCredentials() + const session = sessionVersion + const token = await firebaseUser.getIdToken() + if (session !== sessionVersion) return + setAuthHeader(token) const { uid, email, displayName } = firebaseUser authUser.value = { id: uid, email, displayName } authStateChanged.value = true } async function onIdTokenChanged(firebaseUser: FirebaseUser | null) { - if (firebaseUser == null) { - setApiKey('') - return - } - setAuthHeader(await firebaseUser.getIdToken()) + await onAuthStateChanged(firebaseUser) } async function loadUser() { + const session = sessionVersion const response = await apiFetch<{ data: EntitiesUser }>('/v1/users/me') + if (session !== sessionVersion) return + setApiKey(response.data.api_key) user.value = response.data } async function updateUser(payload: { owner?: string; timezone?: string }) { + const session = sessionVersion const phonesStore = usePhonesStore() if (payload.owner) { phonesStore.setOwner(payload.owner) @@ -69,6 +79,7 @@ export const useAuthStore = defineStore('auth', () => { }, }) + if (session !== sessionVersion) return setApiKey(response.data.api_key) user.value = response.data } @@ -81,22 +92,24 @@ export const useAuthStore = defineStore('auth', () => { } async function rotateApiKey(userId: string): Promise { + const session = sessionVersion const response = await apiFetch<{ data: EntitiesUser }>( `/v1/users/${userId}/api-keys`, { method: 'DELETE', }, ) - user.value = response.data - setApiKey(response.data.api_key) + if (session === sessionVersion) { + user.value = response.data + setApiKey(response.data.api_key) + } return response.data } function resetState() { - user.value = null + clearCredentials() authUser.value = null authStateChanged.value = true - setApiKey('') } function loadPhones() { diff --git a/web/app/utils/errors.test.ts b/web/app/utils/errors.test.ts new file mode 100644 index 000000000..ddf729fca --- /dev/null +++ b/web/app/utils/errors.test.ts @@ -0,0 +1,43 @@ +import assert from 'node:assert/strict' +import { test } from 'node:test' +import { getErrorMessages } from './errors' + +const validationData = { + messages_per_minute: [ + 'The messages_per_minute field value can not be greater than 30', + ], +} + +test('formats fetch validation errors while retaining field keys', () => { + const errors = getErrorMessages({ + status: 422, + data: { data: validationData }, + response: new Response(null, { status: 422 }), + }) + + assert.deepEqual(errors.get('messages_per_minute'), [ + 'The messages per minute value can not be greater than 30', + ]) +}) + +test('still formats Axios validation errors', () => { + const errors = getErrorMessages({ + response: { status: 422, data: { data: validationData } }, + }) + + assert.deepEqual(errors.get('messages_per_minute'), [ + 'The messages per minute value can not be greater than 30', + ]) +}) + +test('does not turn non-validation failures into field errors', () => { + for (const error of [ + null, + undefined, + new Error('Network error'), + { status: 500, data: { data: validationData } }, + { status: 422, data: { message: 'Validation failed' } }, + ]) { + assert.equal(getErrorMessages(error).size(), 0) + } +}) diff --git a/web/app/utils/errors.ts b/web/app/utils/errors.ts index 38bc5f942..e649a97a4 100644 --- a/web/app/utils/errors.ts +++ b/web/app/utils/errors.ts @@ -1,5 +1,6 @@ -import Bag from '~/utils/bag' -import { capitalize } from '~/utils/capitalize' +import Bag from './bag' +import { capitalize } from './capitalize' +import { toApiError } from './api-error' export class ErrorMessages extends Bag {} @@ -21,25 +22,17 @@ const sanitize = (key: string, values: Array): Array => { }) } -interface AxiosLikeError { - response?: { - data?: { data?: Record } - status?: number - } -} - -export const getErrorMessages = (error: AxiosLikeError): ErrorMessages => { +export const getErrorMessages = (error: unknown): ErrorMessages => { const errors = new ErrorMessages() - if ( - error === null || - typeof error.response?.data?.data !== 'object' || - error.response?.data?.data === null || - error.response?.status !== 422 - ) { + const apiError = toApiError(error) + const response = 'response' in apiError ? toApiError(apiError.response) : {} + const data = apiError.data?.data ?? response.data?.data + const status = apiError.status ?? response.status + if (typeof data !== 'object' || data === null || status !== 422) { return errors } - Object.entries(error.response.data.data).forEach(([key, values]) => { + Object.entries(data).forEach(([key, values]) => { errors.addMany(key, sanitize(key, values)) })