From 904cc561e9f1e44c40fa056b0c5c140ed893aa09 Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Tue, 3 Mar 2026 09:32:54 +0000 Subject: [PATCH 1/2] Remove token exchange step from authentication flow Made-with: Cursor --- .../cli-kit/src/private/node/session.test.ts | 107 ++++++++++-------- packages/cli-kit/src/private/node/session.ts | 73 +++--------- .../src/private/node/session/exchange.ts | 2 +- .../ui-extensions-dev-console/package.json | 1 - pnpm-lock.yaml | 10 -- 5 files changed, 75 insertions(+), 118 deletions(-) diff --git a/packages/cli-kit/src/private/node/session.test.ts b/packages/cli-kit/src/private/node/session.test.ts index 08eda6acff..8262b2f3aa 100644 --- a/packages/cli-kit/src/private/node/session.test.ts +++ b/packages/cli-kit/src/private/node/session.test.ts @@ -50,35 +50,13 @@ const validIdentityToken: IdentityToken = { } const validTokens: OAuthSession = { - admin: {token: 'admin_token', storeFqdn: 'mystore.myshopify.com'}, - storefront: 'storefront_token', - partners: 'partners_token', + admin: {token: 'access_token', storeFqdn: 'mystore.myshopify.com'}, + storefront: 'access_token', + partners: 'access_token', userId, } -const appTokens: Record = { - // Admin APIs includes domain in the key - 'mystore.myshopify.com-admin': { - accessToken: 'admin_token', - expiresAt: futureDate, - scopes: ['scope', 'scope2'], - }, - 'storefront-renderer': { - accessToken: 'storefront_token', - expiresAt: futureDate, - scopes: ['scope1'], - }, - partners: { - accessToken: 'partners_token', - expiresAt: futureDate, - scopes: ['scope2'], - }, - 'business-platform': { - accessToken: 'business_platform_token', - expiresAt: futureDate, - scopes: ['scope3'], - }, -} +const appTokens: Record = {} const partnersToken: ApplicationToken = { accessToken: 'custom_partners_token', @@ -162,7 +140,7 @@ describe('ensureAuthenticated when previous session is invalid', () => { const got = await ensureAuthenticated(defaultApplications) // Then - expect(exchangeAccessForApplicationTokens).toBeCalled() + expect(exchangeAccessForApplicationTokens).not.toBeCalled() expect(refreshAccessToken).not.toBeCalled() expect(businessPlatformRequest).toHaveBeenCalled() expect(storeSessions).toHaveBeenCalledOnce() @@ -211,7 +189,7 @@ The CLI is currently unable to prompt for reauthentication.`, ...validIdentityToken, alias: 'user@example.com', }, - applications: appTokens, + applications: {}, }, }, } @@ -220,7 +198,7 @@ The CLI is currently unable to prompt for reauthentication.`, const got = await ensureAuthenticated(defaultApplications) // Then - expect(exchangeAccessForApplicationTokens).toBeCalled() + expect(exchangeAccessForApplicationTokens).not.toBeCalled() expect(refreshAccessToken).not.toBeCalled() expect(storeSessions).toBeCalledWith(expectedSessions) expect(got).toEqual(validTokens) @@ -239,7 +217,7 @@ The CLI is currently unable to prompt for reauthentication.`, const got = await ensureAuthenticated(defaultApplications) // Then - expect(exchangeAccessForApplicationTokens).toBeCalled() + expect(exchangeAccessForApplicationTokens).not.toBeCalled() expect(businessPlatformRequest).toHaveBeenCalled() expect(storeSessions).toHaveBeenCalledOnce() @@ -250,26 +228,20 @@ The CLI is currently unable to prompt for reauthentication.`, expect(got).toEqual(validTokens) }) - test('falls back to userId when no business platform token available', async () => { + test('uses identity token to fetch email during full auth flow', async () => { // Given vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth') vi.mocked(fetchSessions).mockResolvedValue(undefined) - const appTokensWithoutBusinessPlatform = { - 'mystore.myshopify.com-admin': appTokens['mystore.myshopify.com-admin']!, - 'storefront-renderer': appTokens['storefront-renderer']!, - partners: appTokens.partners!, - } - vi.mocked(exchangeAccessForApplicationTokens).mockResolvedValueOnce(appTokensWithoutBusinessPlatform) // When const got = await ensureAuthenticated(defaultApplications) // Then - expect(businessPlatformRequest).not.toHaveBeenCalled() + expect(businessPlatformRequest).toHaveBeenCalledWith(expect.any(String), 'access_token') - // Verify the session was stored with userId as alias (fallback) + // Verify the session was stored with email as alias const storedSession = vi.mocked(storeSessions).mock.calls[0]![0] - expect(storedSession[fqdn]![userId]!.identity.alias).toBe(userId) + expect(storedSession[fqdn]![userId]!.identity.alias).toBe('user@example.com') }) test('executes complete auth flow if requesting additional scopes', async () => { @@ -281,7 +253,7 @@ The CLI is currently unable to prompt for reauthentication.`, const got = await ensureAuthenticated(defaultApplications) // Then - expect(exchangeAccessForApplicationTokens).toBeCalled() + expect(exchangeAccessForApplicationTokens).not.toBeCalled() expect(refreshAccessToken).not.toBeCalled() expect(businessPlatformRequest).toHaveBeenCalled() expect(storeSessions).toHaveBeenCalledOnce() @@ -320,7 +292,12 @@ describe('when existing session is valid', () => { vi.mocked(validateSession).mockResolvedValueOnce('ok') vi.mocked(fetchSessions).mockResolvedValue(validSessions) vi.mocked(getPartnersToken).mockReturnValue('custom_cli_token') - const expected = {...validTokens, partners: 'custom_partners_token'} + const expected = { + admin: {token: 'access_token', storeFqdn: 'mystore.myshopify.com'}, + storefront: 'access_token', + partners: 'custom_partners_token', + userId, + } // When const got = await ensureAuthenticated(defaultApplications) @@ -344,8 +321,16 @@ describe('when existing session is valid', () => { // Then expect(refreshAccessToken).toBeCalled() - expect(exchangeAccessForApplicationTokens).toBeCalled() - expect(storeSessions).toBeCalledWith(validSessions) + expect(exchangeAccessForApplicationTokens).not.toBeCalled() + const expectedSessions = { + [fqdn]: { + [userId]: { + identity: validIdentityToken, + applications: {}, + }, + }, + } + expect(storeSessions).toBeCalledWith(expectedSessions) expect(got).toEqual(validTokens) await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678') await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth') @@ -364,8 +349,16 @@ describe('when existing session is expired', () => { // Then expect(refreshAccessToken).toBeCalled() - expect(exchangeAccessForApplicationTokens).toBeCalled() - expect(storeSessions).toBeCalledWith(validSessions) + expect(exchangeAccessForApplicationTokens).not.toBeCalled() + const expectedSessions = { + [fqdn]: { + [userId]: { + identity: validIdentityToken, + applications: {}, + }, + }, + } + expect(storeSessions).toBeCalledWith(expectedSessions) expect(got).toEqual(validTokens) await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678') await expect(getLastSeenAuthMethod()).resolves.toEqual('device_auth') @@ -385,7 +378,7 @@ describe('when existing session is expired', () => { // Then expect(refreshAccessToken).toBeCalled() - expect(exchangeAccessForApplicationTokens).toBeCalled() + expect(exchangeAccessForApplicationTokens).not.toBeCalled() expect(businessPlatformRequest).toHaveBeenCalled() expect(storeSessions).toHaveBeenCalledOnce() @@ -644,7 +637,15 @@ describe('ensureAuthenticated email fetch functionality', () => { const got = await ensureAuthenticated(defaultApplications) // Then - expect(storeSessions).toBeCalledWith(validSessions) + const expectedSessions = { + [fqdn]: { + [userId]: { + identity: validIdentityToken, + applications: {}, + }, + }, + } + expect(storeSessions).toBeCalledWith(expectedSessions) expect(got).toEqual(validTokens) }) @@ -659,7 +660,15 @@ describe('ensureAuthenticated email fetch functionality', () => { // Then // The email fetch is not called during refresh - the session keeps its existing alias expect(businessPlatformRequest).not.toHaveBeenCalled() - expect(storeSessions).toBeCalledWith(validSessions) + const expectedSessions = { + [fqdn]: { + [userId]: { + identity: validIdentityToken, + applications: {}, + }, + }, + } + expect(storeSessions).toBeCalledWith(expectedSessions) expect(got).toEqual(validTokens) }) diff --git a/packages/cli-kit/src/private/node/session.ts b/packages/cli-kit/src/private/node/session.ts index 46d48594c4..7bb565adab 100644 --- a/packages/cli-kit/src/private/node/session.ts +++ b/packages/cli-kit/src/private/node/session.ts @@ -1,10 +1,7 @@ -import {applicationId} from './session/identity.js' import {validateSession} from './session/validate.js' -import {allDefaultScopes, apiScopes} from './session/scopes.js' +import {allDefaultScopes} from './session/scopes.js' import { - exchangeAccessForApplicationTokens, exchangeCustomPartnerToken, - ExchangeScopes, refreshAccessToken, InvalidGrantError, InvalidRequestError, @@ -293,8 +290,6 @@ The CLI is currently unable to prompt for reauthentication.`, */ async function executeCompleteFlow(applications: OAuthApplications): Promise { const scopes = getFlattenScopes(applications) - const exchangeScopes = getExchangeScopes(applications) - const store = applications.adminApi?.storeFqdn if (firstPartyDev()) { outputDebug(outputContent`Authenticating as Shopify Employee...`) scopes.push('employee') @@ -314,20 +309,18 @@ async function executeCompleteFlow(applications: OAuthApplications): Promise { - // Refresh Identity Token +async function refreshTokens(session: Session, _applications: OAuthApplications): Promise { + // Refresh Identity Token (PCAT) - no exchange needed const identityToken = await refreshAccessToken(session.identity) - // Exchange new identity token for application tokens - const exchangeScopes = getExchangeScopes(applications) - const applicationTokens = await exchangeAccessForApplicationTokens( - identityToken, - exchangeScopes, - applications.adminApi?.storeFqdn, - ) return { identity: identityToken, - applications: applicationTokens, + applications: {}, } } /** * Get the application tokens for a given session. + * With PCAT, the identity token can be used directly for all APIs. * * @param applications - An object containing the applications we need the tokens for. * @param session - The current session. @@ -369,33 +356,26 @@ async function tokensFor(applications: OAuthApplications, session: Session): Pro userId: session.identity.userId, } + const pcatToken = session.identity.accessToken + if (applications.adminApi) { - const appId = applicationId('admin') - const realAppId = `${applications.adminApi.storeFqdn}-${appId}` - const token = session.applications[realAppId]?.accessToken - if (token) { - tokens.admin = {token, storeFqdn: applications.adminApi.storeFqdn} - } + tokens.admin = {token: pcatToken, storeFqdn: applications.adminApi.storeFqdn} } if (applications.partnersApi) { - const appId = applicationId('partners') - tokens.partners = session.applications[appId]?.accessToken + tokens.partners = pcatToken } if (applications.storefrontRendererApi) { - const appId = applicationId('storefront-renderer') - tokens.storefront = session.applications[appId]?.accessToken + tokens.storefront = pcatToken } if (applications.businessPlatformApi) { - const appId = applicationId('business-platform') - tokens.businessPlatform = session.applications[appId]?.accessToken + tokens.businessPlatform = pcatToken } if (applications.appManagementApi) { - const appId = applicationId('app-management') - tokens.appManagement = session.applications[appId]?.accessToken + tokens.appManagement = pcatToken } return tokens @@ -418,27 +398,6 @@ function getFlattenScopes(apps: OAuthApplications): string[] { return allDefaultScopes(requestedScopes) } -/** - * Get the scopes for the given applications. - * - * @param apps - An object containing the applications we need the scopes for. - * @returns An object containing the scopes for each application. - */ -function getExchangeScopes(apps: OAuthApplications): ExchangeScopes { - const adminScope = apps.adminApi?.scopes ?? [] - const partnerScope = apps.partnersApi?.scopes ?? [] - const storefrontScopes = apps.storefrontRendererApi?.scopes ?? [] - const businessPlatformScopes = apps.businessPlatformApi?.scopes ?? [] - const appManagementScopes = apps.appManagementApi?.scopes ?? [] - return { - admin: apiScopes('admin', adminScope), - partners: apiScopes('partners', partnerScope), - storefront: apiScopes('storefront-renderer', storefrontScopes), - businessPlatform: apiScopes('business-platform', businessPlatformScopes), - appManagement: apiScopes('app-management', appManagementScopes), - } -} - function buildIdentityTokenFromEnv( scopes: string[], identityTokenInformation: {accessToken: string; refreshToken: string; userId: string}, diff --git a/packages/cli-kit/src/private/node/session/exchange.ts b/packages/cli-kit/src/private/node/session/exchange.ts index ebf49f3d6a..2b83a24ed6 100644 --- a/packages/cli-kit/src/private/node/session/exchange.ts +++ b/packages/cli-kit/src/private/node/session/exchange.ts @@ -15,7 +15,7 @@ export class InvalidGrantError extends ExtendableError {} export class InvalidRequestError extends ExtendableError {} class InvalidTargetError extends AbortError {} -export interface ExchangeScopes { +interface ExchangeScopes { admin: string[] partners: string[] storefront: string[] diff --git a/packages/ui-extensions-dev-console/package.json b/packages/ui-extensions-dev-console/package.json index 0db840bda6..5eba7dc073 100644 --- a/packages/ui-extensions-dev-console/package.json +++ b/packages/ui-extensions-dev-console/package.json @@ -26,7 +26,6 @@ "devDependencies": { "@shopify/react-testing": "^3.0.0", "@shopify/ui-extensions-test-utils": "3.26.0", - "@types/qrcode.react": "^1.0.2", "@types/react": "16.14.0", "@types/react-dom": "^16.9.11", "@vitejs/plugin-react-refresh": "^1.3.6", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 9494c8dfb3..caa4dc153a 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -760,9 +760,6 @@ importers: '@shopify/ui-extensions-test-utils': specifier: 3.26.0 version: link:../ui-extensions-test-utils - '@types/qrcode.react': - specifier: ^1.0.2 - version: 1.0.5 '@types/react': specifier: 18.3.12 version: 18.3.12 @@ -4116,9 +4113,6 @@ packages: '@types/proper-lockfile@4.1.4': resolution: {integrity: sha512-uo2ABllncSqg9F1D4nugVl9v93RmjxF6LJzQLMLDdPaXCUIDPeOJ21Gbqi43xNKzBi/WQ0Q0dICqufzQbMjipQ==} - '@types/qrcode.react@1.0.5': - resolution: {integrity: sha512-BghPtnlwvrvq8QkGa1H25YnN+5OIgCKFuQruncGWLGJYOzeSKiix/4+B9BtfKF2wf5ja8yfyWYA3OXju995G8w==} - '@types/qs@6.14.0': resolution: {integrity: sha512-eOunJqu0K1923aExK6y8p6fsihYEn/BYuQ4g0CxAAgFc4b/ZLN4CrsRZ55srTdqoiLzU2B2evC+apEIxprEzkQ==} @@ -14056,10 +14050,6 @@ snapshots: dependencies: '@types/retry': 0.12.5 - '@types/qrcode.react@1.0.5': - dependencies: - '@types/react': 18.3.12 - '@types/qs@6.14.0': {} '@types/range-parser@1.2.7': {} From aa7ad7f17b7a623696a6230ce629f480b437ef7e Mon Sep 17 00:00:00 2001 From: Gonzalo Riestra Date: Tue, 3 Mar 2026 12:35:10 +0100 Subject: [PATCH 2/2] Additional cleanup --- .../cli-kit/src/private/node/session.test.ts | 53 ++---- packages/cli-kit/src/private/node/session.ts | 34 +--- .../src/private/node/session/exchange.test.ts | 83 ---------- .../src/private/node/session/exchange.ts | 38 ----- .../src/private/node/session/validate.test.ts | 155 ++---------------- .../src/private/node/session/validate.ts | 54 +----- .../ui-extensions-dev-console/package.json | 1 + pnpm-lock.yaml | 10 ++ 8 files changed, 52 insertions(+), 376 deletions(-) diff --git a/packages/cli-kit/src/private/node/session.test.ts b/packages/cli-kit/src/private/node/session.test.ts index 8262b2f3aa..6b9c9a0729 100644 --- a/packages/cli-kit/src/private/node/session.test.ts +++ b/packages/cli-kit/src/private/node/session.test.ts @@ -7,17 +7,11 @@ import { setLastSeenAuthMethod, setLastSeenUserIdAfterAuth, } from './session.js' -import { - exchangeAccessForApplicationTokens, - exchangeCustomPartnerToken, - refreshAccessToken, - InvalidGrantError, -} from './session/exchange.js' +import {exchangeCustomPartnerToken, refreshAccessToken, InvalidGrantError} from './session/exchange.js' import {allDefaultScopes} from './session/scopes.js' import {store as storeSessions, fetch as fetchSessions, remove as secureRemove} from './session/store.js' -import {ApplicationToken, IdentityToken, Sessions} from './session/schema.js' +import {IdentityToken, Sessions} from './session/schema.js' 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 * as fqdnModule from '../../public/node/context/fqdn.js' @@ -56,26 +50,9 @@ const validTokens: OAuthSession = { userId, } -const appTokens: Record = {} - -const partnersToken: ApplicationToken = { - accessToken: 'custom_partners_token', - expiresAt: futureDate, - scopes: ['scope2'], -} - const fqdn = 'fqdn.com' const validSessions: Sessions = { - [fqdn]: { - [userId]: { - identity: validIdentityToken, - applications: appTokens, - }, - }, -} - -const invalidSessions: Sessions = { [fqdn]: { [userId]: { identity: validIdentityToken, @@ -85,7 +62,6 @@ const invalidSessions: Sessions = { } vi.mock('../../public/node/context/local.js') -vi.mock('./session/identity') vi.mock('./session/authorize') vi.mock('./session/exchange') vi.mock('./session/scopes') @@ -101,11 +77,9 @@ vi.mock('../../public/node/system.js') beforeEach(() => { vi.spyOn(fqdnModule, 'identityFqdn').mockResolvedValue(fqdn) - vi.mocked(exchangeAccessForApplicationTokens).mockResolvedValue(appTokens) vi.mocked(refreshAccessToken).mockResolvedValue(validIdentityToken) - vi.mocked(applicationId).mockImplementation((app) => app) vi.mocked(exchangeCustomPartnerToken).mockResolvedValue({ - accessToken: partnersToken.accessToken, + accessToken: 'custom_partners_token', userId: validIdentityToken.userId, }) vi.mocked(partnersRequest).mockResolvedValue(undefined) @@ -140,7 +114,6 @@ describe('ensureAuthenticated when previous session is invalid', () => { const got = await ensureAuthenticated(defaultApplications) // Then - expect(exchangeAccessForApplicationTokens).not.toBeCalled() expect(refreshAccessToken).not.toBeCalled() expect(businessPlatformRequest).toHaveBeenCalled() expect(storeSessions).toHaveBeenCalledOnce() @@ -180,9 +153,9 @@ The CLI is currently unable to prompt for reauthentication.`, test('executes complete auth flow if session is for a different fqdn', async () => { // Given vi.mocked(validateSession).mockResolvedValueOnce('needs_full_auth') - vi.mocked(fetchSessions).mockResolvedValue(invalidSessions) + vi.mocked(fetchSessions).mockResolvedValue(validSessions) const expectedSessions = { - ...invalidSessions, + ...validSessions, [fqdn]: { [userId]: { identity: { @@ -198,7 +171,7 @@ The CLI is currently unable to prompt for reauthentication.`, const got = await ensureAuthenticated(defaultApplications) // Then - expect(exchangeAccessForApplicationTokens).not.toBeCalled() + expect(refreshAccessToken).not.toBeCalled() expect(storeSessions).toBeCalledWith(expectedSessions) expect(got).toEqual(validTokens) @@ -217,7 +190,7 @@ The CLI is currently unable to prompt for reauthentication.`, const got = await ensureAuthenticated(defaultApplications) // Then - expect(exchangeAccessForApplicationTokens).not.toBeCalled() + expect(businessPlatformRequest).toHaveBeenCalled() expect(storeSessions).toHaveBeenCalledOnce() @@ -253,7 +226,7 @@ The CLI is currently unable to prompt for reauthentication.`, const got = await ensureAuthenticated(defaultApplications) // Then - expect(exchangeAccessForApplicationTokens).not.toBeCalled() + expect(refreshAccessToken).not.toBeCalled() expect(businessPlatformRequest).toHaveBeenCalled() expect(storeSessions).toHaveBeenCalledOnce() @@ -279,7 +252,7 @@ describe('when existing session is valid', () => { const got = await ensureAuthenticated(defaultApplications) // Then - expect(exchangeAccessForApplicationTokens).not.toBeCalled() + expect(refreshAccessToken).not.toBeCalled() expect(got).toEqual(validTokens) await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678') @@ -303,7 +276,7 @@ describe('when existing session is valid', () => { const got = await ensureAuthenticated(defaultApplications) // Then - expect(exchangeAccessForApplicationTokens).not.toBeCalled() + expect(refreshAccessToken).not.toBeCalled() expect(got).toEqual(expected) await expect(getLastSeenUserIdAfterAuth()).resolves.toBe('1234-5678') @@ -321,7 +294,7 @@ describe('when existing session is valid', () => { // Then expect(refreshAccessToken).toBeCalled() - expect(exchangeAccessForApplicationTokens).not.toBeCalled() + const expectedSessions = { [fqdn]: { [userId]: { @@ -349,7 +322,7 @@ describe('when existing session is expired', () => { // Then expect(refreshAccessToken).toBeCalled() - expect(exchangeAccessForApplicationTokens).not.toBeCalled() + const expectedSessions = { [fqdn]: { [userId]: { @@ -378,7 +351,7 @@ describe('when existing session is expired', () => { // Then expect(refreshAccessToken).toBeCalled() - expect(exchangeAccessForApplicationTokens).not.toBeCalled() + expect(businessPlatformRequest).toHaveBeenCalled() expect(storeSessions).toHaveBeenCalledOnce() diff --git a/packages/cli-kit/src/private/node/session.ts b/packages/cli-kit/src/private/node/session.ts index 7bb565adab..ed3cdf9105 100644 --- a/packages/cli-kit/src/private/node/session.ts +++ b/packages/cli-kit/src/private/node/session.ts @@ -220,7 +220,7 @@ For applications: ${outputToken.json(applications)} `) - const validationResult = await validateSession(scopes, applications, currentSession) + const validationResult = await validateSession(scopes, currentSession) let newSession = {} @@ -352,33 +352,15 @@ async function refreshTokens(session: Session, _applications: OAuthApplications) * @param fqdn - The identity FQDN. */ async function tokensFor(applications: OAuthApplications, session: Session): Promise { - const tokens: OAuthSession = { + const token = session.identity.accessToken + return { userId: session.identity.userId, + admin: applications.adminApi ? {token, storeFqdn: applications.adminApi.storeFqdn} : undefined, + partners: applications.partnersApi ? token : undefined, + storefront: applications.storefrontRendererApi ? token : undefined, + businessPlatform: applications.businessPlatformApi ? token : undefined, + appManagement: applications.appManagementApi ? token : undefined, } - - const pcatToken = session.identity.accessToken - - if (applications.adminApi) { - tokens.admin = {token: pcatToken, storeFqdn: applications.adminApi.storeFqdn} - } - - if (applications.partnersApi) { - tokens.partners = pcatToken - } - - if (applications.storefrontRendererApi) { - tokens.storefront = pcatToken - } - - if (applications.businessPlatformApi) { - tokens.businessPlatform = pcatToken - } - - if (applications.appManagementApi) { - tokens.appManagement = pcatToken - } - - return tokens } // Scope Helpers diff --git a/packages/cli-kit/src/private/node/session/exchange.test.ts b/packages/cli-kit/src/private/node/session/exchange.test.ts index 22befb2f7b..8204ee3504 100644 --- a/packages/cli-kit/src/private/node/session/exchange.test.ts +++ b/packages/cli-kit/src/private/node/session/exchange.test.ts @@ -1,5 +1,4 @@ import { - exchangeAccessForApplicationTokens, exchangeCustomPartnerToken, exchangeCliTokenForAppManagementAccessToken, exchangeCliTokenForBusinessPlatformAccessToken, @@ -55,88 +54,6 @@ afterAll(() => { vi.useRealTimers() }) -describe('exchange identity token for application tokens', () => { - const scopes = {admin: [], partners: [], storefront: [], businessPlatform: [], appManagement: []} - - test('returns tokens for all APIs if a store is passed', async () => { - // Given - vi.mocked(shopifyFetch).mockImplementation(async () => Promise.resolve(new Response(JSON.stringify(data)))) - - // When - const got = await exchangeAccessForApplicationTokens(identityToken, scopes, 'storeFQDN') - - // Then - const expected = { - 'app-management': { - accessToken: 'access_token', - expiresAt: expiredDate, - scopes: ['scope', 'scope2'], - }, - partners: { - accessToken: 'access_token', - expiresAt: expiredDate, - scopes: ['scope', 'scope2'], - }, - 'storefront-renderer': { - accessToken: 'access_token', - expiresAt: expiredDate, - scopes: ['scope', 'scope2'], - }, - 'storeFQDN-admin': { - accessToken: 'access_token', - expiresAt: expiredDate, - scopes: ['scope', 'scope2'], - }, - 'business-platform': { - accessToken: 'access_token', - expiresAt: expiredDate, - scopes: ['scope', 'scope2'], - }, - } - expect(got).toEqual(expected) - }) - - test('does not return token for admin if there is no store', async () => { - // Given - const response = new Response(JSON.stringify(data)) - - // Need to do it 3 times because a Response can only be used once - vi.mocked(shopifyFetch) - .mockResolvedValue(response) - .mockResolvedValueOnce(response.clone()) - .mockResolvedValueOnce(response.clone()) - .mockResolvedValueOnce(response.clone()) - - // When - const got = await exchangeAccessForApplicationTokens(identityToken, scopes, undefined) - - // Then - const expected = { - 'app-management': { - accessToken: 'access_token', - expiresAt: expiredDate, - scopes: ['scope', 'scope2'], - }, - partners: { - accessToken: 'access_token', - expiresAt: expiredDate, - scopes: ['scope', 'scope2'], - }, - 'storefront-renderer': { - accessToken: 'access_token', - expiresAt: expiredDate, - scopes: ['scope', 'scope2'], - }, - 'business-platform': { - accessToken: 'access_token', - expiresAt: expiredDate, - scopes: ['scope', 'scope2'], - }, - } - expect(got).toEqual(expected) - }) -}) - describe('refresh access tokens', () => { test('throws an InvalidGrantError when Identity returns invalid_grant', async () => { // Given diff --git a/packages/cli-kit/src/private/node/session/exchange.ts b/packages/cli-kit/src/private/node/session/exchange.ts index 2b83a24ed6..635629cb1c 100644 --- a/packages/cli-kit/src/private/node/session/exchange.ts +++ b/packages/cli-kit/src/private/node/session/exchange.ts @@ -15,44 +15,6 @@ export class InvalidGrantError extends ExtendableError {} export class InvalidRequestError extends ExtendableError {} class InvalidTargetError extends AbortError {} -interface ExchangeScopes { - admin: string[] - partners: string[] - storefront: string[] - businessPlatform: string[] - appManagement: string[] -} - -/** - * Given an identity token, request an application token. - * @param identityToken - access token obtained in a previous step - * @param store - the store to use, only needed for admin API - * @returns An array with the application access tokens. - */ -export async function exchangeAccessForApplicationTokens( - identityToken: IdentityToken, - scopes: ExchangeScopes, - store?: string, -): Promise> { - const token = identityToken.accessToken - - const [partners, storefront, businessPlatform, admin, appManagement] = await Promise.all([ - requestAppToken('partners', token, scopes.partners), - requestAppToken('storefront-renderer', token, scopes.storefront), - requestAppToken('business-platform', token, scopes.businessPlatform), - store ? requestAppToken('admin', token, scopes.admin, store) : {}, - requestAppToken('app-management', token, scopes.appManagement), - ]) - - return { - ...partners, - ...storefront, - ...businessPlatform, - ...admin, - ...appManagement, - } -} - /** * Given an expired access token, refresh it to get a new one. */ diff --git a/packages/cli-kit/src/private/node/session/validate.test.ts b/packages/cli-kit/src/private/node/session/validate.test.ts index 12f8300b68..e42cb58a0b 100644 --- a/packages/cli-kit/src/private/node/session/validate.test.ts +++ b/packages/cli-kit/src/private/node/session/validate.test.ts @@ -1,13 +1,10 @@ import {validateSession} from './validate.js' -import {applicationId} from './identity.js' import {IdentityToken, validateCachedIdentityTokenStructure} from './schema.js' -import {OAuthApplications} from '../session.js' import {expect, describe, test, vi, afterAll, beforeEach} from 'vitest' const pastDate = new Date(2022, 1, 1, 9) const currentDate = new Date(2022, 1, 1, 10) const futureDate = new Date(2022, 1, 1, 11) -const storeName = 'store.myshopify.io' const requestedScopes = ['scope', 'scope2'] const validIdentity: IdentityToken = { @@ -28,197 +25,67 @@ const expiredIdentity: IdentityToken = { alias: '1234-5678', } -const validApplications = { - partners: { - accessToken: 'access_token', - expiresAt: futureDate, - scopes: ['scope'], - }, - 'storefront-renderer': { - accessToken: 'access_token', - expiresAt: futureDate, - scopes: ['scope'], - }, - [`${storeName}-admin`]: { - accessToken: 'access_token', - expiresAt: futureDate, - scopes: ['scope'], - }, -} - -const expiredApplications = { - partners: { - accessToken: 'access_token', - expiresAt: pastDate, - scopes: ['scope'], - }, - 'storefront-renderer': { - accessToken: 'access_token', - expiresAt: pastDate, - scopes: ['scope'], - }, - [`${storeName}-admin`]: { - accessToken: 'access_token', - expiresAt: pastDate, - scopes: ['scope'], - }, -} - -const defaultApps: OAuthApplications = { - partnersApi: {scopes: []}, - adminApi: {scopes: [], storeFqdn: storeName}, - storefrontRendererApi: {scopes: []}, -} - vi.mock('./identity-token-validation') -vi.mock('./identity') vi.mock('./schema') beforeEach(() => { - vi.mocked(applicationId).mockImplementation((id: any) => id) vi.mocked(validateCachedIdentityTokenStructure).mockReturnValue(true) vi.setSystemTime(currentDate) }) afterAll(() => { - // Restore Date mock vi.useRealTimers() }) describe('validateSession', () => { test('returns ok if session is valid', async () => { - // Given const session = { identity: validIdentity, - applications: validApplications, + applications: {}, } - // When - const got = await validateSession(requestedScopes, defaultApps, session) + const got = await validateSession(requestedScopes, session) - // Then expect(got).toBe('ok') }) test('returns needs_full_auth if validateCachedIdentityTokenStructure returns false', async () => { - // Given const session = { identity: validIdentity, - applications: validApplications, + applications: {}, } vi.mocked(validateCachedIdentityTokenStructure).mockReturnValueOnce(false) - // When - const got = await validateSession(requestedScopes, defaultApps, session) + const got = await validateSession(requestedScopes, session) - // Then expect(got).toBe('needs_full_auth') }) test('returns needs_full_auth if there is no session', async () => { - // Given - const session: any = undefined + const got = await validateSession(requestedScopes, undefined) - // When - const got = await validateSession(requestedScopes, defaultApps, session) - - // Then expect(got).toBe('needs_full_auth') }) - test('returns needs_full_auth if there is requested scopes are not included in token', async () => { - // Given + test('returns needs_full_auth if requested scopes are not included in token', async () => { const session = { identity: validIdentity, - applications: validApplications, + applications: {}, } - // When - const got = await validateSession(['random_scope'], defaultApps, session) + const got = await validateSession(['random_scope'], session) - // Then expect(got).toBe('needs_full_auth') }) - test('returns needs_refresh if identity is expired', async () => { - // Given + test('returns needs_refresh if identity token is expired', async () => { const session = { identity: expiredIdentity, - applications: validApplications, - } - - // When - const got = await validateSession(requestedScopes, defaultApps, session) - - // Then - expect(got).toBe('needs_refresh') - }) - - test('returns needs_refresh if requesting partners and is expired', async () => { - // Given - const applications = { - partnersApi: {scopes: []}, - } - const session = { - identity: validIdentity, - applications: expiredApplications, - } - - // When - const got = await validateSession(requestedScopes, applications, session) - - // Then - expect(got).toBe('needs_refresh') - }) - - test('returns needs_refresh if requesting storefront and is expired', async () => { - // Given - const applications = { - storefrontRendererApi: {scopes: []}, - } - const session = { - identity: validIdentity, - applications: expiredApplications, - } - - // When - const got = await validateSession(requestedScopes, applications, session) - - // Then - expect(got).toBe('needs_refresh') - }) - - test('returns needs_refresh if requesting admin and is expired', async () => { - // Given - const applications: OAuthApplications = { - adminApi: {scopes: [], storeFqdn: storeName}, - } - const session = { - identity: validIdentity, - applications: expiredApplications, - } - - // When - const got = await validateSession(requestedScopes, applications, session) - - // Then - expect(got).toBe('needs_refresh') - }) - - test('returns needs_refresh if session does not include requested store', async () => { - // Given - const applications: OAuthApplications = { - adminApi: {scopes: [], storeFqdn: 'NotMyStore'}, - } - const session = { - identity: validIdentity, - applications: validApplications, + applications: {}, } - // When - const got = await validateSession(requestedScopes, applications, session) + const got = await validateSession(requestedScopes, session) - // Then expect(got).toBe('needs_refresh') }) }) diff --git a/packages/cli-kit/src/private/node/session/validate.ts b/packages/cli-kit/src/private/node/session/validate.ts index d4b7979928..4cedd12f87 100644 --- a/packages/cli-kit/src/private/node/session/validate.ts +++ b/packages/cli-kit/src/private/node/session/validate.ts @@ -1,9 +1,7 @@ -import {ApplicationToken, IdentityToken, Session, validateCachedIdentityTokenStructure} from './schema.js' +import {IdentityToken, Session, validateCachedIdentityTokenStructure} from './schema.js' -import {applicationId} from './identity.js' import {sessionConstants} from '../constants.js' import {firstPartyDev} from '../../../public/node/context/local.js' -import {OAuthApplications} from '../session.js' import {outputDebug} from '../../../public/node/output.js' type ValidationResult = 'needs_refresh' | 'needs_full_auth' | 'ok' @@ -18,61 +16,27 @@ function validateScopes(requestedScopes: string[], identity: IdentityToken) { } /** - * Validate if the current session is valid or we need to refresh/re-authenticate + * Validate if the current session is valid or we need to refresh/re-authenticate. + * With PCAT, only the identity token needs validation - no per-application tokens. * @param scopes - requested scopes to validate - * @param applications - requested applications - * @param session - current session with identity and application tokens + * @param session - current session with identity token * @returns 'ok' if the session is valid, 'needs_full_auth' if we need to re-authenticate, 'needs_refresh' if we need to refresh the session */ -export async function validateSession( - scopes: string[], - applications: OAuthApplications, - session: Session | undefined, -): Promise { +export async function validateSession(scopes: string[], session: Session | undefined): Promise { if (!session) return 'needs_full_auth' const scopesAreValid = validateScopes(scopes, session.identity) if (!scopesAreValid) return 'needs_full_auth' - let tokensAreExpired = isTokenExpired(session.identity) - - if (applications.partnersApi) { - const appId = applicationId('partners') - const token = session.applications[appId]! - tokensAreExpired = tokensAreExpired || isTokenExpired(token) - } - - if (applications.appManagementApi) { - const appId = applicationId('app-management') - const token = session.applications[appId]! - tokensAreExpired = tokensAreExpired || isTokenExpired(token) - } - - if (applications.storefrontRendererApi) { - const appId = applicationId('storefront-renderer') - const token = session.applications[appId]! - tokensAreExpired = tokensAreExpired || isTokenExpired(token) - } - - if (applications.adminApi) { - const appId = applicationId('admin') - const realAppId = `${applications.adminApi.storeFqdn}-${appId}` - const token = session.applications[realAppId]! - tokensAreExpired = tokensAreExpired || isTokenExpired(token) - } - - outputDebug(`- Token validation -> It's expired: ${tokensAreExpired}`) if (!validateCachedIdentityTokenStructure(session.identity)) { return 'needs_full_auth' } - if (tokensAreExpired) return 'needs_refresh' + const expired = session.identity.expiresAt < expireThreshold() + outputDebug(`- Token validation -> It's expired: ${expired}`) - return 'ok' -} + if (expired) return 'needs_refresh' -function isTokenExpired(token: ApplicationToken): boolean { - if (!token) return true - return token.expiresAt < expireThreshold() + return 'ok' } function expireThreshold(): Date { diff --git a/packages/ui-extensions-dev-console/package.json b/packages/ui-extensions-dev-console/package.json index 5eba7dc073..0db840bda6 100644 --- a/packages/ui-extensions-dev-console/package.json +++ b/packages/ui-extensions-dev-console/package.json @@ -26,6 +26,7 @@ "devDependencies": { "@shopify/react-testing": "^3.0.0", "@shopify/ui-extensions-test-utils": "3.26.0", + "@types/qrcode.react": "^1.0.2", "@types/react": "16.14.0", "@types/react-dom": "^16.9.11", "@vitejs/plugin-react-refresh": "^1.3.6", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index caa4dc153a..9494c8dfb3 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -760,6 +760,9 @@ importers: '@shopify/ui-extensions-test-utils': specifier: 3.26.0 version: link:../ui-extensions-test-utils + '@types/qrcode.react': + specifier: ^1.0.2 + version: 1.0.5 '@types/react': specifier: 18.3.12 version: 18.3.12 @@ -4113,6 +4116,9 @@ packages: '@types/proper-lockfile@4.1.4': resolution: {integrity: sha512-uo2ABllncSqg9F1D4nugVl9v93RmjxF6LJzQLMLDdPaXCUIDPeOJ21Gbqi43xNKzBi/WQ0Q0dICqufzQbMjipQ==} + '@types/qrcode.react@1.0.5': + resolution: {integrity: sha512-BghPtnlwvrvq8QkGa1H25YnN+5OIgCKFuQruncGWLGJYOzeSKiix/4+B9BtfKF2wf5ja8yfyWYA3OXju995G8w==} + '@types/qs@6.14.0': resolution: {integrity: sha512-eOunJqu0K1923aExK6y8p6fsihYEn/BYuQ4g0CxAAgFc4b/ZLN4CrsRZ55srTdqoiLzU2B2evC+apEIxprEzkQ==} @@ -14050,6 +14056,10 @@ snapshots: dependencies: '@types/retry': 0.12.5 + '@types/qrcode.react@1.0.5': + dependencies: + '@types/react': 18.3.12 + '@types/qs@6.14.0': {} '@types/range-parser@1.2.7': {}