From 37ca62b66aadb2cb80daa280a25a390c0bc2e4af Mon Sep 17 00:00:00 2001 From: "Eli Kent [SSW]" <69125238+kulesy@users.noreply.github.com> Date: Tue, 18 Aug 2026 17:28:42 +1000 Subject: [PATCH] fix(tinacms): skip TinaCloud identity requests when logged out (#7445) --- .changeset/lazy-donkeys-shake.md | 5 + .changeset/tall-jars-smile.md | 5 + .../components/GetCollection.hooks.test.tsx | 108 ++++++++++++++++++ .../src/admin/components/GetCollection.tsx | 56 +++++---- .../src/admin/components/GetDocument.test.tsx | 62 ++++++++++ .../src/admin/components/GetDocument.tsx | 28 ++--- .../tinacms/src/auth/TinaCloudProvider.tsx | 13 ++- .../src/internalClient/authProvider.test.ts | 104 +++++++++++++++++ .../src/internalClient/authProvider.ts | 23 +++- 9 files changed, 362 insertions(+), 42 deletions(-) create mode 100644 .changeset/lazy-donkeys-shake.md create mode 100644 .changeset/tall-jars-smile.md create mode 100644 packages/tinacms/src/admin/components/GetCollection.hooks.test.tsx create mode 100644 packages/tinacms/src/admin/components/GetDocument.test.tsx diff --git a/.changeset/lazy-donkeys-shake.md b/.changeset/lazy-donkeys-shake.md new file mode 100644 index 0000000000..58bb3c9287 --- /dev/null +++ b/.changeset/lazy-donkeys-shake.md @@ -0,0 +1,5 @@ +--- +'tinacms': patch +--- + +Skip TinaCloud identity requests when no auth token is stored. Logged-out admin loads no longer produce misleading 401/CORS console errors; a clear console message now points at the login popup console instead. Also fixes an unawaited auth guard in GetDocument, stops the document view from loading forever when that guard rejects the request, and fixes an unhandled promise rejection when the project settings request fails. diff --git a/.changeset/tall-jars-smile.md b/.changeset/tall-jars-smile.md new file mode 100644 index 0000000000..35f40a3702 --- /dev/null +++ b/.changeset/tall-jars-smile.md @@ -0,0 +1,5 @@ +--- +'tinacms': patch +--- + +Fix the collection list and collection search hanging on the loading screen when the session check says the user is not signed in. Both now settle and render instead of spinning until the page is reloaded. diff --git a/packages/tinacms/src/admin/components/GetCollection.hooks.test.tsx b/packages/tinacms/src/admin/components/GetCollection.hooks.test.tsx new file mode 100644 index 0000000000..157506464e --- /dev/null +++ b/packages/tinacms/src/admin/components/GetCollection.hooks.test.tsx @@ -0,0 +1,108 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import type { TinaCMS } from '@tinacms/toolkit'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { TinaAdminApi } from '../api'; +import { useGetCollection, useSearchCollection } from './GetCollection'; + +const loadedFolder = { loading: false, fullyQualifiedName: '' }; + +const buildCms = (isAuthenticated: boolean, searchResults = []) => + ({ + api: { + tina: { + schema: { getCollection: () => ({ name: 'post', fields: [] }) }, + authProvider: { + isAuthenticated: vi.fn().mockResolvedValue(isAuthenticated), + }, + }, + search: { + supportsClientSideIndexing: () => false, + query: vi.fn().mockResolvedValue({ results: searchResults }), + }, + }, + alerts: { error: vi.fn() }, + }) as unknown as TinaCMS; + +describe('useGetCollection', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('fetches the collection when authenticated', async () => { + const fetchCollection = vi + .spyOn(TinaAdminApi.prototype, 'fetchCollection') + .mockResolvedValue({ name: 'post' }); + + const cms = buildCms(true); + + const { result } = renderHook(() => + useGetCollection(cms, 'post', true, loadedFolder) + ); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(fetchCollection).toHaveBeenCalled(); + expect(result.current.collection).toEqual({ name: 'post' }); + }); + + it('skips the request and stops loading when not authenticated', async () => { + const fetchCollection = vi.spyOn(TinaAdminApi.prototype, 'fetchCollection'); + + const cms = buildCms(false); + + const { result } = renderHook(() => + useGetCollection(cms, 'post', true, loadedFolder) + ); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(fetchCollection).not.toHaveBeenCalled(); + expect(result.current.collection).toBeUndefined(); + }); + + it('keeps loading while the folder is still resolving', async () => { + const fetchCollection = vi.spyOn(TinaAdminApi.prototype, 'fetchCollection'); + + const cms = buildCms(true); + const pendingFolder = { loading: true, fullyQualifiedName: '' }; + + const { result } = renderHook(() => + useGetCollection(cms, 'post', true, pendingFolder) + ); + + await waitFor(() => expect(fetchCollection).not.toHaveBeenCalled()); + expect(result.current.loading).toBe(true); + }); + + it('surfaces a fetch failure instead of loading forever', async () => { + vi.spyOn(TinaAdminApi.prototype, 'fetchCollection').mockRejectedValue( + new Error('boom') + ); + vi.spyOn(console, 'error').mockImplementation(() => {}); + + const cms = buildCms(true); + + const { result } = renderHook(() => + useGetCollection(cms, 'post', true, loadedFolder) + ); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.error).toEqual(new Error('boom')); + }); +}); + +describe('useSearchCollection', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('skips the search and stops loading when not authenticated', async () => { + const cms = buildCms(false); + + const { result } = renderHook(() => + useSearchCollection(cms, 'post', true, loadedFolder, '', 'hello') + ); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(cms.api.search.query).not.toHaveBeenCalled(); + expect(result.current.collection).toBeUndefined(); + }); +}); diff --git a/packages/tinacms/src/admin/components/GetCollection.tsx b/packages/tinacms/src/admin/components/GetCollection.tsx index 855a50a136..69c2a8b26a 100644 --- a/packages/tinacms/src/admin/components/GetCollection.tsx +++ b/packages/tinacms/src/admin/components/GetCollection.tsx @@ -72,12 +72,16 @@ export const useGetCollection = ( let cancelled = false; const fetchCollection = async () => { - if ((await api.isAuthenticated()) && !folder.loading && !cancelled) { - const { name, order } = JSON.parse(sortKey || '{}'); - const validSortKey = isValidSortKey(name, collectionExtra) - ? name - : undefined; - try { + // the effect re-runs once the folder resolves + if (folder.loading) { + return; + } + try { + if ((await api.isAuthenticated()) && !cancelled) { + const { name, order } = JSON.parse(sortKey || '{}'); + const validSortKey = isValidSortKey(name, collectionExtra) + ? name + : undefined; const collection = await api.fetchCollection( collectionName, includeDocuments, @@ -88,15 +92,17 @@ export const useGetCollection = ( filterArgs ); setCollection(collection); - } catch (error) { - cms.alerts.error( - `[${error.name}] GetCollection failed: ${error.message}` - ); - console.error(error); - setCollection(undefined); - setError(error); } + } catch (error) { + cms.alerts.error( + `[${error.name}] GetCollection failed: ${error.message}` + ); + console.error(error); + setCollection(undefined); + setError(error); + } + if (!cancelled) { setLoading(false); } }; @@ -152,8 +158,12 @@ export const useSearchCollection = ( let cancelled = false; const searchCollection = async () => { - if ((await api.isAuthenticated()) && !folder.loading && !cancelled) { - try { + // the effect re-runs once the folder resolves + if (folder.loading) { + return; + } + try { + if ((await api.isAuthenticated()) && !cancelled) { const response = (await cms.api.search.query(search, { limit: 15, cursor: after, @@ -203,15 +213,17 @@ export const useSearchCollection = ( }; setCollection(collectionData); - } catch (error) { - cms.alerts.error( - `[${error.name}] GetCollection failed: ${error.message}` - ); - console.error(error); - setCollection(undefined); - setError(error); } + } catch (error) { + cms.alerts.error( + `[${error.name}] GetCollection failed: ${error.message}` + ); + console.error(error); + setCollection(undefined); + setError(error); + } + if (!cancelled) { setLoading(false); } }; diff --git a/packages/tinacms/src/admin/components/GetDocument.test.tsx b/packages/tinacms/src/admin/components/GetDocument.test.tsx new file mode 100644 index 0000000000..99aec5f3eb --- /dev/null +++ b/packages/tinacms/src/admin/components/GetDocument.test.tsx @@ -0,0 +1,62 @@ +import { renderHook, waitFor } from '@testing-library/react'; +import type { TinaCMS } from '@tinacms/toolkit'; +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { TinaAdminApi } from '../api'; +import { useGetDocument } from './GetDocument'; + +const buildCms = (isAuthenticated: boolean) => + ({ + api: { + tina: { + schema: {}, + authProvider: { + isAuthenticated: vi.fn().mockResolvedValue(isAuthenticated), + }, + }, + }, + alerts: { error: vi.fn() }, + }) as unknown as TinaCMS; + +const renderGetDocument = (cms: TinaCMS) => + renderHook(() => useGetDocument(cms, 'post', 'hello.mdx')); + +describe('useGetDocument', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it('fetches the document when authenticated', async () => { + const fetchDocument = vi + .spyOn(TinaAdminApi.prototype, 'fetchDocument') + .mockResolvedValue({ document: { _values: { title: 'Hello' } } }); + + const { result } = renderGetDocument(buildCms(true)); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(fetchDocument).toHaveBeenCalledWith('post', 'hello.mdx'); + expect(result.current.document).toEqual({ _values: { title: 'Hello' } }); + expect(result.current.error).toBeUndefined(); + }); + + it('skips the request and stops loading when not authenticated', async () => { + const fetchDocument = vi.spyOn(TinaAdminApi.prototype, 'fetchDocument'); + + const { result } = renderGetDocument(buildCms(false)); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(fetchDocument).not.toHaveBeenCalled(); + expect(result.current.document).toBeUndefined(); + }); + + it('surfaces a fetch failure instead of loading forever', async () => { + vi.spyOn(TinaAdminApi.prototype, 'fetchDocument').mockRejectedValue( + new Error('boom') + ); + vi.spyOn(console, 'error').mockImplementation(() => {}); + + const { result } = renderGetDocument(buildCms(true)); + + await waitFor(() => expect(result.current.loading).toBe(false)); + expect(result.current.error).toEqual(new Error('boom')); + }); +}); diff --git a/packages/tinacms/src/admin/components/GetDocument.tsx b/packages/tinacms/src/admin/components/GetDocument.tsx index 3659ef66d4..45a3e52c19 100644 --- a/packages/tinacms/src/admin/components/GetDocument.tsx +++ b/packages/tinacms/src/admin/components/GetDocument.tsx @@ -23,8 +23,8 @@ export const useGetDocument = ( let isCancelled = false; // Add cancellation flag const fetchDocument = async () => { - if (api.isAuthenticated() && !isCancelled) { - try { + try { + if ((await api.isAuthenticated()) && !isCancelled) { const response = await api.fetchDocument( collectionName, relativePath @@ -34,22 +34,22 @@ export const useGetDocument = ( if (!isCancelled) { setDocument(response.document); } - } catch (error) { - // Only handle error if the request hasn't been cancelled - if (!isCancelled) { - cms.alerts.error( - `[${error.name}] GetDocument failed: ${error.message}` - ); - console.error(error); - setDocument(undefined); - setError(error); - } } - + } catch (error) { + // Only handle error if the request hasn't been cancelled if (!isCancelled) { - setLoading(false); + cms.alerts.error( + `[${error.name}] GetDocument failed: ${error.message}` + ); + console.error(error); + setDocument(undefined); + setError(error); } } + + if (!isCancelled) { + setLoading(false); + } }; setLoading(true); diff --git a/packages/tinacms/src/auth/TinaCloudProvider.tsx b/packages/tinacms/src/auth/TinaCloudProvider.tsx index 30b5a01579..2560891fe3 100644 --- a/packages/tinacms/src/auth/TinaCloudProvider.tsx +++ b/packages/tinacms/src/auth/TinaCloudProvider.tsx @@ -523,8 +523,13 @@ export const TinaCloudProvider = ( }, []); React.useEffect(() => { - const setupEditorialWorkflow = () => { - client.getProject().then(async (project) => { + const setupEditorialWorkflow = async () => { + try { + const token = await client.authProvider.getToken(); + if (!token?.access_token && !token?.id_token) { + return; + } + const project = await client.getProject(); if (project?.features?.includes('editorial-workflow')) { cms.flags.set('branch-switcher', true); client.usingEditorialWorkflow = true; @@ -536,7 +541,9 @@ export const TinaCloudProvider = ( setCurrentBranch(project.defaultBranch || 'main'); } } - }); + } catch (e) { + console.error('TinaCMS: unable to load project settings', e); + } }; if (isTinaCloud) { setupEditorialWorkflow(); diff --git a/packages/tinacms/src/internalClient/authProvider.test.ts b/packages/tinacms/src/internalClient/authProvider.test.ts index 860ad689e9..5ee806e820 100644 --- a/packages/tinacms/src/internalClient/authProvider.test.ts +++ b/packages/tinacms/src/internalClient/authProvider.test.ts @@ -12,6 +12,11 @@ const nearExpiryAccessToken = makeJwt({ client_id: 'client-id', }); +const freshAccessToken = makeJwt({ + exp: Math.floor(Date.now() / 1000) + 3600, + client_id: 'client-id', +}); + const buildProvider = () => new TinaCloudAuthProvider({ clientId: 'client-id', @@ -91,3 +96,102 @@ describe('TinaCloudAuthProvider getRefreshedToken', () => { }); }); }); + +describe('TinaCloudAuthProvider getUser', () => { + afterEach(() => { + vi.unstubAllGlobals(); + vi.restoreAllMocks(); + }); + + it('skips the currentUser request when no token is stored', async () => { + const provider = buildProvider(); + const fetchMock = stubFetch({}); + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}); + + const user = await provider.getUser(); + await provider.getUser(); + + expect(user).toBeNull(); + expect(fetchMock).not.toHaveBeenCalled(); + expect(warnSpy).toHaveBeenCalledTimes(1); + }); + + it('fetches currentUser with the stored token', async () => { + const provider = buildProvider(); + provider.setToken({ + access_token: freshAccessToken, + id_token: 'id-token', + refresh_token: 'refresh', + }); + const fetchMock = stubFetch({ id: 'user-1' }); + const fetchWithToken = vi.spyOn(provider, 'fetchWithToken'); + + const user = await provider.getUser(); + + expect(user).toEqual({ id: 'user-1' }); + expect(fetchWithToken).toHaveBeenCalledTimes(1); + expect(fetchMock).toHaveBeenCalledTimes(1); + const [url, init] = fetchMock.mock.calls[0]; + expect(url).toBe( + 'https://identity.example.com/v2/apps/client-id/currentUser' + ); + expect(new Headers(init.headers).get('Authorization')).toBe( + `Bearer ${freshAccessToken}` + ); + }); + + it('returns null and logs the status on a non-2xx response', async () => { + const provider = buildProvider(); + provider.setToken({ + access_token: freshAccessToken, + id_token: 'id-token', + refresh_token: 'refresh', + }); + const fetchMock = vi.fn().mockResolvedValue({ + status: 401, + json: vi.fn().mockResolvedValue({ error: 'unauthorized' }), + }); + vi.stubGlobal('fetch', fetchMock); + const errorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + + const user = await provider.getUser(); + + expect(user).toBeNull(); + expect(errorSpy).toHaveBeenCalledWith( + expect.stringContaining('401'), + 'unauthorized' + ); + }); +}); + +describe('TinaCloudAuthProvider getAccessToken', () => { + afterEach(() => { + vi.unstubAllGlobals(); + }); + + it('returns null when no token is stored', async () => { + expect(await buildProvider().getAccessToken()).toBeNull(); + }); + + it('prefers the access token', async () => { + const provider = buildProvider(); + provider.setToken({ + access_token: freshAccessToken, + id_token: 'id-token', + refresh_token: 'refresh', + }); + + expect(await provider.getAccessToken()).toBe(freshAccessToken); + }); + + it('falls back to the id token', async () => { + const provider = buildProvider(); + provider.getToken = async () => ({ + access_token: null, + id_token: 'id-token', + refresh_token: 'refresh', + }); + + expect(await provider.getAccessToken()).toBe('id-token'); + }); +}); diff --git a/packages/tinacms/src/internalClient/authProvider.ts b/packages/tinacms/src/internalClient/authProvider.ts index 7a9088ce1f..4a7d88e673 100644 --- a/packages/tinacms/src/internalClient/authProvider.ts +++ b/packages/tinacms/src/internalClient/authProvider.ts @@ -22,8 +22,7 @@ export abstract class AbstractAuthProvider implements AuthProvider { */ async fetchWithToken(input: Input, init: Init): FetchReturn { const headers = init?.headers || {}; - const token = await this.getToken(); - const accessToken = token?.access_token ?? token?.id_token; + const accessToken = await this.getAccessToken(); if (accessToken) { headers['Authorization'] = 'Bearer ' + accessToken; } @@ -33,6 +32,11 @@ export abstract class AbstractAuthProvider implements AuthProvider { }); } + async getAccessToken(): Promise { + const token = await this.getToken(); + return token?.access_token ?? token?.id_token ?? null; + } + async authorize(context?: any): Promise { // by default, the existence of a token is enough to be authorized return this.getToken(); @@ -72,6 +76,7 @@ export class TinaCloudAuthProvider extends AbstractAuthProvider { identityApiUrl: string; frontendUrl: string; token: TokenObject; // used with memory storage + hasWarnedNoSession = false; setToken: (_token: TokenObject | null) => void; getToken: () => Promise; @@ -157,12 +162,24 @@ export class TinaCloudAuthProvider extends AbstractAuthProvider { const url = `${this.identityApiUrl}/v2/apps/${this.clientId}/currentUser`; try { + if (!(await this.getAccessToken())) { + if (!this.hasWarnedNoSession) { + this.hasWarnedNoSession = true; + console.warn( + 'TinaCMS: no TinaCloud session found. If login fails, check the console inside the login popup window for the underlying error.' + ); + } + return null; + } const res = await this.fetchWithToken(url, { method: 'GET', }); const val = await res.json(); if (!res.status.toString().startsWith('2')) { - console.error(val.error); + console.error( + `TinaCMS: TinaCloud session check failed (status ${res.status}).`, + val?.error ?? val + ); return null; } return val;