> On Dec. 10, 2015, 11:37 a.m., Jonathan Hurley wrote: > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractPropertyProvider.java, > > lines 202-229 > > <https://reviews.apache.org/r/41107/diff/1/?file=1158775#file1158775line202> > > > > Shouldn't these specific checks be made in the concrete provider? The > > abstract property provider shouldn't even know there's such a thing as a > > metrics provider.
Unfortnately the metrics inforamtion is spread out in multiple PropertyProvider classes and the AbstractPropertyProvider class seems to be the best common denominator. What if the method was renamed to checkAuthorizationForMetrics. As far as the AbstractPropertyProvider not knowing about metrics, several methods in this class reference metrics... for example `org.apache.ambari.server.controller.internal.AbstractPropertyProvider#getComponentMetrics`. - Robert ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/41107/#review109778 ----------------------------------------------------------- On Dec. 10, 2015, 9:04 a.m., Swapan Shridhar wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/41107/ > ----------------------------------------------------------- > > (Updated Dec. 10, 2015, 9:04 a.m.) > > > Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, Nate Cole, > Robert Levas, Sumit Mohanty, and Sid Wagle. > > > Bugs: AMBARI-14192 > https://issues.apache.org/jira/browse/AMBARI-14192 > > > Repository: ambari > > > Description > ------- > > Role Based Access Control support for Metrics. > > > * With the base infrastructure already in place for "Role Based Access > Control(RBAC)", this change introduces the RBAC support for AMbari Metrics. > Before the doing the metrics population, to be send back, it does an > authorization check for the current user in consideration for the VIEW > METRICE permissoions. > > > * The mapping is as follows : > > Resource.InternalType.Cluster -> CLUSTER_VIEW_METRICS > Resource.InternalType.HOST -> HOST_VIEW_METRICS > Resource.InternalType.Component -> SERVICE_VIEW_METRICS > Resource.InternalType.HostComponent -> SERVICE_VIEW_METRICS > > * For a user requesting Metrics and not having Au`thorization, > AuthorizationException is raised. > > > NOTE : > ---- > As of now, UI hangs when 403 code is raised (Sncreenshot attached) for VIEW > user. I am raising a BUG on UI for that. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/AbstractPropertyProvider.java > 4a0c44f > > ambari-server/src/main/java/org/apache/ambari/server/controller/jmx/JMXPropertyProvider.java > 2748dd4 > > ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsPropertyProvider.java > f1c5c81 > > ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsPropertyProviderProxy.java > ac11556 > > ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/MetricsReportPropertyProviderProxy.java > 4d2ce01 > > ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/RestMetricsPropertyProvider.java > b32adda > > ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/ThreadPoolEnabledPropertyProvider.java > 8a35636 > > ambari-server/src/main/java/org/apache/ambari/server/controller/metrics/timeline/AMSPropertyProvider.java > b9f54db > > ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/JMXPropertyProviderTest.java > f0c1280 > > ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/RestMetricsPropertyProviderTest.java > 82b42f2 > > ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/ganglia/GangliaPropertyProviderTest.java > 6fefffe > > ambari-server/src/test/java/org/apache/ambari/server/controller/metrics/timeline/AMSPropertyProviderTest.java > 6b5926b > > ambari-server/src/test/java/org/apache/ambari/server/security/TestAuthenticationFactory.java > 8abe757 > > Diff: https://reviews.apache.org/r/41107/diff/ > > > Testing > ------- > > - Ambari Server Deployment and tested for "View USER (only view > permissions)", 'admin' login(all permissions) and SERVICE OPERATOR role. > - UT : Running, will update the results. > - UNIT tests modified as part of this change, tested : Success. > - > - API : > > > View User : > --------- > > [root@c6401 ambari-server]# curl -u viewUser:aaa > "http://c6401:8080/api/v1/clusters/c1/hosts/c6401.ambari.apache.org?fields=metrics/network/bytes_in[1449532831,1449534631,15],metrics/network/bytes_out[1449532831,1449534631,15]"; > { > "status" : 403, > "message" : "The authenticated user does not have authorization to view > Host metrics" > } > > > Cluster Administrator Role: > -------------------------- > > [root@c6401 ambari-server]# curl -u clusAdmin:aaa > "http://c6401:8080/api/v1/clusters/c1/hosts/c6401.ambari.apache.org?fields=metrics/network/bytes_in[1449532831,1449534631,15],metrics/network/bytes_out[1449532831,1449534631,15]"; > { > "href" : > "http://c6401:8080/api/v1/clusters/c1/hosts/c6401.ambari.apache.org?fields=metrics/network/bytes_in[1449532831,1449534631,15],metrics/network/bytes_out[1449532831,1449534631,15]";, > "Hosts" : { > "cluster_name" : "c1", > "host_name" : "c6401.ambari.apache.org" > } > } > > > Service Operator: > ---------------- > > [root@c6401 ambari-server]# curl -u servOp:aaa > "http://c6401:8080/api/v1/clusters/c1/hosts/c6401.ambari.apache.org?fields=metrics/network/bytes_in[1449532831,1449534631,15],metrics/network/bytes_out[1449532831,1449534631,15]"; > { > "href" : > "http://c6401:8080/api/v1/clusters/c1/hosts/c6401.ambari.apache.org?fields=metrics/network/bytes_in[1449532831,1449534631,15],metrics/network/bytes_out[1449532831,1449534631,15]";, > "Hosts" : { > "cluster_name" : "c1", > "host_name" : "c6401.ambari.apache.org" > } > > > Non-existing User: > ----------------- > > [root@c6401 ambari-server]# curl -u a:aaa > "http://c6401:8080/api/v1/clusters/c1/hosts/c6401.ambari.apache.org?fields=metrics/network/bytes_in[1449532831,1449534631,15],metrics/network/bytes_out[1449532831,1449534631,15]"; > { > "status": 403, > "message": "Full authentication is required to access this resource" > } > > > Admin User: > ---------- > > [root@c6401 ambari-server]# curl -u admin:admin > "http://c6401:8080/api/v1/clusters/c1/hosts/c6401.ambari.apache.org?fields=metrics/network/bytes_in[1449532831,1449534631,15],metrics/network/bytes_out[1449532831,1449534631,15]"; > { > "href" : > "http://c6401:8080/api/v1/clusters/c1/hosts/c6401.ambari.apache.org?fields=metrics/network/bytes_in[1449532831,1449534631,15],metrics/network/bytes_out[1449532831,1449534631,15]";, > "Hosts" : { > "cluster_name" : "c1", > "host_name" : "c6401.ambari.apache.org" > } > } > > > File Attachments > ---------------- > > Screenshot for hang > > https://reviews.apache.org/media/uploaded/files/2015/12/10/9f38adda-2db9-44e1-b9f1-15e88e786817__Screen_Shot_2015-12-10_at_5.13.37_AM.png > > > Thanks, > > Swapan Shridhar > >
