fix: find by _id OR anonymousAccessToken

[MohanNarayana]

Signed-off-by: Mohan Narayana mohan.narayana@mailchimp.com

Resolves #7
Impact: minor
Type: bugfix

Issue

When querying using anonymousAccessToken from DB the search breaks as the anonymousAccessToken is already hashed in the DB. If either cartId or anonymousAccessToken is wrong, the query returns null.

Solution

Add or condition to findOne(). Cart ID is unique in the first place so it should find the right anonymous cart. If cart ID is wrong or cannot find a record then query is tried with anonymousAccessToken.
NOTE: The anonymousAccessToken stored in browser is not hashed and queries correctly without the or condition for the query parameters.

Testing

Run the below query before and after pulling the fix.

{
  anonymousCartByCartId(cartId:"<cartId>", cartToken:"<anonymousAccessToken>"){
    missingItems{
      title
      productConfiguration{
        productId
        productVariantId
      }
    }
    items{
      nodes{
        title
      _id
      }
    }
  }
}

Expected Result

[rc-publisher]

🎉 This PR is included in version 1.3.1 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[derBretti]

Isn't the whole point of using access tokens to secure the API, i.e. to prevent users who know the cart ID from accessing the cart unless they also have a corresponding token? The way it is implemented now, there is little sense in using an access token at all, since the query returns a result as soon as a cart with the ID is found.

[MohanNarayana]

Hi @derBretti, thanks for looking into this PR. The anonymousAccessToken is used as an ID for a cart created without an account (for an anonymous user). The naming is confusing but it does not have anything to do with security for this query. The query is set to find a cart by the cartID or anonymousID.
We appreciate you posting this question and encourage you to bring up any thoughts or issues that you may find in the future.

[derBretti]

If the purpose of using the token wasn't adding security, why should it be used at all? When the cart is created, both, ID and token are returned, so there are no cases in which a client knows the token, but not the ID. Requiring the token in the anonymousCartByCartId request, as defined in cart.graphql, would make little sense. The comment at the token field in type CreateCartPayload also hints at it's intended use.
In practice, not requiring the token to match, means that anybody who knows a cartID (and we should not assume that they are impossible to guess), has access the corresponding cart with all the personal data it might include.
I don't see, why a mechanism that added security and did work, is removed now and why it is removed in one place only.

[tedraykov]

@MohanNarayana The token has to do with authorization (security), just hasn't been implemented completely in the carts plugin in my opinion. Looking at the orders plugin we can see the anonymousAccessToken is used for securing access to the order from either the users that do not have permissions or don't have the correct anonymous access token.

• https://github.com/reactioncommerce/api-plugin-orders/blob/f2a0dd6abf6b0750f4a06774e345fa06ad7c1a0e/src/util/getOrderQuery.js#L25

With this new implementation and using an OR clause we are treating the anonymousAccessToken just as a boolean flag, telling the query trust me this cart is anonymous. This is even a vulnerability because by providing a random string as an anonymousAccessToken and a leaked account cart id, you have full access.

I think we should treat it as a security mechanism that throws an error when the wrong access token is provided for an anonymous cart/order.