Auto merge of #2529 - str4d:2444-rpc-www-authenticate, r=daira

[rpc] Add WWW-Authenticate header to 401 response

Fix cherry-picked from upstream PR bitcoin/bitcoin#7472.

Closes #2444.

src/Makefile.gtest.include

@@ -21,6 +21,7 @@ zcash_gtest_SOURCES += \
 	gtest/test_tautology.cpp \
 	gtest/test_deprecation.cpp \
 	gtest/test_equihash.cpp \
+	gtest/test_httprpc.cpp \
 	gtest/test_joinsplit.cpp \
 	gtest/test_keystore.cpp \
 	gtest/test_noteencryption.cpp \

src/gtest/test_httprpc.cpp

@@ -0,0 +1,62 @@
+#include <gtest/gtest.h>
+#include <gmock/gmock.h>
+
+#include "httprpc.cpp"
+#include "httpserver.h"
+
+using ::testing::Return;
+
+class MockHTTPRequest : public HTTPRequest {
+public:
+    MOCK_METHOD0(GetPeer, CService());
+    MOCK_METHOD0(GetRequestMethod, HTTPRequest::RequestMethod());
+    MOCK_METHOD1(GetHeader, std::pair<bool, std::string>(const std::string& hdr));
+    MOCK_METHOD2(WriteHeader, void(const std::string& hdr, const std::string& value));
+    MOCK_METHOD2(WriteReply, void(int nStatus, const std::string& strReply));
+
+    MockHTTPRequest() : HTTPRequest(nullptr) {}
+    void CleanUp() {
+        // So the parent destructor doesn't try to send a reply
+        replySent = true;
+    }
+};
+
+TEST(HTTPRPC, FailsOnGET) {
+    MockHTTPRequest req;
+    EXPECT_CALL(req, GetRequestMethod())
+        .WillRepeatedly(Return(HTTPRequest::GET));
+    EXPECT_CALL(req, WriteReply(HTTP_BAD_METHOD, "JSONRPC server handles only POST requests"))
+        .Times(1);
+    EXPECT_FALSE(HTTPReq_JSONRPC(&req, ""));
+    req.CleanUp();
+}
+
+TEST(HTTPRPC, FailsWithoutAuthHeader) {
+    MockHTTPRequest req;
+    EXPECT_CALL(req, GetRequestMethod())
+        .WillRepeatedly(Return(HTTPRequest::POST));
+    EXPECT_CALL(req, GetHeader("authorization"))
+        .WillRepeatedly(Return(std::make_pair(false, "")));
+    EXPECT_CALL(req, WriteHeader("WWW-Authenticate", "Basic realm=\"jsonrpc\""))
+        .Times(1);
+    EXPECT_CALL(req, WriteReply(HTTP_UNAUTHORIZED, ""))
+        .Times(1);
+    EXPECT_FALSE(HTTPReq_JSONRPC(&req, ""));
+    req.CleanUp();
+}
+
+TEST(HTTPRPC, FailsWithBadAuth) {
+    MockHTTPRequest req;
+    EXPECT_CALL(req, GetRequestMethod())
+        .WillRepeatedly(Return(HTTPRequest::POST));
+    EXPECT_CALL(req, GetHeader("authorization"))
+        .WillRepeatedly(Return(std::make_pair(true, "Basic spam:eggs")));
+    EXPECT_CALL(req, GetPeer())
+        .WillRepeatedly(Return(CService("127.0.0.1:1337")));
+    EXPECT_CALL(req, WriteHeader("WWW-Authenticate", "Basic realm=\"jsonrpc\""))
+        .Times(1);
+    EXPECT_CALL(req, WriteReply(HTTP_UNAUTHORIZED, ""))
+        .Times(1);
+    EXPECT_FALSE(HTTPReq_JSONRPC(&req, ""));
+    req.CleanUp();
+}

src/httprpc.cpp

@@ -13,6 +13,9 @@
 
 #include <boost/algorithm/string.hpp> // boost::trim
 
+/** WWW-Authenticate to present with 401 Unauthorized response */
+static const char* WWW_AUTH_HEADER_DATA = "Basic realm=\"jsonrpc\"";
+
 /** Simple one-shot callback timer to be used by the RPC mechanism to e.g.
  * re-lock the wellet.
  */
@@ -94,6 +97,7 @@ static bool HTTPReq_JSONRPC(HTTPRequest* req, const std::string &)
     // Check authorization
     std::pair<bool, std::string> authHeader = req->GetHeader("authorization");
     if (!authHeader.first) {
+        req->WriteHeader("WWW-Authenticate", WWW_AUTH_HEADER_DATA);
         req->WriteReply(HTTP_UNAUTHORIZED);
         return false;
     }
@@ -106,6 +110,7 @@ static bool HTTPReq_JSONRPC(HTTPRequest* req, const std::string &)
            shouldn't have their RPC port exposed. */
         MilliSleep(250);
 
+        req->WriteHeader("WWW-Authenticate", WWW_AUTH_HEADER_DATA);
         req->WriteReply(HTTP_UNAUTHORIZED);
         return false;
     }

src/httpserver.h

@@ -56,11 +56,14 @@ class HTTPRequest
 {
 private:
     struct evhttp_request* req;
+
+    // For test access
+protected:
     bool replySent;
 
 public:
     HTTPRequest(struct evhttp_request* req);
-    ~HTTPRequest();
+    virtual ~HTTPRequest();
 
     enum RequestMethod {
         UNKNOWN,
@@ -76,17 +79,17 @@ public:
 
     /** Get CService (address:ip) for the origin of the http request.
      */
-    CService GetPeer();
+    virtual CService GetPeer();
 
     /** Get request method.
      */
-    RequestMethod GetRequestMethod();
+    virtual RequestMethod GetRequestMethod();
 
     /**
      * Get the request header specified by hdr, or an empty string.
      * Return an pair (isPresent,string).
      */
-    std::pair<bool, std::string> GetHeader(const std::string& hdr);
+    virtual std::pair<bool, std::string> GetHeader(const std::string& hdr);
 
     /**
      * Read request body.
@@ -101,7 +104,7 @@ public:
      *
      * @note call this before calling WriteErrorReply or Reply.
      */
-    void WriteHeader(const std::string& hdr, const std::string& value);
+    virtual void WriteHeader(const std::string& hdr, const std::string& value);
 
     /**
      * Write HTTP reply.
@@ -111,7 +114,7 @@ public:
      * @note Can be called only once. As this will give the request back to the
      * main thread, do not call any other HTTPRequest methods after calling this.
      */
-    void WriteReply(int nStatus, const std::string& strReply = "");
+    virtual void WriteReply(int nStatus, const std::string& strReply = "");
 };
 
 /** Event handler closure.