Reaction: [Marketplace] Receive 404 when clicking on link for "Getting Started" email

[zenweasel]

Expected behavior

Should be taken to this page

Actual Behavior

Receive "Not Found" page

Steps to Reproduce the Behavior

1. Register as a user with a valid email
2. Click on the link in the email
3. Observe it takes you to the "Not Found" page

Versions

Node: 7.10.0
NPM: 5.0.3
Meteor Node: 4.8.4
Meteor NPM: 4.6.1
Reaction CLI: 0.11.0
Reaction: 1.5.0
Reaction branch: marketplace
Docker: 17.06.0-ce
commit: b6e18a4b5a35b852bd75648ae395a2f4b77eae81

[jm-wanja]

This issue is caused by the shop prefix on the email verification url, so I think a decision on whether to not use it should be made @zenweasel @spencern .

[spencern]

Why does having a shop prefix on the email verification cause this behavior?

Two thoughts:

1. Users who are invited to a shop should have permissions to view a shop's routes/packages (scoped appropriately). If they don't have that access I don't know how this would ever work.
2. I'm not sure why this should be scoped to a shop/shop prefix. Unless there's a good reason that this verification page should be different for each shop, then I think it should probably be a root level route and perhaps owned by the primary shop. Maybe we need a way to specify that a route is root level in the registry file.

[spencern]

@jm-wanja Are you still working on this? If not what is the progress?

[jm-wanja]

@spencern I have not looked at this for awhile, because I am not quite clear on how to proceed.

[spencern]

@zenweasel @jm-wanja I can't think of any good reasons to keep the shop prefix on the email verification url right now, even if we wanted to attach a shop to a registered user for some reason, that would probably be done better as a query param or similar rather than creating unique user email verification routes for every shop.

My vote: email verification route should be on the root, similar to the Stripe callback

[zenweasel]

@jm-wanja Yes, sorry for letting this get ignored. For all non-product routes I don't think we need to take shop name into account. Hopefully that gives you enough clarity.

[jm-wanja]

This issue is currently being solved by this PR https://github.com/reactioncommerce/reaction/pull/2905
However, there is still an issue with authorization in relation to sign in when one clicks on the verification link.

[aaronjudd]

@jm-wanja not really resolved by #2905 -> yet. That PR has requested changes, and is currently being blocked. Can you clarify your comment However, there is still an issue with authorization in relation to sign in when one clicks on the verification link. isn't this the issue we're trying to resolve?

[jm-wanja]

@aaronjudd Sorry for the the confusion, what I meant to point out was that PR #2905 solves the error where it leads to a 404 page not found which was why this ticket was opened. I'll close that PR until I solve the issue with sign in in incognito mode.

[spencern]

@jm-wanja are you still working on #2905? In general, a PR should use [WIP] in the title to indicate that it's not ready for review yet. I'm not sure that closing the PR for now is necessarily the best way to resolve this as it drops off the PRs page which makes it more difficult to follow up on progress.

[jm-wanja]

@spencern yes I am still working on it. I have made the correction and re-opened it and also added [WIP].

[jm-wanja]

Have not been able to consistently reproduce the blocker where the link leads to the permission denied page as noted in #2905

[spencern]

I just reproduced this issue 30 seconds ago.

@mikemurray this seems to be related to the /shop/:shopSlug vs /:shopSlug issue you solved yesterday with paypal express

[mikemurray]

@spencern Yea, there must be other places where getShopPrefix() is still returning the wrong prefix, or maybe the prefix is generated with some other method wherever the emails are created.

[spencern]

my thoughts as well.

[janus-reith]

I just noticed, that it's not only the verification url, but also when clicking "Your Orders" on the order confirmation mail.
The behaviour here is the opposite: The prefix is added although there is only a single shop, like:
http://localhost:3000/shopname/cart/completed?_id= while the valid url would be
http://localhost:3000/cart/completed?_id=

[spencern]

The issue for the verification URL turned out to be two-fold

1. The route is not accurate for for most shops.
   I'll be changing the email template to always use the root route, there aren't any good reasons to use the shop-prefix here as it just adds complexity and all users are global anyway.

1. The email is not being URI encoded which means that some valid emails, such as emails with a + in them are incorrectly sent to the server.
   I'm updating the verification url to use an encoded email address to solve this problem.

[spencern]

Resolved by #3046