> On March 1, 2016, 4:24 p.m., Robert Levas wrote: > > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql, line 1120 > > <https://reviews.apache.org/r/44109/diff/2/?file=1275018#file1275018line1120> > > > > This is not consistent with the other DDL files. Should this record be > > added here... and in the other DDL files? > > > > Also, there is no addition of this record in the catalog upgrade class.
Removed it. > On March 1, 2016, 4:24 p.m., Robert Levas wrote: > > ambari-server/src/test/java/org/apache/ambari/server/api/services/PersistenceManagerImplTest.java, > > lines 69-70 > > <https://reviews.apache.org/r/44109/diff/2/?file=1275024#file1275024line69> > > > > You will want to clear the security context here or in an `@After` > > task. This is to reset the security context so that information from this > > test does not leak into other tests. > > > > This should be done for all test cases. Added across all relevant *CREATE.sql files. > On March 1, 2016, 4:24 p.m., Robert Levas wrote: > > ambari-server/src/test/java/org/apache/ambari/server/api/services/PersistenceManagerImplTest.java, > > lines 89-90 > > <https://reviews.apache.org/r/44109/diff/2/?file=1275024#file1275024line89> > > > > You will want to clear the security context here or in an `@After` > > task. This is to reset the security context so that information from this > > test does not leak into other tests. > > > > This should be done for all test cases. Fixed. Added @After public void clearAuthentication() { SecurityContextHolder.getContext().setAuthentication(null); } > On March 1, 2016, 4:24 p.m., Robert Levas wrote: > > ambari-server/src/test/java/org/apache/ambari/server/api/services/PersistenceManagerImplTest.java, > > lines 99-100 > > <https://reviews.apache.org/r/44109/diff/2/?file=1275024#file1275024line99> > > > > You will want to clear the security context here or in an `@After` > > task. This is to reset the security context so that information from this > > test does not leak into other tests. > > > > This should be done for all test cases. Fixed. Added @After public void clearAuthentication() { SecurityContextHolder.getContext().setAuthentication(null); } > On March 1, 2016, 4:24 p.m., Robert Levas wrote: > > ambari-server/src/test/java/org/apache/ambari/server/api/services/PersistenceManagerImplTest.java, > > lines 109-110 > > <https://reviews.apache.org/r/44109/diff/2/?file=1275024#file1275024line109> > > > > You will want to clear the security context here or in an `@After` > > task. This is to reset the security context so that information from this > > test does not leak into other tests. > > > > This should be done for all test cases. Fixed. Added @After public void clearAuthentication() { SecurityContextHolder.getContext().setAuthentication(null); } > On March 1, 2016, 4:24 p.m., Robert Levas wrote: > > ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java, > > line 288 > > <https://reviews.apache.org/r/44109/diff/2/?file=1275025#file1275025line288> > > > > If `CLUSTER_MANAGE_USER_PERSISTED_DATA` is granted to VIEW.USERs than > > it needs to be added here. "CLUSTER_MANAGE_USER_PERSISTED_DATA" is not added to the VIEW USER. Fixed the issue above. > On March 1, 2016, 4:24 p.m., Robert Levas wrote: > > ambari-server/src/main/java/org/apache/ambari/server/api/services/persistence/PersistenceManagerImpl.java, > > line 64 > > <https://reviews.apache.org/r/44109/diff/2/?file=1275013#file1275013line64> > > > > The resource id argument should not be `null`. This tells the > > authorization check to not care about which cluster the user is assigned > > the role for. You should look up the relevant cluster's resource id (which > > is not the same as a cluster's id). Dropping it, as at Persist API, we dont have context of cluster present. - Swapan ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/44109/#review121440 ----------------------------------------------------------- On March 1, 2016, 7:31 a.m., Swapan Shridhar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/44109/ > ----------------------------------------------------------- > > (Updated March 1, 2016, 7:31 a.m.) > > > Review request for Ambari, Alejandro Fernandez, Jaimin Jetly, Robert Levas, > and Sumit Mohanty. > > > Bugs: AMBARI-15213 > https://issues.apache.org/jira/browse/AMBARI-15213 > > > Repository: ambari > > > Description > ------- > > * RBAC : Background Jobs window doesn't show when login user role is : > CLUSTER.ADMINISTRATOR, SERVICE.OPERATOR, SERVICE.ADMINISTRATOR and > CLUSTER.OPERATOR. > * Currently, background Jobs window only comes for AMBARI.ADMINISTRATOR. > * > * > * Reason : It happenning because the UI calls for > api/v1/persist/admin-settings-show-bg-<username> and BE returns 404, because > the persistence of user values is not allowed other than Ambari > Adminsistrator. > * > * Change : > > Backend : Created a new Authroization > "CLUSTER.MANAGE_USER_PERSISTED_DATA" which can be imparted to the roles > CLUSTER.ADMINISTRATOR, > SERVICE.OPERATOR, SERVICE.ADMINISTRATOR, CLUSTER.OPERATOR and > AMBARI.ADMINISTRATOR. > Frontend : Earlier it was check authorization : > CLUSTER.UPGRADE_DOWNGRADE_STACK (misnomer) to use > CLUSTER.MANAGE_USER_PERSISTED_DATA. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/api/services/persistence/PersistenceManagerImpl.java > 4db5611 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java > e2a28d0 > > ambari-server/src/main/java/org/apache/ambari/server/security/authorization/RoleAuthorization.java > a77263d > > ambari-server/src/main/java/org/apache/ambari/server/upgrade/UpgradeCatalog240.java > 2f509b4 > ambari-server/src/main/resources/Ambari-DDL-Derby-CREATE.sql 9c61cbc > ambari-server/src/main/resources/Ambari-DDL-MySQL-CREATE.sql 0ebfa40 > ambari-server/src/main/resources/Ambari-DDL-Oracle-CREATE.sql a8cbda3 > ambari-server/src/main/resources/Ambari-DDL-Postgres-CREATE.sql bb47a8a > ambari-server/src/main/resources/Ambari-DDL-Postgres-EMBEDDED-CREATE.sql > 8ce2ba8 > ambari-server/src/main/resources/Ambari-DDL-SQLAnywhere-CREATE.sql b7a764e > ambari-server/src/main/resources/Ambari-DDL-SQLServer-CREATE.sql f60f07a > > ambari-server/src/test/java/org/apache/ambari/server/api/services/PersistenceManagerImplTest.java > 9ff1506 > > ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java > 2b2c276 > > ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java > b30bff3 > > ambari-server/src/test/java/org/apache/ambari/server/upgrade/UpgradeCatalog240Test.java > a909f84 > ambari-web/app/mixins/common/userPref.js 5a531f2 > > Diff: https://reviews.apache.org/r/44109/diff/ > > > Testing > ------- > > - Manual Testing : Tested the behavior for all the roles. Works. > - Upgrade from 2.3 to 2.4 : Works. > - > - Unit tests : Ran locally. Success. > - > - > - > - > - > - > - > - [INFO] Rat check: Summary of files. Unapproved: 0 unknown: 0 generated: 0 > approved: 41 licence. > [INFO] > ------------------------------------------------------------------------ > [INFO] Reactor Summary: > [INFO] > [INFO] Ambari Main ....................................... SUCCESS [7.381s] > [INFO] Apache Ambari Project POM ......................... SUCCESS [0.039s] > [INFO] Ambari Web ........................................ SUCCESS [54.672s] > [INFO] Ambari Views ...................................... SUCCESS [3.637s] > [INFO] Ambari Admin View ................................. SUCCESS [6.886s] > [INFO] ambari-metrics .................................... SUCCESS [0.223s] > [INFO] Ambari Metrics Common ............................. SUCCESS [3.028s] > [INFO] Ambari Metrics Hadoop Sink ........................ SUCCESS [4.706s] > [INFO] Ambari Metrics Flume Sink ......................... SUCCESS [3.094s] > [INFO] Ambari Metrics Kafka Sink ......................... SUCCESS [4.266s] > [INFO] Ambari Metrics Storm Sink ......................... SUCCESS [1.606s] > [INFO] Ambari Metrics Collector .......................... SUCCESS [4:53.861s] > [INFO] Ambari Metrics Monitor ............................ SUCCESS [1.916s] > [INFO] Ambari Metrics Grafana ............................ SUCCESS [24.236s] > [INFO] Ambari Metrics Assembly ........................... SUCCESS [2:21.604s] > [INFO] Ambari Server ..................................... SUCCESS > [1:03:33.924s] > [INFO] Ambari Functional Tests ........................... SUCCESS [0.791s] > [INFO] Ambari Agent ...................................... SUCCESS [19.518s] > [INFO] Ambari Client ..................................... SUCCESS [0.053s] > [INFO] Ambari Python Client .............................. SUCCESS [0.264s] > [INFO] Ambari Groovy Client .............................. SUCCESS [11.817s] > [INFO] Ambari Shell ...................................... SUCCESS [0.044s] > [INFO] Ambari Python Shell ............................... SUCCESS [0.050s] > [INFO] Ambari Groovy Shell ............................... SUCCESS [9.633s] > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 1:13:27.897s > [INFO] Finished at: Mon Feb 29 23:10:37 PST 2016 > [INFO] Final Memory: 91M/963M > [INFO] > ------------------------------------------------------------------------ > > > Thanks, > > Swapan Shridhar > >
