Shopify / shopify-api-ruby

ShopifyAPI is a lightweight gem for accessing the Shopify admin REST and GraphQL web services.
MIT License
1.05k stars 468 forks source link

Use private access token header for Storefront API #1293

Closed bfad closed 5 months ago

bfad commented 6 months ago

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?

sle-c commented 5 months ago

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 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

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.