Feature/service instance scope #221
Conversation
|
We have created an issue in Pivotal Tracker to manage this. You can view the current status of your issue at: http://www.pivotaltracker.com/story/show/70726886. This repo is managed by the 'Runtime' team. |
|
FYI - The Travis issue is in the process of being fixed. They should have new build environment deployed later tonight. |
|
|
||
| context 'any user using client without cloud_controller.write' do |
There was a problem hiding this comment.
Shouldn't there be a test for a user that is granted cloud_controller.write since that was exposed as a possiblity via buildpack_access.rb?
There was a problem hiding this comment.
Good catch. Maybe the best thing to do is remove the write scope requirement: the upload action is only allowable by admins (that is, user/client with cloud_controller.admin scope), so the write scope is irrelevant.
I'm going to make another commit that take outs the write scope requirement. In that case, do you think it's important to include a test for case that the user has write scope but not read scope? It seems like that might be superfluous.
We're trying to separate these two concerns as much as possible: - Token decoding sets the token_info in the SecurityContext. Valid values are stored directly, invalid values are stored as the symbol :invalid_token, and missing values are stored as nil. - Authentication happens completely in the check_authentication method of the base controller model (base.rb). Raises NotAuthenticated if the token is missing; raises InvalidAuthToken if there was an error decoding the token. This method is called before every controller action is dispatched. - Authorization happens during the validate_access method of model_controller. The logic for authorization is maintained in the Allowy access classes. Controller actions are responsible for calling this method (or find_and_validate_access, which defers to validate_access) directly.
scopes This commit changes the way that our `validate_access` helper enforces access to resources. Access is determined in two ways: - First, the client used to access the resource must have appropriate scopes. The scopes necessary to access a particular resource are defined in the resource's access class, in a method with form `<action>_with_token?`. The action passed to `validate_access` is appended with `_with_token?` before being passed to Allowy. For example, enforcment of scopes on the read action for an App resource is found in the `read_with_token?` method in the AppAccess class (app_access.rb) - Second, the user must have access to actions on the resource as defined by the usual methods in the resource's access class. For example, enforcement of authorization on the read action for an App resource is found in the `read?` method in the AppAccess class (app_access.rb) We also included a few minor refactoring/renames. In particular, the upload action in the buildpack_bits_controller now uses the same name (instead of :read_bits) to determine access.
cloud_controller_service_permissions.read scope in addition to cloud_controller.read [finishes #68037500]
This action is only permitted for admin users
This code is unused. The JobsController allows unauthenticated access, and does not use any of the valide_access methods from model_controller, so the JobsAccess is not needed.
All authentication logic happens during the check_authentication method -- before this access class is ever used. Because the desired behavior is that any authenticated user can perform the read action on the class, the access class does need to check any user permissions and only needs to return true.
Feature/service instance scope
|
Reviewed, tests pass, nice work guys. |
Some feature work in the Services team backlog to enforce a new scope necessitated much larger changes in the structure of authentication/authorization code. Because the change is so big, we were hoping for a code review from a pair on the Runtime team.
Our commit messages attempt to describe in detail how each commit refactors the existing behavior. If you have any questions, come find David Sabeti, Nick Calugar, or Alex Stupakov.