From 0a22eaa03704a14f018b546d358217e96a160f05 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 13 Apr 2017 11:45:24 -0700 Subject: [PATCH 1/5] Add HTTP request headers to Firebase TestRuleset request. --- .../endpoints/include/api_manager/request.h | 3 ++ .../api_manager/check_security_rules_test.cc | 39 ++++++------------- .../firebase_rules/firebase_request.cc | 26 +++++++++++++ .../firebase_rules/firebase_request.h | 1 + .../endpoints/src/api_manager/mock_request.h | 1 + 5 files changed, 42 insertions(+), 28 deletions(-) diff --git a/contrib/endpoints/include/api_manager/request.h b/contrib/endpoints/include/api_manager/request.h index 8d96ff356d6..ec80ee20c25 100644 --- a/contrib/endpoints/include/api_manager/request.h +++ b/contrib/endpoints/include/api_manager/request.h @@ -70,6 +70,9 @@ class Request { // Adds a header to backend. If the header exists, overwrite its value virtual utils::Status AddHeaderToBackend(const std::string &key, const std::string &value) = 0; + + // Gets all HTTP request headers. + virtual void GetHeaders(std::map *headers) { } }; } // namespace api_manager diff --git a/contrib/endpoints/src/api_manager/check_security_rules_test.cc b/contrib/endpoints/src/api_manager/check_security_rules_test.cc index cefab67fbab..1ddbc419663 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules_test.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules_test.cc @@ -160,13 +160,13 @@ static const char kDummyBody[] = R"( static const char kDummyAudience[] = "test-audience"; const char kFirstRequest[] = - R"({"testSuite":{"testCases":[{"expectation":"ALLOW","request":{"method":"get","path":"/ListShelves","auth":{"token":{"email":"limin-429@appspot.gserviceaccount.com","email_verified":true,"azp":"limin-429@appspot.gserviceaccount.com","aud":"https://myfirebaseapp.appspot.com","sub":"113424383671131376652","iat":1486575396,"iss":"https://accounts.google.com","exp":1486578996}}}}]}})"; + R"({"testSuite":{"testCases":[{"expectation":"ALLOW","request":{"headers":{"Pragma":"Custom value"},"path":"/ListShelves","method":"get","auth":{"token":{"email":"limin-429@appspot.gserviceaccount.com","email_verified":true,"azp":"limin-429@appspot.gserviceaccount.com","sub":"113424383671131376652","aud":"https://myfirebaseapp.appspot.com","iat":1486575396,"iss":"https://accounts.google.com","exp":1486578996}}}}]}})"; const char kSecondRequest[] = - R"({"testSuite":{"testCases":[{"expectation":"ALLOW","request":{"auth":{"token":{"email":"limin-429@appspot.gserviceaccount.com","azp":"limin-429@appspot.gserviceaccount.com","aud":"https://myfirebaseapp.appspot.com","sub":"113424383671131376652","iss":"https://accounts.google.com","email_verified":true,"iat":1486575396,"exp":1486578996}},"method":"get","path":"/ListShelves"},"functionMocks":[{"function":"f1","args":[{"exactValue":"http://url1"},{"exactValue":"POST"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}}]}]}})"; + R"({"testSuite":{"testCases":[{"expectation":"ALLOW","request":{"headers":{"Pragma":"Custom value"},"auth":{"token":{"email":"limin-429@appspot.gserviceaccount.com","azp":"limin-429@appspot.gserviceaccount.com","iat":1486575396,"email_verified":true,"aud":"https://myfirebaseapp.appspot.com","sub":"113424383671131376652","exp":1486578996,"iss":"https://accounts.google.com"}},"method":"get","path":"/ListShelves"},"functionMocks":[{"function":"f1","args":[{"exactValue":"http://url1"},{"exactValue":"POST"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}}]}]}})"; const char kThirdRequest[] = - R"({"testSuite":{"testCases":[{"expectation":"ALLOW","request":{"method":"get","path":"/ListShelves","auth":{"token":{"email":"limin-429@appspot.gserviceaccount.com","iat":1486575396,"azp":"limin-429@appspot.gserviceaccount.com","exp":1486578996,"email_verified":true,"sub":"113424383671131376652","aud":"https://myfirebaseapp.appspot.com","iss":"https://accounts.google.com"}}},"functionMocks":[{"function":"f2","args":[{"exactValue":"http://url2"},{"exactValue":"GET"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}},{"function":"f3","args":[{"exactValue":"https://url3"},{"exactValue":"GET"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}},{"function":"f1","args":[{"exactValue":"http://url1"},{"exactValue":"POST"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}}]}]}})"; + R"({"testSuite":{"testCases":[{"expectation":"ALLOW","request":{"headers":{"Pragma":"Custom value"},"auth":{"token":{"email":"limin-429@appspot.gserviceaccount.com","exp":1486578996,"iss":"https://accounts.google.com","iat":1486575396,"sub":"113424383671131376652","aud":"https://myfirebaseapp.appspot.com","azp":"limin-429@appspot.gserviceaccount.com","email_verified":true}},"method":"get","path":"/ListShelves"},"functionMocks":[{"function":"f2","args":[{"exactValue":"http://url2"},{"exactValue":"GET"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}},{"function":"f3","args":[{"exactValue":"https://url3"},{"exactValue":"GET"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}},{"function":"f1","args":[{"exactValue":"http://url1"},{"exactValue":"POST"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}}]}]}})"; ::google::protobuf::Value ToValue(const std::string &arg) { ::google::protobuf::Value value; @@ -198,31 +198,6 @@ MATCHER_P3(HTTPRequestMatches, url, method, body, "") { return MessageDifferencer::Equals(actual, expected); } -FunctionCall BuildCall(const std::string &name, const std::string &url, - const std::string &method, const std::string &body, - const std::string &audience) { - FunctionCall func_call; - func_call.set_function(name); - - if (!url.empty()) { - *(func_call.add_args()) = ToValue(url); - } - - if (!method.empty()) { - *(func_call.add_args()) = ToValue(method); - } - - if (!body.empty()) { - *(func_call.add_args()) = ToValue(body); - } - - if (!audience.empty()) { - *(func_call.add_args()) = ToValue(audience); - } - - return func_call; -} - // Get a server configuration that has auth disabled. This should disable // security rules check by default. std::pair GetConfigWithAuthForceDisabled() { @@ -346,6 +321,14 @@ class CheckSecurityRulesTest : public ::testing::Test { ON_CALL(*raw_request_, GetRequestPath()) .WillByDefault(Return(std::string("/ListShelves"))); + ON_CALL(*raw_request_, GetHeaders(_)) + .WillByDefault(testing::Invoke( + [](std::map *map) { + if (map != nullptr) { + (*map)["Pragma"] = "Custom value"; + } + })); + request_context_ = std::make_shared( service_context_, std::move(request)); release_url_ = diff --git a/contrib/endpoints/src/api_manager/firebase_rules/firebase_request.cc b/contrib/endpoints/src/api_manager/firebase_rules/firebase_request.cc index 7a69cbed47c..38143c8e910 100644 --- a/contrib/endpoints/src/api_manager/firebase_rules/firebase_request.cc +++ b/contrib/endpoints/src/api_manager/firebase_rules/firebase_request.cc @@ -38,6 +38,7 @@ const std::string kToken = "token"; const std::string kAuth = "auth"; const std::string kPath = "path"; const std::string kMethod = "method"; +const std::string kHeaders = "headers"; const std::string kHttpGetMethod = "GET"; const std::string kHttpPostMethod = "POST"; const std::string kHttpHeadMethod = "HEAD"; @@ -180,6 +181,7 @@ Status FirebaseRequest::UpdateRulesetRequestBody( ::google::protobuf::Value claims; ::google::protobuf::Value path; ::google::protobuf::Value method; + ::google::protobuf::Value headers; Status status = utils::JsonToProto(context_->auth_claims(), &claims); if (!status.ok()) { @@ -188,6 +190,10 @@ Status FirebaseRequest::UpdateRulesetRequestBody( auto *variables = test_case->mutable_request()->mutable_struct_value(); auto *fields = variables->mutable_fields(); + GetHeaders(&headers); + if (headers.kind_case() != ::google::protobuf::Value::kNullValue) { + (*fields)[kHeaders] = headers; + } path.set_string_value(context_->request()->GetRequestPath()); (*fields)[kPath] = path; @@ -216,6 +222,26 @@ Status FirebaseRequest::UpdateRulesetRequestBody( return status; } +void FirebaseRequest::GetHeaders(::google::protobuf::Value *headers) { + if (headers == nullptr) { + return; + } + + std::map req_headers; + context_->request()->GetHeaders(&req_headers); + if (req_headers.size() == 0) { + headers->set_null_value(::google::protobuf::NullValue::NULL_VALUE); + return; + } + + auto *map = headers->mutable_struct_value()->mutable_fields(); + for (auto const &kvp : req_headers) { + ::google::protobuf::Value value; + value.set_string_value(kvp.second); + (*map)[kvp.first] = value; + } +} + Status FirebaseRequest::ProcessTestRulesetResponse(const std::string &body) { Status status = utils::JsonToProto(body, &response_); if (!status.ok()) { diff --git a/contrib/endpoints/src/api_manager/firebase_rules/firebase_request.h b/contrib/endpoints/src/api_manager/firebase_rules/firebase_request.h index 1a9a22ea699..4c9dc0dbf31 100644 --- a/contrib/endpoints/src/api_manager/firebase_rules/firebase_request.h +++ b/contrib/endpoints/src/api_manager/firebase_rules/firebase_request.h @@ -134,6 +134,7 @@ class FirebaseRequest { std::vector>::const_iterator Find(const proto::TestRulesetResponse::TestResult::FunctionCall &func_call); + void GetHeaders(::google::protobuf::Value *value); // The API manager environment. Primarily used for logging. ApiManagerEnvInterface *env_; diff --git a/contrib/endpoints/src/api_manager/mock_request.h b/contrib/endpoints/src/api_manager/mock_request.h index 5b3bcb03962..757b75061ab 100644 --- a/contrib/endpoints/src/api_manager/mock_request.h +++ b/contrib/endpoints/src/api_manager/mock_request.h @@ -40,6 +40,7 @@ class MockRequest : public Request { MOCK_METHOD0(GetGrpcResponseBytes, int64_t()); MOCK_METHOD0(GetGrpcRequestMessageCounts, int64_t()); MOCK_METHOD0(GetGrpcResponseMessageCounts, int64_t()); + MOCK_METHOD1(GetHeaders, void(std::map *)); }; } // namespace api_manager From 403150084ba70da97ae4f2e55595c8a2ac0034f0 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 13 Apr 2017 12:20:05 -0700 Subject: [PATCH 2/5] Address code review comment. Fix style. --- contrib/endpoints/include/api_manager/request.h | 2 +- .../src/api_manager/check_security_rules_test.cc | 10 +++++----- .../src/api_manager/firebase_rules/firebase_request.cc | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/contrib/endpoints/include/api_manager/request.h b/contrib/endpoints/include/api_manager/request.h index ec80ee20c25..840bb8e4f75 100644 --- a/contrib/endpoints/include/api_manager/request.h +++ b/contrib/endpoints/include/api_manager/request.h @@ -72,7 +72,7 @@ class Request { const std::string &value) = 0; // Gets all HTTP request headers. - virtual void GetHeaders(std::map *headers) { } + virtual void GetHeaders(std::map *headers) {} }; } // namespace api_manager diff --git a/contrib/endpoints/src/api_manager/check_security_rules_test.cc b/contrib/endpoints/src/api_manager/check_security_rules_test.cc index 1ddbc419663..83d78498c09 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules_test.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules_test.cc @@ -160,13 +160,13 @@ static const char kDummyBody[] = R"( static const char kDummyAudience[] = "test-audience"; const char kFirstRequest[] = - R"({"testSuite":{"testCases":[{"expectation":"ALLOW","request":{"headers":{"Pragma":"Custom value"},"path":"/ListShelves","method":"get","auth":{"token":{"email":"limin-429@appspot.gserviceaccount.com","email_verified":true,"azp":"limin-429@appspot.gserviceaccount.com","sub":"113424383671131376652","aud":"https://myfirebaseapp.appspot.com","iat":1486575396,"iss":"https://accounts.google.com","exp":1486578996}}}}]}})"; + R"({"testSuite":{"testCases":[{"expectation":"ALLOW","request":{"headers":{"Pragma":"Custom value"},"path":"/ListShelves","method":"get","auth":{"token":{"email":"limin-429@appspot.gserviceaccount.com","email_verified":true,"azp":"limin-429@appspot.gserviceaccount.com","sub":"113424383671131376652","aud":"https://myfirebaseapp.appspot.com","iat":1486575396,"iss":"https://accounts.google.com","exp":1486578996}}}}]}})"; const char kSecondRequest[] = - R"({"testSuite":{"testCases":[{"expectation":"ALLOW","request":{"headers":{"Pragma":"Custom value"},"auth":{"token":{"email":"limin-429@appspot.gserviceaccount.com","azp":"limin-429@appspot.gserviceaccount.com","iat":1486575396,"email_verified":true,"aud":"https://myfirebaseapp.appspot.com","sub":"113424383671131376652","exp":1486578996,"iss":"https://accounts.google.com"}},"method":"get","path":"/ListShelves"},"functionMocks":[{"function":"f1","args":[{"exactValue":"http://url1"},{"exactValue":"POST"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}}]}]}})"; + R"({"testSuite":{"testCases":[{"expectation":"ALLOW","request":{"headers":{"Pragma":"Custom value"},"auth":{"token":{"email":"limin-429@appspot.gserviceaccount.com","azp":"limin-429@appspot.gserviceaccount.com","iat":1486575396,"email_verified":true,"aud":"https://myfirebaseapp.appspot.com","sub":"113424383671131376652","exp":1486578996,"iss":"https://accounts.google.com"}},"method":"get","path":"/ListShelves"},"functionMocks":[{"function":"f1","args":[{"exactValue":"http://url1"},{"exactValue":"POST"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}}]}]}})"; const char kThirdRequest[] = - R"({"testSuite":{"testCases":[{"expectation":"ALLOW","request":{"headers":{"Pragma":"Custom value"},"auth":{"token":{"email":"limin-429@appspot.gserviceaccount.com","exp":1486578996,"iss":"https://accounts.google.com","iat":1486575396,"sub":"113424383671131376652","aud":"https://myfirebaseapp.appspot.com","azp":"limin-429@appspot.gserviceaccount.com","email_verified":true}},"method":"get","path":"/ListShelves"},"functionMocks":[{"function":"f2","args":[{"exactValue":"http://url2"},{"exactValue":"GET"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}},{"function":"f3","args":[{"exactValue":"https://url3"},{"exactValue":"GET"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}},{"function":"f1","args":[{"exactValue":"http://url1"},{"exactValue":"POST"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}}]}]}})"; + R"({"testSuite":{"testCases":[{"expectation":"ALLOW","request":{"headers":{"Pragma":"Custom value"},"auth":{"token":{"email":"limin-429@appspot.gserviceaccount.com","exp":1486578996,"iss":"https://accounts.google.com","iat":1486575396,"sub":"113424383671131376652","aud":"https://myfirebaseapp.appspot.com","azp":"limin-429@appspot.gserviceaccount.com","email_verified":true}},"method":"get","path":"/ListShelves"},"functionMocks":[{"function":"f2","args":[{"exactValue":"http://url2"},{"exactValue":"GET"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}},{"function":"f3","args":[{"exactValue":"https://url3"},{"exactValue":"GET"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}},{"function":"f1","args":[{"exactValue":"http://url1"},{"exactValue":"POST"},{"exactValue":{"key":"value"}},{"exactValue":"test-audience"}],"result":{"value":{"key":"value"}}}]}]}})"; ::google::protobuf::Value ToValue(const std::string &arg) { ::google::protobuf::Value value; @@ -322,8 +322,8 @@ class CheckSecurityRulesTest : public ::testing::Test { .WillByDefault(Return(std::string("/ListShelves"))); ON_CALL(*raw_request_, GetHeaders(_)) - .WillByDefault(testing::Invoke( - [](std::map *map) { + .WillByDefault( + testing::Invoke([](std::map *map) { if (map != nullptr) { (*map)["Pragma"] = "Custom value"; } diff --git a/contrib/endpoints/src/api_manager/firebase_rules/firebase_request.cc b/contrib/endpoints/src/api_manager/firebase_rules/firebase_request.cc index 38143c8e910..a11d30c72c5 100644 --- a/contrib/endpoints/src/api_manager/firebase_rules/firebase_request.cc +++ b/contrib/endpoints/src/api_manager/firebase_rules/firebase_request.cc @@ -235,7 +235,7 @@ void FirebaseRequest::GetHeaders(::google::protobuf::Value *headers) { } auto *map = headers->mutable_struct_value()->mutable_fields(); - for (auto const &kvp : req_headers) { + for (const auto &kvp : req_headers) { ::google::protobuf::Value value; value.set_string_value(kvp.second); (*map)[kvp.first] = value; From 0c0d0dacbd00f5de2b65b9189a44d27943f1ca09 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 13 Apr 2017 13:47:38 -0700 Subject: [PATCH 3/5] Add comment about why the method cannot be pure virtual method. --- contrib/endpoints/include/api_manager/request.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/contrib/endpoints/include/api_manager/request.h b/contrib/endpoints/include/api_manager/request.h index 840bb8e4f75..746c93ddcbd 100644 --- a/contrib/endpoints/include/api_manager/request.h +++ b/contrib/endpoints/include/api_manager/request.h @@ -71,7 +71,11 @@ class Request { virtual utils::Status AddHeaderToBackend(const std::string &key, const std::string &value) = 0; - // Gets all HTTP request headers. + // Gets all HTTP request headers. This method cannot be made into a pure + // virtual function until all the implementations of this class also implement + // the GetHeaders method. Changing this to a pure virtual function should be + // done with a lot of care and making sure that all implementations of this + // class have implemented this method. virtual void GetHeaders(std::map *headers) {} }; From 6769a20b4918b6750251a05973dfee1ca9025c0a Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 13 Apr 2017 14:10:09 -0700 Subject: [PATCH 4/5] Move CheckSecurityRules before CheckServiceControl in workflow. --- contrib/endpoints/src/api_manager/check_workflow.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/endpoints/src/api_manager/check_workflow.cc b/contrib/endpoints/src/api_manager/check_workflow.cc index 79cd74953db..c9e1a376b6e 100644 --- a/contrib/endpoints/src/api_manager/check_workflow.cc +++ b/contrib/endpoints/src/api_manager/check_workflow.cc @@ -33,10 +33,10 @@ void CheckWorkflow::RegisterAll() { Register(FetchServiceAccountToken); // Authentication checks. Register(CheckAuth); - // Checks service control. - Register(CheckServiceControl); // Check Security Rules. Register(CheckSecurityRules); + // Checks service control. + Register(CheckServiceControl); // Quota control Register(QuotaControl); } From 31c582b9514dac4e03d23380823a57ea7febbe1d Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 13 Apr 2017 16:04:29 -0700 Subject: [PATCH 5/5] Convert GetHeaders to be pure virtual. --- contrib/endpoints/include/api_manager/request.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/endpoints/include/api_manager/request.h b/contrib/endpoints/include/api_manager/request.h index 746c93ddcbd..4b1dde81816 100644 --- a/contrib/endpoints/include/api_manager/request.h +++ b/contrib/endpoints/include/api_manager/request.h @@ -76,7 +76,7 @@ class Request { // the GetHeaders method. Changing this to a pure virtual function should be // done with a lot of care and making sure that all implementations of this // class have implemented this method. - virtual void GetHeaders(std::map *headers) {} + virtual void GetHeaders(std::map *headers) = 0; }; } // namespace api_manager