absmach / magistrala

Industrial IoT Messaging and Device Management Platform
https://www.abstractmachines.fr/magistrala.html
Apache License 2.0
2.46k stars 671 forks source link

Share thing works with users from different domains #2266

Closed nyagamunene closed 3 months ago

nyagamunene commented 3 months ago

What were you trying to achieve?

I'm trying to share and unshare a thing between domain mambers,

What are the expected results?

To successfully share thing between domain members only.

What are the received results?

It is possible to share a thing between members from different domains. Also unshare thing with a correct thing ID and a random user ID passes successfully, and it can be done multiple times successfully.

In what environment did you encounter the issue?

Docker

Steps To Reproduce

JeffMboya commented 3 months ago

@dborovcanin @arvindh123 I tried to recreate this issue and found that user1 who has relationmember in domain1 cannot share thing1 which is in domain1 with user2 who has relation member in domain2. However, I did realize that super admin can sharething1 with user2. Is this supposed to be the case i.e super admin sharing thing between domains?

arvindh123 commented 3 months ago

@dborovcanin @arvindh123 I tried to recreate this issue and found that user1 who has relationmember in domain1 cannot share thing1 which is in domain1 with user2 who has relation member in domain2. However, I did realize that super admin can sharething1 with user2. Is this supposed to be the case i.e super admin sharing thing between domains?

@JeffMboya No it should not happen if user_2 is not in domain_1.

dborovcanin commented 3 months ago

I confirm this is a bug. @JeffMboya please take this one.

nyagamunene commented 3 months ago

@dborovcanin @arvindh123. On further testing with @JeffMboya, there was an oversight which we have caught and the superadmin cannot share a thing between users in different domains. The issue is on unsharing an entity with non exist users. The operation can be done multiple times as per this comment by @WashingtonKK.

arvindh123 commented 3 months ago

@dborovcanin @arvindh123. On further testing with @JeffMboya, there was an oversight which we have caught and the superadmin cannot share a thing between users in different domains. The issue is on unsharing an entity with non exist users. The operation can be done multiple times as per this comment by @WashingtonKK.

Yes , For unhare, the response should be no content , It doesn't tell you it is removed successfully. This made like this to avoid finding the entity ids with brut force. May be we can return 404 instead of no content

JeffMboya commented 3 months ago

It doesn't tell you it is removed successfully. This made like this to avoid finding the entity ids with brut force. May be we can return 404 instead of no content

@nyagamunene for unshare operation, the policy is deleted for all users (whether the user is part of magistrala or non-existent/random) by design