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