reactioncommerce / reaction

Mailchimp Open Commerce is an API-first, headless commerce platform built using Node.js, React, GraphQL. Deployed via Docker and Kubernetes.
https://mailchimp.com/developer/open-commerce/
GNU General Public License v3.0
12.3k stars 2.17k forks source link

[v2.7.0] Link to reset password receive through reset-password email does not work #5726

Closed jmaver-plume closed 4 years ago

jmaver-plume commented 4 years ago

Prerequisites

I have checked code, changelog etc. but I have not seen any mention of reset-password url being removed.

Issue Description

Issue links: #5247

When user forgets password and completes form regarding forgot password, he receives email which links to page where password can be change. Example of link: http://localhost:3000/reset-password/08udZOQl5gWJMg44DtS2BfT9cONg5Fk7hpf7guJ0wHd

When going on this link user should see a form where he can change his password, but now he goes back to sign in page.

Steps to Reproduce

Please provide starting context, i.e. logged in as a user, configure a particular payment method.

  1. Have running reaction.
  2. Go to: http://localhost:3000/reset-password/08udZOQl5gWJMg44DtS2BfT9cONg5Fk7hpf7guJ0wHd
  3. Expect password change form, but receive regular login form.

Versions

2.7.0

jmaver-plume commented 4 years ago

I did some debugging and arrived to the following conclusions:

  1. Issue is present with v2.7.0 and not v2.6.0.
  2. Issue happens because route is not found in login component currentroute.name is 'not-found'

login.js was not changed in update from 2.6 to 2.7, but I saw that register.js was changed and no-meteor/register.js was moved.

jmaver-plume commented 4 years ago

Further inspection leads me to getEnabledPackageRoutes, which receives packages argument.

In version 2.6.0 packages has 40+ elements in array, but in 2.7.0 it is empty so no routes are registrated for use with import { Router } from "/client/api". Is there a new Router version which should be used to get routes?

jmaver-plume commented 4 years ago

Meteor database query for packages returns empty packages, even though I have it data populated in database.

This might be due to data not being available on client side at the moment this call is made.

jmaver-plume commented 4 years ago

By mistake I have noticed that when I am logged in and go to reset-password url I receive form, but when I log out I no longer receive it.

spencern commented 4 years ago

@jm18457 thanks for reporting this issue and for a detailed account of what you've observed and tested.

@manueldelreal this seems like a Critical issue - will you check to see if you can reproduce?

spencern commented 4 years ago

@kieckhafer and @aldeed Does this seem like it could be related to work we've been doing to demeteorize the API?

manueldelreal commented 4 years ago

I have tried this with both a blank db and the example dataset and for both I can get to the form and reset the password. First thing that comes to mind are the storefront url's being different as in both my use cases they are all blank, do you have something in these @jm18457 ?

manueldelreal commented 4 years ago

addendum: it seems that the logged in / logged out status for the admin is the key, I tried again and I can't even get the email to send for resetting the password if my admin account is logged out.

kieckhafer commented 4 years ago

@manueldelreal my first thought is also that the storefront URL's are not set correctly.

You're saying that if you are logged in as the admin somewhere (anywhere, a different browser), it works, but if no one is logged in as admin it doesn't work?

jmaver-plume commented 4 years ago

I have tried this with both a blank db and the example dataset and for both I can get to the form and reset the password. First thing that comes to mind are the storefront url's being different as in both my use cases they are all blank, do you have something in these @jm18457 ?

I can't check what I have in db right now, but I started with a completely fresh installation. Issue also links #5725 because you can't receive email to reset password, without being logged in because entire graphql mutation is protected by token middleware.

When talking logged I mean: Log in to localhost:3000, now go to OAUTH authentication url, the one you get from storefront, localhost:3000/accounts... and if you click forgot password now it will work and reset-password will also be shown.

I tried everything only with admin account, because of fresh installaiton.

manueldelreal commented 4 years ago

@manueldelreal my first thought is also that the storefront URL's are not set correctly.

You're saying that if you are logged in as the admin somewhere (anywhere, a different browser), it works, but if no one is logged in as admin it doesn't work?

It has to be on the same session, only if there's an active admin session in the same browser these flows work (requesting the reset email and resetting the password with the token that was received in the email inbox).

spencern commented 4 years ago

@jm18457 Have you been able to try the fix in #5744?

jmaver-plume commented 4 years ago

@jm18457 Have you been able to try the fix in #5744?

Kinda, I tried solving the issue by regexing path instead of route name which shows not-found, similar to #5744 , but the problem is that issue is with router and not just this particular route.

I think its related to #5247 , because no routes exist.

For example this means that hydra authentication does not work through storefront, because special oauth route is not-found in link.js

garyrvaz commented 4 years ago

So in imports/plugins/core/core/server/publications/collections/packages.js it depends on shopId, self.userId and Reaction.getPrimaryShopId for packages information that the sign in/sign up/reset password link depend on for meta information for oauth. As user is not logged in, all of these values are either null or undefined, due to which Packages subscription for guests will always be an empty array and routes will not get the meta data they need.

So in the above mentioned file, to test my theory I did the following

  1. Removed lines 97-104

    if (!self.userId) return self.ready();
    
    if (!myShopId) {
    myShopId = Reaction.getPrimaryShopId();
    if (!myShopId) {
      return self.ready();
    }
    }
  2. Removed lines 123-128:
    // if admin user, return all shop properties
    if (
    Roles.userIsInRole(self.userId, ["dashboard", "owner", "admin"], myShopId) ||
    Roles.userIsInRole(self.userId, ["owner", "admin"], Roles.GLOBAL_GROUP)
    ) {
    options = {};
    }
  3. Changed line 130: const query = {};
  4. Commented out line 50: // delete registry.route

On doing these steps, the guest login routes worked like before till v2.6.0

garyrvaz commented 4 years ago

But this is just to get the routes working, I have not yet checked what will happen if I register, reset password, login, when the app doesn't have shopId, userId and primaryShopId for a user that is not logged in.

loan-laux commented 4 years ago

I would assume that this regression was introduced when @aldeed removed the code that was creating anonymous users for all visitors. But I'm just curious to know the thought process behind not publishing packages (and hence routes) to non-logged-in users. Is this auth page being deprecated and we weren't made aware of it? Is there a newer way to do it that's not documented yet?

daniel-rosiak commented 4 years ago

Any progress on this? Because of this, as of 2.7 ver. Reaction Commerce is not usable at all.

willopez commented 4 years ago

@aldeed thoughts on this?

kieckhafer commented 4 years ago

We have PR open. I cannot replicate the issue mentioned by @mikemurray in said PR, any one experiencing the problem can check it out and see if the solution works: https://github.com/reactioncommerce/reaction/pull/5744

garyrvaz commented 4 years ago

So in imports/plugins/core/core/server/publications/collections/packages.js it depends on shopId, self.userId and Reaction.getPrimaryShopId for packages information that the sign in/sign up/reset password link depend on for meta information for oauth. As user is not logged in, all of these values are either null or undefined, due to which Packages subscription for guests will always be an empty array and routes will not get the meta data they need.

So in the above mentioned file, to test my theory I did the following

  1. Removed lines 97-104
if (!self.userId) return self.ready();

  if (!myShopId) {
    myShopId = Reaction.getPrimaryShopId();
    if (!myShopId) {
      return self.ready();
    }
  }
  1. Removed lines 123-128:
// if admin user, return all shop properties
  if (
    Roles.userIsInRole(self.userId, ["dashboard", "owner", "admin"], myShopId) ||
    Roles.userIsInRole(self.userId, ["owner", "admin"], Roles.GLOBAL_GROUP)
  ) {
    options = {};
  }
  1. Changed line 130: const query = {};
  2. Commented out line 50: // delete registry.route

On doing these steps, the guest login routes worked like before till v2.6.0

@kieckhafer, running fix-kieckhafer-passwordReset branch, I get a 401 on submitting the form and Network error: Unexpected token U in JSON at position 0 shows under the email input. I don't think these forms will function correctly unless the underlying issue is fixed, as you mentioned in the PR. Also, these are backup forms for forms in imports/plugins/core/accounts/client/containers/auth.js which only renders when package registry meta information is available.

kieckhafer commented 4 years ago

@YuuwakU Ok, i'm seeing the Network error: Unexpected token U in JSON at position 0 when I try to reset my password from the admin. I do not see that error when I try to reset my password from the Storefront.

Storefront uses /account as the login url, admin uses /operator

garyrvaz commented 4 years ago

What I did was click on Sign In from example-storefront, then URL of this type: http://localhost:3000/account/login?action=signin&login_challenge=834ef19ef3b94994a5d3d286d2782f35 loads, I click on the Reset Password from there. Also, when I load a URL like this http://localhost:3000/account/reset-password the sign in form loads, but if I add a trailing slash, the update password form loads, but an update password form only makes sense for a logged in user.

kieckhafer commented 4 years ago

@YuuwakU Can you try the latest version of the branch here, just pushed another update that I think fixes the issue: https://github.com/reactioncommerce/reaction/pull/5744/files

garyrvaz commented 4 years ago

@kieckhafer Trying to check it, but some Oauth2 2.0 client does not exist error happens when I click on sign in or create account from example-storefront.

kieckhafer commented 4 years ago

@jm18457 This should be resolved by #5744, and will be in our next release.

garyrvaz commented 4 years ago

@kieckhafer it works. Took me a while to get the auth pages working due to this: https://github.com/reactioncommerce/example-storefront/issues/606 . Also, I had referened #5738 to this issue as a duplicate as the underlying bug is the same, should I reopen it?

willopez commented 4 years ago

@kieckhafer can this issue be closed?

kieckhafer commented 4 years ago

As far as I'm concerned, yes. @jm18457 You can reopen this ticket if you feel your issue hasn't been resolved.