Skip to content

Commit 0bfa30f

Browse files
rickyromboCopilotCopilot
authored
Promisify OAuth service login (#13892)
Updates OAuth service `loginAsync()` to be promise-based. Keeps `login()` the same. Removes need for `init()` to be called. Also removes some unused types. Updates the OAUTH_URL to be instead the API url from the config now that it redirects, which allows for handling multiple environments. However, since it redirects, removed the origin check on the postmessage since it won't come from the API server url. --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
1 parent 66cd36e commit 0bfa30f

4 files changed

Lines changed: 351 additions & 208 deletions

File tree

.changeset/tricky-zoos-film.md

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
'@audius/sdk': patch
3+
---
4+
5+
Update OAuth service to allow for loginAsync to not require init()
6+
7+
- Promisifies OAuth logins
8+
- Uses API URL rather than hardcoding the production URL (respects environment)
9+
- Fixes minor error handling

packages/sdk/src/sdk/oauth/OAuth.test.ts

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ import { OAuthTokenStore } from './tokenStore'
88
vi.stubGlobal('window', {
99
localStorage: { getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() },
1010
sessionStorage: { getItem: vi.fn(), setItem: vi.fn(), removeItem: vi.fn() },
11+
crypto: { getRandomValues: vi.fn((arr: Uint8Array) => arr.fill(0)) },
1112
addEventListener: vi.fn(),
13+
removeEventListener: vi.fn(),
1214
location: { href: '', origin: 'https://example.com' },
1315
open: vi.fn()
1416
})
@@ -206,3 +208,91 @@ describe('OAuth.refreshAccessToken', () => {
206208
expect(result).toBeNull()
207209
})
208210
})
211+
212+
describe('OAuth message listener lifecycle', () => {
213+
beforeEach(() => {
214+
vi.mocked(window.addEventListener).mockClear()
215+
vi.mocked(window.removeEventListener).mockClear()
216+
vi.mocked(window.open).mockReturnValue({
217+
closed: false,
218+
close: vi.fn()
219+
} as unknown as Window)
220+
vi.mocked(window.localStorage.setItem).mockClear()
221+
vi.mocked(window.localStorage.getItem).mockReturnValue('csrf-token')
222+
vi.mocked(window.sessionStorage.setItem).mockClear()
223+
vi.mocked(window.sessionStorage.getItem).mockReturnValue(null)
224+
})
225+
226+
afterEach(() => {
227+
vi.restoreAllMocks()
228+
})
229+
230+
it('does not attach a message listener in the constructor', () => {
231+
makeOAuth({ basePath: 'https://api.example.com' })
232+
expect(window.addEventListener).not.toHaveBeenCalled()
233+
})
234+
235+
it('attaches a message listener when loginAsync starts (postMessage flow)', async () => {
236+
const oauth = makeOAuth({ basePath: 'https://api.example.com' })
237+
// Kick off a login — don't await so we can inspect immediately
238+
oauth.loginAsync({ redirectUri: 'postMessage' })
239+
// Flush microtasks
240+
await Promise.resolve()
241+
expect(window.addEventListener).toHaveBeenCalledWith(
242+
'message',
243+
expect.any(Function),
244+
false
245+
)
246+
})
247+
248+
it('does not attach a duplicate listener on repeated loginAsync calls', async () => {
249+
const oauth = makeOAuth({ basePath: 'https://api.example.com' })
250+
oauth.loginAsync({ redirectUri: 'postMessage' })
251+
await Promise.resolve()
252+
oauth.loginAsync({ redirectUri: 'postMessage' })
253+
await Promise.resolve()
254+
// Should still only be registered once
255+
const messageAddCalls = vi
256+
.mocked(window.addEventListener)
257+
.mock.calls.filter(([event]) => event === 'message')
258+
expect(messageAddCalls).toHaveLength(1)
259+
})
260+
261+
it('removes the message listener when the login settles', async () => {
262+
const oauth = makeOAuth({ basePath: 'https://api.example.com' })
263+
const loginPromise = oauth.loginAsync({ redirectUri: 'postMessage' })
264+
await Promise.resolve()
265+
266+
// Retrieve the registered handler
267+
const addCall = vi
268+
.mocked(window.addEventListener)
269+
.mock.calls.find(([event]) => event === 'message')
270+
expect(addCall).toBeDefined()
271+
const registeredHandler = addCall![1]
272+
273+
// Settle the login via an error path
274+
;(oauth as any)._settleLogin(new Error('test settle'))
275+
276+
// Await so rejection is handled
277+
await loginPromise.catch(() => {})
278+
279+
expect(window.removeEventListener).toHaveBeenCalledWith(
280+
'message',
281+
registeredHandler,
282+
false
283+
)
284+
})
285+
286+
it('does not attach a listener when redirectUri is not postMessage', async () => {
287+
const oauth = makeOAuth({ basePath: 'https://api.example.com' })
288+
// When redirectUri is a real URL the code does window.location.href = …
289+
// and never enters the postMessage branch — don't await the never-settling promise
290+
oauth.loginAsync({ redirectUri: 'https://myapp.example.com/callback' })
291+
await Promise.resolve()
292+
expect(window.addEventListener).not.toHaveBeenCalledWith(
293+
'message',
294+
expect.any(Function),
295+
false
296+
)
297+
})
298+
})

0 commit comments

Comments
 (0)