-
Notifications
You must be signed in to change notification settings - Fork 3.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HBASE-27693 Support for Hadoop's LDAP Authentication mechanism (Web UI only) #5144
Conversation
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
hbase-http/pom.xml
Outdated
@@ -29,6 +29,11 @@ | |||
<artifactId>hbase-http</artifactId> | |||
<name>Apache HBase - HTTP</name> | |||
<description>HTTP functionality for HBase Servers</description> | |||
<properties> | |||
<!-- Required for testing LDAP integration --> | |||
<apacheds.version>2.0.0.AM26</apacheds.version> |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
hbase-http/src/main/java/org/apache/hadoop/hbase/http/lib/AuthenticationFilterInitializer.java
Show resolved
Hide resolved
* Initializes hadoop-auth AuthenticationFilter which provides support for Kerberos HTTP SPNEGO | ||
* authentication. | ||
* <p> | ||
* It enables anonymous access, simple/pseudo and Kerberos HTTP SPNEGO authentication for Hadoop |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could others please suggest on this?
hbase-http/src/test/java/org/apache/hadoop/hbase/http/TestLdapHttpServer.java
Outdated
Show resolved
Hide resolved
💔 -1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
🎊 +1 overall
This message was automatically generated. |
|
💔 -1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Failures in pipeline are not relevant to this commit. |
This is only for http? |
No, this was tested on different HBase UIs running on both HTTP/HTTPS. It is a layer for HBase UI only. |
@@ -169,6 +169,48 @@ | |||
<artifactId>log4j-slf4j-impl</artifactId> | |||
<scope>test</scope> | |||
</dependency> | |||
<dependency> | |||
<groupId>org.apache.directory.server</groupId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a comment.
I am a little concerned to include apache directory server. It's a big project with lots of dependencies, and there's been no release over 3 years. Very likely this dependency is going to to introduce transitive dependency versions know to be vulnerable. The apache directory server quarterly report last October mentioned a new release is coming up but nothing happens yet.
I understand this is for test scope only, so this is not a real issue.
hbase-http/src/main/java/org/apache/hadoop/hbase/http/lib/AuthenticationFilterInitializer.java
Outdated
Show resolved
Hide resolved
I don't recall how the Hadoop's authentication filter works. But I assume it ultimately uses Hadoop's LdapAuthenticationHandler to implement the core functionality? |
Yes, it will instantiate AuthenticationFilter which will further use LdapAuthenticationHandler to implement core functionality. |
🎊 +1 overall
This message was automatically generated. |
Addressed all the review comments. Please let me know if this looks good. Thank you @Apache9 @jojochuang @NihalJain for the review. |
🎊 +1 overall
This message was automatically generated. |
💔 -1 overall
This message was automatically generated. |
Sorry I did not make it clear. I mean this is only for http/https, not for rpc, right? And you have already answered that this is for UI only. |
Thank you for your response. Yes, it is not for RPC. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 from me
+1 (non-binding) |
nit: we could update jira description to capture its for web ui to avoid any confusion. Also @ydodeja365 please update release note in jira, once changes are merged. |
I'm not very familiar with this area... Anyway, you've already gotten a +1 from a committer :) |
@ydodeja365 if you'd like this feature in branch-2 (HBase 2.6.0) can you submit another PR against branch-2? This feature has dependency on Hadoop so I'd like to have it run through precommit to make sure it doesn't break with Hadoop 2 runtime. |
@jojochuang Raised PR for the same, there were some conflicts with this exact commit, so raised with similar changes: #5213 |
No description provided.