> On Dec. 2, 2015, 9:41 a.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RepositoryVersionResourceProvider.java, > > lines 284-286 > > <https://reviews.apache.org/r/40805/diff/2/?file=1149940#file1149940line284> > > > > Why is this check inside of an if-statement? By virtue of the fact that > > you're in the `updateResourcesAuthorized` method, wouldn't that mean that > > you need to run this check regardless of what data is in the map? > > Robert Levas wrote: > This is basically the crux of the issues forcing us to get deep into the > logic of the resource providers in order to perform authorization checks. > > The `updateResourcesAuthorized` method is _protected_ such that the > authenticated user must have the privileges to _manage stack versions_ *or* > _edit stack repositories_ in order to perform the operation. See line 150: > > ``` > > setRequiredUpdateAuthorizations(EnumSet.of(RoleAuthorization.AMBARI_MANAGE_STACK_VERSIONS, > RoleAuthorization.AMBARI_EDIT_STACK_REPOS)); > ``` > > Once allowed _update resources_, we need to figure out what the user is > tring to do. If the user is trying to set the stack repositories, then we > need to make sure that he is authorized to do so. Hence the if check before > the authorization check. > > Jonathan Hurley wrote: > Are you saying that they might have gotten this far because they have the > RoleAuthorization.AMBARI_MANAGE_STACK_VERSIONS permission but may not have > the RoleAuthorization.AMBARI_EDIT_STACK_REPOS permission so we need to do a > fine grain check?
Correct... Having one of the required authorizations gets you in the door, but may not get you into the VIP room. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/40805/#review108647 ----------------------------------------------------------- On Dec. 1, 2015, 9:24 p.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/40805/ > ----------------------------------------------------------- > > (Updated Dec. 1, 2015, 9:24 p.m.) > > > Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav > Papirkovskyy, Nate Cole, and Sumit Mohanty. > > > Bugs: AMBARI-14114 > https://issues.apache.org/jira/browse/AMBARI-14114 > > > Repository: ambari > > > Description > ------- > > Enforce granular role-based access control for stack version functions: > > | Cluster User | Service Operator | Service > Administrator | Cluster Operator | Cluster Administrator | Administrator > ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|--------------- > > View stack version details | (+) | (+) | (+) > | (+) | (+) | (+) > Manage stack versions | | | > | | | (+) > Edit stack repository URLs | | | > | | | (+) > > Entry points affected: > - GET /api/v1/stacks/:stack_name/versions/:version_id > - GET /api/v1/stacks/:stack_name/versions/:version_id > - PUT /api/v1/stacks/:stack_name/versions/:version_id > - POST /api/v1/stacks/:stack_name/versions/:version_id > - DELETE /api/v1/stacks/:stack_name/versions/:version_id > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/RepositoryVersionResourceProvider.java > 062b0cb > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java > 7f88286 > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/CompatibleRepositoryVersionResourceProviderTest.java > 4e4386e > > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/RepositoryVersionResourceProviderTest.java > dfaef98 > > ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java > 634d840 > > Diff: https://reviews.apache.org/r/40805/diff/ > > > Testing > ------- > > manually tested > > # Local test resuts: > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 59:46.219s > [INFO] Finished at: Mon Nov 30 18:47:07 EST 2015 > [INFO] Final Memory: 66M/923M > [INFO] > ------------------------------------------------------------------------ > > # Jenkins test resuts: > > > Thanks, > > Robert Levas > >
