[impala] 02/02: IMPALA-8868: Fix 401 response when LDAP and Kerberos are enabled

[ta...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

tarmstrong pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git

commit 46976ba4c1d7157d6fda36dbfc2ca9b3d7174a28
Author: Thomas Tauber-Marshall <tm...@cloudera.com>
AuthorDate: Thu Aug 15 12:03:20 2019 -0700

    IMPALA-8868: Fix 401 response when LDAP and Kerberos are enabled
    
    When both kerberos and ldap auth are enabled and an http request is
    not successfully authenticated, THttpServer only sends the
    'WWW-Authenticate: Basic' challenge and doesn't send the
    'WWW-Authenticate: Negotiate' challenge, which can cause clients that
    want to connect with kerberos to fail to authenticate.
    
    This patch fixes this to send both challenges.
    
    Testing:
    - Manually tested in a cluster with both Kerberos and LDAP enabled on
      Impala with connections proxied through Apache Knox, which would
      previously fail.
    
    Change-Id: I138f33783bfd0f8f9b8db242589a9cc75cfd392a
    Reviewed-on: http://gerrit.cloudera.org:8080/14077
    Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
    Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
 be/src/transport/THttpServer.cpp | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/be/src/transport/THttpServer.cpp b/be/src/transport/THttpServer.cpp
index fb2ff11..8db0a1c 100644
--- a/be/src/transport/THttpServer.cpp
+++ b/be/src/transport/THttpServer.cpp
@@ -170,11 +170,15 @@ void THttpServer::headersDone() {
 
   // Determine what type of auth header we got.
   StripWhiteSpace(&auth_value_);
-  string basic_auth_token;
-  bool got_basic_auth = TryStripPrefixString(auth_value_, "Basic ", &basic_auth_token);
-  string negotiate_auth_token;
+  string stripped_basic_auth_token;
+  bool got_basic_auth =
+      TryStripPrefixString(auth_value_, "Basic ", &stripped_basic_auth_token);
+  string basic_auth_token = got_basic_auth ? move(stripped_basic_auth_token) : "";
+  string stripped_negotiate_auth_token;
   bool got_negotiate_auth =
-      TryStripPrefixString(auth_value_, "Negotiate ", &negotiate_auth_token);
+      TryStripPrefixString(auth_value_, "Negotiate ", &stripped_negotiate_auth_token);
+  string negotiate_auth_token =
+      got_negotiate_auth ? move(stripped_negotiate_auth_token) : "";
   // We can only have gotten one type of auth header.
   DCHECK(!got_basic_auth || !got_negotiate_auth);
 
@@ -192,7 +196,8 @@ void THttpServer::headersDone() {
     } else {
       if (got_basic_auth && metrics_enabled_) total_basic_auth_failure_->Increment(1);
     }
-  } else if (has_kerberos_ && (!got_basic_auth || !has_ldap_)) {
+  }
+  if (has_kerberos_ && (!got_basic_auth || !has_ldap_)) {
     bool is_complete;
     if (callbacks_.negotiate_auth_fn(negotiate_auth_token, &is_complete)) {
       // If 'is_complete' is false we want to return a 401.