wire up identity client to current identity callsites

Author: alexanderMontague

packages/cli-kit/src/private/node/session.test.ts

@@ -20,6 +20,8 @@ import {validateSession} from './session/validate.js'
 import {applicationId} from './session/identity.js'
 import {pollForDeviceAuthorization, requestDeviceAuthorization} from './session/device-authorization.js'
 import {getCurrentSessionId} from './conf-store.js'
+import {getIdentityClient} from './clients/identity/instance.js'
+import {IdentityMockClient} from './clients/identity/identity-mock-client.js'
 import * as fqdnModule from '../../public/node/context/fqdn.js'
 import {themeToken} from '../../public/node/context/local.js'
 import {partnersRequest} from '../../public/node/api/partners.js'
@@ -31,7 +33,7 @@ import {vi, describe, expect, test, beforeEach} from 'vitest'
 
 const futureDate = new Date(2022, 1, 1, 11)
 
-const userId = '1234-5678'
+const userId = '08978734-325e-44ce-bc65-34823a8d5180'
 
 const defaultApplications: OAuthApplications = {
   adminApi: {storeFqdn: 'mystore', scopes: []},
@@ -105,6 +107,8 @@ const invalidSessions: Sessions = {
   },
 }
 
+const mockIdentityClient = new IdentityMockClient()
+
 vi.mock('../../public/node/context/local.js')
 vi.mock('./session/identity')
 vi.mock('./session/authorize')
@@ -119,6 +123,7 @@ vi.mock('../../public/node/environment.js')
 vi.mock('./session/device-authorization')
 vi.mock('./conf-store')
 vi.mock('../../public/node/system.js')
+vi.mock('./clients/identity/instance.js')
 
 beforeEach(() => {
   vi.spyOn(fqdnModule, 'identityFqdn').mockResolvedValue(fqdn)
@@ -149,6 +154,11 @@ beforeEach(() => {
       email: 'user@example.com',
     },
   })
+
+  vi.mocked(getIdentityClient).mockImplementation(() => mockIdentityClient)
+  vi.spyOn(mockIdentityClient, 'refreshAccessToken').mockResolvedValue(validIdentityToken)
+  vi.spyOn(mockIdentityClient, 'requestAccessToken').mockResolvedValue(validIdentityToken)
+  vi.spyOn(mockIdentityClient, 'exchangeAccessForApplicationTokens').mockResolvedValue(appTokens)
 })
 
 describe('ensureAuthenticated when previous session is invalid', () => {
@@ -172,7 +182,7 @@ describe('ensureAuthenticated when previous session is invalid', () => {
     expect(storedSession[fqdn]![userId]!.identity.alias).toBe('user@example.com')
 
     // The userID is cached in memory and the secureStore is not accessed again
-    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
+    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('08978734-325e-44ce-bc65-34823a8d5180')
     await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth')
     expect(fetchSessions).toHaveBeenCalledOnce()
   })
@@ -223,7 +233,7 @@ The CLI is currently unable to prompt for reauthentication.`,
     expect(refreshAccessToken).not.toBeCalled()
     expect(storeSessions).toBeCalledWith(expectedSessions)
     expect(got).toEqual(validTokens)
-    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
+    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('08978734-325e-44ce-bc65-34823a8d5180')
     await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth')
     expect(fetchSessions).toHaveBeenCalledOnce()
   })
@@ -290,7 +300,7 @@ The CLI is currently unable to prompt for reauthentication.`,
     expect(storedSession[fqdn]![userId]!.identity.alias).toBe('user@example.com')
 
     expect(got).toEqual(validTokens)
-    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
+    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('08978734-325e-44ce-bc65-34823a8d5180')
     await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth')
     expect(fetchSessions).toHaveBeenCalledOnce()
   })
@@ -309,7 +319,7 @@ describe('when existing session is valid', () => {
     expect(exchangeAccessForApplicationTokens).not.toBeCalled()
     expect(refreshAccessToken).not.toBeCalled()
     expect(got).toEqual(validTokens)
-    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
+    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('08978734-325e-44ce-bc65-34823a8d5180')
     await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth')
     expect(fetchSessions).toHaveBeenCalledOnce()
   })
@@ -328,7 +338,7 @@ describe('when existing session is valid', () => {
     expect(exchangeAccessForApplicationTokens).not.toBeCalled()
     expect(refreshAccessToken).not.toBeCalled()
     expect(got).toEqual(expected)
-    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
+    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('08978734-325e-44ce-bc65-34823a8d5180')
     await expect(getLastSeenAuthMethod()).resolves.toEqual('partners_token')
     expect(fetchSessions).toHaveBeenCalledOnce()
   })
@@ -342,11 +352,11 @@ describe('when existing session is valid', () => {
     const got = await ensureAuthenticated(defaultApplications, process.env, {forceRefresh: true})
 
     // Then
-    expect(refreshAccessToken).toBeCalled()
+    expect(mockIdentityClient.refreshAccessToken).toBeCalled()
     expect(exchangeAccessForApplicationTokens).toBeCalled()
     expect(storeSessions).toBeCalledWith(validSessions)
     expect(got).toEqual(validTokens)
-    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
+    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('08978734-325e-44ce-bc65-34823a8d5180')
     await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth')
     expect(fetchSessions).toHaveBeenCalledOnce()
   })
@@ -362,11 +372,11 @@ describe('when existing session is expired', () => {
     const got = await ensureAuthenticated(defaultApplications)
 
     // Then
-    expect(refreshAccessToken).toBeCalled()
+    expect(mockIdentityClient.refreshAccessToken).toBeCalled()
     expect(exchangeAccessForApplicationTokens).toBeCalled()
     expect(storeSessions).toBeCalledWith(validSessions)
     expect(got).toEqual(validTokens)
-    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
+    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('08978734-325e-44ce-bc65-34823a8d5180')
     await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth')
     expect(fetchSessions).toHaveBeenCalledOnce()
   })
@@ -377,13 +387,13 @@ describe('when existing session is expired', () => {
 
     vi.mocked(validateSession).mockResolvedValueOnce('needs_refresh')
     vi.mocked(fetchSessions).mockResolvedValue(validSessions)
-    vi.mocked(refreshAccessToken).mockRejectedValueOnce(tokenResponseError)
+    vi.spyOn(mockIdentityClient, 'refreshAccessToken').mockRejectedValueOnce(tokenResponseError)
 
     // When
     const got = await ensureAuthenticated(defaultApplications)
 
     // Then
-    expect(refreshAccessToken).toBeCalled()
+    expect(mockIdentityClient.refreshAccessToken).toBeCalled()
     expect(exchangeAccessForApplicationTokens).toBeCalled()
     expect(businessPlatformRequest).toHaveBeenCalled()
     expect(storeSessions).toHaveBeenCalledOnce()
@@ -393,7 +403,7 @@ describe('when existing session is expired', () => {
     expect(storedSession[fqdn]![userId]!.identity.alias).toBe('user@example.com')
 
     expect(got).toEqual(validTokens)
-    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678')
+    await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('08978734-325e-44ce-bc65-34823a8d5180')
     await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth')
     expect(fetchSessions).toHaveBeenCalledOnce()
   })
@@ -667,10 +677,11 @@ describe('ensureAuthenticated email fetch functionality', () => {
     const tokenResponseError = new InvalidGrantError()
     vi.mocked(validateSession).mockResolvedValueOnce('needs_refresh')
     vi.mocked(fetchSessions).mockResolvedValue(validSessions)
-    vi.mocked(refreshAccessToken).mockRejectedValueOnce(tokenResponseError)
+    vi.spyOn(mockIdentityClient, 'refreshAccessToken').mockRejectedValueOnce(tokenResponseError)
+    vi.spyOn(mockIdentityClient, 'requestAccessToken').mockResolvedValueOnce(validIdentityToken)
     vi.mocked(businessPlatformRequest).mockResolvedValueOnce({
       currentUserAccount: {
-        email: 'fallback@example.com',
+        email: 'dev@shopify.com',
       },
     })
 
@@ -679,7 +690,7 @@ describe('ensureAuthenticated email fetch functionality', () => {
 
     // Then
     const storedSession = vi.mocked(storeSessions).mock.calls[0]![0]
-    expect(storedSession[fqdn]![userId]!.identity.alias).toBe('fallback@example.com')
+    expect(storedSession[fqdn]![userId]!.identity.alias).toBe('dev@shopify.com')
     expect(got).toEqual(validTokens)
   })
 

packages/cli-kit/src/private/node/session.ts

@@ -5,16 +5,15 @@ import {
   exchangeAccessForApplicationTokens,
   exchangeCustomPartnerToken,
   ExchangeScopes,
-  refreshAccessToken,
   InvalidGrantError,
   InvalidRequestError,
 } from './session/exchange.js'
 import {IdentityToken, Session, Sessions} from './session/schema.js'
 import * as sessionStore from './session/store.js'
-import {pollForDeviceAuthorization, requestDeviceAuthorization} from './session/device-authorization.js'
 import {isThemeAccessSession} from './api/rest.js'
 import {getCurrentSessionId, setCurrentSessionId} from './conf-store.js'
 import {UserEmailQueryString, UserEmailQuery} from './api/graphql/business-platform-destinations/user-email.js'
+import {getIdentityClient} from './clients/identity/instance.js'
 import {outputContent, outputToken, outputDebug, outputCompleted} from '../../public/node/output.js'
 import {firstPartyDev, themeToken} from '../../public/node/context/local.js'
 import {AbortError} from '../../public/node/error.js'
@@ -306,13 +305,7 @@ async function executeCompleteFlow(applications: OAuthApplications): Promise<Ses
   if (identityTokenInformation) {
     identityToken = buildIdentityTokenFromEnv(scopes, identityTokenInformation)
   } else {
-    // Request a device code to authorize without a browser redirect.
-    outputDebug(outputContent`Requesting device authorization code...`)
-    const deviceAuth = await requestDeviceAuthorization(scopes)
-
-    // Poll for the identity token
-    outputDebug(outputContent`Starting polling for the identity token...`)
-    identityToken = await pollForDeviceAuthorization(deviceAuth.deviceCode, deviceAuth.interval)
+    identityToken = await getIdentityClient().requestAccessToken(scopes)
   }
 
   // Exchange identity token for application tokens
@@ -343,7 +336,7 @@ async function executeCompleteFlow(applications: OAuthApplications): Promise<Ses
  */
 async function refreshTokens(session: Session, applications: OAuthApplications): Promise<Session> {
   // Refresh Identity Token
-  const identityToken = await refreshAccessToken(session.identity)
+  const identityToken = await getIdentityClient().refreshAccessToken(session.identity)
   // Exchange new identity token for application tokens
   const exchangeScopes = getExchangeScopes(applications)
   const applicationTokens = await exchangeAccessForApplicationTokens(

packages/cli-kit/src/private/node/session/device-authorization.test.ts

@@ -3,7 +3,6 @@ import {
   pollForDeviceAuthorization,
   requestDeviceAuthorization,
 } from './device-authorization.js'
-import {clientId} from './identity.js'
 import {IdentityToken} from './schema.js'
 import {exchangeDeviceCodeForAccessToken} from './exchange.js'
 import {identityFqdn} from '../../../public/node/context/fqdn.js'
@@ -50,7 +49,6 @@ describe('requestDeviceAuthorization', () => {
     const response = new Response(JSON.stringify(data))
     vi.mocked(shopifyFetch).mockResolvedValue(response)
     vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
-    vi.mocked(clientId).mockReturnValue('clientId')
 
     // When
     const got = await requestDeviceAuthorization(['scope1', 'scope2'])
@@ -59,7 +57,7 @@ describe('requestDeviceAuthorization', () => {
     expect(shopifyFetch).toBeCalledWith('https://fqdn.com/oauth/device_authorization', {
       method: 'POST',
       headers: {'Content-type': 'application/x-www-form-urlencoded'},
-      body: 'client_id=clientId&scope=scope1 scope2',
+      body: 'client_id=fbdb2649-e327-4907-8f67-908d24cfd7e3&scope=scope1 scope2',
     })
     expect(got).toEqual(dataExpected)
   })
@@ -71,7 +69,6 @@ describe('requestDeviceAuthorization', () => {
     Object.defineProperty(response, 'statusText', {value: 'OK'})
     vi.mocked(shopifyFetch).mockResolvedValue(response)
     vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
-    vi.mocked(clientId).mockReturnValue('clientId')
 
     // When/Then
     await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrowError(
@@ -86,7 +83,6 @@ describe('requestDeviceAuthorization', () => {
     Object.defineProperty(response, 'statusText', {value: 'OK'})
     vi.mocked(shopifyFetch).mockResolvedValue(response)
     vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
-    vi.mocked(clientId).mockReturnValue('clientId')
 
     // When/Then
     await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrowError(
@@ -102,7 +98,6 @@ describe('requestDeviceAuthorization', () => {
     Object.defineProperty(response, 'statusText', {value: 'Not Found'})
     vi.mocked(shopifyFetch).mockResolvedValue(response)
     vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
-    vi.mocked(clientId).mockReturnValue('clientId')
 
     // When/Then
     await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrowError(
@@ -117,7 +112,6 @@ describe('requestDeviceAuthorization', () => {
     Object.defineProperty(response, 'statusText', {value: 'Internal Server Error'})
     vi.mocked(shopifyFetch).mockResolvedValue(response)
     vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
-    vi.mocked(clientId).mockReturnValue('clientId')
 
     // When/Then
     await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrowError(
@@ -134,7 +128,6 @@ describe('requestDeviceAuthorization', () => {
     response.text = vi.fn().mockRejectedValue(new Error('Network error'))
     vi.mocked(shopifyFetch).mockResolvedValue(response)
     vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
-    vi.mocked(clientId).mockReturnValue('clientId')
 
     // When/Then
     await expect(requestDeviceAuthorization(['scope1', 'scope2'])).rejects.toThrowError(

packages/cli-kit/src/private/node/session/device-authorization.ts

@@ -1,4 +1,3 @@
-import {clientId} from './identity.js'
 import {exchangeDeviceCodeForAccessToken} from './exchange.js'
 import {IdentityToken} from './schema.js'
 import {identityFqdn} from '../../../public/node/context/fqdn.js'
@@ -8,6 +7,7 @@ import {AbortError, BugError} from '../../../public/node/error.js'
 import {isCloudEnvironment} from '../../../public/node/context/local.js'
 import {isCI, openURL} from '../../../public/node/system.js'
 import {isTTY, keypress} from '../../../public/node/ui.js'
+import {getIdentityClient} from '../clients/identity/instance.js'
 import {Response} from 'node-fetch'
 
 export interface DeviceAuthorizationResponse {
@@ -31,7 +31,7 @@ export interface DeviceAuthorizationResponse {
  */
 export async function requestDeviceAuthorization(scopes: string[]): Promise<DeviceAuthorizationResponse> {
   const fqdn = await identityFqdn()
-  const identityClientId = clientId()
+  const identityClientId = getIdentityClient().clientId()
   const queryParams = {client_id: identityClientId, scope: scopes.join(' ')}
   const url = `https://${fqdn}/oauth/device_authorization`
 

packages/cli-kit/src/private/node/session/exchange.test.ts

@@ -8,7 +8,7 @@ import {
   refreshAccessToken,
   requestAppToken,
 } from './exchange.js'
-import {applicationId, clientId} from './identity.js'
+import {applicationId} from './identity.js'
 import {IdentityToken} from './schema.js'
 import {shopifyFetch} from '../../../public/node/http.js'
 import {identityFqdn} from '../../../public/node/context/fqdn.js'
@@ -43,7 +43,6 @@ vi.mock('../../../public/node/context/fqdn.js')
 vi.mock('./identity')
 
 beforeEach(() => {
-  vi.mocked(clientId).mockReturnValue('clientId')
   vi.setSystemTime(currentDate)
   vi.mocked(applicationId).mockImplementation((api) => api)
   vi.mocked(identityFqdn).mockResolvedValue('fqdn.com')
@@ -301,7 +300,7 @@ describe.each(tokenExchangeMethods)(
       expect(params.get('grant_type')).toBe(grantType)
       expect(params.get('requested_token_type')).toBe(accessTokenType)
       expect(params.get('subject_token_type')).toBe(accessTokenType)
-      expect(params.get('client_id')).toBe('clientId')
+      expect(params.get('client_id')).toBe('fbdb2649-e327-4907-8f67-908d24cfd7e3')
       expect(params.get('audience')).toBe(expectedApi)
       expect(params.get('scope')).toBe(expectedScopes.join(' '))
       expect(params.get('subject_token')).toBe(cliToken)

packages/cli-kit/src/private/node/session/exchange.ts

@@ -1,5 +1,5 @@
 import {ApplicationToken, IdentityToken} from './schema.js'
-import {applicationId, clientId as getIdentityClientId} from './identity.js'
+import {applicationId} from './identity.js'
 import {tokenExchangeScopes} from './scopes.js'
 import {API} from '../api.js'
 import {identityFqdn} from '../../../public/node/context/fqdn.js'
@@ -8,6 +8,7 @@ import {err, ok, Result} from '../../../public/node/result.js'
 import {AbortError, BugError, ExtendableError} from '../../../public/node/error.js'
 import {setLastSeenAuthMethod, setLastSeenUserIdAfterAuth} from '../session.js'
 import {nonRandomUUID} from '../../../public/node/crypto.js'
+import {getIdentityClient} from '../clients/identity/instance.js'
 import * as jose from 'jose'
 
 export class InvalidGrantError extends ExtendableError {}
@@ -56,14 +57,14 @@ export async function exchangeAccessForApplicationTokens(
  * Given an expired access token, refresh it to get a new one.
  */
 export async function refreshAccessToken(currentToken: IdentityToken): Promise<IdentityToken> {
-  const clientId = getIdentityClientId()
+  const clientId = getIdentityClient().clientId()
   const params = {
     grant_type: 'refresh_token',
     access_token: currentToken.accessToken,
     refresh_token: currentToken.refreshToken,
     client_id: clientId,
   }
-  const tokenResult = await tokenRequest(params)
+  const tokenResult = await getIdentityClient().tokenRequest(params)
   const value = tokenResult.mapError(tokenRequestErrorHandler).valueOrBug()
   return buildIdentityToken(value, currentToken.userId, currentToken.alias)
 }
@@ -141,15 +142,15 @@ type IdentityDeviceError = 'authorization_pending' | 'access_denied' | 'expired_
 export async function exchangeDeviceCodeForAccessToken(
   deviceCode: string,
 ): Promise<Result<IdentityToken, IdentityDeviceError>> {
-  const clientId = await getIdentityClientId()
+  const clientId = getIdentityClient().clientId()
 
   const params = {
     grant_type: 'urn:ietf:params:oauth:grant-type:device_code',
     device_code: deviceCode,
     client_id: clientId,
   }
 
-  const tokenResult = await tokenRequest(params)
+  const tokenResult = await getIdentityClient().tokenRequest(params)
   if (tokenResult.isErr()) {
     return err(tokenResult.error.error as IdentityDeviceError)
   }
@@ -164,7 +165,7 @@ export async function requestAppToken(
   store?: string,
 ): Promise<{[x: string]: ApplicationToken}> {
   const appId = applicationId(api)
-  const clientId = await getIdentityClientId()
+  const clientId = getIdentityClient().clientId()
 
   const params = {
     grant_type: 'urn:ietf:params:oauth:grant-type:token-exchange',
@@ -181,7 +182,7 @@ export async function requestAppToken(
   if (api === 'admin' && store) {
     identifier = `${store}-${appId}`
   }
-  const tokenResult = await tokenRequest(params)
+  const tokenResult = await getIdentityClient().tokenRequest(params)
   const value = tokenResult.mapError(tokenRequestErrorHandler).valueOrBug()
   const appToken = buildApplicationToken(value)
   return {[identifier]: appToken}
@@ -221,7 +222,7 @@ export function tokenRequestErrorHandler({error, store}: {error: string; store?:
   return new AbortError(error)
 }
 
-async function tokenRequest(params: {
+async function _tokenRequest(params: {
   [key: string]: string
 }): Promise<Result<TokenRequestResult, {error: string; store?: string}>> {
   const fqdn = await identityFqdn()