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

fix: STRF-11923 - Handle Missing a New Channels Permission Requirement in Auth Token #1191

Merged
merged 1 commit into from
Apr 10, 2024

Conversation

bc-jz
Copy link
Contributor

@bc-jz bc-jz commented Apr 10, 2024

What?

This PR handles the possibility of missing the Channels auth permission when making a GET for a list of channels in getStoreChannels().

When this update was made I overlooked that the standard Stencil CLI auth token that we recommend providing to 3rd parties does not include the "read channel settings" permission by default. So people utilizing the Stencil CLI token, or any v3 token that does not include "read/manage channel settings" permission, can end up with this error when making start, push, pull, or download commands:
Screenshot 2024-04-10 at 7 14 28 AM

We will be updating that permission on the Stencil CLI token but people with errors will still have to generate a new token to get this permission. As such, this PR will avoid failures if the Channels permission is missing. Instead we will ignore the error and return an unfiltered list of urls the same way we did prior to the change in version 4.7.2.

Refactor
Also incorporates some smart improvements to the fetching of channels that I found in this PR:
#1187

Tickets / Documentation

Previous related pr.

Screenshots (if appropriate)

With this changes in this PR we will now give a "warning" message if the "channels" permission is missing and still proceed with an unfiltered response as we did before:
Screenshot 2024-04-10 at 7 17 32 AM

However, if you do have an updated token that includes the correct permission then we will filter the list of channels returned using this new information:
Screenshot 2024-04-10 at 7 19 12 AM

cc @bigcommerce/storefront-team

.catch((error) => {
if (error.response.data.status === 403) {
console.log(
'WARNING: Version 4.7.2+ of stencil-cli added a request to the Channels resource which requires a permission missing on your current stencil cli auth token. Please generate a new auth token at your convenience which includes the "read-only channels" permission to clear up this warning. You can read more about correcting this issue here: https:/bigcommerce/stencil-cli/issues/1185',
Copy link
Contributor

Choose a reason for hiding this comment

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

Great idea for the warning!

.catch((error) => {
if (error.response.data.status === 403) {
console.log(
'WARNING: Version 4.7.2+ of stencil-cli added a request to the Channels resource which requires a permission missing on your current stencil cli auth token. Please generate a new auth token at your convenience which includes the "read-only channels" permission to clear up this warning. You can read more about correcting this issue here: https:/bigcommerce/stencil-cli/issues/1185',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'WARNING: Version 4.7.2+ of stencil-cli added a request to the Channels resource which requires a permission missing on your current stencil cli auth token. Please generate a new auth token at your convenience which includes the "read-only channels" permission to clear up this warning. You can read more about correcting this issue here: https:/bigcommerce/stencil-cli/issues/1185',
'WARNING: Version 7.4.2+ of stencil-cli added a request to the Channels resource which requires a permission missing on your current stencil cli auth token. Please generate a new auth token at your convenience which includes the "read-only channels" permission to clear up this warning. You can read more about correcting this issue here: https:/bigcommerce/stencil-cli/issues/1185',

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good eye! Updated.

accessToken,
})
.catch((error) => {
if (error.response.data.status === 403) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's

Suggested change
if (error.response.data.status === 403) {
if (error.response.status === 403) {

Copy link
Contributor

Choose a reason for hiding this comment

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

and lets wrap it into try {} catch rather than use promise chain

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ahh, its both actually:

response: {
    status: 403,
    statusText: 'Forbidden',
    ...
    data: {
      status: 403,
      title: "You don't have a required scope to access the endpoint",
      type: 'https://developer.bigcommerce.com/api-docs/getting-started/api-status-codes',
      errors: {}
    }
  }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will go ahead and make the suggested update for simplicity sake.

@bc-jz bc-jz merged commit 20e5fcb into master Apr 10, 2024
6 checks passed
Copy link
Contributor

🎉 This PR is included in version 7.5.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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 this pull request may close these issues.

2 participants