Products created in one shop showing up in all shops

[kathiresh-i2i]

Issue Description

This issue only applies to a marketplace setup.

As a merchant admin, the product grid shows all products available. That is, you can see products from other merchants as well as the marketplace owner's products. You can't edit them though, so there's no direct security issue arising through this bug.

As a user, only those products that belong to the appropriate shop are displayed, which is the expected behaviour, IMHO.
E.g.

• http://localhost:3000 shows all products from all merchants
• http://localhost:3000/shop/merchant1 shows all products from merchant 1
• http://localhost:3000/shop/merchant2 shows all products from merchant 2

Steps to Reproduce

1. As marketplace owner, enable marketplace and create a product
2. Invite two merchants: merchant1 and merchant2
3. As merchant1 admin, create product
4. Observe that the logged-in merchant 2 can see products from marketplace owner and merchant 1

Versions

Node: 8.2.1
NPM: 5.3.0
Meteor Node: 8.9.4
Meteor NPM: 5.6.0
Reaction CLI: 0.29.0
Reaction: 1.10.0
Reaction branch: master
Docker: 17.03.1-ce

[kathiresh-i2i]

Issue Description and resolution

Products created in one shop showing up in all shops.

1. Logged in to 'Testshop1' as 'Testuser1'

2. Created a product called 'Created on 'test shop 1' in 'Testshop 1'

​

3. Logged out

4. Logged in as 'Testuser2' and was automatically redirected to 'Testshop2'

5. The product 'Created on test shop 1' was still visible in 'Testshop2'

​

Changes:

Filepath:

imports/plugins/included/productvariant/containers/productsContainer.js

Line No: 109

// const shopIdOrSlug = Reaction.Router.getParam("shopSlug"); // 

Replace this line with 

const shopIdOrSlug = Reaction.getShopId(); 

After this change was made products were only visible in the stores they were created in.

@prinzdezibel is this solution valid?

[brent-hoover]

@spencern Just want to verify that this is the expected behavior:

http://localhost:3000 shows all products from all merchants
http://localhost:3000/shop/merchant1 shows all products from merchant 1
http://localhost:3000/shop/merchant2 shows all products from merchant 2

[spencern]

That's how it's been designed to this point

[nithin-ideas2it]

@spencern Just would like to know, at which level we can achieve multi-tenant.

we already referred. #357

We assumed shop1 product should not display in shop2. We also trying like a single supplier can have multiple shops, One supplier shops details and products should not be displayed to another.

Could you suggest how to achieve this with the current system?

[brent-hoover]

@nithin-ideas2it I think you have misunderstood Spencer's comment. He is agreeing that the current implementation (assuming that this bug is reported correctly) is incorrect and needs to be fixed.

[nithin-ideas2it]

@zenweasel Sorry I thought to know the multi-tenant structure after the fix so that we can change few things on our side. Let us wait for the update. Thank you.

[spencern]

@nithin-ideas2it I think this is a legitimate bug in our marketplace product publication or subscription code likely and I'd look at a PR that identifies what the problem was and provides a good solution.

I think your proposed fix on line 109 is not ideal because it appears to remove the product filter from the URL and to hard-code it to the active shop.

I'd also look at a PR that creates a method to toggle this behavior on/off from the operator interface if this is the functionality that your multi-tenant marketplace needs, though I'd want to understand the use-case better.

@pmn4 I think you've been working on a multi-tenant marketplace, have you experienced similar issues with products showing up in grids that they aren't supposed to? Any other solutions come to mind?

[pmn4]

tl;dr: I have noticed this, yes.

My memory is fuzzy, but when I first got started with Reaction, I noticed that Primary Shop products had been appearing in Merchant Shops and vice-versa. I didn't get too far into sorting out why before I re-wrote the Products subscription to only show for the current Shop (@kathiresh-i2i's solution would have been much simpler!). Compounding the problem is that I use the domains feature of the Shop schema (hosting my marketplace shops on domains other than that of the Primary Shop), resulting in Reaction.Router.getParam("shopSlug"); always being blank.

My use of Reaction is less like a Marketplace and more like a white-label solution for shops -- I disable the Primary Shop grid, instead showing a grid of the merchant shops -- I had no need for a combined grid page, making the solution much simpler.

It seems like a combination of getShopId() and isPrimaryShop() can get us close to the intended feature.

My question is whether this check should take place server-side in Meteor.publish("Products") instead of in the client. It seems like a small security gap to expose all products via DDP, selecting only those for the current shop.

[spencern]

I hadn't noticed that this was a client-side check. That's a good catch. The general rule should be that the client should never be sent data it doesn't need or shouldn't have and that those filters should take place in the database ideally, or on the server.