hive icon indicating copy to clipboard operation
hive copied to clipboard

HIVE-28457: HS2 WEBUI: LDAP authorization

Open difin opened this issue 1 year ago • 1 comments

What changes were proposed in this pull request?

Added LDAP authentication for HS2 WebUI. When enabled via HiveConf, and user attempts to access HS2 WebUI pages, user will be redirected to a new Login page to enter his LDAP username and password. After that the users' credentials are authenticated with LDAP provider. If successful, a signed cookie will be added to HTTP session for further user interaction with the server. If unsuccessful, user will be redirected again to the Login page.

Why are the changes needed?

To add authentication mechanism to WebUI.

Does this PR introduce any user-facing change?

Yes

Is the change a dependency upgrade?

No

How was this patch tested?

New JUnit integration tests were developed as part of this PR. Also manually tested accessing WebUI on a downstream Hive version in the cloud with LDAP authentication.

There are several SonarCube warnings in ThriftHttpServlet.java, but they are not new. I extracted some code into a separate class without changing functionality.

difin avatar Aug 20 '24 22:08 difin

@difin: the overall design looks good to me, left some code comments do you have screenshots of the login page or the process by any chance?

abstractdog avatar Sep 05 '24 12:09 abstractdog

Hi @abstractdog, Thanks for the review! Here are screenshots of:

  1. login page
  2. An attempt to login with empty creds
  3. Before submitting good creds.
  4. After login.
Screenshot 2024-09-05 at 5 11 09 PM Screenshot 2024-09-05 at 5 11 28 PM Screenshot 2024-09-05 at 5 11 42 PM Screenshot 2024-09-05 at 5 11 45 PM

difin avatar Sep 05 '24 21:09 difin

thanks a lot for addressing comments and screenshots @difin, only a minor nit regarding formatting, other than that it's +1

abstractdog avatar Sep 06 '24 11:09 abstractdog

Thanks a lot for review and +1, @abstractdog! I handled the comment regarding the formatting. I also added one more comment about ignoring exception in ThriftHttpServlet: https://github.com/apache/hive/pull/5399#discussion_r1747272788

difin avatar Sep 06 '24 15:09 difin