Reaction: Cannot view anonymous order when logged in.

[samkelleher]

Checkout as an anonymous customer, you get an order with an ID and token so you can visit the order complete page /order/:id/:token in your storefront.

When you visit this page not logged in, you see the order details, as the token is in the URL.

Now if you login, the order is not found.

It appears that once logged in, the token value is ignored and it will attempt to find the order from your account. The order resolver, when given a order ID and token, should use these to locate the order, and ignore who they are logged in with. If it's presented with only an Order ID and no token, then it should try to find the order from the current account.

[kieckhafer]

Seems like we could rework this getOrderQuery function a little to accommodate this: https://github.com/reactioncommerce/reaction/blob/release-3.0.0/src/core-services/orders/util/getOrderQuery.js#L25-L32

[aldeed]

I think the contextAccountId and token if blocks could just be flipped. Whoever does this should add an integration test to confirm it's fixed and ensure it doesn't get flipped back in the future.

[kieckhafer]

@aldeed do you think if there is contextAccountId and a token we should check to make sure the logged in person is the same person the order belongs to? Maybe something like this?

export async function getOrderQuery(context, selector, shopId, token) {
  const { accountId: contextAccountId } = context;
  const newSelector = { ...selector, shopId };
  const userHasPermission = await context.userHasPermission("reaction:orders", "read", { shopId, legacyRoles: ["orders", "order/fulfillment", "order/view"] });

  // users with any `["orders", "order/fulfillment", "order/view"]` permissions can view any orders
  if (userHasPermission) {
    return newSelector;
  }

  // if only an anonymous access token is provided for this order, any there is no `contextAccountID`, any user can view
  if (token && !contextAccountId) {
    newSelector["anonymousAccessTokens.hashedToken"] = hashToken(token);
    return newSelector;
  }

  // users without any `["orders", "order/fulfillment", "order/view"]` permissions can only view their own orders
  if (contextAccountId) {
    newSelector.accountId = contextAccountId;
    return newSelector;
  }

  throw new ReactionError("access-denied", "Access Denied");
}

Or is a just a token good enough even if it's not for the same user?

[samkelleher]

Thanks for jumping on this @kieckhafer - To clarify, if you have an order token, you should be able to see the order, regardless if you are logged in or not. If I can see an order not logged in, I should still be able to see it logged in.

As far as I know, accounts with an order do not have a token anyway (I'm raising a separate issue to propose that they should) so you still couldn't see an order from a different account.

[aldeed]

@kieckhafer I don't think if (token && !contextAccountId) would actually fix the issue because the problem is not being able to access an order that does have token but does not have accountId, when logged in. The "when logged in" part means that contextAccountId would have a value, and so would token.

I still think just flipping the order of the if else is enough, but it's hard to say for sure without writing some tests to prove it.

As I'm looking more closely at this, I think it would be a lot simpler to just find the document by ID/referenceId, and then compare the token and/or accountId on the found document to determine access. So wherever getOrderQuery is used, we'd instead call a different fn something like validateOrderAccess(context, { order, token }). It might also be easier to use new validatePermissions with ownerId in that case (though token access would still need to be handled specially).

What we want are passing tests for all of these cases:

• context user has "orders:read": allow
• context account matches order.accountId, no token: allow
• context account matches order.accountId, yes token: allow
• context account does not match order.accountId and order.accountId is not null, no token: deny
• context account does not match order.accountId and order.accountId is not null, yes token: allow if token matches?
• context account does not match order.accountId and order.accountId is null, no token: deny
• context account does not match order.accountId and order.accountId is null, yes token: allow if token matches
• context account is null, no token: deny
• context account is null, yes token: allow if token matches

[kieckhafer]

@aldeed there was a PR open for this, which was accidently closed when we deleted the 3.0 branch. Here is the replacement. I don't think there is much more work to go on it, I can take a look at getting it up to par: https://github.com/reactioncommerce/reaction/pull/6092

[kieckhafer]

@aldeed https://github.com/reactioncommerce/reaction/pull/6092 has been updated with the latest and is good for testing.