diff --git a/.changeset/encode-device-authorization-request.md b/.changeset/encode-device-authorization-request.md new file mode 100644 index 00000000000..1e7cbae8923 --- /dev/null +++ b/.changeset/encode-device-authorization-request.md @@ -0,0 +1,5 @@ +--- +'@shopify/cli-kit': patch +--- + +Encode device authorization request parameters safely. diff --git a/packages/cli-kit/src/private/node/session/device-authorization.test.ts b/packages/cli-kit/src/private/node/session/device-authorization.test.ts index e87ab4fc703..7f66b5a4ceb 100644 --- a/packages/cli-kit/src/private/node/session/device-authorization.test.ts +++ b/packages/cli-kit/src/private/node/session/device-authorization.test.ts @@ -63,11 +63,48 @@ 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=clientId&scope=scope1+scope2', }) expect(got).toEqual(dataExpected) }) + test('encodes special characters in authorization scopes', async () => { + // Given + const response = new Response(JSON.stringify(data)) + vi.mocked(shopifyFetch).mockResolvedValue(response) + vi.mocked(identityFqdn).mockResolvedValue('fqdn.com') + vi.mocked(clientId).mockReturnValue('clientId') + const scopes = ['scope&name', 'scope=value', 'scope%value'] + + // When + await requestDeviceAuthorization(scopes) + + // Then + expect(shopifyFetch).toHaveBeenCalledWith('https://fqdn.com/oauth/device_authorization', { + method: 'POST', + headers: {'Content-type': 'application/x-www-form-urlencoded'}, + body: 'client_id=clientId&scope=scope%26name+scope%3Dvalue+scope%25value', + }) + }) + + test('omits empty authorization scope values', async () => { + // Given + const response = new Response(JSON.stringify(data)) + vi.mocked(shopifyFetch).mockResolvedValue(response) + vi.mocked(identityFqdn).mockResolvedValue('fqdn.com') + vi.mocked(clientId).mockReturnValue('clientId') + + // When + await requestDeviceAuthorization([]) + + // Then + expect(shopifyFetch).toHaveBeenCalledWith('https://fqdn.com/oauth/device_authorization', { + method: 'POST', + headers: {'Content-type': 'application/x-www-form-urlencoded'}, + body: 'client_id=clientId', + }) + }) + test('opens the browser directly in an interactive terminal', async () => { // Given const outputInfo = vi.spyOn(output, 'outputInfo') diff --git a/packages/cli-kit/src/private/node/session/device-authorization.ts b/packages/cli-kit/src/private/node/session/device-authorization.ts index 8ac629e7cc0..cb7b5c0c34e 100644 --- a/packages/cli-kit/src/private/node/session/device-authorization.ts +++ b/packages/cli-kit/src/private/node/session/device-authorization.ts @@ -152,10 +152,8 @@ export async function pollForDeviceAuthorization(code: string, interval = 5): Pr } function convertRequestToParams(queryParams: {client_id: string; scope: string}): string { - return Object.entries(queryParams) - .map(([key, value]) => value && `${key}=${value}`) - .filter((hasValue) => Boolean(hasValue)) - .join('&') + // URLSearchParams applies form encoding so scope values cannot change the request structure. + return new URLSearchParams(Object.entries(queryParams).filter(([, value]) => Boolean(value))).toString() } /**