-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tighter Crossbar topic/rpc ACLs #30
Comments
@joelee484 @janeeisenstein @fcurella Thoughts? |
I like enforcing the data model's access constraints as currently documented and implemented in the Simpl code. In particular, only leaders should be able to subscribe to runs. Once that’s in place, preventing players from subscribing to worlds they’re not in becomes less urgent , but should still be done. With the addition of game level RPC's for wharton-simpl-admin, I'd like to be able to limit their access to admin users. Would that be covered by your second proposal? |
Could we leverage something like django-guardian and provide API views on simpl-games-api for model services to check for permissions? Basicall pass a user id, content-type and object id and it returns True/False. Then leave it to the specific game implementation to do whatever they want with that info. In addition, we could also provide some convenience functions or decorator for common scenarios like |
@janeeisenstein "admin" level RPC calls makes sense to me, I'll add that into the mix. |
@fcurella wouldn't repeated calls back to the API for things like Decisions/Results get cumbersome? |
@frankwiles Currently, an RPC user is required to be registered as either a leader or player of the game -- except when the RPC is defined at the game level. I'm not sure that's a bad thing, but it is a thing. |
Assuming that row-level permission is a pretty common scenario (use-case: players can only access a specific run), at some point the permission needs to be persisted in the API service in order to survive a restart of the model service. Querying every time would be cumbersome in terms of network chattiness, but I think we can alleviate that with some light caching on the model service. As for 'cumbersome' from an implementor perspective (ie: coding a sim), that's why we should write a set of decorators or functions for at least the most common cases. |
@fcurella At Wharton, we currently limit players to being able to access one active run at a time. Players actually have free access to at most one world as well as their private scenarios. This is supported using the current simpl-modelservice and default frontend view code. Please explain how an individual player might attempt to access other runs. |
@janeeisenstein using runs was a wrong example on my part. Runs are already related to Users in IIRC a |
Decisions are associated with roles rather than users. That said, there are real world use cases for hiding some of the world information that is currently loaded into a player's browser. Currently, all of a world's decisions and results are shared with all players in the world. For competitive games, it makes sense to support only loading decisions and results created with the current player's role. Currently, all the runusers in the player's world are loaded into the browser. There is a use case to support hiding runuser information of a world's players from each other for privacy reasons. @frankwiles what use cases were you given for implementing this feature? I'd like to know whether hiding currently shared data belongs to this issue or should go into separate issues. |
@fcurella Filtering occurs in the simpl-modelservice to ensure users access only data that is appropriate for them. Perhaps review the current simpl-modelservice and simpl-react code and see where a user might gain access to inappropriate data via WAMP? |
@frankwiles only leaders/admins should be able to invoke |
We've discovered a small security issue with how we only check for authenticated users when allowing pub/sub and rpc calls and not whether or not that particular topic or call makes sense for that particular user.
I've got a client who is offering to fund the fix and development of more granular controls. I'm opening this ticket to start the discussion and planning with the rest of the team.
For RPC calls I think the easiest thing to do is:
@register_leader_only
decorator which denies access for any non-leader logins. This would cover many of the issues with most users.When it comes to players however, I think it gets a bit more complicated. Do we just "enforce" the data model? i.e. I'm not allowed to subscribe to Runs or Worlds I'm not a part of in the API.
Or do we provide a real ACL like system going forward? I think first stab is just a "enforce or don't enforce" the data model settings option but if anyone has any brilliant ideas I'm listening!
The text was updated successfully, but these errors were encountered: