Skip to content

Commit a745e43

Browse files
rickyrombodylanjeffersraymondjacobsonCopilotgithub-actions[bot]
committed
Simplify OAuth refreshToken middleware by using OAuth service in the middleware (#13805)
Removes the deps on usersApi by removing the `isWriteAccessGranted` method (which is redundant to the devApps APIs) and the `verifyToken` method (which is redundant with the usersApi) and calling fetch manually internally. Then, this allows the OAuth service to be initialized before other APIs so that it can be used in the config for middlewares to all other APIs. --------- Co-authored-by: Dylan Jeffers <dylan@audius.co> Co-authored-by: Ray Jacobson <ray@audius.co> Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
1 parent 8b47656 commit a745e43

11 files changed

Lines changed: 508 additions & 290 deletions

File tree

packages/mobile/src/harmony-native/components/Artwork.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,8 @@ export const Artwork = (props: ArtworkProps) => {
8181
borderRadius={borderRadius}
8282
border='default'
8383
shadow={shadow}
84+
w='100%'
85+
h='100%'
8486
style={{ borderWidth }}
8587
>
8688
{isLoading && hasImageSource ? (

packages/sdk/src/sdk/createSdkWithServices.ts

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,11 @@ import { developmentConfig } from './config/development'
2929
import { productionConfig } from './config/production'
3030
import {
3131
addAppInfoMiddleware,
32-
addRequestSignatureMiddleware
32+
addRequestSignatureMiddleware,
33+
addTokenRefreshMiddleware
3334
} from './middleware'
3435
import { OAuth } from './oauth'
36+
import { OAuthTokenStore } from './oauth/tokenStore'
3537
import {
3638
PaymentRouterClient,
3739
getDefaultPaymentRouterClientConfig
@@ -133,7 +135,7 @@ export const createSdkWithServices = (config: SdkConfig) => {
133135
)
134136
}
135137

136-
// Initialize APIs
138+
// Initialize APIs (also creates tokenStore and oauth)
137139
const apis = initializeApis({
138140
config,
139141
apiKey,
@@ -142,18 +144,7 @@ export const createSdkWithServices = (config: SdkConfig) => {
142144
services
143145
})
144146

145-
// Initialize OAuth
146-
const oauth = isBrowser
147-
? new OAuth({
148-
appName,
149-
apiKey,
150-
usersApi: apis.users,
151-
logger: services.logger
152-
})
153-
: undefined
154-
155147
return {
156-
oauth,
157148
...apis
158149
}
159150
}
@@ -460,11 +451,36 @@ const initializeApis = ({
460451
})
461452
]
462453

454+
// Token store for PKCE flow — provides dynamic accessToken to Configuration
455+
const tokenStore = new OAuthTokenStore()
456+
457+
// Auto-refresh middleware — intercepts 401s and retries with a fresh token.
458+
const oauth =
459+
typeof window !== 'undefined'
460+
? new OAuth({
461+
apiKey,
462+
tokenStore,
463+
basePath
464+
})
465+
: undefined
466+
467+
if (apiKey && oauth) {
468+
middleware.push(
469+
addTokenRefreshMiddleware({
470+
oauth
471+
})
472+
)
473+
}
474+
475+
const bearerToken = 'bearerToken' in config ? config.bearerToken : undefined
476+
463477
const apiClientConfig = new Configuration({
464478
fetchApi: fetch,
465479
middleware,
466480
basePath,
467-
accessToken: 'bearerToken' in config ? config.bearerToken : undefined
481+
// Static bearerToken takes precedence; otherwise use the dynamic store
482+
// so PKCE login can inject tokens after construction.
483+
accessToken: bearerToken ?? tokenStore.asAccessTokenProvider()
468484
})
469485

470486
const tracks = new TracksApi(apiClientConfig, services)
@@ -507,6 +523,8 @@ const initializeApis = ({
507523
const search = new SearchApi(apiClientConfig)
508524

509525
return {
526+
oauth,
527+
tokenStore,
510528
tracks,
511529
users,
512530
albums,

packages/sdk/src/sdk/createSdkWithoutServices.ts

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,16 @@ export const createSdkWithoutServices = (config: SdkConfig) => {
5959
// Token store for PKCE flow — provides dynamic accessToken to Configuration
6060
const tokenStore = new OAuthTokenStore()
6161

62+
// Initialize OAuth early so it can be passed to middleware
63+
const oauth =
64+
typeof window !== 'undefined'
65+
? new OAuth({
66+
apiKey,
67+
tokenStore,
68+
basePath
69+
})
70+
: undefined
71+
6272
if (apiSecret || services?.audiusWalletClient) {
6373
middleware.push(
6474
addRequestSignatureMiddleware({
@@ -87,12 +97,10 @@ export const createSdkWithoutServices = (config: SdkConfig) => {
8797
}
8898

8999
// Auto-refresh middleware — intercepts 401s and retries with a fresh token.
90-
if (apiKey) {
100+
if (apiKey && oauth) {
91101
middleware.push(
92102
addTokenRefreshMiddleware({
93-
tokenStore,
94-
apiKey,
95-
basePath
103+
oauth
96104
})
97105
)
98106
}
@@ -106,17 +114,8 @@ export const createSdkWithoutServices = (config: SdkConfig) => {
106114
accessToken: bearerToken ?? tokenStore.asAccessTokenProvider()
107115
})
108116

109-
// Initialize OAuth
117+
// Initialize API clients
110118
const usersApi = new UsersApi(apiConfig)
111-
const oauth =
112-
typeof window !== 'undefined'
113-
? new OAuth({
114-
apiKey,
115-
usersApi,
116-
tokenStore,
117-
basePath
118-
})
119-
: undefined
120119

121120
return {
122121
oauth,
Lines changed: 40 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,9 @@
11
import { describe, it, expect, vi, beforeEach } from 'vitest'
22

3-
import { OAuthTokenStore } from '../oauth/tokenStore'
3+
import type { OAuth } from '../oauth/OAuth'
44

55
import { addTokenRefreshMiddleware } from './addTokenRefreshMiddleware'
66

7-
// Mock cross-fetch used by the middleware
8-
vi.mock('cross-fetch', () => ({
9-
default: vi.fn()
10-
}))
11-
12-
import crossFetch from 'cross-fetch'
13-
const mockCrossFetch = vi.mocked(crossFetch)
14-
157
// Minimal fetch mock helper
168
function mockResponse(status: number, body?: object): Response {
179
return new Response(body ? JSON.stringify(body) : null, {
@@ -20,18 +12,24 @@ function mockResponse(status: number, body?: object): Response {
2012
})
2113
}
2214

23-
describe('addTokenRefreshMiddleware', () => {
24-
let tokenStore: OAuthTokenStore
25-
const apiKey = 'test-api-key'
26-
const basePath = 'https://api.example.com/v1'
15+
function createMockOAuth(
16+
refreshBehaviour: () => Promise<string | null>,
17+
hasRefreshToken = true
18+
): OAuth {
19+
return {
20+
hasRefreshToken,
21+
refreshAccessToken: refreshBehaviour
22+
} as unknown as OAuth
23+
}
2724

25+
describe('addTokenRefreshMiddleware', () => {
2826
beforeEach(() => {
29-
tokenStore = new OAuthTokenStore()
3027
vi.restoreAllMocks()
3128
})
3229

3330
it('passes through non-401 responses unchanged', async () => {
34-
const mw = addTokenRefreshMiddleware({ tokenStore, apiKey, basePath })
31+
const oauth = createMockOAuth(async () => 'token')
32+
const mw = addTokenRefreshMiddleware({ oauth })
3533
const response = mockResponse(200, { data: 'ok' })
3634

3735
const result = await mw.post!({
@@ -44,8 +42,10 @@ describe('addTokenRefreshMiddleware', () => {
4442
expect(result).toBe(response)
4543
})
4644

47-
it('passes through 401 when no refresh token is available', async () => {
48-
const mw = addTokenRefreshMiddleware({ tokenStore, apiKey, basePath })
45+
it('passes through 401 without calling refreshAccessToken when unauthenticated', async () => {
46+
const refreshFn = vi.fn()
47+
const oauth = createMockOAuth(refreshFn, false)
48+
const mw = addTokenRefreshMiddleware({ oauth })
4949
const response = mockResponse(401)
5050

5151
const result = await mw.post!({
@@ -56,28 +56,30 @@ describe('addTokenRefreshMiddleware', () => {
5656
})
5757

5858
expect(result).toBe(response)
59+
expect(refreshFn).not.toHaveBeenCalled()
5960
})
6061

61-
it('refreshes and retries on 401 when refresh token exists', async () => {
62-
tokenStore.setTokens('expired-access', 'valid-refresh')
62+
it('passes through 401 when refresh returns null (no refresh token)', async () => {
63+
const oauth = createMockOAuth(async () => null)
64+
const mw = addTokenRefreshMiddleware({ oauth })
65+
const response = mockResponse(401)
6366

64-
const refreshResponse = mockResponse(200, {
65-
access_token: 'new-access',
66-
refresh_token: 'new-refresh',
67-
token_type: 'Bearer',
68-
expires_in: 3600,
69-
scope: 'write'
67+
const result = await mw.post!({
68+
fetch,
69+
url: 'https://api.example.com/v1/users/me',
70+
init: {},
71+
response
7072
})
7173

72-
const retryResponse = mockResponse(200, { data: 'success' })
73-
74-
// Mock cross-fetch for the refresh call
75-
mockCrossFetch.mockResolvedValueOnce(refreshResponse)
74+
expect(result).toBe(response)
75+
})
7676

77-
// The retry fetch provided in context
77+
it('refreshes and retries on 401 when refresh succeeds', async () => {
78+
const oauth = createMockOAuth(async () => 'new-access')
79+
const retryResponse = mockResponse(200, { data: 'success' })
7880
const contextFetch = vi.fn().mockResolvedValueOnce(retryResponse)
7981

80-
const mw = addTokenRefreshMiddleware({ tokenStore, apiKey, basePath })
82+
const mw = addTokenRefreshMiddleware({ oauth })
8183

8284
const result = await mw.post!({
8385
fetch: contextFetch,
@@ -89,19 +91,7 @@ describe('addTokenRefreshMiddleware', () => {
8991
response: mockResponse(401)
9092
})
9193

92-
// Refresh endpoint was called
93-
expect(mockCrossFetch).toHaveBeenCalledWith(
94-
`${basePath}/oauth/token`,
95-
expect.objectContaining({
96-
method: 'POST'
97-
})
98-
)
99-
100-
// Token store was updated
101-
expect(tokenStore.accessToken).toBe('new-access')
102-
expect(tokenStore.refreshToken).toBe('new-refresh')
103-
104-
// Original request was retried
94+
// Original request was retried with new token
10595
expect(contextFetch).toHaveBeenCalledWith(
10696
'https://api.example.com/v1/tracks/123',
10797
expect.objectContaining({
@@ -115,12 +105,8 @@ describe('addTokenRefreshMiddleware', () => {
115105
})
116106

117107
it('surfaces 401 when refresh fails', async () => {
118-
tokenStore.setTokens('expired-access', 'revoked-refresh')
119-
120-
// Refresh returns 400 (invalid_grant)
121-
mockCrossFetch.mockResolvedValueOnce(mockResponse(400))
122-
123-
const mw = addTokenRefreshMiddleware({ tokenStore, apiKey, basePath })
108+
const oauth = createMockOAuth(async () => null)
109+
const mw = addTokenRefreshMiddleware({ oauth })
124110
const original401 = mockResponse(401)
125111

126112
const result = await mw.post!({
@@ -130,80 +116,14 @@ describe('addTokenRefreshMiddleware', () => {
130116
response: original401
131117
})
132118

133-
// Returns the original 401 — doesn't swallow it
134119
expect(result).toBe(original401)
135120
})
136121

137-
it('surfaces 401 when refresh endpoint returns invalid JSON', async () => {
138-
tokenStore.setTokens('expired-access', 'valid-refresh')
139-
140-
// Return 200 but with garbage body
141-
mockCrossFetch.mockResolvedValueOnce(
142-
new Response('not json', { status: 200 })
143-
)
144-
145-
const mw = addTokenRefreshMiddleware({ tokenStore, apiKey, basePath })
146-
const original401 = mockResponse(401)
147-
148-
const result = await mw.post!({
149-
fetch,
150-
url: 'https://api.example.com/v1/tracks/123',
151-
init: {},
152-
response: original401
122+
it('surfaces 401 when refreshAccessToken throws', async () => {
123+
const oauth = createMockOAuth(async () => {
124+
throw new Error('network failure')
153125
})
154-
155-
expect(result).toBe(original401)
156-
})
157-
158-
it('surfaces 401 when refresh response is missing required fields', async () => {
159-
tokenStore.setTokens('expired-access', 'valid-refresh')
160-
161-
// Return 200 but only access_token (no refresh_token)
162-
mockCrossFetch.mockResolvedValueOnce(
163-
mockResponse(200, { access_token: 'new-access' })
164-
)
165-
166-
const mw = addTokenRefreshMiddleware({ tokenStore, apiKey, basePath })
167-
const original401 = mockResponse(401)
168-
169-
const result = await mw.post!({
170-
fetch,
171-
url: 'https://api.example.com/v1/tracks/123',
172-
init: {},
173-
response: original401
174-
})
175-
176-
expect(result).toBe(original401)
177-
})
178-
179-
it('surfaces 401 when network error occurs during refresh', async () => {
180-
tokenStore.setTokens('expired-access', 'valid-refresh')
181-
182-
mockCrossFetch.mockRejectedValueOnce(new Error('network failure'))
183-
184-
const mw = addTokenRefreshMiddleware({ tokenStore, apiKey, basePath })
185-
const original401 = mockResponse(401)
186-
187-
const result = await mw.post!({
188-
fetch,
189-
url: 'https://api.example.com/v1/tracks/123',
190-
init: {},
191-
response: original401
192-
})
193-
194-
expect(result).toBe(original401)
195-
})
196-
197-
it('surfaces 401 when refresh token is cleared between check and exchange', async () => {
198-
tokenStore.setTokens('expired-access', 'about-to-be-cleared')
199-
200-
// Simulate: refresh token exists at guard check, but the exchange returns
201-
// empty strings (server rejected it). The middleware should not store empty tokens.
202-
mockCrossFetch.mockResolvedValueOnce(
203-
mockResponse(200, { access_token: '', refresh_token: '' })
204-
)
205-
206-
const mw = addTokenRefreshMiddleware({ tokenStore, apiKey, basePath })
126+
const mw = addTokenRefreshMiddleware({ oauth })
207127
const original401 = mockResponse(401)
208128

209129
const result = await mw.post!({
@@ -213,7 +133,6 @@ describe('addTokenRefreshMiddleware', () => {
213133
response: original401
214134
})
215135

216-
// Empty tokens are rejected by the validation
217136
expect(result).toBe(original401)
218137
})
219138
})

0 commit comments

Comments
 (0)