[GitHub] [trafficcontrol] rawlinp commented on pull request #5848: User Permissions Blueprint

[GitBox <gi...@apache.org>]

rawlinp commented on pull request #5848:
URL: https://github.com/apache/trafficcontrol/pull/5848#issuecomment-891994128


   > If Roles are taken out of the API, then would we not still need to make API changes, so that the configuration file contents can be returned through the API?
   
   Not necessarily, we don't _need_ the API to return which permissions belong to which roles.
   
   > How would we migrate existing Roles to the on-disk configuration? How could that migration be undone for downgrades?
   
   Since User Permissions are optional, it seems like we'll still have to define privilege levels for every route we add. So, I'd think our existing roles wouldn't even belong in the on-disk config. Their permissions would continue to be defined by the privilege levels we assign to routes. If people created custom roles with different privilege levels, they will have to translate those custom roles into the new User Permissions model.
   
   > Testing this would also be difficult to do in the current framework, since creating or deleting Roles would require restarting Traffic Ops, which for the moment makes it a manual process during testing.
   
   Testing permissions would be pretty easy via unit tests in this manner. You just simply update TO's internal memory representation (e.g. `map[string]map[string]struct{}`), then make an HTTP request. For the most part, you wouldn't have to mock the DB very much, since the request would be 403'd before even needing most DB queries. You'd probably just have to mock the DB for the user data (i.e. which role the user has). Long-term, testing via this method would probably be easier than writing API tests just to verify permissions functionality. In fact, the API tests we currently have to test privileges are pretty convoluted and confusing to begin with, since they require creating a new session, logging into the new session, and using the new session to attempt the requests.
   
   > I also think that being able to manipulate user permissions through Traffic Portal, and in particular the ability to create a new service account with permissions for only a few endpoints it needs quickly through Traffic Portal, would be a shame to trade away for the very manual process
   
   It wouldn't be a shame if the trade-offs were well-understood and taken into consideration. I think creating roles and assigning permissions will be one of the most rare operations that ever occurs in an ATC environment. The default roles have covered 99% of all cases up to this point, we just want something better now (for security). Since better security is the main goal here, it might make sense to make the implementation of this security feature as secure as possible.
   
   > of ensuring everyone's okay with restarting TO, ssh in and edit a configuration file, then perform the restart and signal the all-clear (or, using Ansible as you said, which would probably be a longer process but safer than ssh-ing manually)
   
   It wouldn't necessarily require a restart; we could make it reloadable if restarting TO was a problem. IMO, the way TO is designed, restarting is not really an issue in terms of availability. We can rolling-restart TO and still maintain 100% availability. Also, I would argue that most permissions requests wouldn't be so time-sensitive that we'd need to be able to quickly use TP to add permissions. If something _was_ really time-sensitive, then any admin should be able to perform the time-sensitive task or get the user the time-sensitive data.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@trafficcontrol.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org