fixup! feat: add setRequestToken and fetchRequestToken methods

Author: susnux

lib/globals.d.ts globals.d.tslib/globals.d.ts renamed to globals.d.ts

@@ -7,7 +7,7 @@ import type { NextcloudUser } from './user.ts'
 
 declare module '@nextcloud/event-bus' {
 	export interface NextcloudEvents {
-		'csrf-token-update': { token: string }
+		'csrf-token-update': { token: string, _internal?: true }
 		// mapping of 'event name' => 'event type'
 		'user:info:changed': NextcloudUser
 	}

lib/eventbus.d.ts

@@ -10,17 +10,23 @@ export interface CsrfTokenObserver {
 	(token: string): void
 }
 
+_subscribeToTokenUpdates() // TODO: remove once we drop support for Nextcloud 33.0.1 and before
+
 /**
  * Get current request token
  *
  * @return Current request token or null if not set
  */
 export function getRequestToken(): string | null {
+	if (globalThis._nc_auth_requestToken) {
+		return globalThis._nc_auth_requestToken
+	}
+
 	if (globalThis.document) {
+		// for service workers or other contexts without DOM we need to safeguard this
 		return document.head.dataset.requesttoken ?? null
 	}
-	// for service workers or other contexts without DOM, we keep the token in memory
-	return globalThis._nc_auth_requestToken ?? null
+	return null
 }
 
 /**
@@ -35,12 +41,18 @@ export function setRequestToken(token: string): void {
 		throw new Error('Invalid CSRF token given', { cause: { token } })
 	}
 
+	if (globalThis._nc_auth_requestToken === token) {
+		// token is the same as before, no need to update and especially no need to notify the observers
+		return
+	}
+
+	globalThis._nc_auth_requestToken = token
 	if (globalThis.document) {
+		// For DOM environments we also set the token to the DOM, so it is available for legacy code
 		document.head.dataset.requesttoken = token
-	} else {
-		globalThis._nc_auth_requestToken = token
 	}
-	emit('csrf-token-update', { token })
+
+	emit('csrf-token-update', { token, _internal: true })
 }
 
 /**
@@ -62,40 +74,34 @@ export async function fetchRequestToken(): Promise<string> {
 	return token
 }
 
-const _observers: CsrfTokenObserver[] = []
 /**
  * Add an observer which is called when the CSRF token changes
  *
  * @param observer The observer
  */
 export function onRequestTokenUpdate(observer: CsrfTokenObserver): void {
-	_subscribeToTokenUpdates()
-	_observers.push(observer)
+	subscribe('csrf-token-update', async ({ token }) => {
+		try {
+			observer(token)
+		} catch (error) {
+			// we cannot use the logger as the logger uses this library = circular dependency
+			// eslint-disable-next-line no-console
+			console.error('Error updating CSRF token observer', error)
+		}
+	})
 }
 
-let _initialized = false
 /**
  * Subscribe to token update events from server.
  *
- * This is legacy and not needed once all supported server versions use `setRequestToken` of this library.
+ * @todo - This is legacy and not needed once all supported server versions use `setRequestToken` of this library.
  */
 function _subscribeToTokenUpdates(): void {
-	if (_initialized) {
-		return
-	}
-
-	_initialized = true
 	// Listen to server event and keep token in sync
-	subscribe('csrf-token-update', (event) => {
-		setRequestToken(event.token)
-		for (const observer of _observers) {
-			try {
-				observer(event.token)
-			} catch (error) {
-				// we cannot use the logger as the logger uses this library = circular dependency
-				// eslint-disable-next-line no-console
-				console.error('Error updating CSRF token observer', error)
-			}
+	subscribe('csrf-token-update', ({ token, _internal }) => {
+		if (!_internal) {
+			// Only update the token if the event is not emitted from this library, otherwise we would end in a loop
+			setRequestToken(token)
 		}
 	})
 }

lib/requestToken.ts

@@ -0,0 +1,67 @@
+/**
+ * SPDX-FileCopyrightText: 2019 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+
+import { emit } from '@nextcloud/event-bus'
+import { beforeEach, describe, expect, it, test, vi } from 'vitest'
+
+test('it does set the new token form legacy event', async () => {
+	const { getRequestToken } = await import('../lib/requestToken.ts')
+	mockToken('old-token')
+
+	// older server versions emitted the event directly without using this libary
+	emit('csrf-token-update', { token: 'new-token' })
+	expect(getRequestToken()).toBe('new-token')
+})
+
+describe('request token observers', () => {
+	beforeEach(() => {
+		vi.resetModules()
+		vi.resetAllMocks()
+		mockToken(undefined)
+	})
+
+	it('request token observer is called', async () => {
+		const { onRequestTokenUpdate, setRequestToken } = await import('../lib/requestToken.ts')
+		const observer = vi.fn(() => { })
+
+		onRequestTokenUpdate(observer)
+		expect(observer).not.toBeCalled()
+
+		setRequestToken('token123')
+		expect(observer).toBeCalledTimes(1)
+		expect(observer).toBeCalledWith('token123')
+	})
+
+	it('handle exception in observer', async () => {
+		const { onRequestTokenUpdate, setRequestToken } = await import('../lib/requestToken.ts')
+		const spy = vi.spyOn(window.console, 'error')
+		const observer = vi.fn(() => {
+			throw new Error('!Error!')
+		})
+		// silence the console
+		spy.mockImplementationOnce(() => {})
+
+		onRequestTokenUpdate(observer)
+		setRequestToken('token123')
+
+		expect(observer).toBeCalledTimes(1)
+		expect(spy).toHaveBeenCalledOnce()
+	})
+})
+
+/**
+ * Mock the request token directly so we can it reading it.
+ *
+ * @param token - The CSRF token to mock
+ */
+function mockToken(token?: string) {
+	if (token === undefined) {
+		delete document.head.dataset.requesttoken
+		delete globalThis._nc_auth_requestToken
+	} else {
+		document.head.dataset.requesttoken = token
+		globalThis._nc_auth_requestToken = token
+	}
+}

test/requestToken-observers.test.ts

@@ -6,7 +6,7 @@
 import { http, HttpResponse } from 'msw'
 import { setupServer } from 'msw/node'
 import { beforeAll, beforeEach, describe, expect, it, vi } from 'vitest'
-import { fetchRequestToken, getRequestToken, onRequestTokenUpdate, setRequestToken } from '../lib/requestToken.ts'
+import { fetchRequestToken, getRequestToken, setRequestToken } from '../lib/requestToken.ts'
 
 const eventbus = vi.hoisted(() => ({ emit: vi.fn(), subscribe: vi.fn() }))
 vi.mock('@nextcloud/event-bus', () => eventbus)
@@ -15,14 +15,24 @@ const server = setupServer()
 
 describe('getRequestToken', () => {
 	it('can read the token from DOM', () => {
-		mockToken('tokenmock-123')
+		mockToken('tokenmock-123', undefined)
 		expect(getRequestToken()).toBe('tokenmock-123')
 	})
 
 	it('can handle missing token', () => {
 		mockToken(undefined)
 		expect(getRequestToken()).toBeNull()
 	})
+
+	it('can handle cache token', () => {
+		mockToken('cached-token')
+		expect(getRequestToken()).toBe('cached-token')
+	})
+
+	it('prioritizes cached token', () => {
+		mockToken('dom-token', 'cached-token')
+		expect(getRequestToken()).toBe('cached-token')
+	})
 })
 
 describe('setRequestToken', () => {
@@ -33,7 +43,7 @@ describe('setRequestToken', () => {
 	it('does emit an event on change', () => {
 		setRequestToken('new-token')
 		expect(eventbus.emit).toBeCalledTimes(1)
-		expect(eventbus.emit).toBeCalledWith('csrf-token-update', { token: 'new-token' })
+		expect(eventbus.emit).toBeCalledWith('csrf-token-update', expect.objectContaining({ token: 'new-token' }))
 	})
 
 	it('does set the new token to the DOM', () => {
@@ -62,50 +72,6 @@ describe('setRequestToken', () => {
 	})
 })
 
-describe('request token observers', () => {
-	beforeEach(() => {
-		mockToken(undefined)
-	})
-
-	it('can update token by event', async () => {
-		expect(getRequestToken()).toBeNull()
-
-		onRequestTokenUpdate(() => {})
-		expect(eventbus.subscribe).toBeCalledWith('csrf-token-update', expect.any(Function))
-		eventbus.subscribe.mock.calls[0][1]({ token: 'token123' })
-
-		expect(getRequestToken()).toBe('token123')
-	})
-
-	it('request token observer is called', async () => {
-		const observer = vi.fn(() => { })
-
-		onRequestTokenUpdate(observer)
-		expect(observer).not.toBeCalled()
-
-		expect(eventbus.subscribe).toBeCalledWith('csrf-token-update', expect.any(Function))
-		eventbus.subscribe.mock.calls[0][1]({ token: 'token123' })
-
-		expect(observer).toBeCalledTimes(1)
-	})
-
-	it('handle exception in observer', async () => {
-		const spy = vi.spyOn(window.console, 'error')
-		const observer = vi.fn(() => {
-			throw new Error('!Error!')
-		})
-		// silence the console
-		spy.mockImplementationOnce(() => {})
-
-		onRequestTokenUpdate(observer)
-		expect(eventbus.subscribe).toBeCalledWith('csrf-token-update', expect.any(Function))
-		eventbus.subscribe.mock.calls[0][1]({ token: 'token123' })
-
-		expect(observer).toBeCalledTimes(1)
-		expect(spy).toHaveBeenCalledOnce()
-	})
-})
-
 describe('fetchRequestToken', () => {
 	const successfullCsrf = http.get('/index.php/csrftoken', () => {
 		return HttpResponse.json({ token: 'new-token' })
@@ -127,20 +93,19 @@ describe('fetchRequestToken', () => {
 
 	beforeEach(() => {
 		vi.resetAllMocks()
+		mockToken('oldToken')
 	})
 
 	it('correctly parses response', async () => {
 		server.use(successfullCsrf)
 
-		mockToken('oldToken')
 		const token = await fetchRequestToken()
 		expect(token).toBe('new-token')
 	})
 
 	it('sets the token', async () => {
 		server.use(successfullCsrf)
 
-		mockToken('oldToken')
 		await fetchRequestToken()
 		expect(getRequestToken()).toBe('new-token')
 	})
@@ -150,7 +115,7 @@ describe('fetchRequestToken', () => {
 
 		await fetchRequestToken()
 		expect(eventbus.emit).toHaveBeenCalledOnce()
-		expect(eventbus.emit).toBeCalledWith('csrf-token-update', { token: 'new-token' })
+		expect(eventbus.emit).toBeCalledWith('csrf-token-update', expect.objectContaining({ token: 'new-token' }))
 	})
 
 	it('handles 403 error due to invalid cookies', async () => {
@@ -182,11 +147,18 @@ describe('fetchRequestToken', () => {
  * Mock the request token directly so we can it reading it.
  *
  * @param token - The CSRF token to mock
+ * @param internalToken - The internal (cached version of the DOM token) token to mock, if null the `token` will be used as internal token
  */
-function mockToken(token?: string) {
+function mockToken(token?: string, internalToken: string | undefined | null = null) {
 	if (token === undefined) {
 		delete document.head.dataset.requesttoken
 	} else {
 		document.head.dataset.requesttoken = token
 	}
+
+	if (internalToken === null) {
+		globalThis._nc_auth_requestToken = token
+	} else {
+		globalThis._nc_auth_requestToken = internalToken
+	}
 }

test/requestToken.test.ts

@@ -0,0 +1,4 @@
+{
+  "extends": "../tsconfig.json",
+  "include": [".", "../lib", "../*.d.ts"],
+}

test/tsconfig.json

@@ -1,5 +1,5 @@
 {
-  "include": ["./lib"],
+  "include": ["./lib", "./*.d.ts"],
   "compilerOptions": {
     "allowSyntheticDefaultImports": true,
     "allowImportingTsExtensions": true,