You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@ambari.apache.org by Keta Patel <pa...@us.ibm.com> on 2017/04/18 05:33:29 UTC

Review Request 58493: AMBARI-20768: Local Ambari user with no cluster role must not be able to access Logsearch UI

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

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

Re: Review Request 58493: AMBARI-20768: Local Ambari user with no cluster role must not be able to access Logsearch UI

Posted by Oliver Szabo <os...@hortonworks.com>.


> On April 20, 2017, 11:14 a.m., Oliver Szabo wrote:
> > Ship It!
> 
> Keta Patel wrote:
>     Thank you!
>     Could you please help me with pushing in the change?

thanks for the contribution.
patches are merged:
- trunk: 1c37ffc435995fc898941837a2cdcdffd51d06bc
- branch-2.5: 682bd23194db38ddfeff2743888a9dee91bf514d

You can submit this review request


- Oliver


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


On April 20, 2017, 11:13 a.m., Keta Patel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58493/
> -----------------------------------------------------------
> 
> (Updated April 20, 2017, 11:13 a.m.)
> 
> 
> Review request for Ambari, Di Li, Miklos Gergely, and Oliver Szabo.
> 
> 
> 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 issue:
> 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.
> 
> 
> DESIRED BEHAVIOR:
> =================
> 1. A local user with no role assigned to it, must not be able to access Logsearch UI.
> 
> Note: The description has been updated by removing the aspect of correcting the behavior for Ambari Administrator role 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/2/
> 
> 
> Testing
> -------
> 
> The patch *AMBARI-20768.patch* contains the fix for this issue. The fix involves correction in 1 place 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*.
> 
> The results of the logsearch tests after applying the patch are shown in the screenshot "all_tests_successful.png" on the Jira.
> 
> Note: The description for testing has been updated by removing the aspect of correcting the behavior for Ambari Administrator role for the Logsearch UI.
> 
> 
> Thanks,
> 
> Keta Patel
> 
>

Re: Review Request 58493: AMBARI-20768: Local Ambari user with no cluster role must not be able to access Logsearch UI

Posted by Keta Patel <ke...@in.ibm.com>.


> On April 20, 2017, 4:44 p.m., Oliver Szabo wrote:
> > Ship It!
> 
> Keta Patel wrote:
>     Thank you!
>     Could you please help me with pushing in the change?
> 
> Oliver Szabo wrote:
>     thanks for the contribution.
>     patches are merged:
>     - trunk: 1c37ffc435995fc898941837a2cdcdffd51d06bc
>     - branch-2.5: 682bd23194db38ddfeff2743888a9dee91bf514d
>     
>     You can submit this review request

Thank you!


- Keta


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


On April 20, 2017, 4:43 p.m., Keta Patel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58493/
> -----------------------------------------------------------
> 
> (Updated April 20, 2017, 4:43 p.m.)
> 
> 
> Review request for Ambari, Di Li, Miklos Gergely, and Oliver Szabo.
> 
> 
> 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 issue:
> 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.
> 
> 
> DESIRED BEHAVIOR:
> =================
> 1. A local user with no role assigned to it, must not be able to access Logsearch UI.
> 
> Note: The description has been updated by removing the aspect of correcting the behavior for Ambari Administrator role 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/2/
> 
> 
> Testing
> -------
> 
> The patch *AMBARI-20768.patch* contains the fix for this issue. The fix involves correction in 1 place 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*.
> 
> The results of the logsearch tests after applying the patch are shown in the screenshot "all_tests_successful.png" on the Jira.
> 
> Note: The description for testing has been updated by removing the aspect of correcting the behavior for Ambari Administrator role for the Logsearch UI.
> 
> 
> Thanks,
> 
> Keta Patel
> 
>

Re: Review Request 58493: AMBARI-20768: Local Ambari user with no cluster role must not be able to access Logsearch UI

Posted by Keta Patel <ke...@in.ibm.com>.


> On April 20, 2017, 4:44 p.m., Oliver Szabo wrote:
> > Ship It!

Thank you!
Could you please help me with pushing in the change?


- Keta


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


On April 20, 2017, 4:43 p.m., Keta Patel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58493/
> -----------------------------------------------------------
> 
> (Updated April 20, 2017, 4:43 p.m.)
> 
> 
> Review request for Ambari, Di Li, Miklos Gergely, and Oliver Szabo.
> 
> 
> 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 issue:
> 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.
> 
> 
> DESIRED BEHAVIOR:
> =================
> 1. A local user with no role assigned to it, must not be able to access Logsearch UI.
> 
> Note: The description has been updated by removing the aspect of correcting the behavior for Ambari Administrator role 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/2/
> 
> 
> Testing
> -------
> 
> The patch *AMBARI-20768.patch* contains the fix for this issue. The fix involves correction in 1 place 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*.
> 
> The results of the logsearch tests after applying the patch are shown in the screenshot "all_tests_successful.png" on the Jira.
> 
> Note: The description for testing has been updated by removing the aspect of correcting the behavior for Ambari Administrator role for the Logsearch UI.
> 
> 
> Thanks,
> 
> Keta Patel
> 
>

Re: Review Request 58493: AMBARI-20768: Local Ambari user with no cluster role must not be able to access Logsearch UI

Posted by Oliver Szabo <os...@hortonworks.com>.

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


Ship it!




Ship It!

- Oliver Szabo


On April 20, 2017, 11:13 a.m., Keta Patel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58493/
> -----------------------------------------------------------
> 
> (Updated April 20, 2017, 11:13 a.m.)
> 
> 
> Review request for Ambari, Di Li, Miklos Gergely, and Oliver Szabo.
> 
> 
> 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 issue:
> 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.
> 
> 
> DESIRED BEHAVIOR:
> =================
> 1. A local user with no role assigned to it, must not be able to access Logsearch UI.
> 
> Note: The description has been updated by removing the aspect of correcting the behavior for Ambari Administrator role 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/2/
> 
> 
> Testing
> -------
> 
> The patch *AMBARI-20768.patch* contains the fix for this issue. The fix involves correction in 1 place 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*.
> 
> The results of the logsearch tests after applying the patch are shown in the screenshot "all_tests_successful.png" on the Jira.
> 
> Note: The description for testing has been updated by removing the aspect of correcting the behavior for Ambari Administrator role for the Logsearch UI.
> 
> 
> Thanks,
> 
> Keta Patel
> 
>

Re: Review Request 58493: AMBARI-20768: Local Ambari user with no cluster role must not be able to access Logsearch UI

Posted by Keta Patel <ke...@in.ibm.com>.

(Updated April 20, 2017, 4:43 p.m.)

Review request for Ambari, Di Li, Miklos Gergely, and Oliver Szabo.

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

Repository: ambari

Description (updated)
-------

A local Ambari user with no cluster roles assigned to it can successfully log into the Logsearch UI.

DESIRED BEHAVIOR:
=================
1. A local user with no role assigned to it, must not be able to access Logsearch UI.

Note: The description has been updated by removing the aspect of correcting the behavior for Ambari Administrator role 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/2/

Testing (updated)
-------

The patch *AMBARI-20768.patch* contains the fix for this issue. The fix involves correction in 1 place 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*.

The results of the logsearch tests after applying the patch are shown in the screenshot "all_tests_successful.png" on the Jira.

Note: The description for testing has been updated by removing the aspect of correcting the behavior for Ambari Administrator role for the Logsearch UI.

Thanks,

Keta Patel

Re: Review Request 58493: AMBARI-20768: Local Ambari user with no cluster role must not be able to access Logsearch UI

Posted by Keta Patel <ke...@in.ibm.com>.


> On April 20, 2017, 3:22 p.m., Miklos Gergely wrote:
> > ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProvider.java
> > Line 126 (original), 126 (patched)
> > <https://reviews.apache.org/r/58493/diff/2/?file=1695295#file1695295line126>
> >
> >     As this is the only change that remained, and this is a bug fix not related to AMBARI.ADMINISTRATOR privileges please modify the description of the review request and the bug.

Hello Miklos,
Thank you for the suggestion. I have updated the description on both the ReviewBoard and the Jira.


- Keta


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


On April 20, 2017, 4:43 p.m., Keta Patel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58493/
> -----------------------------------------------------------
> 
> (Updated April 20, 2017, 4:43 p.m.)
> 
> 
> Review request for Ambari, Di Li, Miklos Gergely, and Oliver Szabo.
> 
> 
> 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 issue:
> 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.
> 
> 
> DESIRED BEHAVIOR:
> =================
> 1. A local user with no role assigned to it, must not be able to access Logsearch UI.
> 
> Note: The description has been updated by removing the aspect of correcting the behavior for Ambari Administrator role 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/2/
> 
> 
> Testing
> -------
> 
> The patch *AMBARI-20768.patch* contains the fix for this issue. The fix involves correction in 1 place 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*.
> 
> The results of the logsearch tests after applying the patch are shown in the screenshot "all_tests_successful.png" on the Jira.
> 
> Note: The description for testing has been updated by removing the aspect of correcting the behavior for Ambari Administrator role for the Logsearch UI.
> 
> 
> Thanks,
> 
> Keta Patel
> 
>

Re: Review Request 58493: AMBARI-20768: Local Ambari user with no cluster role must not be able to access Logsearch UI

Posted by Miklos Gergely <mg...@hortonworks.com>.

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


Fix it, then Ship it!




Ship It!


ambari-logsearch/ambari-logsearch-portal/src/main/java/org/apache/ambari/logsearch/web/security/LogsearchExternalServerAuthenticationProvider.java
Line 126 (original), 126 (patched)
<https://reviews.apache.org/r/58493/#comment245583>

    As this is the only change that remained, and this is a bug fix not related to AMBARI.ADMINISTRATOR privileges please modify the description of the review request and the bug.


- Miklos Gergely


On April 20, 2017, 5:37 a.m., Keta Patel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58493/
> -----------------------------------------------------------
> 
> (Updated April 20, 2017, 5:37 a.m.)
> 
> 
> Review request for Ambari, Di Li, Miklos Gergely, and Oliver Szabo.
> 
> 
> 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/2/
> 
> 
> 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
> 
>

Re: Review Request 58493: AMBARI-20768: Local Ambari user with no cluster role must not be able to access Logsearch UI

Posted by Keta Patel <ke...@in.ibm.com>.

(Updated April 20, 2017, 11:07 a.m.)

Review request for Ambari, Di Li, Miklos Gergely, and Oliver Szabo.

Changes
-------

The patch "AMBARI-20768_branch-2.5_updated.patch" has been updated with suggestions from the 2 comments from Oliver Szabo.

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.

Diffs (updated)
-----

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/2/

Changes: https://reviews.apache.org/r/58493/diff/1-2/

Testing
-------

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

Re: Review Request 58493: AMBARI-20768: Local Ambari user with no cluster role must not be able to access Logsearch UI

Posted by Keta Patel <ke...@in.ibm.com>.


> On April 18, 2017, 8:35 p.m., Oliver Szabo wrote:
> > 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/diff/1/?file=1693595#file1693595line131>
> >
> >     Do not use inline comments, maybe its better to use private methods with names that describe the behaviour.

Hello Oliver,
Thank you for the usggestion. I have removed inline comments from the patch. I have also removed the logic to test for "AMBARI.ADMINISTRATOR" as per your suggestion. The new patch is meant for branch-2.5.0, it is named as "AMBARI-20768_branch-2.5_updated.patch" on the Jira.

Thank you,
Keta


- Keta


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


On April 20, 2017, 11:07 a.m., Keta Patel wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/58493/
> -----------------------------------------------------------
> 
> (Updated April 20, 2017, 11:07 a.m.)
> 
> 
> Review request for Ambari, Di Li, Miklos Gergely, and Oliver Szabo.
> 
> 
> 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/2/
> 
> 
> 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
> 
>

Re: Review Request 58493: AMBARI-20768: Local Ambari user with no cluster role must not be able to access Logsearch UI

Posted by Oliver Szabo <os...@hortonworks.com>.

-----------------------------------------------------------
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
> 
>