Skip to content

Encode device authorization request parameters - #8270

Open
dmerand wants to merge 3 commits into
mainfrom
donald/device-auth-form-encoding
Open

Encode device authorization request parameters#8270
dmerand wants to merge 3 commits into
mainfrom
donald/device-auth-form-encoding

Conversation

@dmerand

@dmerand dmerand commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

The device authorization request built its application/x-www-form-urlencoded body 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 in exchange.ts already uses URLSearchParams for the same reason.

WHAT is this pull request doing?

  • Build the device authorization body with URLSearchParams, matching the existing token-request precedent.
  • Preserve the current behavior of omitting empty values (an empty scope string stays out of the body).
  • Pin the request in tests: exact URL, method, Content-Type, and a hand-written encoded body (spaces as +, %26/%3D/%25 for reserved characters), so the expectation does not depend on URLSearchParams itself.

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 example shopify theme list). Device login completes as before; the request body is now correctly encoded.

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing — I've identified the correct bump type (patch for bug fixes · minor for new features · major for breaking changes) and added a changeset with pnpm changeset add

Assisted-By: devx/bb1c6644-dd32-425e-b0c8-33f7efa1bb93
@github-actions github-actions Bot added the Area: @shopify/cli @shopify/cli package issues label Aug 8, 2026
Assisted-By: devx/05a127b1-5522-4d26-afd2-fdde93868c5e
@dmerand
dmerand marked this pull request as ready for review August 8, 2026 15:03
@dmerand
dmerand requested a review from a team as a code owner August 8, 2026 15:03
Copilot AI lite review requested due to automatic review settings August 8, 2026 15:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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-kit documenting 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area: @shopify/cli @shopify/cli package issues

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants