Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use private access token header for Storefront API #1293

Closed
bfad opened this issue Mar 15, 2024 · 4 comments · Fixed by #1302
Closed

Use private access token header for Storefront API #1293

bfad opened this issue Mar 15, 2024 · 4 comments · Fixed by #1302
Labels

Comments

@bfad
Copy link
Contributor

bfad commented Mar 15, 2024

Overview/summary

The current code for the Storefront API uses the public access token with the X-Shopify-Storefront-Access-Token header. I would like to be able to instead use the private access token with the Shopify-Storefront-Private-Token header instead. I'm unsure if the current behavior of using the public token is a bug or a feature that should continue to be allowed. The documentation for public access tokens indicate they should be used for client-side requests while I would presume that this library is meant for server-side queries and should be using the private token instead. I would be happy to put together a PR for this, but I don't know which direction to take: should we add the ability to use the private key, or should the public key usage be replaced?

@bfad bfad changed the title Use private access token and buyer IP header for Storefront API Use private access token header for Storefront API Mar 15, 2024
@sle-c
Copy link
Contributor

sle-c commented Mar 26, 2024

It could be that this gem is quite old and the Storefront-Private-Token is introduced and this library was not updated to use it. With the current code, developers can pass it additional headers and use the Shopify-Storefront-Private-Token header if necessary. I'd lean toward adding the ability to use the private key. cc: @Shopify/client-libraries-app-templates

@paulomarg
Copy link
Contributor

paulomarg commented Mar 26, 2024

Actually, using the private access token might be the right choice for this package - it only provides support for server-side requests, in which case the private token makes more sense because we can use the token we get back from OAuth, correct?

Si is correct in that when this was first implemented, private tokens weren't available, so I'd consider it a bug fix and not a change - i.e. we should stop using the public token in favour of using the private one. That being said, we should ideally avoid making it a breaking change, so we should probably still have the ability to use a public token, so we don't break apps that already implemented that.

@bfad
Copy link
Contributor Author

bfad commented Mar 27, 2024

@paulomarg

in which case the private token makes more sense because we can use the token we get back from OAuth, correct?

I don't think OAuth enters into this. The private token itself is passed to Storefront API requests via Shopify-Storefront-Private-Token. I think it's just a shared secret.

Si is correct in that when this was first implemented, private tokens weren't available, so I'd consider it a bug fix and not a change - i.e. we should stop using the public token in favour of using the private one. That being said, we should ideally avoid making it a breaking change, so we should probably still have the ability to use a public token, so we don't break apps that already implemented that.

We can have the ability to use both, but if we make the private key the default (which I think we should do) that will cause a breaking change for people currently using the client with the public key. Do you think we should make that breaking change now? Alternatively we could keep the public key the default for now but enable using either public or private. This would allow for a deprecation period until we want to make a breaking change release.

@paulomarg
Copy link
Contributor

I think it's just a shared secret.

IIRC the token you use for the Admin API also serves as a private token for the SFAPI, but I could be wrong! We do use the access token we get from the OAuth process successfully in the JS library.

Alternatively we could keep the public key the default for now but enable using either public or private. This would allow for a deprecation period until we want to make a breaking change release.

I prefer that approach, because we just recently released some breaking changes. I think long term the public token might not make much sense for a server-to-server call since it's more aggressively throttled, but I'd prefer to have a deprecation period to give folks time to move over (or forcibly set it to use public) before we flip the switch.

@sle-c sle-c added the bug label Mar 28, 2024
@bfad bfad closed this as completed in #1302 Apr 3, 2024
paulomarg pushed a commit that referenced this issue Apr 4, 2024
This allows for using [the Storefront private access token][1] with the
library's Storefront GraphQL client. Since this client is probably going
to be running on a server where the token can be secured, using the
private access token should be preferred. This commit doesn't make using
the private token the default to avoid an immediate breaking change. We
want to give clients time to see the deprecation and update their
scripts.

Closes #1293

[1]: https://shopify.dev/docs/api/usage/authentication#getting-started-with-private-access
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants