IMPALA-11436: Change search bind authentication parameters

Impala's search bind authentication intends to mimic Spring's behaviour.
However, the login username and user dn paremeters were swapped for
group searches compared to Spring. This change intends to align these
parameters.

For user search, Spring uses {0} to replace the login username.
Meanwhile, during group search {0} is used to replace the login user dn
and {1} is used to replace the login username.

Testing:
 - Ran LdapSearchBindImpalaShellTest frontend tests

Change-Id: I9808566a348f7c6200b0571fbc05e67f720f2075
Reviewed-on: http://gerrit.cloudera.org:8080/18819
Reviewed-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
Tested-by: Impala Public Jenkins <impala-public-jenkins@cloudera.com>
This commit is contained in:
Tamas Mate
2022-08-05 15:07:23 +02:00
parent 7ca11dfc7f
commit 51126259be
3 changed files with 30 additions and 23 deletions

View File

@@ -42,11 +42,12 @@ using std::string;
namespace impala {
// Permitted patterns in the user/group filter
const string USER_NAME_PATTERN = "{0}";
const string USER_DN_PATTERN = "{1}";
const string USER_SEARCH_LOGIN_NAME_PATTERN = "{0}";
const string GROUP_SEARCH_LOGIN_NAME_PATTERN = "{1}";
const string GROUP_SEARCH_USER_DN_PATTERN = "{0}";
// Default ldap filters
const string DEFAULT_USER_FILTER = "(&(objectClass=user)(sAMAccountName={0}))";
const string DEFAULT_GROUP_FILTER = "(&(objectClass=group)(member={1})";
const string DEFAULT_GROUP_FILTER = "(&(objectClass=group)(member={0})";
Status LdapSearchBind::ValidateFlags() {
RETURN_IF_ERROR(ImpalaLdap::ValidateFlags());
@@ -92,10 +93,10 @@ bool LdapSearchBind::LdapCheckPass(const char* user, const char* pass, unsigned
if (!success) return false;
VLOG(2) << "LDAP bind successful";
// Escape special characters and replace the USER_NAME_PATTERN in the filter.
// Escape special characters and replace the USER_SEARCH_LOGIN_NAME_PATTERN.
string filter = string(user_filter_);
string escaped_user = EscapeFilterProperty(user);
replace_all(filter, USER_NAME_PATTERN, escaped_user);
replace_all(filter, USER_SEARCH_LOGIN_NAME_PATTERN, escaped_user);
// Execute the LDAP search and try to retrieve the user dn
VLOG(1) << "Trying LDAP user search for: " << user;
@@ -134,15 +135,15 @@ bool LdapSearchBind::LdapCheckFilters(string username) {
if (!success) return false;
VLOG(2) << "LDAP bind successful";
// Substitute the USER_NAME_PATTERN and USER_DN_PATTERN patterns for the group search.
// USER_DN_PATTERN requires to determine the user dn and therefore an additional LDAP
// search.
// Substitute the USER_SEARCH_LOGIN_NAME_PATTERN and GROUP_SEARCH_USER_DN_PATTERN
// patterns for the group search. This needs an additional LDAP search to determine
// the GROUP_SEARCH_USER_DN_PATTERN.
string group_filter = group_filter_;
string escaped_username = EscapeFilterProperty(username);
replace_all(group_filter, USER_NAME_PATTERN, escaped_username);
if (group_filter.find(USER_DN_PATTERN) != string::npos) {
replace_all(group_filter, GROUP_SEARCH_LOGIN_NAME_PATTERN, escaped_username);
if (group_filter.find(GROUP_SEARCH_USER_DN_PATTERN) != string::npos) {
string user_filter = user_filter_;
replace_all(user_filter, USER_NAME_PATTERN, escaped_username);
replace_all(user_filter, USER_SEARCH_LOGIN_NAME_PATTERN, escaped_username);
vector<string> user_dns =
LdapSearchObject(ld, FLAGS_ldap_user_search_basedn.c_str(), user_filter.c_str());
if (user_dns.size() != 1) {
@@ -153,9 +154,9 @@ bool LdapSearchBind::LdapCheckFilters(string username) {
ldap_unbind_ext(ld, nullptr, nullptr);
return false;
}
// Escape the characters in the the DN then replace the USER_DN_PATTERN.
// Escape the characters in the the DN then replace the GROUP_SEARCH_DN_PATTERN.
string escaped_user_dn = EscapeFilterProperty(user_dns[0]);
replace_all(group_filter, USER_DN_PATTERN, escaped_user_dn);
replace_all(group_filter, GROUP_SEARCH_USER_DN_PATTERN, escaped_user_dn);
}
// Execute LDAP search for the group

View File

@@ -323,7 +323,7 @@ under the License.
<dd>
<p>
A comma separated list of user names. If specified, users must be
A comma separated list of usernames. If specified, users must be
on this list for authentication to succeed
</p>
</dd>
@@ -459,7 +459,7 @@ under the License.
<p>
LDAP filter that will be used during LDAP search, it can contain
<codeph>{0}</codeph> pattern which will be replaced with the
user name. The default value is <codeph>
username. The default value is <codeph>
(&amp;(objectClass=user)(sAMAccountName={0}))</codeph>.
</p>
</dd>
@@ -475,11 +475,17 @@ under the License.
<dd>
<p>
LDAP filter that will be used during LDAP group search, it can
contain <codeph>{0}</codeph> pattern which will be replaced with
the user name and/or <codeph>{1}</codeph> which will be replaced
contain <codeph>{1}</codeph> pattern which will be replaced with
the username and/or <codeph>{0}</codeph> which will be replaced
with the user DN. The default value is <codeph>
(&amp;(objectClass=group)(member={1})</codeph>.
(&amp;(objectClass=group)(member={0})</codeph>.
</p>
<note>
The behaviour of this flag has been changed between Impala 4.1 and Impala 4.2
to comply with Spring LDAP. Previously <codeph>{0}</codeph> was for username
and <codeph>{1}</codeph> for user dn, these paramters were swapped, now
<codeph>{0}</codeph> is for user dn and <codeph>{1}</codeph> is for username.
</note>
</dd>
</dlentry>
@@ -617,7 +623,7 @@ under the License.
Sets the user. Per Active Directory, the user is the short username, not the full
LDAP distinguished name. If your LDAP settings include a search base, use the
<codeph>--ldap_bind_pattern</codeph> on the <cmdname>impalad</cmdname> daemon to
translate the short user name from <cmdname>impala-shell</cmdname> automatically
translate the short username from <cmdname>impala-shell</cmdname> automatically
to the fully qualified name.
</p>
</dd>

View File

@@ -71,7 +71,7 @@ public class LdapSearchBindImpalaShellTest extends LdapImpalaShellTest {
setUp(String.format("--ldap_user_search_basedn=dc=myorg,dc=com "
+ "--ldap_group_search_basedn=ou=Groups,dc=myorg,dc=com "
+ "--ldap_user_filter=(&(objectClass=person)(cn={0})(!(cn=%s))) "
+ "--ldap_group_filter=(uniqueMember={1})",
+ "--ldap_group_filter=(uniqueMember={0})",
TEST_USER_2));
testLdapFiltersImpl();
}
@@ -89,7 +89,7 @@ public class LdapSearchBindImpalaShellTest extends LdapImpalaShellTest {
setUp(String.format("--ldap_user_search_basedn=dc=myorg,dc=com "
+ "--ldap_group_search_basedn=ou=Groups,dc=myorg,dc=com "
+ "--ldap_user_filter=(&(objectClass=person)(cn={0})(!(cn=%s))) "
+ "--ldap_group_filter=(&(cn=%s)(uniqueMember={1}))",
+ "--ldap_group_filter=(&(cn=%s)(uniqueMember={0}))",
TEST_USER_2, TEST_USER_GROUP));
testLdapFiltersImpl();
}
@@ -107,7 +107,7 @@ public class LdapSearchBindImpalaShellTest extends LdapImpalaShellTest {
setUp(String.format("--ldap_user_search_basedn=dc=myorg,dc=com "
+ "--ldap_group_search_basedn=ou=Groups,dc=myorg,dc=com "
+ "--ldap_user_filter=(&(objectClass=person)(cn={0})(!(cn=Test2Ldap))) "
+ "--ldap_group_filter=(&(cn=group1)(uniqueMember={1})) "
+ "--ldap_group_filter=(&(cn=group1)(uniqueMember={0})) "
+ "--authorized_proxy_user_config=%s=* ",
TEST_USER_4));
testLdapFiltersWithProxyImpl();
@@ -145,7 +145,7 @@ public class LdapSearchBindImpalaShellTest extends LdapImpalaShellTest {
setUp("--ldap_user_search_basedn=dc=myorg,dc=com "
+ "--ldap_group_search_basedn=ou=Groups,dc=myorg,dc=com "
+ "--ldap_user_filter=(cn={0}) "
+ "--ldap_group_filter=(uniqueMember={1}) ");
+ "--ldap_group_filter=(uniqueMember={0}) ");
String query = "select logged_in_user()";
// Authentications should succeed with user who has escaped character in its DN