[kudu] 02/02: webserver: port fix for IMPALA-9001 to Kudu

[ad...@apache.org]

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

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

commit c29ee5556a76bda9d8c15ae9b25164296a7d0ad9
Author: Adar Dembo <ad...@cloudera.com>
AuthorDate: Tue Oct 8 16:25:17 2019 -0700

    webserver: port fix for IMPALA-9001 to Kudu
    
    It's not a straight cherry-pick because the Impala fix includes a change to
    Thrift authn, and because Kudu can leverage EasyCurl to test the change.
    
    Change-Id: I9231677beb5990772736cf349c88f4b2b3344a7c
    Reviewed-on: http://gerrit.cloudera.org:8080/14393
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
 src/kudu/server/webserver-test.cc | 11 +++++++++++
 src/kudu/server/webserver.cc      | 23 +++++++++++++++++------
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/src/kudu/server/webserver-test.cc b/src/kudu/server/webserver-test.cc
index 8266c1c..2581ed0 100644
--- a/src/kudu/server/webserver-test.cc
+++ b/src/kudu/server/webserver-test.cc
@@ -234,6 +234,17 @@ TEST_F(SpnegoWebserverTest, TestInvalidHeaders) {
   EXPECT_STR_CONTAINS(buf_.ToString(), "Invalid token was supplied");
 }
 
+// Test that if no authorization header at all is provided, the response
+// contains an empty "WWW-Authenticate: Negotiate" header.
+TEST_F(SpnegoWebserverTest, TestNoAuthHeader) {
+  const string& url = strings::Substitute("http://$0/", addr_.ToString());
+  EasyCurl c;
+  c.set_return_headers(true);
+  ASSERT_EQ(c.FetchURL(url, &buf_).ToString(),
+            "Remote error: HTTP 401");
+  ASSERT_STR_CONTAINS(buf_.ToString(), "WWW-Authenticate: Negotiate\r\n");
+}
+
 // Test all single-bit-flips of a well-formed token, to make sure we don't
 // crash.
 //
diff --git a/src/kudu/server/webserver.cc b/src/kudu/server/webserver.cc
index 794aa26..7e34cd5 100644
--- a/src/kudu/server/webserver.cc
+++ b/src/kudu/server/webserver.cc
@@ -139,17 +139,28 @@ Sockaddr GetRemoteAddress(const struct sq_request_info* req) {
 }
 
 
-// Performs a step of SPNEGO authorization by parsing the HTTP Authorization header
-// 'authz_header' and running it through GSSAPI. If authentication fails or the header
-// is invalid, a bad Status will be returned (and the other out-parameters left untouched).
+// Performs a step of SPNEGO authorization by parsing the HTTP Authorization
+// header 'authz_header' and running it through GSSAPI.
 //
+// If authentication fails or the header is invalid, a bad Status will be
+// returned (and the other out-parameters left untouched). Otherwise, the
+// out-parameters will be written to, and the function will return either OK or
+// Incomplete depending on whether additional SPNEGO steps are required.
 Status RunSpnegoStep(const char* authz_header, string* resp_header,
                      string* authn_user) {
+  static const char* const kNegotiateStr = "WWW-Authenticate: Negotiate";
+  static const Status kIncomplete = Status::Incomplete("authn incomplete");
+
   VLOG(2) << "Handling Authorization header "
           << (authz_header ? KUDU_REDACT(authz_header) : "<null>");
 
+  if (!authz_header) {
+    *resp_header = kNegotiateStr;
+    return kIncomplete;
+  }
+
   string neg_token;
-  if (authz_header && !TryStripPrefixString(authz_header, "Negotiate ", &neg_token)) {
+  if (!TryStripPrefixString(authz_header, "Negotiate ", &neg_token)) {
     return Status::InvalidArgument("bad Negotiate header");
   }
 
@@ -160,9 +171,9 @@ Status RunSpnegoStep(const char* authz_header, string* resp_header,
   VLOG(2) << "SPNEGO step complete, response token: " << KUDU_REDACT(resp_token_b64);
 
   if (!resp_token_b64.empty()) {
-    *resp_header = Substitute("WWW-Authenticate: Negotiate $0", resp_token_b64);
+    *resp_header = Substitute("$0 $1", kNegotiateStr, resp_token_b64);
   }
-  return is_complete ? Status::OK() : Status::Incomplete("authn incomplete");
+  return is_complete ? Status::OK() : kIncomplete;
 }
 
 }  // anonymous namespace