Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16252 )

Change subject: IMPALA-9988 (part 2): Integrate ldap filters and 
impala.doas.user
......................................................................


Patch Set 2:

(3 comments)

The implementation looks good, my questions are about the desired behaviour.

http://gerrit.cloudera.org:8080/#/c/16252/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16252/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-9988 (part 2): Integrate ldap filters and impala.doas.user
I was trying to understand the expected interaction between LDAP and Kerberos 
here.

It seems like the filters are applied when delegating if LDAP and Kerberos are 
both enabled regardless of how the client authenticated. I think this could 
make sense, since Kerberos auth doesn't check group membership.

I was wondering if they should also be applied if the filters are configured 
and only Kerberos auth is enabled. Or if we should explicitly document when the 
LDAP filters don't apply

I think the same thing also applies to the initial Kerberos authentication - 
should it be checking LDAP filters then too?

Some of this is outside the scope of this particular patch, but I wanted to 
understand what end-point we should be aiming for.


http://gerrit.cloudera.org:8080/#/c/16252/2/be/src/util/ldap-util.cc
File be/src/util/ldap-util.cc:

http://gerrit.cloudera.org:8080/#/c/16252/2/be/src/util/ldap-util.cc@61
PS2, Line 61: DEFINE_string(ldap_bind_password, "", "Password for 
--ldap_bind_dn");
For other password command line flags, we actually have a command that is run, 
so that the password doesn't need to be present on the command line. Should we 
replicate this pattern here?

ssl_private_key_password_cmd, webserver_private_key_password_cmd,
webserver_password_file

are what I was looking at


http://gerrit.cloudera.org:8080/#/c/16252/2/be/src/util/ldap-util.cc@228
PS2, Line 228:   if (!success) {
nit: one line?



--
To view, visit http://gerrit.cloudera.org:8080/16252
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I9ca8e1a0466288225efbe05b2d0068b8241df070
Gerrit-Change-Number: 16252
Gerrit-PatchSet: 2
Gerrit-Owner: Thomas Tauber-Marshall <tmarsh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Wed, 05 Aug 2020 18:09:35 +0000
Gerrit-HasComments: Yes

Reply via email to