From db719ed6b8e8e6dbe1b7da13e56424ef2e50add8 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Wed, 8 Feb 2017 12:47:11 -0800 Subject: [PATCH 1/4] Unit tests for check security rules. --- contrib/endpoints/src/api_manager/BUILD | 15 + .../src/api_manager/check_security_rules.cc | 12 +- .../api_manager/check_security_rules_test.cc | 416 ++++++++++++++++++ 3 files changed, 437 insertions(+), 6 deletions(-) create mode 100644 contrib/endpoints/src/api_manager/check_security_rules_test.cc diff --git a/contrib/endpoints/src/api_manager/BUILD b/contrib/endpoints/src/api_manager/BUILD index ffe4f060b7c..39e85eacac5 100644 --- a/contrib/endpoints/src/api_manager/BUILD +++ b/contrib/endpoints/src/api_manager/BUILD @@ -279,3 +279,18 @@ cc_test( "//external:googletest_main", ], ) + +cc_test( + name = "check_security_rules_test", + size = "small", + srcs = [ + "check_security_rules_test.cc", + "mock_request.h", + ], + linkstatic = 1, + deps = [ + ":api_manager", + ":mock_api_manager_environment", + "//external:googletest_main", + ], +) diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index 45fa30ae7bd..73bb4d2e008 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -43,10 +43,10 @@ const char kFirebaseCreateMethod[] = "create"; const char kFirebaseGetMethod[] = "get"; const char kFirebaseDeleteMethod[] = "delete"; const char kFirebaseUpdateMethod[] = "update"; -const char kV1[] = "v1/"; +const char kV1[] = "/v1"; const char kTestQuery[] = ":test?alt=json"; -const char kProjects[] = "projects/"; -const char kReleases[] = "/releases/"; +const char kProjects[] = "/projects"; +const char kReleases[] = "/releases"; const char kRulesetName[] = "rulesetName"; const char kTestResults[] = "testResults"; const char kState[] = "state"; @@ -72,13 +72,13 @@ std::string GetReleaseName(const context::RequestContext &context) { std::string GetRulesetTestUri(const context::RequestContext &context, const std::string &ruleset_id) { return context.service_context()->config()->GetFirebaseServer() + kV1 + - ruleset_id + kTestQuery; + "/" + ruleset_id + kTestQuery; } std::string GetReleaseUrl(const context::RequestContext &context) { return context.service_context()->config()->GetFirebaseServer() + kV1 + - kProjects + context.service_context()->project_id() + kReleases + - GetReleaseName(context); + kProjects + "/" + context.service_context()->project_id() + + kReleases + "/" + GetReleaseName(context); } std::string GetOperation(const std::string &httpMethod) { diff --git a/contrib/endpoints/src/api_manager/check_security_rules_test.cc b/contrib/endpoints/src/api_manager/check_security_rules_test.cc new file mode 100644 index 00000000000..400d91c8e17 --- /dev/null +++ b/contrib/endpoints/src/api_manager/check_security_rules_test.cc @@ -0,0 +1,416 @@ +// Copyright 2016 Google Inc. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +//////////////////////////////////////////////////////////////////////////////// +// + +#include "contrib/endpoints/src/api_manager/check_security_rules.h" +#include "contrib/endpoints/src/api_manager/context/request_context.h" +#include "contrib/endpoints/src/api_manager/context/service_context.h" +#include "contrib/endpoints/src/api_manager/mock_api_manager_environment.h" +#include "contrib/endpoints/src/api_manager/mock_request.h" + +using ::testing::_; +using ::testing::AllOf; +using ::testing::Invoke; +using ::testing::Property; +using ::testing::Return; +using ::testing::StrCaseEq; +using ::testing::StrEq; +using ::testing::StrNe; + +using ::google::api_manager::utils::Status; +using ::google::protobuf::util::error::Code; + +namespace google { +namespace api_manager { + +namespace { +const char kServiceName[] = R"( +name: "myfirebaseapp.appspot.com")"; + +const char kBadServiceName[] = R"( +name: "badService.appspot.com")"; + +const char kProducerProjectId[] = R"( +producer_project_id: "myfirebaseapp" +)"; + +const char kApis[] = R"( +apis { + name: "Bookstore" + version: "v1" + methods { + name: "ListShelves" + request_type_url: "types.googleapis.com/google.protobuf.Empty" + response_type_url: "types.googleapis.com/Bookstore.ListShelvesResponse" + } + methods { + name: "ListBooks" + request_type_url: "types.googleapis.com/google.protobuf.Empty" + response_type_url: "types.googleapis.com/Bookstore.ListBooksResponse" + } + methods { + name: "CreateBook" + request_type_url: "types.googleapis.com/Bookstore.CreateBookRequest" + response_type_url: "types.googleapis.com/Bookstore.Book" + } +})"; + +const char kAuthentication[] = R"( +authentication { + providers { + id: "issuer1" + issuer: "https://issuer1.com" + } + providers { + id: "issuer2" + issuer: "https://issuer2.com" + jwks_uri: "https://issuer2.com/pubkey" + } + rules { + selector: "ListShelves" + requirements { + provider_id: "issuer1" + } + requirements { + provider_id: "issuer2" + } + } +})"; + +const char kHttp[] = R"( +http { + rules { + selector: "ListShelves" + get: "/ListShelves" + } +} +control { + environment: "http://127.0.0.1:8081" +})"; + +const char kJwtEmailPayload[] = R"({"iss":"https://accounts.google.com","iat":1486575396,"exp":1486578996,"aud":"https://myfirebaseapp.appspot.com","sub":"113424383671131376652","email_verified":true,"azp":"limin-429@appspot.gserviceaccount.com","email":"limin-429@appspot.gserviceaccount.com"})"; + +static const char kServerConfig[] = R"( +api_check_security_rules_config { + firebase_server: "https://myfirebaseserver.com" +})"; + +static const char kRelease[] = R"( +{ + "name": "projects/myfirebaseapp/releases/myfirebaseapp.appspot.com:v1", + "rulesetName": "projects/myfirebaseapp/rulesets/99045fc0-a5e4-47e2-a665-f88593594b6b", + "createTime": "2017-01-10T16:52:27.764111Z", + "updateTime": "2017-01-10T16:52:27.764111Z" +})"; + +static const char kReleaseError[] = R"( +{ + "error": { + "code": 404, + "message": "Requested entity was not found.", + "status": "NOT_FOUND", + } +})"; + +static const char kTestResultFailure[] = R"( +{ + "testResults": [ + { + "state": "FAILURE" + } + ] +} +)"; + +static const char kTestResultSuccess[] = R"( +{ + "testResults": [ + { + "state": "SUCCESS" + } + ] +} +)"; + +std::pair GetConfigWithAuthForceDisabled() { + std::string service_config = std::string(kServiceName) + kApis + kAuthentication + kHttp; + const char server_config[] = R"( +api_authentication_config { +force_disable: true +} +api_check_security_rules_config { + firebase_server: "https://myfirebaseserver.com" +} +)"; + return std::make_pair(service_config, server_config); +} + +std::pair GetConfigWithNoAuth() { + std::string service_config = std::string(kServiceName) + kApis + kHttp; + return std::make_pair(service_config, std::string(kServerConfig)); +} + +std::pair GetConfigWithoutApis() { + std::string service_config = std::string(kServiceName) + kAuthentication + kHttp; + + return std::make_pair(service_config, std::string(kServerConfig)); +} + +std::pair GetConfigWithoutServer() { + std::string service_config = std::string(kServiceName) + + kApis + kAuthentication + kHttp; + return std::make_pair(service_config, ""); +} + +std::pair GetValidConfig() { + std::string service_config = std::string(kServiceName) + + kApis + kAuthentication + kHttp; + return std::make_pair(service_config, kServerConfig); +} + + +class CheckDisableSecurityRulesTest : + public ::testing::TestWithParam>{ + public: + void SetUp() { + std::unique_ptr env( + new ::testing::NiceMock()); + raw_env_ = env.get(); + + std::string service_config; + std::string server_config; + + std::tie(service_config, server_config) = GetParam(); + std::unique_ptr config = Config::Create(raw_env_, service_config, + server_config); + + ASSERT_TRUE(config != nullptr); + + service_context_ = std::make_shared( + std::move(env), std::move(config)); + + ASSERT_TRUE(service_context_.get() != nullptr); + + std::unique_ptr request( + new ::testing::NiceMock()); + raw_request = request.get(); + + request_context_ = std::make_shared( + service_context_, std::move(request)); + EXPECT_CALL(*raw_env_, DoRunHTTPRequest(_)) + .Times(0); + } + + MockApiManagerEnvironment *raw_env_; + MockRequest *raw_request; + std::shared_ptr request_context_; + std::shared_ptr service_context_; +}; + +TEST_P(CheckDisableSecurityRulesTest, CheckAuthzDisabled) { + CheckSecurityRules(request_context_, + [](Status status) { ASSERT_TRUE(status.ok()); }); +} + +INSTANTIATE_TEST_CASE_P(ConfigToDisableFirebaseRulesCheck, + CheckDisableSecurityRulesTest, + testing::Values(GetConfigWithNoAuth(), + GetConfigWithoutApis(), + GetConfigWithoutServer())); + +class CheckSecurityRulesTest : public ::testing::Test { + public: + + void SetUp(std::string service_config, std::string server_config) { + + std::unique_ptr env( + new ::testing::NiceMock()); + raw_env_ = env.get(); + + std::unique_ptr config = Config::Create(raw_env_, service_config, + server_config); + ASSERT_TRUE(config != nullptr); + + service_context_ = std::make_shared( + std::move(env), std::move(config)); + + ASSERT_TRUE(service_context_.get() != nullptr); + + std::unique_ptr request( + new ::testing::NiceMock()); + raw_request_ = request.get(); + + ON_CALL(*raw_request_, GetRequestHTTPMethod()) + .WillByDefault(Return(std::string("GET"))); + + ON_CALL(*raw_request_, GetRequestPath()) + .WillByDefault(Return(std::string("/ListShelves"))); + + request_context_ = std::make_shared( + service_context_, std::move(request)); + release_url_ = "https://myfirebaseserver.com/v1/projects/myfirebaseapp/" + "releases/myfirebaseapp.appspot.com:v1"; + + ruleset_test_url_ = "https://myfirebaseserver.com/v1" + "/projects/myfirebaseapp/rulesets/99045fc0-a5e4-47e2-a665-f88593594b6b" + ":test?alt=json"; + } + + MockApiManagerEnvironment *raw_env_; + MockRequest *raw_request_; + std::shared_ptr request_context_; + std::shared_ptr service_context_; + std::string release_url_; + std::string ruleset_test_url_; +}; + +TEST_F(CheckSecurityRulesTest, CheckAuthzFailGetRelease) { + std::string service_config = std::string(kBadServiceName) + kProducerProjectId + + kApis + kAuthentication + kHttp; + std::string server_config = kServerConfig; + SetUp(service_config, server_config); + + EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( + Property(&HTTPRequest::url, StrNe(release_url_)), + Property(&HTTPRequest::method, StrCaseEq("GET"))))) + .WillOnce(Invoke([] (HTTPRequest *req) { + + std::map empty; + std::string body(kReleaseError); + req->OnComplete( + Status(Code::NOT_FOUND, "Requested entity was not found"), + std::move(empty), std::move(body)); + + })); + + EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( + Property(&HTTPRequest::url, StrEq(ruleset_test_url_)), + Property(&HTTPRequest::method, StrCaseEq("POST"))))) + .Times(0); + + auto ptr = this; + CheckSecurityRules(request_context_, + [ptr](Status status) { + ptr->raw_env_->LogInfo(status.ToString()); + ASSERT_TRUE(!status.ok()); }); +} + +TEST_F(CheckSecurityRulesTest, CheckAuthzFailTestRuleset) { + std::string service_config = std::string(kServiceName) + kProducerProjectId + + kApis + kAuthentication + kHttp; + std::string server_config = kServerConfig; + SetUp(service_config, server_config); + + request_context_->set_auth_claims(kJwtEmailPayload); + EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( + Property(&HTTPRequest::url, StrEq(release_url_)), + Property(&HTTPRequest::method, StrCaseEq("GET"))))) + .WillOnce(Invoke([] (HTTPRequest *req) { + + std::map empty; + std::string body(kRelease); + req->OnComplete(Status::OK, + std::move(empty), std::move(body)); + + })); + + EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( + Property(&HTTPRequest::url, StrEq(ruleset_test_url_)), + Property(&HTTPRequest::method, StrCaseEq("POST"))))) + .WillOnce(Invoke([] (HTTPRequest *req) { + std::map empty; + std::string body; + req->OnComplete(Status(Code::INTERNAL, "Cannot talk to server"), + std::move(empty), std::move(body)); + })); + + CheckSecurityRules(request_context_, + [] (Status status) { + ASSERT_FALSE(status.ok()); }); +} + +TEST_F(CheckSecurityRulesTest, CheckAutzFailWithTestResultFailure) { + std::string service_config = std::string(kServiceName) + kProducerProjectId + + kApis + kAuthentication + kHttp; + std::string server_config = kServerConfig; + SetUp(service_config, server_config); + + request_context_->set_auth_claims(kJwtEmailPayload); + EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( + Property(&HTTPRequest::url, StrEq(release_url_)), + Property(&HTTPRequest::method, StrCaseEq("GET"))))) + .WillOnce(Invoke([] (HTTPRequest *req) { + + std::map empty; + std::string body(kRelease); + req->OnComplete(Status::OK, + std::move(empty), std::move(body)); + + })); + + EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( + Property(&HTTPRequest::url, StrEq(ruleset_test_url_)), + Property(&HTTPRequest::method, StrCaseEq("POST"))))) + .WillOnce(Invoke([] (HTTPRequest *req) { + std::map empty; + std::string body = kTestResultFailure; + req->OnComplete(Status::OK, + std::move(empty), std::move(body)); + })); + + CheckSecurityRules(request_context_, + [] (Status status) { + ASSERT_TRUE(status.CanonicalCode() == Code::PERMISSION_DENIED); }); + +} + +TEST_F(CheckSecurityRulesTest, CheckAuthzSuccess) { + std::string service_config = std::string(kServiceName) + kProducerProjectId + + kApis + kAuthentication + kHttp; + std::string server_config = kServerConfig; + SetUp(service_config, server_config); + + request_context_->set_auth_claims(kJwtEmailPayload); + EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( + Property(&HTTPRequest::url, StrEq(release_url_)), + Property(&HTTPRequest::method, StrCaseEq("GET"))))) + .WillOnce(Invoke([] (HTTPRequest *req) { + + std::map empty; + std::string body(kRelease); + req->OnComplete(Status::OK, + std::move(empty), std::move(body)); + + })); + + EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( + Property(&HTTPRequest::url, StrEq(ruleset_test_url_)), + Property(&HTTPRequest::method, StrCaseEq("POST"))))) + .WillOnce(Invoke([] (HTTPRequest *req) { + std::map empty; + std::string body = kTestResultSuccess; + req->OnComplete(Status::OK, + std::move(empty), std::move(body)); + })); + + CheckSecurityRules(request_context_, + [] (Status status) { + ASSERT_TRUE(status.ok()); }); +} + +} +} // namespace api_manager +} // namespace google From 309f1fa319e826ee7f13b91b83d85211f6d3220e Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Wed, 8 Feb 2017 12:50:15 -0800 Subject: [PATCH 2/4] format --- .../src/api_manager/check_security_rules.cc | 8 +- .../api_manager/check_security_rules_test.cc | 227 +++++++++--------- 2 files changed, 116 insertions(+), 119 deletions(-) diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index 73bb4d2e008..845bc4cf6bb 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -71,14 +71,14 @@ std::string GetReleaseName(const context::RequestContext &context) { std::string GetRulesetTestUri(const context::RequestContext &context, const std::string &ruleset_id) { - return context.service_context()->config()->GetFirebaseServer() + kV1 + - "/" + ruleset_id + kTestQuery; + return context.service_context()->config()->GetFirebaseServer() + kV1 + "/" + + ruleset_id + kTestQuery; } std::string GetReleaseUrl(const context::RequestContext &context) { return context.service_context()->config()->GetFirebaseServer() + kV1 + - kProjects + "/" + context.service_context()->project_id() + - kReleases + "/" + GetReleaseName(context); + kProjects + "/" + context.service_context()->project_id() + kReleases + + "/" + GetReleaseName(context); } std::string GetOperation(const std::string &httpMethod) { 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 400d91c8e17..e01b345bf85 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules_test.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules_test.cc @@ -101,7 +101,8 @@ control { environment: "http://127.0.0.1:8081" })"; -const char kJwtEmailPayload[] = R"({"iss":"https://accounts.google.com","iat":1486575396,"exp":1486578996,"aud":"https://myfirebaseapp.appspot.com","sub":"113424383671131376652","email_verified":true,"azp":"limin-429@appspot.gserviceaccount.com","email":"limin-429@appspot.gserviceaccount.com"})"; +const char kJwtEmailPayload[] = + R"({"iss":"https://accounts.google.com","iat":1486575396,"exp":1486578996,"aud":"https://myfirebaseapp.appspot.com","sub":"113424383671131376652","email_verified":true,"azp":"limin-429@appspot.gserviceaccount.com","email":"limin-429@appspot.gserviceaccount.com"})"; static const char kServerConfig[] = R"( api_check_security_rules_config { @@ -146,7 +147,8 @@ static const char kTestResultSuccess[] = R"( )"; std::pair GetConfigWithAuthForceDisabled() { - std::string service_config = std::string(kServiceName) + kApis + kAuthentication + kHttp; + std::string service_config = + std::string(kServiceName) + kApis + kAuthentication + kHttp; const char server_config[] = R"( api_authentication_config { force_disable: true @@ -164,26 +166,26 @@ std::pair GetConfigWithNoAuth() { } std::pair GetConfigWithoutApis() { - std::string service_config = std::string(kServiceName) + kAuthentication + kHttp; + std::string service_config = + std::string(kServiceName) + kAuthentication + kHttp; return std::make_pair(service_config, std::string(kServerConfig)); } std::pair GetConfigWithoutServer() { - std::string service_config = std::string(kServiceName) + - kApis + kAuthentication + kHttp; + std::string service_config = + std::string(kServiceName) + kApis + kAuthentication + kHttp; return std::make_pair(service_config, ""); } std::pair GetValidConfig() { - std::string service_config = std::string(kServiceName) + - kApis + kAuthentication + kHttp; + std::string service_config = + std::string(kServiceName) + kApis + kAuthentication + kHttp; return std::make_pair(service_config, kServerConfig); } - -class CheckDisableSecurityRulesTest : - public ::testing::TestWithParam>{ +class CheckDisableSecurityRulesTest + : public ::testing::TestWithParam> { public: void SetUp() { std::unique_ptr env( @@ -194,8 +196,8 @@ class CheckDisableSecurityRulesTest : std::string server_config; std::tie(service_config, server_config) = GetParam(); - std::unique_ptr config = Config::Create(raw_env_, service_config, - server_config); + std::unique_ptr config = + Config::Create(raw_env_, service_config, server_config); ASSERT_TRUE(config != nullptr); @@ -210,8 +212,7 @@ class CheckDisableSecurityRulesTest : request_context_ = std::make_shared( service_context_, std::move(request)); - EXPECT_CALL(*raw_env_, DoRunHTTPRequest(_)) - .Times(0); + EXPECT_CALL(*raw_env_, DoRunHTTPRequest(_)).Times(0); } MockApiManagerEnvironment *raw_env_; @@ -233,15 +234,13 @@ INSTANTIATE_TEST_CASE_P(ConfigToDisableFirebaseRulesCheck, class CheckSecurityRulesTest : public ::testing::Test { public: - void SetUp(std::string service_config, std::string server_config) { - std::unique_ptr env( new ::testing::NiceMock()); raw_env_ = env.get(); - std::unique_ptr config = Config::Create(raw_env_, service_config, - server_config); + std::unique_ptr config = + Config::Create(raw_env_, service_config, server_config); ASSERT_TRUE(config != nullptr); service_context_ = std::make_shared( @@ -261,13 +260,15 @@ class CheckSecurityRulesTest : public ::testing::Test { request_context_ = std::make_shared( service_context_, std::move(request)); - release_url_ = "https://myfirebaseserver.com/v1/projects/myfirebaseapp/" + release_url_ = + "https://myfirebaseserver.com/v1/projects/myfirebaseapp/" "releases/myfirebaseapp.appspot.com:v1"; - ruleset_test_url_ = "https://myfirebaseserver.com/v1" + ruleset_test_url_ = + "https://myfirebaseserver.com/v1" "/projects/myfirebaseapp/rulesets/99045fc0-a5e4-47e2-a665-f88593594b6b" ":test?alt=json"; - } + } MockApiManagerEnvironment *raw_env_; MockRequest *raw_request_; @@ -278,139 +279,135 @@ class CheckSecurityRulesTest : public ::testing::Test { }; TEST_F(CheckSecurityRulesTest, CheckAuthzFailGetRelease) { - std::string service_config = std::string(kBadServiceName) + kProducerProjectId - + kApis + kAuthentication + kHttp; + std::string service_config = std::string(kBadServiceName) + + kProducerProjectId + kApis + kAuthentication + + kHttp; std::string server_config = kServerConfig; SetUp(service_config, server_config); EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( - Property(&HTTPRequest::url, StrNe(release_url_)), - Property(&HTTPRequest::method, StrCaseEq("GET"))))) - .WillOnce(Invoke([] (HTTPRequest *req) { + Property(&HTTPRequest::url, StrNe(release_url_)), + Property(&HTTPRequest::method, StrCaseEq("GET"))))) + .WillOnce(Invoke([](HTTPRequest *req) { - std::map empty; - std::string body(kReleaseError); - req->OnComplete( - Status(Code::NOT_FOUND, "Requested entity was not found"), - std::move(empty), std::move(body)); + std::map empty; + std::string body(kReleaseError); + req->OnComplete( + Status(Code::NOT_FOUND, "Requested entity was not found"), + std::move(empty), std::move(body)); - })); + })); - EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( - Property(&HTTPRequest::url, StrEq(ruleset_test_url_)), - Property(&HTTPRequest::method, StrCaseEq("POST"))))) - .Times(0); + EXPECT_CALL(*raw_env_, + DoRunHTTPRequest( + AllOf(Property(&HTTPRequest::url, StrEq(ruleset_test_url_)), + Property(&HTTPRequest::method, StrCaseEq("POST"))))) + .Times(0); auto ptr = this; - CheckSecurityRules(request_context_, - [ptr](Status status) { - ptr->raw_env_->LogInfo(status.ToString()); - ASSERT_TRUE(!status.ok()); }); + CheckSecurityRules(request_context_, [ptr](Status status) { + ptr->raw_env_->LogInfo(status.ToString()); + ASSERT_TRUE(!status.ok()); + }); } TEST_F(CheckSecurityRulesTest, CheckAuthzFailTestRuleset) { std::string service_config = std::string(kServiceName) + kProducerProjectId + - kApis + kAuthentication + kHttp; + kApis + kAuthentication + kHttp; std::string server_config = kServerConfig; SetUp(service_config, server_config); request_context_->set_auth_claims(kJwtEmailPayload); EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( - Property(&HTTPRequest::url, StrEq(release_url_)), - Property(&HTTPRequest::method, StrCaseEq("GET"))))) - .WillOnce(Invoke([] (HTTPRequest *req) { - - std::map empty; - std::string body(kRelease); - req->OnComplete(Status::OK, - std::move(empty), std::move(body)); - - })); - - EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( - Property(&HTTPRequest::url, StrEq(ruleset_test_url_)), - Property(&HTTPRequest::method, StrCaseEq("POST"))))) - .WillOnce(Invoke([] (HTTPRequest *req) { - std::map empty; - std::string body; - req->OnComplete(Status(Code::INTERNAL, "Cannot talk to server"), - std::move(empty), std::move(body)); - })); + Property(&HTTPRequest::url, StrEq(release_url_)), + Property(&HTTPRequest::method, StrCaseEq("GET"))))) + .WillOnce(Invoke([](HTTPRequest *req) { + + std::map empty; + std::string body(kRelease); + req->OnComplete(Status::OK, std::move(empty), std::move(body)); + + })); + + EXPECT_CALL(*raw_env_, + DoRunHTTPRequest( + AllOf(Property(&HTTPRequest::url, StrEq(ruleset_test_url_)), + Property(&HTTPRequest::method, StrCaseEq("POST"))))) + .WillOnce(Invoke([](HTTPRequest *req) { + std::map empty; + std::string body; + req->OnComplete(Status(Code::INTERNAL, "Cannot talk to server"), + std::move(empty), std::move(body)); + })); CheckSecurityRules(request_context_, - [] (Status status) { - ASSERT_FALSE(status.ok()); }); + [](Status status) { ASSERT_FALSE(status.ok()); }); } TEST_F(CheckSecurityRulesTest, CheckAutzFailWithTestResultFailure) { std::string service_config = std::string(kServiceName) + kProducerProjectId + - kApis + kAuthentication + kHttp; + kApis + kAuthentication + kHttp; std::string server_config = kServerConfig; SetUp(service_config, server_config); request_context_->set_auth_claims(kJwtEmailPayload); EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( - Property(&HTTPRequest::url, StrEq(release_url_)), - Property(&HTTPRequest::method, StrCaseEq("GET"))))) - .WillOnce(Invoke([] (HTTPRequest *req) { - - std::map empty; - std::string body(kRelease); - req->OnComplete(Status::OK, - std::move(empty), std::move(body)); - - })); - - EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( - Property(&HTTPRequest::url, StrEq(ruleset_test_url_)), - Property(&HTTPRequest::method, StrCaseEq("POST"))))) - .WillOnce(Invoke([] (HTTPRequest *req) { - std::map empty; - std::string body = kTestResultFailure; - req->OnComplete(Status::OK, - std::move(empty), std::move(body)); - })); - - CheckSecurityRules(request_context_, - [] (Status status) { - ASSERT_TRUE(status.CanonicalCode() == Code::PERMISSION_DENIED); }); - + Property(&HTTPRequest::url, StrEq(release_url_)), + Property(&HTTPRequest::method, StrCaseEq("GET"))))) + .WillOnce(Invoke([](HTTPRequest *req) { + + std::map empty; + std::string body(kRelease); + req->OnComplete(Status::OK, std::move(empty), std::move(body)); + + })); + + EXPECT_CALL(*raw_env_, + DoRunHTTPRequest( + AllOf(Property(&HTTPRequest::url, StrEq(ruleset_test_url_)), + Property(&HTTPRequest::method, StrCaseEq("POST"))))) + .WillOnce(Invoke([](HTTPRequest *req) { + std::map empty; + std::string body = kTestResultFailure; + req->OnComplete(Status::OK, std::move(empty), std::move(body)); + })); + + CheckSecurityRules(request_context_, [](Status status) { + ASSERT_TRUE(status.CanonicalCode() == Code::PERMISSION_DENIED); + }); } TEST_F(CheckSecurityRulesTest, CheckAuthzSuccess) { std::string service_config = std::string(kServiceName) + kProducerProjectId + - kApis + kAuthentication + kHttp; + kApis + kAuthentication + kHttp; std::string server_config = kServerConfig; SetUp(service_config, server_config); request_context_->set_auth_claims(kJwtEmailPayload); EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( - Property(&HTTPRequest::url, StrEq(release_url_)), - Property(&HTTPRequest::method, StrCaseEq("GET"))))) - .WillOnce(Invoke([] (HTTPRequest *req) { - - std::map empty; - std::string body(kRelease); - req->OnComplete(Status::OK, - std::move(empty), std::move(body)); - - })); - - EXPECT_CALL(*raw_env_, DoRunHTTPRequest(AllOf( - Property(&HTTPRequest::url, StrEq(ruleset_test_url_)), - Property(&HTTPRequest::method, StrCaseEq("POST"))))) - .WillOnce(Invoke([] (HTTPRequest *req) { - std::map empty; - std::string body = kTestResultSuccess; - req->OnComplete(Status::OK, - std::move(empty), std::move(body)); - })); + Property(&HTTPRequest::url, StrEq(release_url_)), + Property(&HTTPRequest::method, StrCaseEq("GET"))))) + .WillOnce(Invoke([](HTTPRequest *req) { + + std::map empty; + std::string body(kRelease); + req->OnComplete(Status::OK, std::move(empty), std::move(body)); + + })); + + EXPECT_CALL(*raw_env_, + DoRunHTTPRequest( + AllOf(Property(&HTTPRequest::url, StrEq(ruleset_test_url_)), + Property(&HTTPRequest::method, StrCaseEq("POST"))))) + .WillOnce(Invoke([](HTTPRequest *req) { + std::map empty; + std::string body = kTestResultSuccess; + req->OnComplete(Status::OK, std::move(empty), std::move(body)); + })); CheckSecurityRules(request_context_, - [] (Status status) { - ASSERT_TRUE(status.ok()); }); + [](Status status) { ASSERT_TRUE(status.ok()); }); } - } -} // namespace api_manager -} // namespace google +} // namespace api_manager +} // namespace google From f339f2f856e638676fb3940fa14a0c7bb323d0a6 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 9 Feb 2017 15:03:54 -0800 Subject: [PATCH 3/4] Address review comments. --- .../api_manager/check_security_rules_test.cc | 82 +++++++++++++++---- 1 file changed, 65 insertions(+), 17 deletions(-) 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 e01b345bf85..f9a90567a49 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules_test.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules_test.cc @@ -1,4 +1,4 @@ -// Copyright 2016 Google Inc. All Rights Reserved. +// Copyright 2017 Google Inc. All Rights Reserved. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -37,16 +37,22 @@ namespace google { namespace api_manager { namespace { +// Service name to be used. const char kServiceName[] = R"( -name: "myfirebaseapp.appspot.com")"; +name: "myfirebaseapp.appspot.com" +)"; +// Bad service name that will result in bad release name const char kBadServiceName[] = R"( -name: "badService.appspot.com")"; +name: "badService.appspot.com" +)"; +// Producer project Id used to create the release URL const char kProducerProjectId[] = R"( producer_project_id: "myfirebaseapp" )"; +// APIs definition. const char kApis[] = R"( apis { name: "Bookstore" @@ -68,6 +74,7 @@ apis { } })"; +// Authentication part of service config const char kAuthentication[] = R"( authentication { providers { @@ -90,6 +97,7 @@ authentication { } })"; +// Http part of service config const char kHttp[] = R"( http { rules { @@ -101,14 +109,17 @@ control { environment: "http://127.0.0.1:8081" })"; +// Jwt payload to be used const char kJwtEmailPayload[] = R"({"iss":"https://accounts.google.com","iat":1486575396,"exp":1486578996,"aud":"https://myfirebaseapp.appspot.com","sub":"113424383671131376652","email_verified":true,"azp":"limin-429@appspot.gserviceaccount.com","email":"limin-429@appspot.gserviceaccount.com"})"; +// Firebase Server config static const char kServerConfig[] = R"( api_check_security_rules_config { firebase_server: "https://myfirebaseserver.com" })"; +// The response to GetRelease call to firebase server. static const char kRelease[] = R"( { "name": "projects/myfirebaseapp/releases/myfirebaseapp.appspot.com:v1", @@ -117,6 +128,7 @@ static const char kRelease[] = R"( "updateTime": "2017-01-10T16:52:27.764111Z" })"; +// Error response for GetRelease on bad release name static const char kReleaseError[] = R"( { "error": { @@ -126,6 +138,7 @@ static const char kReleaseError[] = R"( } })"; +// TestRuleset returns Failure which means unauthorized access. static const char kTestResultFailure[] = R"( { "testResults": [ @@ -136,6 +149,7 @@ static const char kTestResultFailure[] = R"( } )"; +// TestRuleset call to Firebase response on success. static const char kTestResultSuccess[] = R"( { "testResults": [ @@ -146,12 +160,14 @@ static const char kTestResultSuccess[] = R"( } )"; +// Get a server configuration that has auth disabled. This should disable +// security rules check by default. std::pair GetConfigWithAuthForceDisabled() { std::string service_config = std::string(kServiceName) + kApis + kAuthentication + kHttp; const char server_config[] = R"( api_authentication_config { -force_disable: true + force_disable: true } api_check_security_rules_config { firebase_server: "https://myfirebaseserver.com" @@ -160,11 +176,15 @@ api_check_security_rules_config { return std::make_pair(service_config, server_config); } +// Get service configuration with no authentication member field. This will +// disable auth and will also disable security rules check. std::pair GetConfigWithNoAuth() { std::string service_config = std::string(kServiceName) + kApis + kHttp; return std::make_pair(service_config, std::string(kServerConfig)); } +// Get Service configuration with no apis. This will result in the version field +// no present and should disable security rules check. std::pair GetConfigWithoutApis() { std::string service_config = std::string(kServiceName) + kAuthentication + kHttp; @@ -172,32 +192,36 @@ std::pair GetConfigWithoutApis() { return std::make_pair(service_config, std::string(kServerConfig)); } +// There is no firebase server configuration. std::pair GetConfigWithoutServer() { std::string service_config = std::string(kServiceName) + kApis + kAuthentication + kHttp; return std::make_pair(service_config, ""); } +// Get a valid configuration. This will enable security check rules. std::pair GetValidConfig() { std::string service_config = std::string(kServiceName) + kApis + kAuthentication + kHttp; return std::make_pair(service_config, kServerConfig); } +// This test class is parameterized and creates Config object based on the +// service and server configuration provided. class CheckDisableSecurityRulesTest : public ::testing::TestWithParam> { public: void SetUp() { std::unique_ptr env( new ::testing::NiceMock()); - raw_env_ = env.get(); + MockApiManagerEnvironment *raw_env = env.get(); std::string service_config; std::string server_config; std::tie(service_config, server_config) = GetParam(); std::unique_ptr config = - Config::Create(raw_env_, service_config, server_config); + Config::Create(raw_env, service_config, server_config); ASSERT_TRUE(config != nullptr); @@ -208,30 +232,31 @@ class CheckDisableSecurityRulesTest std::unique_ptr request( new ::testing::NiceMock()); - raw_request = request.get(); request_context_ = std::make_shared( service_context_, std::move(request)); - EXPECT_CALL(*raw_env_, DoRunHTTPRequest(_)).Times(0); + EXPECT_CALL(*raw_env, DoRunHTTPRequest(_)).Times(0); } - MockApiManagerEnvironment *raw_env_; - MockRequest *raw_request; std::shared_ptr request_context_; std::shared_ptr service_context_; }; +// Paramterized test that will check for various configurations that will +// disable auth. TEST_P(CheckDisableSecurityRulesTest, CheckAuthzDisabled) { CheckSecurityRules(request_context_, [](Status status) { ASSERT_TRUE(status.ok()); }); } +// Invoke the tests on CheckDisableSecurityRulesTest with various parameters. INSTANTIATE_TEST_CASE_P(ConfigToDisableFirebaseRulesCheck, CheckDisableSecurityRulesTest, testing::Values(GetConfigWithNoAuth(), GetConfigWithoutApis(), GetConfigWithoutServer())); +// Class that sets up the required objects to test various scenarios. class CheckSecurityRulesTest : public ::testing::Test { public: void SetUp(std::string service_config, std::string server_config) { @@ -278,6 +303,10 @@ class CheckSecurityRulesTest : public ::testing::Test { std::string ruleset_test_url_; }; +// If the release name is bad, then check the following: +// 1. Ensure that GetRuleset request is inovked on bad release name. +// 2. In this case return Status with NOT_FOUND +// 3. Ensure that there are no mor HTTP calls made to firbase TestRuleset TEST_F(CheckSecurityRulesTest, CheckAuthzFailGetRelease) { std::string service_config = std::string(kBadServiceName) + kProducerProjectId + kApis + kAuthentication + @@ -305,12 +334,18 @@ TEST_F(CheckSecurityRulesTest, CheckAuthzFailGetRelease) { .Times(0); auto ptr = this; - CheckSecurityRules(request_context_, [ptr](Status status) { - ptr->raw_env_->LogInfo(status.ToString()); - ASSERT_TRUE(!status.ok()); - }); + CheckSecurityRules(request_context_, + [ptr](Status status) { ASSERT_TRUE(!status.ok()); }); } +// Check that the right status is returned when TestRuleset completes with an +// error. This is modelled as an internal error. +// 1. Ensure that GetRelease is invoked on the correct release url and correct +// method. +// 2. The mock will respond with the ruleset Id. +// 3. Ensure that TestRuleset is invoked on the the righ URl and method. +// 4. In this case, mock will return an INTERNAL ERROR. +// 5. Make sure that status is not OK in this case. TEST_F(CheckSecurityRulesTest, CheckAuthzFailTestRuleset) { std::string service_config = std::string(kServiceName) + kProducerProjectId + kApis + kAuthentication + kHttp; @@ -340,11 +375,18 @@ TEST_F(CheckSecurityRulesTest, CheckAuthzFailTestRuleset) { std::move(empty), std::move(body)); })); - CheckSecurityRules(request_context_, - [](Status status) { ASSERT_FALSE(status.ok()); }); + CheckSecurityRules(request_context_, [](Status status) { + ASSERT_TRUE(status.CanonicalCode() == Code::INTERNAL); + }); } -TEST_F(CheckSecurityRulesTest, CheckAutzFailWithTestResultFailure) { +// Check behavior when TestResultset return a "FAILURE" message. +// 1. Ensure GetRelease is invoked properly and in this case mock responds with +// the ruelset Id. +// 2. Ensure that the TestResultset is invoked correctly and respond wit ha +// Status::OK but with Failure body. +// 3. Asser that the final status returned is PERMISSION DENIED. +TEST_F(CheckSecurityRulesTest, CheckAuthzFailWithTestResultFailure) { std::string service_config = std::string(kServiceName) + kProducerProjectId + kApis + kAuthentication + kHttp; std::string server_config = kServerConfig; @@ -377,6 +419,12 @@ TEST_F(CheckSecurityRulesTest, CheckAutzFailWithTestResultFailure) { }); } +// Check for success case. +// 1. Ensure GetRelease is invoked properly and in this case mock responds with +// the ruelset Id. +// 2. Ensure that the TestResultset is invoked correctly and respond wit ha +// Status::OK but with SUCCESS body. +// 3. Asser that the final status returned is OK. TEST_F(CheckSecurityRulesTest, CheckAuthzSuccess) { std::string service_config = std::string(kServiceName) + kProducerProjectId + kApis + kAuthentication + kHttp; From 92f606704b5b2b0e3c17a33e1271c19aaf5d7a97 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Fri, 10 Feb 2017 12:47:20 -0800 Subject: [PATCH 4/4] Fix typos --- .../endpoints/src/api_manager/check_security_rules_test.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) 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 f9a90567a49..97c555bc4df 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules_test.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules_test.cc @@ -184,7 +184,7 @@ std::pair GetConfigWithNoAuth() { } // Get Service configuration with no apis. This will result in the version field -// no present and should disable security rules check. +// not present and should disable security rules check. std::pair GetConfigWithoutApis() { std::string service_config = std::string(kServiceName) + kAuthentication + kHttp; @@ -306,7 +306,7 @@ class CheckSecurityRulesTest : public ::testing::Test { // If the release name is bad, then check the following: // 1. Ensure that GetRuleset request is inovked on bad release name. // 2. In this case return Status with NOT_FOUND -// 3. Ensure that there are no mor HTTP calls made to firbase TestRuleset +// 3. Ensure that there are no more HTTP calls made to firbase TestRuleset TEST_F(CheckSecurityRulesTest, CheckAuthzFailGetRelease) { std::string service_config = std::string(kBadServiceName) + kProducerProjectId + kApis + kAuthentication +