You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ambari.apache.org by Robert Levas <rl...@hortonworks.com> on 2015/11/30 15:37:15 UTC

Review Request 40793: Enforce granular role-based access control for cluster functions

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/
-----------------------------------------------------------

Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.


Bugs: AMBARI-14072
    https://issues.apache.org/jira/browse/AMBARI-14072


Repository: ambari


Description
-------

Enforce granular role-based access control for cluster functions:

                           | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
Create new clusters        |              |                  |                       |                  |                       |(+)            
Rename clusters            |              |                  |                       |                  |                       |(+)            

Entry points affected:
* PUT /api/v1/clusters/:cluster_name 
* POST /api/v1/clusters/:cluster_name 

# Note: Read-only requests (GET) are not protected so that the front end is not broken.


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0e3e7b8 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
  ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ca3ca36 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
  ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
  ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
  ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
  ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
  ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
  ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 

Diff: https://reviews.apache.org/r/40793/diff/


Testing
-------

Manually tested

# Local test results:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 58:54.570s
[INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
[INFO] Final Memory: 67M/1340M
[INFO] ------------------------------------------------------------------------

#Jenkins test results: PENDING


Thanks,

Robert Levas


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Robert Levas <rl...@hortonworks.com>.

> On Nov. 30, 2015, 2:20 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java, lines 67-68
> > <https://reviews.apache.org/r/40793/diff/1/?file=1148360#file1148360line67>
> >
> >     Can this be one pattern?  Somewhere the API would have to restrict to "clusters only I can see", but you still have to grant to /clusters if someone can see (say) /clusters/c1

Technically this is temporary code to disable the Authorization filter code while the role-based access control stuff is being slowly implemented. But I will fix it.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108365
-----------------------------------------------------------


On Nov. 30, 2015, 9:37 a.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 9:37 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> * PUT /api/v1/clusters/:cluster_name 
> * POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0e3e7b8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ca3ca36 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108365
-----------------------------------------------------------



ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java (lines 67 - 68)
<https://reviews.apache.org/r/40793/#comment167806>

    Can this be one pattern?  Somewhere the API would have to restrict to "clusters only I can see", but you still have to grant to /clusters if someone can see (say) /clusters/c1


- Nate Cole


On Nov. 30, 2015, 9:37 a.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 9:37 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> * PUT /api/v1/clusters/:cluster_name 
> * POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0e3e7b8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ca3ca36 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Robert Levas <rl...@hortonworks.com>.

> On Dec. 1, 2015, 6 p.m., Alejandro Fernandez wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java, line 186
> > <https://reviews.apache.org/r/40793/diff/2/?file=1149350#file1149350line186>
> >
> >     Should these be enums or string constants?

This was existing code just moved around. It wasnt part of the patch, so I think it shouldn't be touched.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108578
-----------------------------------------------------------


On Dec. 1, 2015, 5:35 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2015, 5:35 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> - PUT /api/v1/clusters/:cluster_name 
> - POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0e3e7b8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ca3ca36 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Alejandro Fernandez <af...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108578
-----------------------------------------------------------

Ship it!



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java (line 185)
<https://reviews.apache.org/r/40793/#comment168029>

    Should these be enums or string constants?


- Alejandro Fernandez


On Dec. 1, 2015, 10:35 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2015, 10:35 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> - PUT /api/v1/clusters/:cluster_name 
> - POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0e3e7b8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ca3ca36 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108887
-----------------------------------------------------------

Ship it!


Let's create a Jira to track the movement of stuff from AMC into the Cluster Resource Provider; for now we can push this in to get to our tech preview.

- Jonathan Hurley


On Dec. 1, 2015, 9:34 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2015, 9:34 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> - PUT /api/v1/clusters/:cluster_name 
> - POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java c0dc342 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java e2ec5e0 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/
-----------------------------------------------------------

(Updated Dec. 1, 2015, 9:34 p.m.)


Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.


Changes
-------

Removed `-` from test case method names.


Bugs: AMBARI-14072
    https://issues.apache.org/jira/browse/AMBARI-14072


Repository: ambari


Description
-------

Enforce granular role-based access control for cluster functions:

                           | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
Create new clusters        |              |                  |                       |                  |                       |(+)            
Rename clusters            |              |                  |                       |                  |                       |(+)            

Entry points affected:
- PUT /api/v1/clusters/:cluster_name 
- POST /api/v1/clusters/:cluster_name 

# Note: Read-only requests (GET) are not protected so that the front end is not broken.


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java c0dc342 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
  ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java e2ec5e0 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
  ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
  ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
  ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
  ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
  ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
  ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 

Diff: https://reviews.apache.org/r/40793/diff/


Testing
-------

Manually tested

# Local test results:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 58:54.570s
[INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
[INFO] Final Memory: 67M/1340M
[INFO] ------------------------------------------------------------------------

#Jenkins test results: PENDING


Thanks,

Robert Levas


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/
-----------------------------------------------------------

(Updated Dec. 1, 2015, 5:35 p.m.)


Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.


Bugs: AMBARI-14072
    https://issues.apache.org/jira/browse/AMBARI-14072


Repository: ambari


Description (updated)
-------

Enforce granular role-based access control for cluster functions:

                           | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
Create new clusters        |              |                  |                       |                  |                       |(+)            
Rename clusters            |              |                  |                       |                  |                       |(+)            

Entry points affected:
- PUT /api/v1/clusters/:cluster_name 
- POST /api/v1/clusters/:cluster_name 

# Note: Read-only requests (GET) are not protected so that the front end is not broken.


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0e3e7b8 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
  ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ca3ca36 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
  ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
  ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
  ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
  ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
  ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
  ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 

Diff: https://reviews.apache.org/r/40793/diff/


Testing
-------

Manually tested

# Local test results:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 58:54.570s
[INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
[INFO] Final Memory: 67M/1340M
[INFO] ------------------------------------------------------------------------

#Jenkins test results: PENDING


Thanks,

Robert Levas


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Nate Cole <nc...@hortonworks.com>.

> On Dec. 1, 2015, 9:57 a.m., Jonathan Hurley wrote:
> > I have a concern about the way that we're coupling the security logic here. The AMC class is a mess - it really is just a massive steaming pot of "anything goes" - there's no real separation of concerns and no real contract. It feels like we're just catching a few places where front-end logic eventually gets funneled. What about trying to catch this stuff at a lower level? Maybe in the DAOs?
> > 
> > What about catching it at a higher level? Could there be a branch new request processor that inspects the incoming request, picks it apart to see the different things it's trying to do and throws auth exceptions there? This way we wouldn't be trying to find all of the spots sprinkeled throughout the code that we need to intercept.

I like the service-level (endpoint) approach.

DAOs feel like it would get complicated fast, especially when performing joins across tables - trying to decide if the user is authenticated to update rows of one table, but not another (if that's a valid use case).


- Nate


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108503
-----------------------------------------------------------


On Nov. 30, 2015, 5:43 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 5:43 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> * PUT /api/v1/clusters/:cluster_name 
> * POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0e3e7b8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ca3ca36 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Robert Levas <rl...@hortonworks.com>.

> On Dec. 1, 2015, 9:57 a.m., Jonathan Hurley wrote:
> > I have a concern about the way that we're coupling the security logic here. The AMC class is a mess - it really is just a massive steaming pot of "anything goes" - there's no real separation of concerns and no real contract. It feels like we're just catching a few places where front-end logic eventually gets funneled. What about trying to catch this stuff at a lower level? Maybe in the DAOs?
> > 
> > What about catching it at a higher level? Could there be a branch new request processor that inspects the incoming request, picks it apart to see the different things it's trying to do and throws auth exceptions there? This way we wouldn't be trying to find all of the spots sprinkeled throughout the code that we need to intercept.
> 
> Nate Cole wrote:
>     I like the service-level (endpoint) approach.
>     
>     DAOs feel like it would get complicated fast, especially when performing joins across tables - trying to decide if the user is authenticated to update rows of one table, but not another (if that's a valid use case).
> 
> Robert Levas wrote:
>     I agree and think this logic should be at a high(er) level.  However doing so may hinder performance since we need to essentially parse the request multiple times - at least once to determine authorzation and at least once to perform the operation. It will also duplicate logic needed to detemine what the user is trying to do.  
>     
>     Essentually protecting this type of API is very difficult.  We really need a higher-level API for Ambari rather than exposing a web-based SQL front-end. 
>     
>     Regarding the AMC code, I suppose we can move the relevant resource provider code back to the resource provider, but I was trying to limit the amount of code changes. If you look at the methods being altered, they are the ones directly related to request handling.  The lower-level methods are untouched.
> 
> Robert Levas wrote:
>     @jurley, would it be better if the request-specific methods were moved from the AMC and into the proper resource provider?  For example, moving `org.apache.ambari.server.controller.AmbariManagementControllerImpl#createCluster` to `org.apache.ambari.server.controller.internal.ClusterResourceProvider#createCluster`
> 
> Jonathan Hurley wrote:
>     I think it would be better if we did that, yes. It would at least try to keep with the consistency of the implementation approach here.

Moving the relevant methods from the AMC is too large of a task for this effort.  It would require many test cases to be updated along with a few other non-test-related classes.  I think a new JIRA should be created for this.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108503
-----------------------------------------------------------


On Dec. 1, 2015, 9:34 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2015, 9:34 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> - PUT /api/v1/clusters/:cluster_name 
> - POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java c0dc342 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java e2ec5e0 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Robert Levas <rl...@hortonworks.com>.

> On Dec. 1, 2015, 9:57 a.m., Jonathan Hurley wrote:
> > I have a concern about the way that we're coupling the security logic here. The AMC class is a mess - it really is just a massive steaming pot of "anything goes" - there's no real separation of concerns and no real contract. It feels like we're just catching a few places where front-end logic eventually gets funneled. What about trying to catch this stuff at a lower level? Maybe in the DAOs?
> > 
> > What about catching it at a higher level? Could there be a branch new request processor that inspects the incoming request, picks it apart to see the different things it's trying to do and throws auth exceptions there? This way we wouldn't be trying to find all of the spots sprinkeled throughout the code that we need to intercept.
> 
> Nate Cole wrote:
>     I like the service-level (endpoint) approach.
>     
>     DAOs feel like it would get complicated fast, especially when performing joins across tables - trying to decide if the user is authenticated to update rows of one table, but not another (if that's a valid use case).
> 
> Robert Levas wrote:
>     I agree and think this logic should be at a high(er) level.  However doing so may hinder performance since we need to essentially parse the request multiple times - at least once to determine authorzation and at least once to perform the operation. It will also duplicate logic needed to detemine what the user is trying to do.  
>     
>     Essentually protecting this type of API is very difficult.  We really need a higher-level API for Ambari rather than exposing a web-based SQL front-end. 
>     
>     Regarding the AMC code, I suppose we can move the relevant resource provider code back to the resource provider, but I was trying to limit the amount of code changes. If you look at the methods being altered, they are the ones directly related to request handling.  The lower-level methods are untouched.

@jurley, would it be better if the request-specific methods were moved from the AMC and into the proper resource provider?  For example, moving `org.apache.ambari.server.controller.AmbariManagementControllerImpl#createCluster` to `org.apache.ambari.server.controller.internal.ClusterResourceProvider#createCluster`


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108503
-----------------------------------------------------------


On Dec. 1, 2015, 9:34 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2015, 9:34 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> - PUT /api/v1/clusters/:cluster_name 
> - POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java c0dc342 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java e2ec5e0 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Jonathan Hurley <jh...@hortonworks.com>.

> On Dec. 1, 2015, 9:57 a.m., Jonathan Hurley wrote:
> > I have a concern about the way that we're coupling the security logic here. The AMC class is a mess - it really is just a massive steaming pot of "anything goes" - there's no real separation of concerns and no real contract. It feels like we're just catching a few places where front-end logic eventually gets funneled. What about trying to catch this stuff at a lower level? Maybe in the DAOs?
> > 
> > What about catching it at a higher level? Could there be a branch new request processor that inspects the incoming request, picks it apart to see the different things it's trying to do and throws auth exceptions there? This way we wouldn't be trying to find all of the spots sprinkeled throughout the code that we need to intercept.
> 
> Nate Cole wrote:
>     I like the service-level (endpoint) approach.
>     
>     DAOs feel like it would get complicated fast, especially when performing joins across tables - trying to decide if the user is authenticated to update rows of one table, but not another (if that's a valid use case).
> 
> Robert Levas wrote:
>     I agree and think this logic should be at a high(er) level.  However doing so may hinder performance since we need to essentially parse the request multiple times - at least once to determine authorzation and at least once to perform the operation. It will also duplicate logic needed to detemine what the user is trying to do.  
>     
>     Essentually protecting this type of API is very difficult.  We really need a higher-level API for Ambari rather than exposing a web-based SQL front-end. 
>     
>     Regarding the AMC code, I suppose we can move the relevant resource provider code back to the resource provider, but I was trying to limit the amount of code changes. If you look at the methods being altered, they are the ones directly related to request handling.  The lower-level methods are untouched.
> 
> Robert Levas wrote:
>     @jurley, would it be better if the request-specific methods were moved from the AMC and into the proper resource provider?  For example, moving `org.apache.ambari.server.controller.AmbariManagementControllerImpl#createCluster` to `org.apache.ambari.server.controller.internal.ClusterResourceProvider#createCluster`

I think it would be better if we did that, yes. It would at least try to keep with the consistency of the implementation approach here.


- Jonathan


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108503
-----------------------------------------------------------


On Dec. 1, 2015, 9:34 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2015, 9:34 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> - PUT /api/v1/clusters/:cluster_name 
> - POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java c0dc342 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java e2ec5e0 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Robert Levas <rl...@hortonworks.com>.

> On Dec. 1, 2015, 9:57 a.m., Jonathan Hurley wrote:
> > I have a concern about the way that we're coupling the security logic here. The AMC class is a mess - it really is just a massive steaming pot of "anything goes" - there's no real separation of concerns and no real contract. It feels like we're just catching a few places where front-end logic eventually gets funneled. What about trying to catch this stuff at a lower level? Maybe in the DAOs?
> > 
> > What about catching it at a higher level? Could there be a branch new request processor that inspects the incoming request, picks it apart to see the different things it's trying to do and throws auth exceptions there? This way we wouldn't be trying to find all of the spots sprinkeled throughout the code that we need to intercept.
> 
> Nate Cole wrote:
>     I like the service-level (endpoint) approach.
>     
>     DAOs feel like it would get complicated fast, especially when performing joins across tables - trying to decide if the user is authenticated to update rows of one table, but not another (if that's a valid use case).
> 
> Robert Levas wrote:
>     I agree and think this logic should be at a high(er) level.  However doing so may hinder performance since we need to essentially parse the request multiple times - at least once to determine authorzation and at least once to perform the operation. It will also duplicate logic needed to detemine what the user is trying to do.  
>     
>     Essentually protecting this type of API is very difficult.  We really need a higher-level API for Ambari rather than exposing a web-based SQL front-end. 
>     
>     Regarding the AMC code, I suppose we can move the relevant resource provider code back to the resource provider, but I was trying to limit the amount of code changes. If you look at the methods being altered, they are the ones directly related to request handling.  The lower-level methods are untouched.
> 
> Robert Levas wrote:
>     @jurley, would it be better if the request-specific methods were moved from the AMC and into the proper resource provider?  For example, moving `org.apache.ambari.server.controller.AmbariManagementControllerImpl#createCluster` to `org.apache.ambari.server.controller.internal.ClusterResourceProvider#createCluster`
> 
> Jonathan Hurley wrote:
>     I think it would be better if we did that, yes. It would at least try to keep with the consistency of the implementation approach here.
> 
> Robert Levas wrote:
>     Moving the relevant methods from the AMC is too large of a task for this effort.  It would require many test cases to be updated along with a few other non-test-related classes.  I think a new JIRA should be created for this.

Moving the ResourceProvider-specific methods will be address in https://issues.apache.org/jira/browse/AMBARI-14201.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108503
-----------------------------------------------------------


On Dec. 1, 2015, 9:34 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Dec. 1, 2015, 9:34 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> - PUT /api/v1/clusters/:cluster_name 
> - POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java c0dc342 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java e2ec5e0 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Robert Levas <rl...@hortonworks.com>.

> On Dec. 1, 2015, 9:57 a.m., Jonathan Hurley wrote:
> > I have a concern about the way that we're coupling the security logic here. The AMC class is a mess - it really is just a massive steaming pot of "anything goes" - there's no real separation of concerns and no real contract. It feels like we're just catching a few places where front-end logic eventually gets funneled. What about trying to catch this stuff at a lower level? Maybe in the DAOs?
> > 
> > What about catching it at a higher level? Could there be a branch new request processor that inspects the incoming request, picks it apart to see the different things it's trying to do and throws auth exceptions there? This way we wouldn't be trying to find all of the spots sprinkeled throughout the code that we need to intercept.
> 
> Nate Cole wrote:
>     I like the service-level (endpoint) approach.
>     
>     DAOs feel like it would get complicated fast, especially when performing joins across tables - trying to decide if the user is authenticated to update rows of one table, but not another (if that's a valid use case).

I agree and think this logic should be at a high(er) level.  However doing so may hinder performance since we need to essentially parse the request multiple times - at least once to determine authorzation and at least once to perform the operation. It will also duplicate logic needed to detemine what the user is trying to do.  

Essentually protecting this type of API is very difficult.  We really need a higher-level API for Ambari rather than exposing a web-based SQL front-end. 

Regarding the AMC code, I suppose we can move the relevant resource provider code back to the resource provider, but I was trying to limit the amount of code changes. If you look at the methods being altered, they are the ones directly related to request handling.  The lower-level methods are untouched.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108503
-----------------------------------------------------------


On Nov. 30, 2015, 5:43 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 5:43 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> * PUT /api/v1/clusters/:cluster_name 
> * POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0e3e7b8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ca3ca36 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Jonathan Hurley <jh...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108503
-----------------------------------------------------------


I have a concern about the way that we're coupling the security logic here. The AMC class is a mess - it really is just a massive steaming pot of "anything goes" - there's no real separation of concerns and no real contract. It feels like we're just catching a few places where front-end logic eventually gets funneled. What about trying to catch this stuff at a lower level? Maybe in the DAOs?

What about catching it at a higher level? Could there be a branch new request processor that inspects the incoming request, picks it apart to see the different things it's trying to do and throws auth exceptions there? This way we wouldn't be trying to find all of the spots sprinkeled throughout the code that we need to intercept.

- Jonathan Hurley


On Nov. 30, 2015, 5:43 p.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 5:43 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> * PUT /api/v1/clusters/:cluster_name 
> * POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0e3e7b8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ca3ca36 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/
-----------------------------------------------------------

(Updated Nov. 30, 2015, 5:43 p.m.)


Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.


Bugs: AMBARI-14072
    https://issues.apache.org/jira/browse/AMBARI-14072


Repository: ambari


Description
-------

Enforce granular role-based access control for cluster functions:

                           | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
Create new clusters        |              |                  |                       |                  |                       |(+)            
Rename clusters            |              |                  |                       |                  |                       |(+)            

Entry points affected:
* PUT /api/v1/clusters/:cluster_name 
* POST /api/v1/clusters/:cluster_name 

# Note: Read-only requests (GET) are not protected so that the front end is not broken.


Diffs (updated)
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
  ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0e3e7b8 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
  ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ca3ca36 
  ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
  ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
  ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
  ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
  ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
  ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
  ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
  ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 

Diff: https://reviews.apache.org/r/40793/diff/


Testing
-------

Manually tested

# Local test results:
[INFO] ------------------------------------------------------------------------
[INFO] BUILD SUCCESS
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 58:54.570s
[INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
[INFO] Final Memory: 67M/1340M
[INFO] ------------------------------------------------------------------------

#Jenkins test results: PENDING


Thanks,

Robert Levas


Re: Review Request 40793: Enforce granular role-based access control for cluster functions

Posted by Nate Cole <nc...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/40793/#review108366
-----------------------------------------------------------

Ship it!


Ship It!

- Nate Cole


On Nov. 30, 2015, 9:37 a.m., Robert Levas wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/40793/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2015, 9:37 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Myroslav Papirkovskyy, Nate Cole, and Sumit Mohanty.
> 
> 
> Bugs: AMBARI-14072
>     https://issues.apache.org/jira/browse/AMBARI-14072
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Enforce granular role-based access control for cluster functions:
> 
>                            | Cluster User | Service Operator | Service Administrator | Cluster Operator | Cluster Administrator | Administrator
> ---------------------------|--------------|------------------|-----------------------|------------------|-----------------------|---------------
> View status information    |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View configuration         |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> View stack version details |(+)           |(+)               |(+)                    |(+)               |(+)                    |(+)            
> Enable/disable Kerberos    |              |                  |                       |                  |(+)                    |(+)            
> Upgrade/downgrade stack    |              |                  |                       |                  |(+)                    |(+)            
> Create new clusters        |              |                  |                       |                  |                       |(+)            
> Rename clusters            |              |                  |                       |                  |                       |(+)            
> 
> Entry points affected:
> * PUT /api/v1/clusters/:cluster_name 
> * POST /api/v1/clusters/:cluster_name 
> 
> # Note: Read-only requests (GET) are not protected so that the front end is not broken.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/services/ClusterService.java 4954a96 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementController.java b446121 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/AmbariManagementControllerImpl.java 0e3e7b8 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ClusterResourceProvider.java 84c13b9 
>   ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java 7f88286 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariCustomCommandExecutionHelperTest.java baa394c 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerImplTest.java ca3ca36 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/AmbariManagementControllerTest.java 3bf6cad 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/BackgroundCustomCommandExecutionTest.java 30be261 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/RefreshYarnCapacitySchedulerReleaseConfigTest.java e93a479 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ClusterResourceProviderTest.java 84de604 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/JMXHostProviderTest.java 2c6905d 
>   ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java 634d840 
>   ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java d4b7d5a 
>   ambari-server/src/test/java/org/apache/ambari/server/state/ConfigHelperTest.java bdb5156 
>   ambari-server/src/test/java/org/apache/ambari/server/state/UpgradeHelperTest.java fa6598c 
>   ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalogTest.java 319b9fe 
> 
> Diff: https://reviews.apache.org/r/40793/diff/
> 
> 
> Testing
> -------
> 
> Manually tested
> 
> # Local test results:
> [INFO] ------------------------------------------------------------------------
> [INFO] BUILD SUCCESS
> [INFO] ------------------------------------------------------------------------
> [INFO] Total time: 58:54.570s
> [INFO] Finished at: Mon Nov 30 07:53:59 EST 2015
> [INFO] Final Memory: 67M/1340M
> [INFO] ------------------------------------------------------------------------
> 
> #Jenkins test results: PENDING
> 
> 
> Thanks,
> 
> Robert Levas
> 
>