Client credentials flow for GraphiQL #2887

amcaplan · 2023-09-26T16:26:11Z

WHY are these changes introduced?

Fixes https://github.com/Shopify/develop-app-management/issues/1494

Replaces the existing, somewhat messy solution for authentication (OAuth with redirect to GraphiQL) with the client credentials flow that can happen entirely behind the scenes.

This means:

No messing with OAuth callback URLs
GraphiQL works for extension-only apps

WHAT is this pull request doing?

Replaces the existing solution with a behind-the-scenes client credentials OAuth flow.

In case the user isn't logged in yet, we've added a page instructing them to click a link to install the app in a new tab. Once the app is installed, the tab will be closed and they'll be redirected to GraphiQL.

There have been a few other related changes:

The dependency on @shopify/shopify-api was removed. We are now getting API versions straight from the source of truth, and requests are being forwarded to the API in a more raw fashion. (I believe this should work the same in spin, but we should be sure of that. I know the npm package did have some special spin handling.)
Thanks to dropping the dependency, app scopes are no longer required!
Added an explanatory warning when credentials can't be refreshed
Styling the page required exposing from cli-kit the style.css asset we use to show simple pages like the post-login page.
The GraphiQL URL was added to the sticky footer.
CLI kit was modified to have some extra available functions for getting information on available API versions. This was mostly just code shuffling and exposing new functions. No APIs should have been broken.

How to test your changes?

Enable the client_credentials beta flag for your app (this requirement should go away in the next few days)
p shopify app dev --path /path/to/your/app
Wait for the app to load, and tap g

Be sure to try with the app previously installed and uninstalled.

Measuring impact

How do we know this change was effective? Please choose one:

n/a - this doesn't need measurement, e.g. a linting rule or a bug-fix
Existing analytics will cater for this addition
PR includes analytics changes to measure impact

Checklist

I've considered possible cross-platform impacts (Mac, Linux, Windows)
I've considered possible documentation changes
I've made sure that any changes to dev or deploy have been reflected in the internal flowchart.

github-actions · 2023-09-26T16:26:32Z

Thanks for your contribution!

Depending on what you are working on, you may want to request a review from a Shopify team:

Themes: @shopify/theme-code-tools
UI extensions: @shopify/ui-extensions-cli
- Checkout UI extensions: @shopify/checkout-ui-extensions-api-stewardship
Hydrogen: @shopify/hydrogen
Other: @shopify/cli-foundations

github-actions · 2023-09-26T21:00:30Z

Coverage report

St.^❔	Category	Percentage	Covered / Total
🟡	Statements	72.69% (-0.16% 🔻)	6138/8444
🟡	Branches	70.02% (+0.15% 🔼)	2968/4239
🟡	Functions	71.41% (-0.1% 🔻)	1571/2200
🟡	Lines	73.87% (-0.18% 🔻)	5829/7891

Show files with reduced coverage 🔻

St.^❔	File	Statements	Branches	Functions	Lines
🔴	`...` / graphiql.ts	40% (-10% 🔻)	100% (+50% 🔼)	33.33%	40% (-10% 🔻)
🟢	`...` / setup-dev-processes.ts	95.35% (-0.11% 🔻)	68% (-2.37% 🔻)	90%	94.74% (-0.13% 🔻)
🟢	`...` / Dev.tsx	93.42% (+0.09% 🔼)	84.44% (-1.6% 🔻)	89.47%	94.37% (+0.08% 🔼)

Test suite run success

1434 tests passing in 665 suites.

Report generated by 🧪jest coverage report action from 060bd75

teddyhwang · 2023-11-09T15:02:47Z

packages/app/src/cli/services/dev/graphiql/server.ts

+        client_secret: apiSecret,
+        grant_type: 'client_credentials',
+      })
+      const tokenResponse = await fetch(`https://${storeFqdn}/admin/oauth/access_token?${queryString}`, {


are there any plans to have this in the @shopify/shopify-api library?

Great question for @paulomarg

I don't think it impacts our efforts, though, as there are significant advantages to eliminating the library from our code (specifically no longer requiring app scopes).

I agree it's good to not have the full shopify-api dependency. Maybe worth moving it to CLI Kit though to start with?

Actually, I retract what I said. This is only available on dev stores (for now), so it shouldn't be in the library.

RE cli-kit, I'm not sure offhand. Usually my inclination is to leave things in individual packages until there is a defined use case in multiple packages.

FWIW, we are in the process of adding token exchange support to the library. https://github.com/Shopify/shopify-api-js/pull/1036/files#diff-b96b5dd8c43f3122c7def35d0d4a4190cfc9e2318de6f5b970c285fcb5b6ec02R29

packages/app/src/cli/services/dev/ui/components/Dev.tsx

shauns · 2023-11-13T09:53:51Z

packages/app/src/cli/services/dev/graphiql/server.ts

+        client_secret: apiSecret,
+        grant_type: 'client_credentials',
+      })
+      const tokenResponse = await fetch(`https://${storeFqdn}/admin/oauth/access_token?${queryString}`, {


I agree it's good to not have the full shopify-api dependency. Maybe worth moving it to CLI Kit though to start with?

packages/app/src/cli/services/dev/graphiql/server.ts

Using client credentials, we no longer require this code, most prominently that which updates the callback URLs, since we no longer perform an in-browser OAuth flow with a callback URL.

…pi dependency

…stalled yet

This was only necessary when using the `@shopify/shopify-api` package. But now we've gotten rid of it. And there is plenty you can do without scopes, so we should still enable GraphiQL.

github-actions · 2023-11-13T19:27:50Z

Differences in type declarations

We detected differences in the type declarations generated by Typescript for this branch compared to the baseline ('main' branch). Please, review them to ensure they are backward-compatible. Here are some important things to keep in mind:

Some seemingly private modules might be re-exported through public modules.
If the branch is behind main you might see odd diffs, rebase main into this branch.

New type declarations

We found no new type declarations in this PR

Existing type declarations

packages/cli-kit/dist/public/node/api/admin.d.ts

@@ -9,6 +9,21 @@ import { AdminSession } from '../session.js';
  * @returns The response of the query of generic type <T>.
  */
 export declare function adminRequest<T>(query: string, session: AdminSession, variables?: GraphQLVariables): Promise<T>;
+/**
+ * GraphQL query to retrieve all supported API versions.
+ *
+ * @param session - Shopify admin session including token and Store FQDN.
+ * @returns - An array of supported API versions.
+ */
+export declare function supportedApiVersions(session: AdminSession): Promise<string[]>;
+/**
+ * Returns the Admin API URL for the given store and version.
+ *
+ * @param store - Store FQDN.
+ * @param version - API version.
+ * @returns - Admin API URL.
+ */
+export declare function adminUrl(store: string, version: string | undefined): string;
 /**
  * Executes a REST request against the Admin API.
  *

amcaplan force-pushed the client-credentials-flow-for-graphiql branch 3 times, most recently from d53cd7a to 0e700d8 Compare September 26, 2023 20:49

amcaplan force-pushed the client-credentials-flow-for-graphiql branch 2 times, most recently from e111d36 to 0be302d Compare September 28, 2023 08:22

amcaplan force-pushed the client-credentials-flow-for-graphiql branch 5 times, most recently from c6561a5 to c25ec53 Compare November 7, 2023 19:36

amcaplan marked this pull request as ready for review November 7, 2023 19:55

This comment has been minimized.

Sign in to view

amcaplan force-pushed the client-credentials-flow-for-graphiql branch from c25ec53 to 0b8b1ff Compare November 9, 2023 14:41

teddyhwang reviewed Nov 9, 2023

View reviewed changes

martinlaws mentioned this pull request Nov 11, 2023

Update styles for GraphiQL top bar #3093

Closed

6 tasks

shauns approved these changes Nov 13, 2023

View reviewed changes

amcaplan added 12 commits November 13, 2023 21:20

Use client credentials flow to get session

00dc224

Show GraphiQL regardless of updating OAuth URLs

a5da490

Using client credentials, we no longer require this code, most prominently that which updates the callback URLs, since we no longer perform an in-browser OAuth flow with a callback URL.

Lint fixes

6ada2d9

Use built-in query to check API versions, removing @shopify/shopify-a…

5d3afc8

…pi dependency

Linty improvements

f99b02b

Lint fixes

18da8c4

Give user clear instructions for how to proceed when the app isn't in…

c6a4530

…stalled yet

Autoclose install tab after install

d4fc5a6

Style install-app page better

ed70b64

Include GraphiQL URL in sticky dev footer

87b6ad9

Lint fixes

116efb4

Remove scopes requirement for GraphiQL

a336b17

This was only necessary when using the `@shopify/shopify-api` package. But now we've gotten rid of it. And there is plenty you can do without scopes, so we should still enable GraphiQL.

Removed unused scopes information

53c2238

amcaplan force-pushed the client-credentials-flow-for-graphiql branch from 0b8b1ff to 53c2238 Compare November 13, 2023 19:23

Add changesets

060bd75

amcaplan added this pull request to the merge queue Nov 13, 2023

Merged via the queue into main with commit 2c20518 Nov 13, 2023

amcaplan deleted the client-credentials-flow-for-graphiql branch November 13, 2023 19:44

shopify-shipit bot temporarily deployed to nightly November 14, 2023 14:14 Inactive

shopify-shipit bot temporarily deployed to production December 6, 2023 13:21 Inactive

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Client credentials flow for GraphiQL #2887

Client credentials flow for GraphiQL #2887

amcaplan commented Sep 26, 2023 •

edited

Loading

github-actions bot commented Sep 26, 2023

github-actions bot commented Sep 26, 2023 •

edited

Loading

This comment has been minimized.

teddyhwang Nov 9, 2023

amcaplan Nov 12, 2023

shauns Nov 13, 2023

amcaplan Nov 13, 2023

paulomarg Nov 13, 2023

shauns Nov 13, 2023

github-actions bot commented Nov 13, 2023

Client credentials flow for GraphiQL #2887

Client credentials flow for GraphiQL #2887

Conversation

amcaplan commented Sep 26, 2023 • edited Loading

WHY are these changes introduced?

WHAT is this pull request doing?

How to test your changes?

Measuring impact

Checklist

github-actions bot commented Sep 26, 2023

github-actions bot commented Sep 26, 2023 • edited Loading

Coverage report

Test suite run success

This comment has been minimized.

teddyhwang Nov 9, 2023

Choose a reason for hiding this comment

amcaplan Nov 12, 2023

Choose a reason for hiding this comment

shauns Nov 13, 2023

Choose a reason for hiding this comment

amcaplan Nov 13, 2023

Choose a reason for hiding this comment

paulomarg Nov 13, 2023

Choose a reason for hiding this comment

shauns Nov 13, 2023

Choose a reason for hiding this comment

github-actions bot commented Nov 13, 2023

Differences in type declarations

New type declarations

Existing type declarations

amcaplan commented Sep 26, 2023 •

edited

Loading

github-actions bot commented Sep 26, 2023 •

edited

Loading