Encode device authorization request parameters - #8270
Conversation
Assisted-By: devx/bb1c6644-dd32-425e-b0c8-33f7efa1bb93
Assisted-By: devx/05a127b1-5522-4d26-afd2-fdde93868c5e
There was a problem hiding this comment.
Pull request overview
This PR fixes device-authorization request body construction by switching from manual string concatenation to proper application/x-www-form-urlencoded encoding via URLSearchParams, aligning it with the existing token exchange behavior and preventing scope values from altering request structure.
Changes:
- Build the device authorization request body using
URLSearchParams(with empty-value omission preserved). - Update and extend tests to cover reserved-character encoding and omission of empty scopes.
- Add a patch changeset for
@shopify/cli-kitdocumenting the user-visible bug fix.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/cli-kit/src/private/node/session/device-authorization.ts | Uses URLSearchParams to safely form-encode device authorization POST body while omitting empty values. |
| packages/cli-kit/src/private/node/session/device-authorization.test.ts | Updates existing body assertion and adds coverage for reserved-character encoding and empty-scope omission. |
| .changeset/encode-device-authorization-request.md | Adds a patch changeset describing the encoding fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Assisted-By: devx/36a3e4d9-c136-46e2-a221-9934ba078d6f
| expect(got).toEqual(dataExpected) | ||
| }) | ||
|
|
||
| test('encodes special characters in authorization scopes', async () => { |
There was a problem hiding this comment.
Are we testing the behavior of URLSearchParams? I feel like we could get away with much simpler testing here...
| .map(([key, value]) => value && `${key}=${value}`) | ||
| .filter((hasValue) => Boolean(hasValue)) | ||
| .join('&') | ||
| // URLSearchParams applies form encoding so scope values cannot change the request structure. |
There was a problem hiding this comment.
I don't think we need a lot of justification for the use of URLSearchParams over a hand-rolled conversion function 😆
WHY are these changes introduced?
The device authorization request built its
application/x-www-form-urlencodedbody by string concatenation without encoding. Scopes are joined with spaces, so the body carried literal spaces, and any&,=, or%in a value could change the request structure. It works today only because the server is lenient. The token endpoint inexchange.tsalready usesURLSearchParamsfor the same reason.WHAT is this pull request doing?
URLSearchParams, matching the existing token-request precedent.Content-Type, and a hand-written encoded body (spaces as+,%26/%3D/%25for reserved characters), so the expectation does not depend onURLSearchParamsitself.One pre-existing assertion changes from the unencoded body to the encoded form — that wire change is the fix. No change to prompts, returned data, or requested scopes.
How to test your changes?
Run
shopify auth logout, then any authenticated command (for exampleshopify theme list). Device login completes as before; the request body is now correctly encoded.Checklist
patchfor bug fixes ·minorfor new features ·majorfor breaking changes) and added a changeset withpnpm changeset add