----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/58493/#review172211 -----------------------------------------------------------
ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProvider.java Lines 131 (patched) <https://reviews.apache.org/r/58493/#comment245311> Do not use inline comments, maybe its better to use private methods with names that describe the behaviour. ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProvider.java Lines 132 (patched) <https://reviews.apache.org/r/58493/#comment245310> Do not use AMBARI.ADMINISTRATOR as a constant, better to be a configuration property (external auth admin role or something like that). Also if you change the behaviour you will need to change the description of the parameter in the logsearch stack code (in ambari-server common-services) Anyway if the "AMBARI.ADMINISTRATOR" role is not in the roles allowed list I would not expect the user to log into Log Search UI. (that not violates Ambari Administrator privilege as that is a different service). On Ambari UI side, Ambari uses its own rest API to get logSearch data, and for that (internally) it uses the default logsearch admin user (+ password), but to access to that rest endpoint you need the proper privileges, that should work as you expected on the Ambari UI. Also that means you will need to test your changes with the default admin/password as well (which you can set during logsearch install through Ambari) I would say as your first point seems to be an issue, maybe the second one is not. - Oliver Szabo On April 18, 2017, 5:33 a.m., Keta Patel wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/58493/ > ----------------------------------------------------------- > > (Updated April 18, 2017, 5:33 a.m.) > > > Review request for Ambari. > > > Bugs: AMBARI-20768 > https://issues.apache.org/jira/browse/AMBARI-20768 > > > Repository: ambari > > > Description > ------- > > A local Ambari user with no cluster roles assigned to it can successfully log > into the Logsearch UI. > > Logsearch service exercises restriction on who can access its UI using a > property "logsearch.roles.allowed". This property is a comma-separated list > of roles to be allowed access to Logsearch UI. This defect deals with the > following 2 issues: > 1. If Logsearch service requires that only certain roles be allowed to access > its UI, then a local Ambari user with no roles must not be allowed to access > the UI. > 2. If some user with privilege to edit the config properties, updates > "logsearch.roles.allowed" by removing the "AMBARI.ADMINISTRATOR" role from > its list, then the Ambari Admins will not be able to access the Logsearch UI. > This violates the Ambari Administrator privilege which must be able to access > all frames of Ambari UI as well as perform all UI operations. > > > DESIRED BEHAVIOR: > ================= > 1. A local user with no role assigned to it, must not be able to access > Logsearch UI. > 2. Ambari Administrators must be always be allowed to access the Logsearch > UI. No user is allowed to revoke this access right of Ambari Administrator > for the Logsearch UI. > > > Diffs > ----- > > > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProvider.java > e23f0a2 > > > Diff: https://reviews.apache.org/r/58493/diff/1/ > > > Testing > ------- > > The patch *AMBARI-20768.patch* contains the fix for this issue. The fix > involves correction in 2 places in the > LogsearchExternalServerAuthenticationProvider class. > 1. In order to prevent a local user with no cluster roles assigned to it from > logging into Logsearch UI, we return *false*. > 2. We implicitly check whether the user is an Ambari Administrator or not, > thus removing the requirement of having "AMBARI.ADMINISTRATOR" role in the > "logsearch.roles.allowed" property on the UI. Now, even if some user removes > the "AMBARI.ADMINISTRATOR" property from the UI, it will not affect the > Ambari admin's accessibility to the Logsearch UI. Ambari Admins will always > be allowed to login. > > The results of the logsearch tests after applying the patch are shown in the > screenshot "all_tests_successful.png" on the Jira. > > > Thanks, > > Keta Patel > >
