{"action":"created","issue":{"url":"https://api.github.com/repos/salopensource/sal/issues/210","repository_url":"https://api.github.com/repos/salopensource/sal","labels_url":"https://api.github.com/repos/salopensource/sal/issues/210/labels{/name}","comments_url":"https://api.github.com/repos/salopensource/sal/issues/210/comments","events_url":"https://api.github.com/repos/salopensource/sal/issues/210/events","html_url":"https://github.com/salopensource/sal/pull/210","id":300377255,"number":210,"title":"Refactor and audit access code","user":{"login":"sheagcraig","id":3568072,"avatar_url":"https://avatars1.githubusercontent.com/u/3568072?v=4","gravatar_id":"","url":"https://api.github.com/users/sheagcraig","html_url":"https://github.com/sheagcraig","followers_url":"https://api.github.com/users/sheagcraig/followers","following_url":"https://api.github.com/users/sheagcraig/following{/other_user}","gists_url":"https://api.github.com/users/sheagcraig/gists{/gist_id}","starred_url":"https://api.github.com/users/sheagcraig/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sheagcraig/subscriptions","organizations_url":"https://api.github.com/users/sheagcraig/orgs","repos_url":"https://api.github.com/users/sheagcraig/repos","events_url":"https://api.github.com/users/sheagcraig/events{/privacy}","received_events_url":"https://api.github.com/users/sheagcraig/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":3,"created_at":"2018-02-26T20:05:23Z","updated_at":"2018-03-01T20:34:47Z","closed_at":null,"author_association":"CONTRIBUTOR","pull_request":{"url":"https://api.github.com/repos/salopensource/sal/pulls/210","html_url":"https://github.com/salopensource/sal/pull/210","diff_url":"https://github.com/salopensource/sal/pull/210.diff","patch_url":"https://github.com/salopensource/sal/pull/210.patch"},"body":"This PR creates a `level_required` decorator, and `staff_required` decorator, and an `access_required` decorator to greatly eliminate redundant permissions testing, and redirection or error handling code.\r\n\r\n- `level_required` allows you to decorate a view function with `UserProfile.level` values that should be allowed to proceed.\r\n- `staff_required` allows you to decorate a view function that should only allow staff users to access them (these are entirely user management views).\r\n- `access_required` allows you to specify that a view should only proceed for users who are either global admins, or have permission on the `BusinessUnit` that a view is retrieving data from. (This could be a `BusinessUnit`, a `MachineGroup`, or a `Machine`).\r\n\r\nAdditionally, as this was no minor undertaking, I broke up the `server.views` into three separate modules based on function (UI views, settings views, and non-UI views). Whenever I worked on some code, if it had unused code, redundant code, poor style, etc, I updated it.\r\n\r\nFor example, in several places the context for a view is initially populated with `csrf(request)`. But then later in the view function the context is assigned a dict, overwriting the previous data. So those were all removed.\r\n\r\nTests are included for the decorators.\r\n\r\nAnnoyingly, I discovered a mistake in the (re-enabled due to another PR from me) the setting of \"never\" for your Sal version notification policy, which is fixed with a6d4b77e88f1be8183f30395546b717ddcecb3c9\r\n\r\nA number of lax permissions mistakes were discovered and corrected. I can summarize more specifically if desired, but these basically fell into several permutations:\r\n1. Views which had no links to unprivileged users presented in the UI were still available if a properly formed request was made.\r\n2. Some settings endpoints were available to any authenticated user.\r\n3. Views were missing `login_required` decorator; this is minor as they all had more restrictive decorators additionally, but I put `login_required` in for consistency. (explicit is better than implicit and whatnot).\r\n4. A properly formed request could get `machine_detail_facter` and `machine_detail_conditions` data with an authenticated but unprivileged user.\r\n5. An unprivileged user who was a member of a Business Unit could delete machines!\r\n\r\nDuring the course of this PR I also figured out how to override the `User` admin page, so I removed the `UserProfile` admin page and replaced the stock `UserAdmin` with a customized one that includes an inline `UserProfile`. (It's better!)\r\n\r\nThe \"X Machine\" delete-this-computer displayed on a machine detail view, even if the viewing user didn't have permission to actually delete a machine. The modal would not display, but I have thus removed the button entirely from the navbar.\r\n\r\nThe base template included a line that was supposed to show the logged in user's level. However, it had a typo and didn't display. Also, there's no easy way to know who you are logged in as, so I added a menu item that includes username and level to the menu.\r\n\r\n# Outstanding Issues\r\n- We need to determine what Sal should do when a user should not have access to something. As it stands, there is a mix of redirects, 403, and 404.\r\n- RW users are able to create, edit, and delete machines, and create and edit machine groups; but not delete them. Is this an oversight, or intended?"},"comment":{"url":"https://api.github.com/repos/salopensource/sal/issues/comments/369722180","html_url":"https://github.com/salopensource/sal/pull/210#issuecomment-369722180","issue_url":"https://api.github.com/repos/salopensource/sal/issues/210","id":369722180,"user":{"login":"sheagcraig","id":3568072,"avatar_url":"https://avatars1.githubusercontent.com/u/3568072?v=4","gravatar_id":"","url":"https://api.github.com/users/sheagcraig","html_url":"https://github.com/sheagcraig","followers_url":"https://api.github.com/users/sheagcraig/followers","following_url":"https://api.github.com/users/sheagcraig/following{/other_user}","gists_url":"https://api.github.com/users/sheagcraig/gists{/gist_id}","starred_url":"https://api.github.com/users/sheagcraig/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/sheagcraig/subscriptions","organizations_url":"https://api.github.com/users/sheagcraig/orgs","repos_url":"https://api.github.com/users/sheagcraig/repos","events_url":"https://api.github.com/users/sheagcraig/events{/privacy}","received_events_url":"https://api.github.com/users/sheagcraig/received_events","type":"User","site_admin":false},"created_at":"2018-03-01T20:34:47Z","updated_at":"2018-03-01T20:34:47Z","author_association":"CONTRIBUTOR","body":"Let's call this good for now.\r\n\r\nI have another chunk of code that I've identified to work on, but it can easily be completely separate from this. _hint_ it's plugins."}}
{"actor":{"display_login":"sheagcraig"}}