Skip to content
This repository has been archived by the owner on Jan 30, 2023. It is now read-only.

Fix inconsistency in authentication path prefix to remove trailing slash #29

Merged
merged 1 commit into from
Oct 22, 2020

Conversation

devjones
Copy link
Contributor

@devjones devjones commented Oct 18, 2020

WHY are these changes introduced?

There are inconsistent assumptions around whether the authentication middleware should use a trailing slash in the prefix. The createShopifyAuth function assumes no trailing slash, but requestStorageAccess assumes trailing slash. This leads to a 404 error when requestStorageAccess attempts to access a url in the form: customprefixauth/inline?shop=. Note the missing slash between customprefix and auth/.

WHAT is this pull request doing?

Update the requestStorageAccess function to assume no trailing slash in the prefix.

Type of change

  • Patch: Bug (non-breaking change which fixes an issue)
  • Minor: New feature (non-breaking change which adds functionality)
  • Major: Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

  • I have added a changelog entry, prefixed by the type of change noted above
  • I have added/updated tests for this change

@raffi1300
Copy link

The prefix is broken on the master branch - can confirm.

Trying out your fixes in my forked repo and it solves the prefix issue. Hopefully the official package can be updated.

Thanks!

@tanema
Copy link

tanema commented Oct 22, 2020

Thanks for the contribution!

@nmalzieu
Copy link

nmalzieu commented Nov 9, 2020

Hi @tanema do you know when this will be released? When is the next release? Thx!

@yoadsn
Copy link

yoadsn commented Nov 23, 2020

@tanema Asking again - when do you expect this to be released? This breaks the Auth flow.
Thanks.

@yoadsn
Copy link

yoadsn commented Nov 24, 2020

So... until we get this fix - if you use Express (for example) just do:


// Working around an existing bug
// https://github.com/Shopify/koa-shopify-auth/issues/13
const BUG_SHOPIFY_ROOT_PATH = '/shopifyauth';
const BUG_PATCHED_SHOPIFY_ROOT_PATH = '/shopify/auth';

    if (req.path.startsWith(BUG_SHOPIFY_ROOT_PATH)) {
      res.redirect(
        req.url.replace(BUG_SHOPIFY_ROOT_PATH, BUG_PATCHED_SHOPIFY_ROOT_PATH)
      );
    }

🙄

@mkevinosullivan mkevinosullivan temporarily deployed to production December 1, 2020 18:12 Inactive
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants