From 348f06120412fa2e2b14a1fa92245f2e855e6794 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 26 Jan 2017 15:23:04 -0800 Subject: [PATCH 01/21] Enable ESP to invoke Firebase Security rules. --- contrib/endpoints/src/api_manager/auth.h | 2 + .../auth/lib/auth_jwt_validator.cc | 5 +- .../api_manager/auth/service_account_token.h | 1 + .../endpoints/src/api_manager/check_auth.cc | 4 + .../src/api_manager/check_security_rules.cc | 275 +++++++++++++++--- .../src/api_manager/check_security_rules.h | 74 ++++- .../api_manager/context/request_context.cc | 15 + .../src/api_manager/context/request_context.h | 15 + .../api_manager/context/service_context.cc | 6 + 9 files changed, 347 insertions(+), 50 deletions(-) diff --git a/contrib/endpoints/src/api_manager/auth.h b/contrib/endpoints/src/api_manager/auth.h index aff735bc1aa..cf4cc270616 100644 --- a/contrib/endpoints/src/api_manager/auth.h +++ b/contrib/endpoints/src/api_manager/auth.h @@ -40,6 +40,8 @@ struct UserInfo { // Authorized party of the incoming JWT. // See http://openid.net/specs/openid-connect-core-1_0.html#IDToken std::string authorized_party; + // String of claims + char *claims; // Returns audiences as a comma separated strings. std::string AudiencesAsString() const { diff --git a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc index 8da6ef2166c..f51c1c9be0a 100644 --- a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc +++ b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc @@ -699,12 +699,15 @@ grpc_jwt_verifier_status JwtValidatorImpl::FillUserInfoAndSetExp( // Optional field. const grpc_json *grpc_json = grpc_jwt_claims_json(claims_); + + char *json_str = grpc_json_dump_to_string(const_cast<::grpc_json *>(grpc_json), 0); + user_info->claims = json_str; + const char *email = GetStringValue(grpc_json, "email"); user_info->email = email == nullptr ? "" : email; const char *authorized_party = GetStringValue(grpc_json, "azp"); user_info->authorized_party = authorized_party == nullptr ? "" : authorized_party; - exp_ = system_clock::from_time_t(grpc_jwt_claims_expires_at(claims_).tv_sec); return GRPC_JWT_VERIFIER_OK; diff --git a/contrib/endpoints/src/api_manager/auth/service_account_token.h b/contrib/endpoints/src/api_manager/auth/service_account_token.h index 211377449f4..51e99c65b89 100644 --- a/contrib/endpoints/src/api_manager/auth/service_account_token.h +++ b/contrib/endpoints/src/api_manager/auth/service_account_token.h @@ -64,6 +64,7 @@ class ServiceAccountToken { enum JWT_TOKEN_TYPE { JWT_TOKEN_FOR_SERVICE_CONTROL = 0, JWT_TOKEN_FOR_CLOUD_TRACING, + JWT_TOKEN_FOR_FIREBASE, JWT_TOKEN_TYPE_MAX, }; // Set audience. Only calcualtes JWT token with specified audience. diff --git a/contrib/endpoints/src/api_manager/check_auth.cc b/contrib/endpoints/src/api_manager/check_auth.cc index ce9a2c159ff..7b6238ac2bb 100644 --- a/contrib/endpoints/src/api_manager/check_auth.cc +++ b/contrib/endpoints/src/api_manager/check_auth.cc @@ -213,7 +213,9 @@ void AuthChecker::LookupJwtCache() { if (cache_hit) { CheckAudience(true); } else { + env_->LogInfo("Before Parse JWT"); ParseJwt(); + env_->LogInfo("After Parse JWT"); } } @@ -243,6 +245,8 @@ void AuthChecker::CheckAudience(bool cache_hit) { context_->set_auth_audience(audience); context_->set_auth_authorized_party(user_info_.authorized_party); + context_->set_auth_claims(user_info_.claims); + // Remove http/s header and trailing '/' for issuer. std::string issuer = utils::GetUrlContent(user_info_.issuer); if (!context_->method()->isIssuerAllowed(issuer)) { diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index b1d6b90ac36..41f696fe483 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -14,92 +14,271 @@ // //////////////////////////////////////////////////////////////////////////////// #include "contrib/endpoints/src/api_manager/check_security_rules.h" +#include "contrib/endpoints/src/api_manager/auth/lib/json_util.h" +#include +#include -#include - -#include "contrib/endpoints/include/api_manager/api_manager.h" -#include "contrib/endpoints/include/api_manager/request.h" - +using ::google::api_manager::auth::GetProperty; +using ::google::api_manager::auth::GetStringValue; using ::google::api_manager::utils::Status; +using ::google::protobuf::util::error::Code; namespace google { namespace api_manager { -namespace { - const char kFirebaseServerStaging[] = "https://staging-firebaserules.sandbox.googleapis.com/"; -// An AuthzChecker object is created for every incoming request. It does -// authorizaiton by calling Firebase Rules service. -class AuthzChecker : public std::enable_shared_from_this { - public: - AuthzChecker(std::shared_ptr context, - std::function continuation); +const char kFirebaseService[] = + "/google.firebase.rules.v1.FirebaseRulesService"; + +const char kFailedFirebaseReleaseFetch[] = "Failed to fetch Firebase Release"; +const char kFailedFirebaseTest[] = "Failed to execute Firebase Test"; +const char kInvalidResponse[] = "Invalid JSON response from Firebase Service"; + +AuthzChecker::AuthzChecker(ApiManagerEnvInterface *env, + auth::ServiceAccountToken *sa_token) + : env_(env), + sa_token_(sa_token) {} + +void AuthzChecker::Check( + std::shared_ptr context, + std::function final_continuation) { + // TODO: Check service config to see if "useSecurityRules" is specified. + // If so, call Firebase Rules service TestRuleset API. - void Check(); + if (!context->service_context()->RequireAuth() || + context->method() == nullptr || !context->method()->auth()) { + env_->LogDebug( + std::string("Autherization and JWT validation was not performed") + + " skipping authorization"); + final_continuation(Status::OK); + return; + } - private: - // Helper function to send a http GET request. - void HttpFetch(const std::string &url, const std::string &request_body, - std::function continuation); + // Fetch the Release attributes. + auto pChecker = GetPtr(); + HttpFetch(GetReleaseUrl(context), std::string("GET"), std::string(""), + [context, final_continuation, pChecker] (Status status, + std::string &&body) { + std::string ruleset_id; + if (status.ok()) { + pChecker->env_->LogDebug( + std::string("GetReleasName succeeded with ") + body); + std::tie(status, ruleset_id) = pChecker->ParseReleaseResponse(&body); + } else { + pChecker->env_->LogError(std::string("GetReleaseName for ") + + pChecker->GetReleaseUrl(context) + + " with status " + status.ToString()); + status = Status(Code::INTERNAL, kFailedFirebaseReleaseFetch); + } - // Get Auth token for accessing Firebase Rules service. - const std::string &GetAuthToken(); + // If the parsing of the release body is successful, then call the + // Test Api for firebase rules service. + if (status.ok()) { + pChecker->CheckAuth(ruleset_id, context, final_continuation); + } else { + final_continuation(status); + } + }); +} - // Request context. - std::shared_ptr context_; +void AuthzChecker::CheckAuth( + std::string ruleset_id, + std::shared_ptr context, + std::function continuation) { + auto pChecker = GetPtr(); - // Pointer to access ESP running environment. - ApiManagerEnvInterface *env_; + HttpFetch(std::string(kFirebaseServerStaging) + "v1/" + ruleset_id + + ":test?alt=json", + std::string("POST"), BuildTestRequestBody(context), + [context, continuation, pChecker, ruleset_id] + (Status status, std::string &&body) { - // The final continuation function. - std::function on_done_; -}; + if (status.ok()) { + pChecker->env_->LogDebug( + std::string("Test API succeeded with ") + body); + status = pChecker->ParseTestResponse(context, &body); + } else { + pChecker->env_->LogError(std::string("Test API failed with ") + + status.ToString()); + status = Status(Code::INTERNAL, kFailedFirebaseTest); + } -AuthzChecker::AuthzChecker(std::shared_ptr context, - std::function continuation) - : context_(context), - env_(context_->service_context()->env()), - on_done_(continuation) {} + continuation(status); + }); +} -void AuthzChecker::Check() { - // TODO: Check service config to see if "useSecurityRules" is specified. - // If so, call Firebase Rules service TestRuleset API. +std::pair AuthzChecker::ParseReleaseResponse( + std::string *json_str) { + grpc_json *json = grpc_json_parse_string_with_len( + const_cast(json_str->data()), json_str->length()); + + if (!json) { + return std::make_pair(Status(Code::INVALID_ARGUMENT, kInvalidResponse), + ""); + } + + const char *ruleset_id = GetStringValue(json, "rulesetName"); + std::pair result = std::make_pair(Status::OK, + ruleset_id); + if (ruleset_id == nullptr) { + env_->LogError("Empty ruleset Id received from firebase service"); + result =std::make_pair(Status(Code::INTERNAL, kInvalidResponse), ""); + } else { + env_->LogInfo(std::string("Received ruleset Id: ") + ruleset_id); + } + + grpc_json_destroy(json); + return result; +} + +Status AuthzChecker::ParseTestResponse( + std::shared_ptr context, + std::string *json_str) { + grpc_json *json = grpc_json_parse_string_with_len( + const_cast(json_str->data()), json_str->length()); + + + if (!json) { + return Status(Code::INVALID_ARGUMENT, + "Invalid JSON response from Firebase Service"); + } + + Status status = Status::OK; + Status invalid = Status(Code::INTERNAL, kInvalidResponse); + + const grpc_json *testResults = GetProperty(json, "testResults"); + if (testResults == nullptr) { + env_->LogError("TestResults are null"); + status = invalid; + } else { + const char *result = GetStringValue(testResults->child, "state"); + if (result == nullptr) { + env_->LogInfo("Result state is empty"); + status = invalid; + } else if (std::string(result) != "SUCCESS") { + status = Status(Code::PERMISSION_DENIED, + std::string("Unauthorized ") + + context->request()->GetRequestHTTPMethod() + + " access to resource " + context->request()->GetRequestPath(), + Status::AUTH); + } + } + + grpc_json_destroy(json); + return status; } -const std::string &AuthzChecker::GetAuthToken() { - // TODO: Get Auth token for accessing Firebase Rules service. - static std::string empty; - return empty; +std::string AuthzChecker::GetOperation(std::string httpMethod) { + if (httpMethod == "POST") { + return std::string("create"); + } + + if (httpMethod == "GET" || httpMethod == "HEAD" || httpMethod == "OPTIONS") { + return std::string("get"); + } + + if (httpMethod == "DELETE") { + return std::string("delete"); + } + + return std::string("update"); +} + +std::string AuthzChecker::BuildTestRequestBody( + std::shared_ptr context) { + + std::shared_ptr ss(new std::ostringstream); + + int openCount = 0; + *(ss.get()) << "{" + << Stringify("test_cases") + ": " + << "[ {"; + ++openCount; + AddToBody("service_name", context->service_context()->service_name(), false, ss); + AddToBody("resource_path", context->request()->GetRequestPath(), + false, ss); + AddToBody("operation", GetOperation(context->request()->GetRequestHTTPMethod()), + false, ss); + AddToBody("expectation", "ALLOW", false, ss); + AddToBody("variables", ss); + ++openCount; + AddToBody("request", ss); + ++openCount; + AddToBody("auth", ss); + ++openCount; + + *(ss.get()) << Stringify("token") + ": " << context->auth_claims(); + + while(openCount-- > 0) { + *(ss.get()) << "} "; + } + + *(ss.get()) << "] }"; + return ss->str(); +} + +void AuthzChecker::AddToBody(const std::string &key, + std::shared_ptr ss) { + *(ss.get()) << Stringify(key.c_str()) + ": " + "{"; +} +void AuthzChecker::AddToBody(const std::string &key, const std::string &value, + bool end, std::shared_ptr ss) { + *(ss.get()) << Stringify(key.c_str()) + ": " + Stringify(value.c_str()); + if (!end) { + *(ss.get()) << ", "; + } +} + +const std::string AuthzChecker::GetReleaseName( + std::shared_ptr request_context) { + return request_context->service_context()->service_name() + ":" + + request_context->service_context()->service().apis(0).version(); +} + +const std::string AuthzChecker::GetReleaseUrl( + std::shared_ptr request_context) { + return std::string(kFirebaseServerStaging) + "v1/projects/" + + request_context->service_context()->project_id() + "/releases/" + + GetReleaseName(request_context); } void AuthzChecker::HttpFetch( - const std::string &url, const std::string &request_body, + const std::string &url, const std::string &method, + const std::string &request_body, std::function continuation) { + + env_->LogInfo(std::string("Issue HTTP Request to url :") + url + + " method : " + method + " body: " + request_body); + std::unique_ptr request(new HTTPRequest([continuation]( Status status, std::map &&, std::string &&body) { continuation(status, std::move(body)); })); + if (!request) { continuation(Status(Code::INTERNAL, "Out of memory"), ""); return; } - request->set_method("POST") + std::string token = GetAuthToken(); + request->set_method(method) .set_url(url) - .set_auth_token(GetAuthToken()) - .set_header("Content-Type", "application/json") + .set_auth_token(token); + if (method != "GET") { + request->set_header("Content-Type", "application/json") .set_body(request_body); + } + env_->RunHTTPRequest(std::move(request)); } -} // namespace - void CheckSecurityRules(std::shared_ptr context, std::function continuation) { - std::shared_ptr authzChecker = - std::make_shared(context, continuation); - authzChecker->Check(); + std::shared_ptr checker(new AuthzChecker( + context->service_context()->env(), + context->service_context()->service_account_token())); + checker->Check(context, continuation); } } // namespace api_manager diff --git a/contrib/endpoints/src/api_manager/check_security_rules.h b/contrib/endpoints/src/api_manager/check_security_rules.h index bc971c48786..70fdc1fee21 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.h +++ b/contrib/endpoints/src/api_manager/check_security_rules.h @@ -15,12 +15,84 @@ #ifndef API_MANAGER_CHECK_SECURITY_RULES_H_ #define API_MANAGER_CHECK_SECURITY_RULES_H_ -#include "contrib/endpoints/include/api_manager/utils/status.h" +#include "contrib/endpoints/include/api_manager/api_manager.h" +#include "contrib/endpoints/src/api_manager/auth/service_account_token.h" #include "contrib/endpoints/src/api_manager/context/request_context.h" +#include "contrib/endpoints/include/api_manager/utils/status.h" + +#include +using ::google::api_manager::utils::Status; namespace google { namespace api_manager { +class AuthzChecker : public std::enable_shared_from_this { + public: + // Constructor + AuthzChecker(ApiManagerEnvInterface *env, + auth::ServiceAccountToken *sa_token); + + // Check for Authorization success or failure + void Check(std::shared_ptr context, + std::function continuation); + + private: + + // Helper method that invokes the test firebase service api. + void CheckAuth(std::string ruleset_id, + std::shared_ptr context, + std::function continuation); + + // Parse the respose for GET RELEASE API call + std::pair ParseReleaseResponse(std::string *json_str); + + // Parses the response for the TEST API call + Status ParseTestResponse(std::shared_ptr context, + std::string *json_str); + + // Builds the request body for the TESP API call. + std::string BuildTestRequestBody( + std::shared_ptr context); + + void AddToBody(const std::string &key, + std::shared_ptr ss); + + void AddToBody(const std::string &key, const std::string &value, + bool end, std::shared_ptr ss); + + // Get the release name to used in Firebase API call + const std::string GetReleaseName( + std::shared_ptr request_context); + + // Get the URL for the Release request. + const std::string GetReleaseUrl( + std::shared_ptr request_context); + + // Invoke the HTTP call + void HttpFetch(const std::string &url, const std::string &method, + const std::string &request_body, + std::function continuation); + + + // Get the auth token for Firebase service + const std::string &GetAuthToken() { + return sa_token_->GetAuthToken( + auth::ServiceAccountToken::JWT_TOKEN_FOR_FIREBASE); + } + + // Get Firebase specific operation Id based on the http Method. + std::string GetOperation(std::string httpMethod); + + const std::string Stringify(const char *s) { + return std::string("\"") + s + "\""; + } + + std::shared_ptr GetPtr() { return shared_from_this(); } + + ApiManagerEnvInterface *env_; + auth::ServiceAccountToken *sa_token_; +}; + // This function checks security rules for a given request. // It is called by CheckWorkflow class when processing a request. void CheckSecurityRules(std::shared_ptr context, diff --git a/contrib/endpoints/src/api_manager/context/request_context.cc b/contrib/endpoints/src/api_manager/context/request_context.cc index cbf4e925e37..4ac9489c35e 100644 --- a/contrib/endpoints/src/api_manager/context/request_context.cc +++ b/contrib/endpoints/src/api_manager/context/request_context.cc @@ -20,6 +20,14 @@ #include #include +extern "C" { +#include "grpc/grpc.h" +#include "grpc/support/alloc.h" +#include "grpc/support/log.h" +#include "grpc/support/string_util.h" +#include "grpc/support/sync.h" +} + using ::google::api_manager::utils::Status; namespace google { @@ -71,6 +79,7 @@ RequestContext::RequestContext(std::shared_ptr service_context, std::unique_ptr request) : service_context_(service_context), request_(std::move(request)), + auth_claims_(nullptr), is_first_report_(true) { start_time_ = std::chrono::system_clock::now(); last_report_time_ = std::chrono::steady_clock::now(); @@ -114,6 +123,12 @@ RequestContext::RequestContext(std::shared_ptr service_context, } } +RequestContext::~RequestContext() { + if (auth_claims_) { + gpr_free(auth_claims_); + } +} + void RequestContext::ExtractApiKey() { bool api_key_defined = false; auto url_queries = method()->api_key_url_query_parameters(); diff --git a/contrib/endpoints/src/api_manager/context/request_context.h b/contrib/endpoints/src/api_manager/context/request_context.h index 5b29c271aad..d94baf18652 100644 --- a/contrib/endpoints/src/api_manager/context/request_context.h +++ b/contrib/endpoints/src/api_manager/context/request_context.h @@ -36,6 +36,8 @@ class RequestContext { RequestContext(std::shared_ptr service_context, std::unique_ptr request); + ~RequestContext(); + // Get the ApiManagerImpl object. context::ServiceContext *service_context() { return service_context_.get(); } @@ -112,6 +114,16 @@ class RequestContext { last_report_time_ = tp; } + void set_auth_claims(char *claims) { + auth_claims_ = claims; + } + + char *auth_claims() { + return auth_claims_; + } + + const std::string GetAuthToken(); + private: // Fill OperationInfo void FillOperationInfo(service_control::OperationInfo *info); @@ -163,6 +175,9 @@ class RequestContext { // Report(). std::string auth_authorized_party_; + // Auth Claims: This is the decoded payload of the JWT token + char *auth_claims_; + // Used by cloud tracing. std::unique_ptr cloud_trace_; diff --git a/contrib/endpoints/src/api_manager/context/service_context.cc b/contrib/endpoints/src/api_manager/context/service_context.cc index d8fc9dfc409..3ab7250a877 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.cc +++ b/contrib/endpoints/src/api_manager/context/service_context.cc @@ -44,6 +44,9 @@ const int kIntermediateReportInterval = 10; const char kHTTPHeadMethod[] = "HEAD"; const char kHTTPGetMethod[] = "GET"; + +const char kFirebaseAudience[] = + "https://staging-firebaserules.sandbox.googleapis.com/google.firebase.rules.v1.FirebaseRulesService"; } ServiceContext::ServiceContext(std::unique_ptr env, @@ -69,6 +72,9 @@ ServiceContext::ServiceContext(std::unique_ptr env, ->service_control_config() .intermediate_report_min_interval(); } + + service_account_token_.SetAudience( + auth::ServiceAccountToken::JWT_TOKEN_FOR_FIREBASE, kFirebaseAudience); } MethodCallInfo ServiceContext::GetMethodCallInfo( From 6339697fee9d79a1a7f5dc66608f984d4f0c5627 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Fri, 27 Jan 2017 11:42:01 -0800 Subject: [PATCH 02/21] Address code review comments. --- contrib/endpoints/src/api_manager/auth.h | 2 + .../auth/lib/auth_jwt_validator.cc | 5 +- .../api_manager/auth/service_account_token.h | 1 + .../endpoints/src/api_manager/check_auth.cc | 2 + .../src/api_manager/check_security_rules.cc | 328 +++++++++++++++--- .../src/api_manager/check_security_rules.h | 7 +- .../api_manager/context/request_context.cc | 1 + .../src/api_manager/context/request_context.h | 13 + .../api_manager/context/service_context.cc | 6 + 9 files changed, 322 insertions(+), 43 deletions(-) diff --git a/contrib/endpoints/src/api_manager/auth.h b/contrib/endpoints/src/api_manager/auth.h index aff735bc1aa..e69718e95f6 100644 --- a/contrib/endpoints/src/api_manager/auth.h +++ b/contrib/endpoints/src/api_manager/auth.h @@ -40,6 +40,8 @@ struct UserInfo { // Authorized party of the incoming JWT. // See http://openid.net/specs/openid-connect-core-1_0.html#IDToken std::string authorized_party; + // String of claims + std::string claims; // Returns audiences as a comma separated strings. std::string AudiencesAsString() const { diff --git a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc index 8da6ef2166c..fc73125b013 100644 --- a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc +++ b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc @@ -699,12 +699,15 @@ grpc_jwt_verifier_status JwtValidatorImpl::FillUserInfoAndSetExp( // Optional field. const grpc_json *grpc_json = grpc_jwt_claims_json(claims_); + + char *json_str = grpc_json_dump_to_string(const_cast<::grpc_json *>(grpc_json), 0); + user_info->claims = json_str == nullptr ? "" : json_str; + const char *email = GetStringValue(grpc_json, "email"); user_info->email = email == nullptr ? "" : email; const char *authorized_party = GetStringValue(grpc_json, "azp"); user_info->authorized_party = authorized_party == nullptr ? "" : authorized_party; - exp_ = system_clock::from_time_t(grpc_jwt_claims_expires_at(claims_).tv_sec); return GRPC_JWT_VERIFIER_OK; diff --git a/contrib/endpoints/src/api_manager/auth/service_account_token.h b/contrib/endpoints/src/api_manager/auth/service_account_token.h index 211377449f4..51e99c65b89 100644 --- a/contrib/endpoints/src/api_manager/auth/service_account_token.h +++ b/contrib/endpoints/src/api_manager/auth/service_account_token.h @@ -64,6 +64,7 @@ class ServiceAccountToken { enum JWT_TOKEN_TYPE { JWT_TOKEN_FOR_SERVICE_CONTROL = 0, JWT_TOKEN_FOR_CLOUD_TRACING, + JWT_TOKEN_FOR_FIREBASE, JWT_TOKEN_TYPE_MAX, }; // Set audience. Only calcualtes JWT token with specified audience. diff --git a/contrib/endpoints/src/api_manager/check_auth.cc b/contrib/endpoints/src/api_manager/check_auth.cc index ce9a2c159ff..b4aae8bb2a1 100644 --- a/contrib/endpoints/src/api_manager/check_auth.cc +++ b/contrib/endpoints/src/api_manager/check_auth.cc @@ -243,6 +243,8 @@ void AuthChecker::CheckAudience(bool cache_hit) { context_->set_auth_audience(audience); context_->set_auth_authorized_party(user_info_.authorized_party); + context_->set_auth_claims(user_info_.claims); + // Remove http/s header and trailing '/' for issuer. std::string issuer = utils::GetUrlContent(user_info_.issuer); if (!context_->method()->isIssuerAllowed(issuer)) { diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index b1d6b90ac36..2baa1704863 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -14,92 +14,338 @@ // //////////////////////////////////////////////////////////////////////////////// #include "contrib/endpoints/src/api_manager/check_security_rules.h" +#include "contrib/endpoints/src/api_manager/auth/lib/json_util.h" +#include +#include -#include - -#include "contrib/endpoints/include/api_manager/api_manager.h" -#include "contrib/endpoints/include/api_manager/request.h" - +using ::google::api_manager::auth::GetProperty; +using ::google::api_manager::auth::GetStringValue; using ::google::api_manager::utils::Status; +using ::google::protobuf::util::error::Code; namespace google { namespace api_manager { -namespace { - const char kFirebaseServerStaging[] = "https://staging-firebaserules.sandbox.googleapis.com/"; -// An AuthzChecker object is created for every incoming request. It does -// authorizaiton by calling Firebase Rules service. +const char kFirebaseService[] = + "/google.firebase.rules.v1.FirebaseRulesService"; + +const char kFailedFirebaseReleaseFetch[] = "Failed to fetch Firebase Release"; +const char kFailedFirebaseTest[] = "Failed to execute Firebase Test"; +const char kInvalidResponse[] = "Invalid JSON response from Firebase Service"; + class AuthzChecker : public std::enable_shared_from_this { public: - AuthzChecker(std::shared_ptr context, - std::function continuation); + // Constructor + AuthzChecker(ApiManagerEnvInterface *env, + auth::ServiceAccountToken *sa_token); - void Check(); + // Check for Authorization success or failure + void Check(std::shared_ptr context, + std::function continuation); private: - // Helper function to send a http GET request. - void HttpFetch(const std::string &url, const std::string &request_body, + + // Helper method that invokes the test firebase service api. + void FirebaseCheck(std::string ruleset_id, + std::shared_ptr context, + std::function continuation); + + // Parse the respose for GET RELEASE API call + Status ParseReleaseResponse(const std::string &json_str, + std::string *ruleset_id); + + // Parses the response for the TEST API call + Status ParseTestResponse(std::shared_ptr context, + const std::string &json_str); + + // Builds the request body for the TESP API call. + std::string BuildTestRequestBody( + std::shared_ptr context); + + void AddToBody(const std::string &key, + std::ostringstream &ss); + + void AddToBody(const std::string &key, const std::string &value, + bool end, std::ostringstream &ss); + + // Get the release name to used in Firebase API call + const std::string GetReleaseName( + std::shared_ptr request_context); + + // Get the URL for the Release request. + const std::string GetReleaseUrl( + std::shared_ptr request_context); + + // Invoke the HTTP call + void HttpFetch(const std::string &url, const std::string &method, + const std::string &request_body, std::function continuation); - // Get Auth token for accessing Firebase Rules service. - const std::string &GetAuthToken(); - // Request context. - std::shared_ptr context_; + // Get the auth token for Firebase service + const std::string &GetAuthToken() { + return sa_token_->GetAuthToken( + auth::ServiceAccountToken::JWT_TOKEN_FOR_FIREBASE); + } - // Pointer to access ESP running environment. - ApiManagerEnvInterface *env_; + // Get Firebase specific operation Id based on the http Method. + std::string GetOperation(std::string httpMethod); + + const std::string Stringify(const char *s) { + return std::string("\"") + (s == nullptr ? "" : s) + "\""; + } + + std::shared_ptr GetPtr() { return shared_from_this(); } - // The final continuation function. - std::function on_done_; + ApiManagerEnvInterface *env_; + auth::ServiceAccountToken *sa_token_; }; -AuthzChecker::AuthzChecker(std::shared_ptr context, - std::function continuation) - : context_(context), - env_(context_->service_context()->env()), - on_done_(continuation) {} +AuthzChecker::AuthzChecker(ApiManagerEnvInterface *env, + auth::ServiceAccountToken *sa_token) + : env_(env), + sa_token_(sa_token) {} -void AuthzChecker::Check() { +void AuthzChecker::Check( + std::shared_ptr context, + std::function final_continuation) { // TODO: Check service config to see if "useSecurityRules" is specified. // If so, call Firebase Rules service TestRuleset API. + + if (!context->service_context()->RequireAuth() || + context->method() == nullptr || !context->method()->auth()) { + env_->LogDebug( + std::string("Autherization and JWT validation was not performed") + + " skipping authorization"); + final_continuation(Status::OK); + return; + } + + // Fetch the Release attributes. + auto pchecker = GetPtr(); + HttpFetch(GetReleaseUrl(context), std::string("GET"), std::string(""), + [context, final_continuation, pchecker] (Status status, + std::string &&body) { + std::string ruleset_id; + if (status.ok()) { + pchecker->env_->LogDebug( + std::string("GetReleasName succeeded with ") + body); + status = pchecker->ParseReleaseResponse(body, &ruleset_id); + } else { + pchecker->env_->LogError(std::string("GetReleaseName for ") + + pchecker->GetReleaseUrl(context) + + " with status " + status.ToString()); + status = Status(Code::INTERNAL, kFailedFirebaseReleaseFetch); + } + + // If the parsing of the release body is successful, then call the + // Test Api for firebase rules service. + if (status.ok()) { + pchecker->FirebaseCheck(ruleset_id, context, final_continuation); + } else { + final_continuation(status); + } + }); } -const std::string &AuthzChecker::GetAuthToken() { - // TODO: Get Auth token for accessing Firebase Rules service. - static std::string empty; - return empty; +void AuthzChecker::FirebaseCheck( + std::string ruleset_id, + std::shared_ptr context, + std::function continuation) { + auto pchecker = GetPtr(); + + HttpFetch(std::string(kFirebaseServerStaging) + "v1/" + ruleset_id + + ":test?alt=json", + std::string("POST"), BuildTestRequestBody(context), + [context, continuation, pchecker, ruleset_id] + (Status status, std::string &&body) { + + if (status.ok()) { + pchecker->env_->LogDebug( + std::string("Test API succeeded with ") + body); + status = pchecker->ParseTestResponse(context, body); + } else { + pchecker->env_->LogError(std::string("Test API failed with ") + + status.ToString()); + status = Status(Code::INTERNAL, kFailedFirebaseTest); + } + + continuation(status); + }); +} + +Status AuthzChecker::ParseReleaseResponse(const std::string &json_str, + std::string *ruleset_id) { + grpc_json *json = grpc_json_parse_string_with_len( + const_cast(json_str.data()), json_str.length()); + + if (!json) { + return Status(Code::INVALID_ARGUMENT, kInvalidResponse); + } + + Status status = Status::OK; + const char *id = GetStringValue(json, "rulesetName"); + (*ruleset_id) = (id == nullptr) ? "" : GetStringValue(json, "rulesetName"); + + if (ruleset_id->empty()) { + env_->LogError("Empty ruleset Id received from firebase service"); + status = Status(Code::INTERNAL, kInvalidResponse); + } else { + env_->LogInfo(std::string("Received ruleset Id: ") + *ruleset_id); + } + + grpc_json_destroy(json); + return status; +} + +Status AuthzChecker::ParseTestResponse( + std::shared_ptr context, + const std::string &json_str) { + grpc_json *json = grpc_json_parse_string_with_len( + const_cast(json_str.data()), json_str.length()); + + + if (!json) { + return Status(Code::INVALID_ARGUMENT, + "Invalid JSON response from Firebase Service"); + } + + Status status = Status::OK; + Status invalid = Status(Code::INTERNAL, kInvalidResponse); + + const grpc_json *testResults = GetProperty(json, "testResults"); + if (testResults == nullptr) { + env_->LogError("TestResults are null"); + status = invalid; + } else { + const char *result = GetStringValue(testResults->child, "state"); + if (result == nullptr) { + env_->LogInfo("Result state is empty"); + status = invalid; + } else if (std::string(result) != "SUCCESS") { + status = Status(Code::PERMISSION_DENIED, + std::string("Unauthorized ") + + context->request()->GetRequestHTTPMethod() + + " access to resource " + context->request()->GetRequestPath(), + Status::AUTH); + } + } + + grpc_json_destroy(json); + return status; +} + +std::string AuthzChecker::GetOperation(std::string httpMethod) { + if (httpMethod == "POST") { + return std::string("create"); + } + + if (httpMethod == "GET" || httpMethod == "HEAD" || httpMethod == "OPTIONS") { + return std::string("get"); + } + + if (httpMethod == "DELETE") { + return std::string("delete"); + } + + return std::string("update"); +} + +std::string AuthzChecker::BuildTestRequestBody( + std::shared_ptr context) { + + std::ostringstream ss; + + int openCount = 0; + ss << "{" + << Stringify("test_cases") + ": " + << "[ {"; + ++openCount; + AddToBody("service_name", context->service_context()->service_name(), false, ss); + AddToBody("resource_path", context->request()->GetRequestPath(), + false, ss); + AddToBody("operation", GetOperation(context->request()->GetRequestHTTPMethod()), + false, ss); + AddToBody("expectation", "ALLOW", false, ss); + AddToBody("variables", ss); + ++openCount; + AddToBody("request", ss); + ++openCount; + AddToBody("auth", ss); + ++openCount; + + ss << Stringify("token") + ": " << context->auth_claims(); + + while(openCount-- > 0) { + ss << "} "; + } + + ss << "] }"; + return ss.str(); +} + +void AuthzChecker::AddToBody(const std::string &key, + std::ostringstream &ss) { + ss << Stringify(key.c_str()) + ": " + "{"; +} +void AuthzChecker::AddToBody(const std::string &key, const std::string &value, + bool end, std::ostringstream &ss) { + ss << Stringify(key.c_str()) + ": " + Stringify(value.c_str()); + if (!end) { + ss << ", "; + } +} + +const std::string AuthzChecker::GetReleaseName( + std::shared_ptr request_context) { + return request_context->service_context()->service_name() + ":" + + request_context->service_context()->service().apis(0).version(); +} + +const std::string AuthzChecker::GetReleaseUrl( + std::shared_ptr request_context) { + return std::string(kFirebaseServerStaging) + "v1/projects/" + + request_context->service_context()->project_id() + "/releases/" + + GetReleaseName(request_context); } void AuthzChecker::HttpFetch( - const std::string &url, const std::string &request_body, + const std::string &url, const std::string &method, + const std::string &request_body, std::function continuation) { + + env_->LogInfo(std::string("Issue HTTP Request to url :") + url + + " method : " + method + " body: " + request_body); + std::unique_ptr request(new HTTPRequest([continuation]( Status status, std::map &&, std::string &&body) { continuation(status, std::move(body)); })); + if (!request) { continuation(Status(Code::INTERNAL, "Out of memory"), ""); return; } - request->set_method("POST") + request->set_method(method) .set_url(url) - .set_auth_token(GetAuthToken()) - .set_header("Content-Type", "application/json") + .set_auth_token(GetAuthToken()); + if (method != "GET") { + request->set_header("Content-Type", "application/json") .set_body(request_body); + } + env_->RunHTTPRequest(std::move(request)); } -} // namespace - void CheckSecurityRules(std::shared_ptr context, std::function continuation) { - std::shared_ptr authzChecker = - std::make_shared(context, continuation); - authzChecker->Check(); + std::shared_ptr checker = std::make_shared( + context->service_context()->env(), + context->service_context()->service_account_token()); + checker->Check(context, continuation); } } // namespace api_manager diff --git a/contrib/endpoints/src/api_manager/check_security_rules.h b/contrib/endpoints/src/api_manager/check_security_rules.h index bc971c48786..88820f6f47b 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.h +++ b/contrib/endpoints/src/api_manager/check_security_rules.h @@ -15,8 +15,13 @@ #ifndef API_MANAGER_CHECK_SECURITY_RULES_H_ #define API_MANAGER_CHECK_SECURITY_RULES_H_ -#include "contrib/endpoints/include/api_manager/utils/status.h" +#include "contrib/endpoints/include/api_manager/api_manager.h" +#include "contrib/endpoints/src/api_manager/auth/service_account_token.h" #include "contrib/endpoints/src/api_manager/context/request_context.h" +#include "contrib/endpoints/include/api_manager/utils/status.h" + +#include +using ::google::api_manager::utils::Status; namespace google { namespace api_manager { diff --git a/contrib/endpoints/src/api_manager/context/request_context.cc b/contrib/endpoints/src/api_manager/context/request_context.cc index cbf4e925e37..8a66338afb6 100644 --- a/contrib/endpoints/src/api_manager/context/request_context.cc +++ b/contrib/endpoints/src/api_manager/context/request_context.cc @@ -71,6 +71,7 @@ RequestContext::RequestContext(std::shared_ptr service_context, std::unique_ptr request) : service_context_(service_context), request_(std::move(request)), + auth_claims_(nullptr), is_first_report_(true) { start_time_ = std::chrono::system_clock::now(); last_report_time_ = std::chrono::steady_clock::now(); diff --git a/contrib/endpoints/src/api_manager/context/request_context.h b/contrib/endpoints/src/api_manager/context/request_context.h index 5b29c271aad..b99b52ccfa4 100644 --- a/contrib/endpoints/src/api_manager/context/request_context.h +++ b/contrib/endpoints/src/api_manager/context/request_context.h @@ -112,6 +112,16 @@ class RequestContext { last_report_time_ = tp; } + void set_auth_claims(std::string &claims) { + auth_claims_ = claims; + } + + std::string &auth_claims() { + return auth_claims_; + } + + const std::string GetAuthToken(); + private: // Fill OperationInfo void FillOperationInfo(service_control::OperationInfo *info); @@ -163,6 +173,9 @@ class RequestContext { // Report(). std::string auth_authorized_party_; + // Auth Claims: This is the decoded payload of the JWT token + std::string auth_claims_; + // Used by cloud tracing. std::unique_ptr cloud_trace_; diff --git a/contrib/endpoints/src/api_manager/context/service_context.cc b/contrib/endpoints/src/api_manager/context/service_context.cc index d8fc9dfc409..3ab7250a877 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.cc +++ b/contrib/endpoints/src/api_manager/context/service_context.cc @@ -44,6 +44,9 @@ const int kIntermediateReportInterval = 10; const char kHTTPHeadMethod[] = "HEAD"; const char kHTTPGetMethod[] = "GET"; + +const char kFirebaseAudience[] = + "https://staging-firebaserules.sandbox.googleapis.com/google.firebase.rules.v1.FirebaseRulesService"; } ServiceContext::ServiceContext(std::unique_ptr env, @@ -69,6 +72,9 @@ ServiceContext::ServiceContext(std::unique_ptr env, ->service_control_config() .intermediate_report_min_interval(); } + + service_account_token_.SetAudience( + auth::ServiceAccountToken::JWT_TOKEN_FOR_FIREBASE, kFirebaseAudience); } MethodCallInfo ServiceContext::GetMethodCallInfo( From b0e48c7e8e3f22d7ad86058a68bd2a1f6cd91cdc Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Fri, 27 Jan 2017 11:57:52 -0800 Subject: [PATCH 03/21] Remove some debug logs --- contrib/endpoints/src/api_manager/check_auth.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/contrib/endpoints/src/api_manager/check_auth.cc b/contrib/endpoints/src/api_manager/check_auth.cc index 7b6238ac2bb..b4aae8bb2a1 100644 --- a/contrib/endpoints/src/api_manager/check_auth.cc +++ b/contrib/endpoints/src/api_manager/check_auth.cc @@ -213,9 +213,7 @@ void AuthChecker::LookupJwtCache() { if (cache_hit) { CheckAudience(true); } else { - env_->LogInfo("Before Parse JWT"); ParseJwt(); - env_->LogInfo("After Parse JWT"); } } From 6f10a20ec48acc6b124df59886b16d8915e71bc2 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Tue, 31 Jan 2017 14:32:26 -0800 Subject: [PATCH 04/21] Add proto file to capture TestRulesetRequest. --- contrib/endpoints/src/api_manager/BUILD | 14 ++ contrib/endpoints/src/api_manager/auth.h | 1 + .../auth/lib/auth_jwt_validator.cc | 4 + .../src/api_manager/check_security_rules.cc | 167 ++++++++++-------- .../src/api_manager/check_security_rules.h | 6 +- .../src/api_manager/context/request_context.h | 4 +- .../api_manager/proto/security_rules.proto | 58 ++++++ 7 files changed, 173 insertions(+), 81 deletions(-) create mode 100644 contrib/endpoints/src/api_manager/proto/security_rules.proto diff --git a/contrib/endpoints/src/api_manager/BUILD b/contrib/endpoints/src/api_manager/BUILD index 38671d6e90e..ffe4f060b7c 100644 --- a/contrib/endpoints/src/api_manager/BUILD +++ b/contrib/endpoints/src/api_manager/BUILD @@ -38,6 +38,19 @@ cc_proto_library( visibility = ["//visibility:public"], ) +cc_proto_library( + name = "security_rules_proto", + srcs = [ + "proto/security_rules.proto", + ], + default_runtime = "//external:protobuf", + protoc = "//external:protoc", + visibility = ["//visibility:public"], + deps = [ + "//external:cc_wkt_protos", + ], +) + cc_library( name = "auth_headers", hdrs = [ @@ -99,6 +112,7 @@ cc_library( ":auth_headers", ":impl_headers", ":server_config_proto", + ":security_rules_proto", "//contrib/endpoints/src/api_manager/auth", "//contrib/endpoints/src/api_manager/cloud_trace", "//contrib/endpoints/src/api_manager/context", diff --git a/contrib/endpoints/src/api_manager/auth.h b/contrib/endpoints/src/api_manager/auth.h index e69718e95f6..2cb2824c051 100644 --- a/contrib/endpoints/src/api_manager/auth.h +++ b/contrib/endpoints/src/api_manager/auth.h @@ -15,6 +15,7 @@ #ifndef API_MANAGER_AUTH_H_ #define API_MANAGER_AUTH_H_ +#include #include #include #include diff --git a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc index fc73125b013..44c5ec92e19 100644 --- a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc +++ b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc @@ -700,6 +700,10 @@ grpc_jwt_verifier_status JwtValidatorImpl::FillUserInfoAndSetExp( // Optional field. const grpc_json *grpc_json = grpc_jwt_claims_json(claims_); + /*for (grpc_json *cur = json->child; cur != nullptr; cur = cur->next) { + user_info->claims.emplace(std::string(cur->key), std::string(cur->value)); + } */ + char *json_str = grpc_json_dump_to_string(const_cast<::grpc_json *>(grpc_json), 0); user_info->claims = json_str == nullptr ? "" : json_str; diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index 2e1390d3f7c..af91d90e0ae 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -15,16 +15,20 @@ //////////////////////////////////////////////////////////////////////////////// #include "contrib/endpoints/src/api_manager/check_security_rules.h" #include "contrib/endpoints/src/api_manager/auth/lib/json_util.h" +#include "contrib/endpoints/src/api_manager/proto/security_rules.pb.h" +#include "contrib/endpoints/src/api_manager/utils/marshalling.h" #include #include using ::google::api_manager::auth::GetProperty; using ::google::api_manager::auth::GetStringValue; using ::google::api_manager::utils::Status; +using ::google::protobuf::Map; using ::google::protobuf::util::error::Code; namespace google { namespace api_manager { +namespace { const char kFirebaseServerStaging[] = "https://staging-firebaserules.sandbox.googleapis.com/"; @@ -36,6 +40,8 @@ const char kFailedFirebaseReleaseFetch[] = "Failed to fetch Firebase Release"; const char kFailedFirebaseTest[] = "Failed to execute Firebase Test"; const char kInvalidResponse[] = "Invalid JSON response from Firebase Service"; +// An AuthzChecker object is created for every incoming request. It does +// authorizaiton by calling Firebase Rules service. class AuthzChecker : public std::enable_shared_from_this { public: // Constructor @@ -49,7 +55,7 @@ class AuthzChecker : public std::enable_shared_from_this { private: // Helper method that invokes the test firebase service api. - void FirebaseCheck(std::string ruleset_id, + void FirebaseCheck(std::string &ruleset_id, std::shared_ptr context, std::function continuation); @@ -62,23 +68,21 @@ class AuthzChecker : public std::enable_shared_from_this { const std::string &json_str); // Builds the request body for the TESP API call. - std::string BuildTestRequestBody( - std::shared_ptr context); - - void AddToBody(const std::string &key, - std::ostringstream &ss); - - void AddToBody(const std::string &key, const std::string &value, - bool end, std::ostringstream &ss); + Status BuildTestRequestBody( + std::shared_ptr context, + std::string &result_string); // Get the release name to used in Firebase API call - const std::string GetReleaseName( + static const std::string GetReleaseName( std::shared_ptr request_context); // Get the URL for the Release request. - const std::string GetReleaseUrl( + static const std::string GetReleaseUrl( std::shared_ptr request_context); + static const std::string GetRulesetTestUri( + std::string &ruleset_id); + // Invoke the HTTP call void HttpFetch(const std::string &url, const std::string &method, const std::string &request_body, @@ -92,11 +96,10 @@ class AuthzChecker : public std::enable_shared_from_this { } // Get Firebase specific operation Id based on the http Method. - std::string GetOperation(std::string httpMethod); + static std::string GetOperation(std::string &httpMethod); - const std::string Stringify(const char *s) { - return std::string("\"") + (s == nullptr ? "" : s) + "\""; - } + void SetProtoValue(::google::protobuf::Value &head, + std::string key, ::google::protobuf::Value &value); std::shared_ptr GetPtr() { return shared_from_this(); } @@ -125,18 +128,18 @@ void AuthzChecker::Check( } // Fetch the Release attributes. - auto pchecker = GetPtr(); - HttpFetch(GetReleaseUrl(context), std::string("GET"), std::string(""), - [context, final_continuation, pchecker] (Status status, + auto checker = GetPtr(); + HttpFetch(AuthzChecker::GetReleaseUrl(context), "GET", "", + [context, final_continuation, checker] (Status status, std::string &&body) { std::string ruleset_id; if (status.ok()) { - pchecker->env_->LogDebug( + checker->env_->LogDebug( std::string("GetReleasName succeeded with ") + body); - status = pchecker->ParseReleaseResponse(body, &ruleset_id); + status = checker->ParseReleaseResponse(body, &ruleset_id); } else { - pchecker->env_->LogError(std::string("GetReleaseName for ") - + pchecker->GetReleaseUrl(context) + checker->env_->LogError(std::string("GetReleaseName for ") + + AuthzChecker::GetReleaseUrl(context) + " with status " + status.ToString()); status = Status(Code::INTERNAL, kFailedFirebaseReleaseFetch); } @@ -144,7 +147,7 @@ void AuthzChecker::Check( // If the parsing of the release body is successful, then call the // Test Api for firebase rules service. if (status.ok()) { - pchecker->FirebaseCheck(ruleset_id, context, final_continuation); + checker->FirebaseCheck(ruleset_id, context, final_continuation); } else { final_continuation(status); } @@ -152,23 +155,28 @@ void AuthzChecker::Check( } void AuthzChecker::FirebaseCheck( - std::string ruleset_id, + std::string &ruleset_id, std::shared_ptr context, std::function continuation) { - auto pchecker = GetPtr(); + auto checker = GetPtr(); + + std::string body; + Status status = BuildTestRequestBody(context, body); + if (!status.ok()) { + continuation(status); + return; + } - HttpFetch(std::string(kFirebaseServerStaging) + "v1/" + ruleset_id + - ":test?alt=json", - std::string("POST"), BuildTestRequestBody(context), - [context, continuation, pchecker, ruleset_id] + HttpFetch(AuthzChecker::GetRulesetTestUri(ruleset_id), "POST", body, + [context, continuation, checker, ruleset_id] (Status status, std::string &&body) { if (status.ok()) { - pchecker->env_->LogDebug( + checker->env_->LogDebug( std::string("Test API succeeded with ") + body); - status = pchecker->ParseTestResponse(context, body); + status = checker->ParseTestResponse(context, body); } else { - pchecker->env_->LogError(std::string("Test API failed with ") + checker->env_->LogError(std::string("Test API failed with ") + status.ToString()); status = Status(Code::INTERNAL, kFailedFirebaseTest); } @@ -238,7 +246,7 @@ Status AuthzChecker::ParseTestResponse( return status; } -std::string AuthzChecker::GetOperation(std::string httpMethod) { +std::string AuthzChecker::GetOperation(std::string &httpMethod) { if (httpMethod == "POST") { return std::string("create"); } @@ -254,49 +262,55 @@ std::string AuthzChecker::GetOperation(std::string httpMethod) { return std::string("update"); } -std::string AuthzChecker::BuildTestRequestBody( - std::shared_ptr context) { - - std::ostringstream ss; - - int openCount = 0; - ss << "{" - << Stringify("test_cases") + ": " - << "[ {"; - ++openCount; - AddToBody("service_name", context->service_context()->service_name(), false, ss); - AddToBody("resource_path", context->request()->GetRequestPath(), - false, ss); - AddToBody("operation", GetOperation(context->request()->GetRequestHTTPMethod()), - false, ss); - AddToBody("expectation", "ALLOW", false, ss); - AddToBody("variables", ss); - ++openCount; - AddToBody("request", ss); - ++openCount; - AddToBody("auth", ss); - ++openCount; - - ss << Stringify("token") + ": " << context->auth_claims(); - - while(openCount-- > 0) { - ss << "} "; +Status AuthzChecker::BuildTestRequestBody( + std::shared_ptr context, + std::string &result_string) { + + proto::TestRulesetRequest request; + proto::TestRulesetRequest::TestCase *test_case = request.add_test_cases(); + std::string httpMethod = context->request()->GetRequestHTTPMethod(); + + test_case->set_service_name(context->service_context()->service_name()); + test_case->set_resource_path(context->request()->GetRequestPath()); + test_case->set_operation(AuthzChecker::GetOperation(httpMethod)); + test_case->set_expectation(proto::TestRulesetRequest::TestCase::ALLOW); + + ::google::protobuf::Value auth; + ::google::protobuf::Value token; + ::google::protobuf::Value claims; + + Status status = utils::JsonToProto(context->auth_claims(), &claims); + if (!status.ok()) { + env_->LogError(std::string("Error creating Protobuf from claims") + + status.ToString()); + return status; } - ss << "] }"; - return ss.str(); -} + SetProtoValue(token, "token", claims); + SetProtoValue(auth, "auth", token); -void AuthzChecker::AddToBody(const std::string &key, - std::ostringstream &ss) { - ss << Stringify(key.c_str()) + ": " + "{"; -} -void AuthzChecker::AddToBody(const std::string &key, const std::string &value, - bool end, std::ostringstream &ss) { - ss << Stringify(key.c_str()) + ": " + Stringify(value.c_str()); - if (!end) { - ss << ", "; + Map *variables = + test_case->mutable_variables(); + (*variables)["request"] = auth; + + status = utils::ProtoToJson(request, &result_string, + utils::JsonOptions::DEFAULT); + if (status.ok()) { + env_->LogDebug(std::string("PRotobuf to JSON string = ") + result_string); + } else { + env_->LogError(std::string("Error creating TestRulesetRequest") + + status.ToString()); } + + return status; +} + +void AuthzChecker::SetProtoValue(::google::protobuf::Value &head, + std::string key, + ::google::protobuf::Value &value) { + ::google::protobuf::Struct *s = head.mutable_struct_value(); + Map *fields = s->mutable_fields(); + (*fields)[key] = value; } const std::string AuthzChecker::GetReleaseName( @@ -305,11 +319,16 @@ const std::string AuthzChecker::GetReleaseName( + request_context->service_context()->service().apis(0).version(); } +const std::string AuthzChecker::GetRulesetTestUri(std::string &ruleset_id) { + return std::string(kFirebaseServerStaging) + "v1/" + ruleset_id + + ":test?alt=json"; +} + const std::string AuthzChecker::GetReleaseUrl( std::shared_ptr request_context) { return std::string(kFirebaseServerStaging) + "v1/projects/" + request_context->service_context()->project_id() + "/releases/" - + GetReleaseName(request_context); + + AuthzChecker::GetReleaseName(request_context); } void AuthzChecker::HttpFetch( @@ -341,6 +360,8 @@ void AuthzChecker::HttpFetch( env_->RunHTTPRequest(std::move(request)); } +} // namespace + void CheckSecurityRules(std::shared_ptr context, std::function continuation) { std::shared_ptr checker = std::make_shared( diff --git a/contrib/endpoints/src/api_manager/check_security_rules.h b/contrib/endpoints/src/api_manager/check_security_rules.h index 88820f6f47b..19dbab349cb 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.h +++ b/contrib/endpoints/src/api_manager/check_security_rules.h @@ -15,13 +15,9 @@ #ifndef API_MANAGER_CHECK_SECURITY_RULES_H_ #define API_MANAGER_CHECK_SECURITY_RULES_H_ -#include "contrib/endpoints/include/api_manager/api_manager.h" -#include "contrib/endpoints/src/api_manager/auth/service_account_token.h" -#include "contrib/endpoints/src/api_manager/context/request_context.h" #include "contrib/endpoints/include/api_manager/utils/status.h" - +#include "contrib/endpoints/src/api_manager/context/request_context.h" #include -using ::google::api_manager::utils::Status; namespace google { namespace api_manager { diff --git a/contrib/endpoints/src/api_manager/context/request_context.h b/contrib/endpoints/src/api_manager/context/request_context.h index b99b52ccfa4..0a42e9e13bd 100644 --- a/contrib/endpoints/src/api_manager/context/request_context.h +++ b/contrib/endpoints/src/api_manager/context/request_context.h @@ -112,7 +112,7 @@ class RequestContext { last_report_time_ = tp; } - void set_auth_claims(std::string &claims) { + void set_auth_claims(const std::string &claims) { auth_claims_ = claims; } @@ -120,8 +120,6 @@ class RequestContext { return auth_claims_; } - const std::string GetAuthToken(); - private: // Fill OperationInfo void FillOperationInfo(service_control::OperationInfo *info); diff --git a/contrib/endpoints/src/api_manager/proto/security_rules.proto b/contrib/endpoints/src/api_manager/proto/security_rules.proto new file mode 100644 index 00000000000..9a09d1c7e43 --- /dev/null +++ b/contrib/endpoints/src/api_manager/proto/security_rules.proto @@ -0,0 +1,58 @@ +// 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. +// 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. +// +//////////////////////////////////////////////////////////////////////////////// +// +syntax = "proto3"; + +package google.api_manager.proto; + +import "google/protobuf/struct.proto"; + +message TestRulesetRequest { + message TestCase { + // The set of supported test case expectations. + enum Expectation { + EXPECTATION_UNSPECIFIED = 0; // Unspecified expectation. + ALLOW = 1; // Expect an allowed result. + DENY = 2; // Expect a denied result. + } + + // The name of the service that is the subject of the test case. + string service_name = 1; + + // The RESTful resource path of the mock `request`. + string resource_path = 2; + + // The `request` `operation`. The operation will typically be one of `get`, + // `list`, `create`, `update`, or `delete`. Services also may provide custom + // operations. + string operation = 3; + + // Test expectation. + Expectation expectation = 4; + + // (-- + // Variables and fake resources need to be updated to support multiple + // services and the standardized `request` definition. + // --) + + // Optional set of variable values to use during evaluation. + map variables = 5; + + } + + // The set of test cases to run against the `Source` if it is well-formed. + repeated TestCase test_cases = 3; +} From c6ae0fadacbfc5da0d144bb7718dd3aceda3d7b9 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Tue, 31 Jan 2017 15:17:23 -0800 Subject: [PATCH 05/21] clang-format files --- .../auth/lib/auth_jwt_validator.cc | 3 +- .../src/api_manager/check_security_rules.cc | 98 +++++++++---------- .../src/api_manager/check_security_rules.h | 2 +- .../src/api_manager/context/request_context.h | 8 +- .../api_manager/context/service_context.cc | 3 +- .../api_manager/proto/security_rules.proto | 1 - 6 files changed, 51 insertions(+), 64 deletions(-) diff --git a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc index 44c5ec92e19..c809c8eb481 100644 --- a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc +++ b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc @@ -704,7 +704,8 @@ grpc_jwt_verifier_status JwtValidatorImpl::FillUserInfoAndSetExp( user_info->claims.emplace(std::string(cur->key), std::string(cur->value)); } */ - char *json_str = grpc_json_dump_to_string(const_cast<::grpc_json *>(grpc_json), 0); + char *json_str = + grpc_json_dump_to_string(const_cast<::grpc_json *>(grpc_json), 0); user_info->claims = json_str == nullptr ? "" : json_str; const char *email = GetStringValue(grpc_json, "email"); diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index af91d90e0ae..eb5fb28cb52 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -14,11 +14,11 @@ // //////////////////////////////////////////////////////////////////////////////// #include "contrib/endpoints/src/api_manager/check_security_rules.h" +#include +#include #include "contrib/endpoints/src/api_manager/auth/lib/json_util.h" #include "contrib/endpoints/src/api_manager/proto/security_rules.pb.h" #include "contrib/endpoints/src/api_manager/utils/marshalling.h" -#include -#include using ::google::api_manager::auth::GetProperty; using ::google::api_manager::auth::GetStringValue; @@ -53,11 +53,10 @@ class AuthzChecker : public std::enable_shared_from_this { std::function continuation); private: - // Helper method that invokes the test firebase service api. void FirebaseCheck(std::string &ruleset_id, - std::shared_ptr context, - std::function continuation); + std::shared_ptr context, + std::function continuation); // Parse the respose for GET RELEASE API call Status ParseReleaseResponse(const std::string &json_str, @@ -68,9 +67,8 @@ class AuthzChecker : public std::enable_shared_from_this { const std::string &json_str); // Builds the request body for the TESP API call. - Status BuildTestRequestBody( - std::shared_ptr context, - std::string &result_string); + Status BuildTestRequestBody(std::shared_ptr context, + std::string &result_string); // Get the release name to used in Firebase API call static const std::string GetReleaseName( @@ -80,15 +78,13 @@ class AuthzChecker : public std::enable_shared_from_this { static const std::string GetReleaseUrl( std::shared_ptr request_context); - static const std::string GetRulesetTestUri( - std::string &ruleset_id); + static const std::string GetRulesetTestUri(std::string &ruleset_id); // Invoke the HTTP call void HttpFetch(const std::string &url, const std::string &method, const std::string &request_body, std::function continuation); - // Get the auth token for Firebase service const std::string &GetAuthToken() { return sa_token_->GetAuthToken( @@ -98,8 +94,8 @@ class AuthzChecker : public std::enable_shared_from_this { // Get Firebase specific operation Id based on the http Method. static std::string GetOperation(std::string &httpMethod); - void SetProtoValue(::google::protobuf::Value &head, - std::string key, ::google::protobuf::Value &value); + void SetProtoValue(::google::protobuf::Value &head, std::string key, + ::google::protobuf::Value &value); std::shared_ptr GetPtr() { return shared_from_this(); } @@ -109,8 +105,7 @@ class AuthzChecker : public std::enable_shared_from_this { AuthzChecker::AuthzChecker(ApiManagerEnvInterface *env, auth::ServiceAccountToken *sa_token) - : env_(env), - sa_token_(sa_token) {} + : env_(env), sa_token_(sa_token) {} void AuthzChecker::Check( std::shared_ptr context, @@ -121,8 +116,8 @@ void AuthzChecker::Check( if (!context->service_context()->RequireAuth() || context->method() == nullptr || !context->method()->auth()) { env_->LogDebug( - std::string("Autherization and JWT validation was not performed") - + " skipping authorization"); + std::string("Autherization and JWT validation was not performed") + + " skipping authorization"); final_continuation(Status::OK); return; } @@ -130,17 +125,17 @@ void AuthzChecker::Check( // Fetch the Release attributes. auto checker = GetPtr(); HttpFetch(AuthzChecker::GetReleaseUrl(context), "GET", "", - [context, final_continuation, checker] (Status status, - std::string &&body) { + [context, final_continuation, checker](Status status, + std::string &&body) { std::string ruleset_id; if (status.ok()) { checker->env_->LogDebug( std::string("GetReleasName succeeded with ") + body); status = checker->ParseReleaseResponse(body, &ruleset_id); } else { - checker->env_->LogError(std::string("GetReleaseName for ") - + AuthzChecker::GetReleaseUrl(context) - + " with status " + status.ToString()); + checker->env_->LogError(std::string("GetReleaseName for ") + + AuthzChecker::GetReleaseUrl(context) + + " with status " + status.ToString()); status = Status(Code::INTERNAL, kFailedFirebaseReleaseFetch); } @@ -155,8 +150,7 @@ void AuthzChecker::Check( } void AuthzChecker::FirebaseCheck( - std::string &ruleset_id, - std::shared_ptr context, + std::string &ruleset_id, std::shared_ptr context, std::function continuation) { auto checker = GetPtr(); @@ -168,16 +162,16 @@ void AuthzChecker::FirebaseCheck( } HttpFetch(AuthzChecker::GetRulesetTestUri(ruleset_id), "POST", body, - [context, continuation, checker, ruleset_id] - (Status status, std::string &&body) { + [context, continuation, checker, ruleset_id](Status status, + std::string &&body) { if (status.ok()) { checker->env_->LogDebug( std::string("Test API succeeded with ") + body); status = checker->ParseTestResponse(context, body); } else { - checker->env_->LogError(std::string("Test API failed with ") - + status.ToString()); + checker->env_->LogError(std::string("Test API failed with ") + + status.ToString()); status = Status(Code::INTERNAL, kFailedFirebaseTest); } @@ -215,7 +209,6 @@ Status AuthzChecker::ParseTestResponse( grpc_json *json = grpc_json_parse_string_with_len( const_cast(json_str.data()), json_str.length()); - if (!json) { return Status(Code::INVALID_ARGUMENT, "Invalid JSON response from Firebase Service"); @@ -235,10 +228,11 @@ Status AuthzChecker::ParseTestResponse( status = invalid; } else if (std::string(result) != "SUCCESS") { status = Status(Code::PERMISSION_DENIED, - std::string("Unauthorized ") - + context->request()->GetRequestHTTPMethod() - + " access to resource " + context->request()->GetRequestPath(), - Status::AUTH); + std::string("Unauthorized ") + + context->request()->GetRequestHTTPMethod() + + " access to resource " + + context->request()->GetRequestPath(), + Status::AUTH); } } @@ -265,7 +259,6 @@ std::string AuthzChecker::GetOperation(std::string &httpMethod) { Status AuthzChecker::BuildTestRequestBody( std::shared_ptr context, std::string &result_string) { - proto::TestRulesetRequest request; proto::TestRulesetRequest::TestCase *test_case = request.add_test_cases(); std::string httpMethod = context->request()->GetRequestHTTPMethod(); @@ -281,8 +274,8 @@ Status AuthzChecker::BuildTestRequestBody( Status status = utils::JsonToProto(context->auth_claims(), &claims); if (!status.ok()) { - env_->LogError(std::string("Error creating Protobuf from claims") - + status.ToString()); + env_->LogError(std::string("Error creating Protobuf from claims") + + status.ToString()); return status; } @@ -293,13 +286,13 @@ Status AuthzChecker::BuildTestRequestBody( test_case->mutable_variables(); (*variables)["request"] = auth; - status = utils::ProtoToJson(request, &result_string, - utils::JsonOptions::DEFAULT); + status = + utils::ProtoToJson(request, &result_string, utils::JsonOptions::DEFAULT); if (status.ok()) { env_->LogDebug(std::string("PRotobuf to JSON string = ") + result_string); } else { - env_->LogError(std::string("Error creating TestRulesetRequest") - + status.ToString()); + env_->LogError(std::string("Error creating TestRulesetRequest") + + status.ToString()); } return status; @@ -315,29 +308,28 @@ void AuthzChecker::SetProtoValue(::google::protobuf::Value &head, const std::string AuthzChecker::GetReleaseName( std::shared_ptr request_context) { - return request_context->service_context()->service_name() + ":" - + request_context->service_context()->service().apis(0).version(); + return request_context->service_context()->service_name() + ":" + + request_context->service_context()->service().apis(0).version(); } const std::string AuthzChecker::GetRulesetTestUri(std::string &ruleset_id) { return std::string(kFirebaseServerStaging) + "v1/" + ruleset_id + - ":test?alt=json"; + ":test?alt=json"; } const std::string AuthzChecker::GetReleaseUrl( std::shared_ptr request_context) { - return std::string(kFirebaseServerStaging) + "v1/projects/" - + request_context->service_context()->project_id() + "/releases/" - + AuthzChecker::GetReleaseName(request_context); + return std::string(kFirebaseServerStaging) + "v1/projects/" + + request_context->service_context()->project_id() + "/releases/" + + AuthzChecker::GetReleaseName(request_context); } void AuthzChecker::HttpFetch( const std::string &url, const std::string &method, const std::string &request_body, std::function continuation) { - env_->LogInfo(std::string("Issue HTTP Request to url :") + url + - " method : " + method + " body: " + request_body); + " method : " + method + " body: " + request_body); std::unique_ptr request(new HTTPRequest([continuation]( Status status, std::map &&, @@ -348,19 +340,17 @@ void AuthzChecker::HttpFetch( return; } - request->set_method(method) - .set_url(url) - .set_auth_token(GetAuthToken()); + request->set_method(method).set_url(url).set_auth_token(GetAuthToken()); if (method != "GET") { - request->set_header("Content-Type", "application/json") - .set_body(request_body); + request->set_header("Content-Type", "application/json") + .set_body(request_body); } env_->RunHTTPRequest(std::move(request)); } -} // namespace +} // namespace void CheckSecurityRules(std::shared_ptr context, std::function continuation) { diff --git a/contrib/endpoints/src/api_manager/check_security_rules.h b/contrib/endpoints/src/api_manager/check_security_rules.h index 19dbab349cb..abf968815bb 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.h +++ b/contrib/endpoints/src/api_manager/check_security_rules.h @@ -15,9 +15,9 @@ #ifndef API_MANAGER_CHECK_SECURITY_RULES_H_ #define API_MANAGER_CHECK_SECURITY_RULES_H_ +#include #include "contrib/endpoints/include/api_manager/utils/status.h" #include "contrib/endpoints/src/api_manager/context/request_context.h" -#include namespace google { namespace api_manager { diff --git a/contrib/endpoints/src/api_manager/context/request_context.h b/contrib/endpoints/src/api_manager/context/request_context.h index 0a42e9e13bd..f89d4f22d3a 100644 --- a/contrib/endpoints/src/api_manager/context/request_context.h +++ b/contrib/endpoints/src/api_manager/context/request_context.h @@ -112,13 +112,9 @@ class RequestContext { last_report_time_ = tp; } - void set_auth_claims(const std::string &claims) { - auth_claims_ = claims; - } + void set_auth_claims(const std::string &claims) { auth_claims_ = claims; } - std::string &auth_claims() { - return auth_claims_; - } + std::string &auth_claims() { return auth_claims_; } private: // Fill OperationInfo diff --git a/contrib/endpoints/src/api_manager/context/service_context.cc b/contrib/endpoints/src/api_manager/context/service_context.cc index 3ab7250a877..d5b6cc5c341 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.cc +++ b/contrib/endpoints/src/api_manager/context/service_context.cc @@ -46,7 +46,8 @@ const char kHTTPHeadMethod[] = "HEAD"; const char kHTTPGetMethod[] = "GET"; const char kFirebaseAudience[] = - "https://staging-firebaserules.sandbox.googleapis.com/google.firebase.rules.v1.FirebaseRulesService"; + "https://staging-firebaserules.sandbox.googleapis.com/" + "google.firebase.rules.v1.FirebaseRulesService"; } ServiceContext::ServiceContext(std::unique_ptr env, diff --git a/contrib/endpoints/src/api_manager/proto/security_rules.proto b/contrib/endpoints/src/api_manager/proto/security_rules.proto index 9a09d1c7e43..ce8d2690fe0 100644 --- a/contrib/endpoints/src/api_manager/proto/security_rules.proto +++ b/contrib/endpoints/src/api_manager/proto/security_rules.proto @@ -50,7 +50,6 @@ message TestRulesetRequest { // Optional set of variable values to use during evaluation. map variables = 5; - } // The set of test cases to run against the `Source` if it is well-formed. From 01b50bfde5c7b199fe1128d8f4e52095cd60790d Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Tue, 31 Jan 2017 15:22:26 -0800 Subject: [PATCH 06/21] Resolve a merge issue with previous commit --- .../endpoints/src/api_manager/context/request_context.cc | 8 -------- 1 file changed, 8 deletions(-) diff --git a/contrib/endpoints/src/api_manager/context/request_context.cc b/contrib/endpoints/src/api_manager/context/request_context.cc index 0c8e70dc511..cbf4e925e37 100644 --- a/contrib/endpoints/src/api_manager/context/request_context.cc +++ b/contrib/endpoints/src/api_manager/context/request_context.cc @@ -20,14 +20,6 @@ #include #include -extern "C" { -#include "grpc/grpc.h" -#include "grpc/support/alloc.h" -#include "grpc/support/log.h" -#include "grpc/support/string_util.h" -#include "grpc/support/sync.h" -} - using ::google::api_manager::utils::Status; namespace google { From 1a6704c37b135af61fa37cc994c9de54bb07d186 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Tue, 31 Jan 2017 16:01:58 -0800 Subject: [PATCH 07/21] Allow security rules to disabled via serverconfig --- .../endpoints/src/api_manager/check_security_rules.cc | 2 +- .../endpoints/src/api_manager/context/service_context.cc | 6 +++++- .../endpoints/src/api_manager/context/service_context.h | 7 +++++++ .../endpoints/src/api_manager/proto/server_config.proto | 9 +++++++++ 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index eb5fb28cb52..4d9068d3ca7 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -113,7 +113,7 @@ void AuthzChecker::Check( // TODO: Check service config to see if "useSecurityRules" is specified. // If so, call Firebase Rules service TestRuleset API. - if (!context->service_context()->RequireAuth() || + if (!context->service_context()->RequireRulesCheck() || context->method() == nullptr || !context->method()->auth()) { env_->LogDebug( std::string("Autherization and JWT validation was not performed") + diff --git a/contrib/endpoints/src/api_manager/context/service_context.cc b/contrib/endpoints/src/api_manager/context/service_context.cc index d5b6cc5c341..7f4c98580d0 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.cc +++ b/contrib/endpoints/src/api_manager/context/service_context.cc @@ -60,7 +60,11 @@ ServiceContext::ServiceContext(std::unique_ptr env, is_auth_force_disabled_(config_->server_config() && config_->server_config() ->api_authentication_config() - .force_disable()) { + .force_disable()), + is_check_security_rules_disabled_(config_->server_config() && + config_->server_config() + ->api_check_security_rules_config() + .force_disable()) { intermediate_report_interval_ = kIntermediateReportInterval; // Check server_config override. diff --git a/contrib/endpoints/src/api_manager/context/service_context.h b/contrib/endpoints/src/api_manager/context/service_context.h index 5633ca3dd88..31d01acda3d 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.h +++ b/contrib/endpoints/src/api_manager/context/service_context.h @@ -65,6 +65,10 @@ class ServiceContext { return !is_auth_force_disabled_ && config_->HasAuth(); } + bool RequireRulesCheck() const { + return RequireAuth() && !is_check_security_rules_disabled_; + } + auth::Certs &certs() { return certs_; } auth::JwtCache &jwt_cache() { return jwt_cache_; } @@ -126,6 +130,9 @@ class ServiceContext { // Is auth force-disabled bool is_auth_force_disabled_; + // Is check security rules disabled + bool is_check_security_rules_disabled_; + // The time interval for grpc intermediate report. int64_t intermediate_report_interval_; }; diff --git a/contrib/endpoints/src/api_manager/proto/server_config.proto b/contrib/endpoints/src/api_manager/proto/server_config.proto index 343414aceae..ab9e4717f07 100644 --- a/contrib/endpoints/src/api_manager/proto/server_config.proto +++ b/contrib/endpoints/src/api_manager/proto/server_config.proto @@ -39,6 +39,9 @@ message ServerConfig { // Envoy/esp talks to Mixer, has to specify this field. MixerOptions mixer_options = 6; + // Server config used for API authorization via Firebase Rules. + ApiCheckSecurityRulesConfig api_check_security_rules_config = 7; + // Experimental flags Experimental experimental = 999; } @@ -143,6 +146,12 @@ message ApiAuthenticationConfig { bool force_disable = 1; } +// Server config for API Authorization via Firebase Rules +message ApiCheckSecurityRulesConfig { + // Allows to disable the API authorization + bool force_disable = 1; +} + message MixerOptions { // For envoy, it is the cluster name for mixer server. string mixer_server = 1; From 6351cfab238e0864217d2f28b4a57a5c441cc095 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Wed, 1 Feb 2017 09:44:13 -0800 Subject: [PATCH 08/21] format file --- contrib/endpoints/src/api_manager/context/service_context.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/contrib/endpoints/src/api_manager/context/service_context.cc b/contrib/endpoints/src/api_manager/context/service_context.cc index 7f4c98580d0..057f196ed3e 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.cc +++ b/contrib/endpoints/src/api_manager/context/service_context.cc @@ -63,8 +63,8 @@ ServiceContext::ServiceContext(std::unique_ptr env, .force_disable()), is_check_security_rules_disabled_(config_->server_config() && config_->server_config() - ->api_check_security_rules_config() - .force_disable()) { + ->api_check_security_rules_config() + .force_disable()) { intermediate_report_interval_ = kIntermediateReportInterval; // Check server_config override. From 32d0023afa26468c924b13ca8b40053db6cbf844 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Wed, 1 Feb 2017 10:54:56 -0800 Subject: [PATCH 09/21] Addressed Wayne's review comments. --- .../auth/lib/auth_jwt_validator.cc | 4 - .../src/api_manager/check_security_rules.cc | 146 ++++++++---------- 2 files changed, 64 insertions(+), 86 deletions(-) diff --git a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc index c809c8eb481..6ccd05eb0e8 100644 --- a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc +++ b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc @@ -700,10 +700,6 @@ grpc_jwt_verifier_status JwtValidatorImpl::FillUserInfoAndSetExp( // Optional field. const grpc_json *grpc_json = grpc_jwt_claims_json(claims_); - /*for (grpc_json *cur = json->child; cur != nullptr; cur = cur->next) { - user_info->claims.emplace(std::string(cur->key), std::string(cur->value)); - } */ - char *json_str = grpc_json_dump_to_string(const_cast<::grpc_json *>(grpc_json), 0); user_info->claims = json_str == nullptr ? "" : json_str; diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index 4d9068d3ca7..6a0d6e3fa05 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -40,6 +40,47 @@ const char kFailedFirebaseReleaseFetch[] = "Failed to fetch Firebase Release"; const char kFailedFirebaseTest[] = "Failed to execute Firebase Test"; const char kInvalidResponse[] = "Invalid JSON response from Firebase Service"; +void SetProtoValue(std::string key, ::google::protobuf::Value &value, + ::google::protobuf::Value *head) { + ::google::protobuf::Struct *s = head->mutable_struct_value(); + Map *fields = s->mutable_fields(); + (*fields)[key] = value; +} + +const std::string GetReleaseName( + std::shared_ptr request_context) { + return request_context->service_context()->service_name() + ":" + + request_context->service_context()->service().apis(0).version(); +} + +const std::string GetRulesetTestUri(std::string &ruleset_id) { + return std::string(kFirebaseServerStaging) + "v1/" + ruleset_id + + ":test?alt=json"; +} + +const std::string GetReleaseUrl( + std::shared_ptr request_context) { + return std::string(kFirebaseServerStaging) + "v1/projects/" + + request_context->service_context()->project_id() + "/releases/" + + GetReleaseName(request_context); +} + +std::string GetOperation(std::string &httpMethod) { + if (httpMethod == "POST") { + return std::string("create"); + } + + if (httpMethod == "GET" || httpMethod == "HEAD" || httpMethod == "OPTIONS") { + return std::string("get"); + } + + if (httpMethod == "DELETE") { + return std::string("delete"); + } + + return std::string("update"); +} + // An AuthzChecker object is created for every incoming request. It does // authorizaiton by calling Firebase Rules service. class AuthzChecker : public std::enable_shared_from_this { @@ -54,9 +95,9 @@ class AuthzChecker : public std::enable_shared_from_this { private: // Helper method that invokes the test firebase service api. - void FirebaseCheck(std::string &ruleset_id, - std::shared_ptr context, - std::function continuation); + void CallTest(std::string &ruleset_id, + std::shared_ptr context, + std::function continuation); // Parse the respose for GET RELEASE API call Status ParseReleaseResponse(const std::string &json_str, @@ -68,17 +109,7 @@ class AuthzChecker : public std::enable_shared_from_this { // Builds the request body for the TESP API call. Status BuildTestRequestBody(std::shared_ptr context, - std::string &result_string); - - // Get the release name to used in Firebase API call - static const std::string GetReleaseName( - std::shared_ptr request_context); - - // Get the URL for the Release request. - static const std::string GetReleaseUrl( - std::shared_ptr request_context); - - static const std::string GetRulesetTestUri(std::string &ruleset_id); + std::string *result_string); // Invoke the HTTP call void HttpFetch(const std::string &url, const std::string &method, @@ -91,12 +122,6 @@ class AuthzChecker : public std::enable_shared_from_this { auth::ServiceAccountToken::JWT_TOKEN_FOR_FIREBASE); } - // Get Firebase specific operation Id based on the http Method. - static std::string GetOperation(std::string &httpMethod); - - void SetProtoValue(::google::protobuf::Value &head, std::string key, - ::google::protobuf::Value &value); - std::shared_ptr GetPtr() { return shared_from_this(); } ApiManagerEnvInterface *env_; @@ -124,7 +149,7 @@ void AuthzChecker::Check( // Fetch the Release attributes. auto checker = GetPtr(); - HttpFetch(AuthzChecker::GetReleaseUrl(context), "GET", "", + HttpFetch(GetReleaseUrl(context), "GET", "", [context, final_continuation, checker](Status status, std::string &&body) { std::string ruleset_id; @@ -134,7 +159,7 @@ void AuthzChecker::Check( status = checker->ParseReleaseResponse(body, &ruleset_id); } else { checker->env_->LogError(std::string("GetReleaseName for ") + - AuthzChecker::GetReleaseUrl(context) + + GetReleaseUrl(context) + " with status " + status.ToString()); status = Status(Code::INTERNAL, kFailedFirebaseReleaseFetch); } @@ -142,26 +167,25 @@ void AuthzChecker::Check( // If the parsing of the release body is successful, then call the // Test Api for firebase rules service. if (status.ok()) { - checker->FirebaseCheck(ruleset_id, context, final_continuation); + checker->CallTest(ruleset_id, context, final_continuation); } else { final_continuation(status); } }); } -void AuthzChecker::FirebaseCheck( - std::string &ruleset_id, std::shared_ptr context, - std::function continuation) { - auto checker = GetPtr(); - +void AuthzChecker::CallTest(std::string &ruleset_id, + std::shared_ptr context, + std::function continuation) { std::string body; - Status status = BuildTestRequestBody(context, body); + Status status = BuildTestRequestBody(context, &body); if (!status.ok()) { continuation(status); return; } - HttpFetch(AuthzChecker::GetRulesetTestUri(ruleset_id), "POST", body, + auto checker = GetPtr(); + HttpFetch(GetRulesetTestUri(ruleset_id), "POST", body, [context, continuation, checker, ruleset_id](Status status, std::string &&body) { @@ -190,13 +214,13 @@ Status AuthzChecker::ParseReleaseResponse(const std::string &json_str, Status status = Status::OK; const char *id = GetStringValue(json, "rulesetName"); - (*ruleset_id) = (id == nullptr) ? "" : GetStringValue(json, "rulesetName"); + (*ruleset_id) = (id == nullptr) ? "" : id; if (ruleset_id->empty()) { env_->LogError("Empty ruleset Id received from firebase service"); status = Status(Code::INTERNAL, kInvalidResponse); } else { - env_->LogInfo(std::string("Received ruleset Id: ") + *ruleset_id); + env_->LogDebug(std::string("Received ruleset Id: ") + *ruleset_id); } grpc_json_destroy(json); @@ -240,32 +264,16 @@ Status AuthzChecker::ParseTestResponse( return status; } -std::string AuthzChecker::GetOperation(std::string &httpMethod) { - if (httpMethod == "POST") { - return std::string("create"); - } - - if (httpMethod == "GET" || httpMethod == "HEAD" || httpMethod == "OPTIONS") { - return std::string("get"); - } - - if (httpMethod == "DELETE") { - return std::string("delete"); - } - - return std::string("update"); -} - Status AuthzChecker::BuildTestRequestBody( std::shared_ptr context, - std::string &result_string) { + std::string *result_string) { proto::TestRulesetRequest request; - proto::TestRulesetRequest::TestCase *test_case = request.add_test_cases(); - std::string httpMethod = context->request()->GetRequestHTTPMethod(); + auto *test_case = request.add_test_cases(); + auto httpMethod = context->request()->GetRequestHTTPMethod(); test_case->set_service_name(context->service_context()->service_name()); test_case->set_resource_path(context->request()->GetRequestPath()); - test_case->set_operation(AuthzChecker::GetOperation(httpMethod)); + test_case->set_operation(GetOperation(httpMethod)); test_case->set_expectation(proto::TestRulesetRequest::TestCase::ALLOW); ::google::protobuf::Value auth; @@ -279,17 +287,17 @@ Status AuthzChecker::BuildTestRequestBody( return status; } - SetProtoValue(token, "token", claims); - SetProtoValue(auth, "auth", token); + SetProtoValue("token", claims, &token); + SetProtoValue("auth", token, &auth); Map *variables = test_case->mutable_variables(); (*variables)["request"] = auth; status = - utils::ProtoToJson(request, &result_string, utils::JsonOptions::DEFAULT); + utils::ProtoToJson(request, result_string, utils::JsonOptions::DEFAULT); if (status.ok()) { - env_->LogDebug(std::string("PRotobuf to JSON string = ") + result_string); + env_->LogDebug(std::string("PRotobuf to JSON string = ") + *result_string); } else { env_->LogError(std::string("Error creating TestRulesetRequest") + status.ToString()); @@ -298,32 +306,6 @@ Status AuthzChecker::BuildTestRequestBody( return status; } -void AuthzChecker::SetProtoValue(::google::protobuf::Value &head, - std::string key, - ::google::protobuf::Value &value) { - ::google::protobuf::Struct *s = head.mutable_struct_value(); - Map *fields = s->mutable_fields(); - (*fields)[key] = value; -} - -const std::string AuthzChecker::GetReleaseName( - std::shared_ptr request_context) { - return request_context->service_context()->service_name() + ":" + - request_context->service_context()->service().apis(0).version(); -} - -const std::string AuthzChecker::GetRulesetTestUri(std::string &ruleset_id) { - return std::string(kFirebaseServerStaging) + "v1/" + ruleset_id + - ":test?alt=json"; -} - -const std::string AuthzChecker::GetReleaseUrl( - std::shared_ptr request_context) { - return std::string(kFirebaseServerStaging) + "v1/projects/" + - request_context->service_context()->project_id() + "/releases/" + - AuthzChecker::GetReleaseName(request_context); -} - void AuthzChecker::HttpFetch( const std::string &url, const std::string &method, const std::string &request_body, From 75d6e7a8044bcd120b5a3c6cd3e6f7729f3b2955 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Wed, 1 Feb 2017 11:01:54 -0800 Subject: [PATCH 10/21] Add firebase server to Server Config. --- contrib/endpoints/src/api_manager/proto/server_config.proto | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/contrib/endpoints/src/api_manager/proto/server_config.proto b/contrib/endpoints/src/api_manager/proto/server_config.proto index ab9e4717f07..a6c03a91a3c 100644 --- a/contrib/endpoints/src/api_manager/proto/server_config.proto +++ b/contrib/endpoints/src/api_manager/proto/server_config.proto @@ -148,8 +148,11 @@ message ApiAuthenticationConfig { // Server config for API Authorization via Firebase Rules message ApiCheckSecurityRulesConfig { + // Firebase server to use. + string firebase_server = 1; + // Allows to disable the API authorization - bool force_disable = 1; + bool force_disable = 2; } message MixerOptions { From b573770393a8bc1482f1bd7f13b6688ef1c6b449 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Wed, 1 Feb 2017 11:43:58 -0800 Subject: [PATCH 11/21] Address Lizan's review comments --- contrib/endpoints/src/api_manager/check_security_rules.cc | 5 +++-- contrib/endpoints/src/api_manager/context/request_context.h | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index 6a0d6e3fa05..7cfe0b08ccf 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -39,6 +39,7 @@ const char kFirebaseService[] = const char kFailedFirebaseReleaseFetch[] = "Failed to fetch Firebase Release"; const char kFailedFirebaseTest[] = "Failed to execute Firebase Test"; const char kInvalidResponse[] = "Invalid JSON response from Firebase Service"; +const char kTestSuccess[] = "SUCCESS"; void SetProtoValue(std::string key, ::google::protobuf::Value &value, ::google::protobuf::Value *head) { @@ -141,7 +142,7 @@ void AuthzChecker::Check( if (!context->service_context()->RequireRulesCheck() || context->method() == nullptr || !context->method()->auth()) { env_->LogDebug( - std::string("Autherization and JWT validation was not performed") + + "Autherization and JWT validation was not performed" " skipping authorization"); final_continuation(Status::OK); return; @@ -250,7 +251,7 @@ Status AuthzChecker::ParseTestResponse( if (result == nullptr) { env_->LogInfo("Result state is empty"); status = invalid; - } else if (std::string(result) != "SUCCESS") { + } else if (std::string(result) != kTestSuccess) { status = Status(Code::PERMISSION_DENIED, std::string("Unauthorized ") + context->request()->GetRequestHTTPMethod() + diff --git a/contrib/endpoints/src/api_manager/context/request_context.h b/contrib/endpoints/src/api_manager/context/request_context.h index f89d4f22d3a..5b8328507a9 100644 --- a/contrib/endpoints/src/api_manager/context/request_context.h +++ b/contrib/endpoints/src/api_manager/context/request_context.h @@ -114,7 +114,7 @@ class RequestContext { void set_auth_claims(const std::string &claims) { auth_claims_ = claims; } - std::string &auth_claims() { return auth_claims_; } + const std::string &auth_claims() { return auth_claims_; } private: // Fill OperationInfo From 9887846480487f595b77dabb393f00016145a017 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Wed, 1 Feb 2017 12:11:43 -0800 Subject: [PATCH 12/21] Address review comments. --- contrib/endpoints/src/api_manager/auth.h | 1 - contrib/endpoints/src/api_manager/check_security_rules.cc | 6 +++--- contrib/endpoints/src/api_manager/check_security_rules.h | 1 - 3 files changed, 3 insertions(+), 5 deletions(-) diff --git a/contrib/endpoints/src/api_manager/auth.h b/contrib/endpoints/src/api_manager/auth.h index 2cb2824c051..e69718e95f6 100644 --- a/contrib/endpoints/src/api_manager/auth.h +++ b/contrib/endpoints/src/api_manager/auth.h @@ -15,7 +15,6 @@ #ifndef API_MANAGER_AUTH_H_ #define API_MANAGER_AUTH_H_ -#include #include #include #include diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index 7cfe0b08ccf..f8d805e59cc 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -54,7 +54,7 @@ const std::string GetReleaseName( request_context->service_context()->service().apis(0).version(); } -const std::string GetRulesetTestUri(std::string &ruleset_id) { +const std::string GetRulesetTestUri(const std::string &ruleset_id) { return std::string(kFirebaseServerStaging) + "v1/" + ruleset_id + ":test?alt=json"; } @@ -96,7 +96,7 @@ class AuthzChecker : public std::enable_shared_from_this { private: // Helper method that invokes the test firebase service api. - void CallTest(std::string &ruleset_id, + void CallTest(const std::string &ruleset_id, std::shared_ptr context, std::function continuation); @@ -175,7 +175,7 @@ void AuthzChecker::Check( }); } -void AuthzChecker::CallTest(std::string &ruleset_id, +void AuthzChecker::CallTest(const std::string &ruleset_id, std::shared_ptr context, std::function continuation) { std::string body; diff --git a/contrib/endpoints/src/api_manager/check_security_rules.h b/contrib/endpoints/src/api_manager/check_security_rules.h index abf968815bb..bc971c48786 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.h +++ b/contrib/endpoints/src/api_manager/check_security_rules.h @@ -15,7 +15,6 @@ #ifndef API_MANAGER_CHECK_SECURITY_RULES_H_ #define API_MANAGER_CHECK_SECURITY_RULES_H_ -#include #include "contrib/endpoints/include/api_manager/utils/status.h" #include "contrib/endpoints/src/api_manager/context/request_context.h" From 7a2abe716055fa6d179e85d12e49d3151ab17e4f Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Wed, 1 Feb 2017 15:43:00 -0800 Subject: [PATCH 13/21] Disable check rules service by default. --- .../auth/lib/auth_jwt_validator.cc | 1 + .../src/api_manager/check_security_rules.cc | 84 ++++++++++--------- .../api_manager/context/service_context.cc | 6 +- .../src/api_manager/context/service_context.h | 11 +-- .../src/api_manager/proto/server_config.proto | 3 - 5 files changed, 51 insertions(+), 54 deletions(-) diff --git a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc index 6ccd05eb0e8..2673594d14a 100644 --- a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc +++ b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc @@ -703,6 +703,7 @@ grpc_jwt_verifier_status JwtValidatorImpl::FillUserInfoAndSetExp( char *json_str = grpc_json_dump_to_string(const_cast<::grpc_json *>(grpc_json), 0); user_info->claims = json_str == nullptr ? "" : json_str; + gpr_free(json_str); const char *email = GetStringValue(grpc_json, "email"); user_info->email = email == nullptr ? "" : email; diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index f8d805e59cc..4c034df7713 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -41,29 +41,36 @@ const char kFailedFirebaseTest[] = "Failed to execute Firebase Test"; const char kInvalidResponse[] = "Invalid JSON response from Firebase Service"; const char kTestSuccess[] = "SUCCESS"; -void SetProtoValue(std::string key, ::google::protobuf::Value &value, +const std::string GetFirebaseServer(context::RequestContext &context) { + return context.service_context() + ->config() + ->server_config() + ->api_check_security_rules_config() + .firebase_server(); +} + +void SetProtoValue(const std::string &key, + const ::google::protobuf::Value &value, ::google::protobuf::Value *head) { ::google::protobuf::Struct *s = head->mutable_struct_value(); Map *fields = s->mutable_fields(); (*fields)[key] = value; } -const std::string GetReleaseName( - std::shared_ptr request_context) { - return request_context->service_context()->service_name() + ":" + - request_context->service_context()->service().apis(0).version(); +const std::string GetReleaseName(context::RequestContext &context) { + return context.service_context()->service_name() + ":" + + context.service_context()->service().apis(0).version(); } -const std::string GetRulesetTestUri(const std::string &ruleset_id) { - return std::string(kFirebaseServerStaging) + "v1/" + ruleset_id + - ":test?alt=json"; +const std::string GetRulesetTestUri(context::RequestContext &context, + const std::string &ruleset_id) { + return GetFirebaseServer(context) + "v1/" + ruleset_id + ":test?alt=json"; } -const std::string GetReleaseUrl( - std::shared_ptr request_context) { - return std::string(kFirebaseServerStaging) + "v1/projects/" + - request_context->service_context()->project_id() + "/releases/" + - GetReleaseName(request_context); +const std::string GetReleaseUrl(context::RequestContext &context) { + return GetFirebaseServer(context) + "v1/projects/" + + context.service_context()->project_id() + "/releases/" + + GetReleaseName(context); } std::string GetOperation(std::string &httpMethod) { @@ -105,11 +112,11 @@ class AuthzChecker : public std::enable_shared_from_this { std::string *ruleset_id); // Parses the response for the TEST API call - Status ParseTestResponse(std::shared_ptr context, + Status ParseTestResponse(context::RequestContext &context, const std::string &json_str); // Builds the request body for the TESP API call. - Status BuildTestRequestBody(std::shared_ptr context, + Status BuildTestRequestBody(context::RequestContext &context, std::string *result_string); // Invoke the HTTP call @@ -139,18 +146,16 @@ void AuthzChecker::Check( // TODO: Check service config to see if "useSecurityRules" is specified. // If so, call Firebase Rules service TestRuleset API. - if (!context->service_context()->RequireRulesCheck() || + if (!context->service_context()->IsRulesCheckEnabled() || context->method() == nullptr || !context->method()->auth()) { - env_->LogDebug( - "Autherization and JWT validation was not performed" - " skipping authorization"); + env_->LogDebug("Skipping Firebase Rules checks since it is disabled."); final_continuation(Status::OK); return; } // Fetch the Release attributes. auto checker = GetPtr(); - HttpFetch(GetReleaseUrl(context), "GET", "", + HttpFetch(GetReleaseUrl(*context.get()), "GET", "", [context, final_continuation, checker](Status status, std::string &&body) { std::string ruleset_id; @@ -160,7 +165,7 @@ void AuthzChecker::Check( status = checker->ParseReleaseResponse(body, &ruleset_id); } else { checker->env_->LogError(std::string("GetReleaseName for ") + - GetReleaseUrl(context) + + GetReleaseUrl(*context.get()) + " with status " + status.ToString()); status = Status(Code::INTERNAL, kFailedFirebaseReleaseFetch); } @@ -179,21 +184,21 @@ void AuthzChecker::CallTest(const std::string &ruleset_id, std::shared_ptr context, std::function continuation) { std::string body; - Status status = BuildTestRequestBody(context, &body); + Status status = BuildTestRequestBody(*context.get(), &body); if (!status.ok()) { continuation(status); return; } auto checker = GetPtr(); - HttpFetch(GetRulesetTestUri(ruleset_id), "POST", body, + HttpFetch(GetRulesetTestUri(*context.get(), ruleset_id), "POST", body, [context, continuation, checker, ruleset_id](Status status, std::string &&body) { if (status.ok()) { checker->env_->LogDebug( std::string("Test API succeeded with ") + body); - status = checker->ParseTestResponse(context, body); + status = checker->ParseTestResponse(*context.get(), body); } else { checker->env_->LogError(std::string("Test API failed with ") + status.ToString()); @@ -215,7 +220,7 @@ Status AuthzChecker::ParseReleaseResponse(const std::string &json_str, Status status = Status::OK; const char *id = GetStringValue(json, "rulesetName"); - (*ruleset_id) = (id == nullptr) ? "" : id; + *ruleset_id = (id == nullptr) ? "" : id; if (ruleset_id->empty()) { env_->LogError("Empty ruleset Id received from firebase service"); @@ -228,9 +233,8 @@ Status AuthzChecker::ParseReleaseResponse(const std::string &json_str, return status; } -Status AuthzChecker::ParseTestResponse( - std::shared_ptr context, - const std::string &json_str) { +Status AuthzChecker::ParseTestResponse(context::RequestContext &context, + const std::string &json_str) { grpc_json *json = grpc_json_parse_string_with_len( const_cast(json_str.data()), json_str.length()); @@ -254,9 +258,9 @@ Status AuthzChecker::ParseTestResponse( } else if (std::string(result) != kTestSuccess) { status = Status(Code::PERMISSION_DENIED, std::string("Unauthorized ") + - context->request()->GetRequestHTTPMethod() + + context.request()->GetRequestHTTPMethod() + " access to resource " + - context->request()->GetRequestPath(), + context.request()->GetRequestPath(), Status::AUTH); } } @@ -265,15 +269,14 @@ Status AuthzChecker::ParseTestResponse( return status; } -Status AuthzChecker::BuildTestRequestBody( - std::shared_ptr context, - std::string *result_string) { +Status AuthzChecker::BuildTestRequestBody(context::RequestContext &context, + std::string *result_string) { proto::TestRulesetRequest request; auto *test_case = request.add_test_cases(); - auto httpMethod = context->request()->GetRequestHTTPMethod(); + auto httpMethod = context.request()->GetRequestHTTPMethod(); - test_case->set_service_name(context->service_context()->service_name()); - test_case->set_resource_path(context->request()->GetRequestPath()); + test_case->set_service_name(context.service_context()->service_name()); + test_case->set_resource_path(context.request()->GetRequestPath()); test_case->set_operation(GetOperation(httpMethod)); test_case->set_expectation(proto::TestRulesetRequest::TestCase::ALLOW); @@ -281,7 +284,7 @@ Status AuthzChecker::BuildTestRequestBody( ::google::protobuf::Value token; ::google::protobuf::Value claims; - Status status = utils::JsonToProto(context->auth_claims(), &claims); + Status status = utils::JsonToProto(context.auth_claims(), &claims); if (!status.ok()) { env_->LogError(std::string("Error creating Protobuf from claims") + status.ToString()); @@ -291,8 +294,7 @@ Status AuthzChecker::BuildTestRequestBody( SetProtoValue("token", claims, &token); SetProtoValue("auth", token, &auth); - Map *variables = - test_case->mutable_variables(); + auto *variables = test_case->mutable_variables(); (*variables)["request"] = auth; status = @@ -311,8 +313,8 @@ void AuthzChecker::HttpFetch( const std::string &url, const std::string &method, const std::string &request_body, std::function continuation) { - env_->LogInfo(std::string("Issue HTTP Request to url :") + url + - " method : " + method + " body: " + request_body); + env_->LogDebug(std::string("Issue HTTP Request to url :") + url + + " method : " + method + " body: " + request_body); std::unique_ptr request(new HTTPRequest([continuation]( Status status, std::map &&, diff --git a/contrib/endpoints/src/api_manager/context/service_context.cc b/contrib/endpoints/src/api_manager/context/service_context.cc index 057f196ed3e..d5b6cc5c341 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.cc +++ b/contrib/endpoints/src/api_manager/context/service_context.cc @@ -60,11 +60,7 @@ ServiceContext::ServiceContext(std::unique_ptr env, is_auth_force_disabled_(config_->server_config() && config_->server_config() ->api_authentication_config() - .force_disable()), - is_check_security_rules_disabled_(config_->server_config() && - config_->server_config() - ->api_check_security_rules_config() - .force_disable()) { + .force_disable()) { intermediate_report_interval_ = kIntermediateReportInterval; // Check server_config override. diff --git a/contrib/endpoints/src/api_manager/context/service_context.h b/contrib/endpoints/src/api_manager/context/service_context.h index 31d01acda3d..29de9a508e6 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.h +++ b/contrib/endpoints/src/api_manager/context/service_context.h @@ -65,8 +65,12 @@ class ServiceContext { return !is_auth_force_disabled_ && config_->HasAuth(); } - bool RequireRulesCheck() const { - return RequireAuth() && !is_check_security_rules_disabled_; + bool IsRulesCheckEnabled() const { + return RequireAuth() && config_->server_config() && + !config_->server_config() + ->api_check_security_rules_config() + .firebase_server() + .empty(); } auth::Certs &certs() { return certs_; } @@ -130,9 +134,6 @@ class ServiceContext { // Is auth force-disabled bool is_auth_force_disabled_; - // Is check security rules disabled - bool is_check_security_rules_disabled_; - // The time interval for grpc intermediate report. int64_t intermediate_report_interval_; }; diff --git a/contrib/endpoints/src/api_manager/proto/server_config.proto b/contrib/endpoints/src/api_manager/proto/server_config.proto index a6c03a91a3c..9e325c3f1a0 100644 --- a/contrib/endpoints/src/api_manager/proto/server_config.proto +++ b/contrib/endpoints/src/api_manager/proto/server_config.proto @@ -150,9 +150,6 @@ message ApiAuthenticationConfig { message ApiCheckSecurityRulesConfig { // Firebase server to use. string firebase_server = 1; - - // Allows to disable the API authorization - bool force_disable = 2; } message MixerOptions { From b871e40e0e76ace79bddc10bbbceff01ea9d17b7 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 2 Feb 2017 09:32:19 -0800 Subject: [PATCH 14/21] Address more review comments. --- .../auth/lib/auth_jwt_validator.cc | 4 +- .../src/api_manager/check_security_rules.cc | 78 ++++++++++++------- .../src/api_manager/context/request_context.h | 4 +- .../api_manager/context/service_context.cc | 10 ++- .../src/api_manager/context/service_context.h | 1 + 5 files changed, 61 insertions(+), 36 deletions(-) diff --git a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc index 2673594d14a..abe298a8f74 100644 --- a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc +++ b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc @@ -703,7 +703,9 @@ grpc_jwt_verifier_status JwtValidatorImpl::FillUserInfoAndSetExp( char *json_str = grpc_json_dump_to_string(const_cast<::grpc_json *>(grpc_json), 0); user_info->claims = json_str == nullptr ? "" : json_str; - gpr_free(json_str); + if (json_str != nullptr) { + gpr_free(json_str); + } const char *email = GetStringValue(grpc_json, "email"); user_info->email = email == nullptr ? "" : email; diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index 4c034df7713..f515bf8f401 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -33,15 +33,33 @@ namespace { const char kFirebaseServerStaging[] = "https://staging-firebaserules.sandbox.googleapis.com/"; -const char kFirebaseService[] = - "/google.firebase.rules.v1.FirebaseRulesService"; - const char kFailedFirebaseReleaseFetch[] = "Failed to fetch Firebase Release"; const char kFailedFirebaseTest[] = "Failed to execute Firebase Test"; const char kInvalidResponse[] = "Invalid JSON response from Firebase Service"; const char kTestSuccess[] = "SUCCESS"; - -const std::string GetFirebaseServer(context::RequestContext &context) { +const char kHttpGetMethod[] = "GET"; +const char kHttpPostMethod[] = "POST"; +const char kHttpHeadMethod[] = "HEAD"; +const char kHttpOptionsMethod[] = "OPTIONS"; +const char kHttpDeleteMethod[] = "DELETE"; +const char kFirebaseCreateMethod[] = "create"; +const char kFirebaseGetMethod[] = "get"; +const char kFirebaseDeleteMethod[] = "delete"; +const char kFirebaseUpdateMethod[] = "update"; +const char kV1[] = "v1/"; +const char kTestQuery[] = ":test?alt=json"; +const char kProjects[] = "projects/"; +const char kReleases[] = "/releases/"; +const char kRulesetName[] = "rulesetName"; +const char kTestResults[] = "testResults"; +const char kState[] = "state"; +const char kToken[] = "token"; +const char kAuth[] = "auth"; +const char kRequest[] = "request"; +const char kContentType[] = "Content-Type"; +const char kApplication[] = "application/json"; + +const std::string GetFirebaseServer(const context::RequestContext &context) { return context.service_context() ->config() ->server_config() @@ -57,36 +75,37 @@ void SetProtoValue(const std::string &key, (*fields)[key] = value; } -const std::string GetReleaseName(context::RequestContext &context) { +const std::string GetReleaseName(const context::RequestContext &context) { return context.service_context()->service_name() + ":" + context.service_context()->service().apis(0).version(); } -const std::string GetRulesetTestUri(context::RequestContext &context, +const std::string GetRulesetTestUri(const context::RequestContext &context, const std::string &ruleset_id) { - return GetFirebaseServer(context) + "v1/" + ruleset_id + ":test?alt=json"; + return GetFirebaseServer(context) + kV1 + ruleset_id + kTestQuery; } -const std::string GetReleaseUrl(context::RequestContext &context) { - return GetFirebaseServer(context) + "v1/projects/" + - context.service_context()->project_id() + "/releases/" + +const std::string GetReleaseUrl(const context::RequestContext &context) { + return GetFirebaseServer(context) + kV1 + kProjects + + context.service_context()->project_id() + kReleases + GetReleaseName(context); } -std::string GetOperation(std::string &httpMethod) { - if (httpMethod == "POST") { - return std::string("create"); +const std::string GetOperation(const std::string &httpMethod) { + if (httpMethod == kHttpPostMethod) { + return kFirebaseCreateMethod; } - if (httpMethod == "GET" || httpMethod == "HEAD" || httpMethod == "OPTIONS") { - return std::string("get"); + if (httpMethod == kHttpGetMethod || httpMethod == kHttpHeadMethod || + httpMethod == kHttpOptionsMethod) { + return kFirebaseGetMethod; } - if (httpMethod == "DELETE") { - return std::string("delete"); + if (httpMethod == kHttpDeleteMethod) { + return kFirebaseDeleteMethod; } - return std::string("update"); + return kFirebaseUpdateMethod; } // An AuthzChecker object is created for every incoming request. It does @@ -155,7 +174,7 @@ void AuthzChecker::Check( // Fetch the Release attributes. auto checker = GetPtr(); - HttpFetch(GetReleaseUrl(*context.get()), "GET", "", + HttpFetch(GetReleaseUrl(*context), kHttpGetMethod, "", [context, final_continuation, checker](Status status, std::string &&body) { std::string ruleset_id; @@ -191,7 +210,7 @@ void AuthzChecker::CallTest(const std::string &ruleset_id, } auto checker = GetPtr(); - HttpFetch(GetRulesetTestUri(*context.get(), ruleset_id), "POST", body, + HttpFetch(GetRulesetTestUri(*context, ruleset_id), kHttpPostMethod, body, [context, continuation, checker, ruleset_id](Status status, std::string &&body) { @@ -219,7 +238,7 @@ Status AuthzChecker::ParseReleaseResponse(const std::string &json_str, } Status status = Status::OK; - const char *id = GetStringValue(json, "rulesetName"); + const char *id = GetStringValue(json, kRulesetName); *ruleset_id = (id == nullptr) ? "" : id; if (ruleset_id->empty()) { @@ -246,12 +265,12 @@ Status AuthzChecker::ParseTestResponse(context::RequestContext &context, Status status = Status::OK; Status invalid = Status(Code::INTERNAL, kInvalidResponse); - const grpc_json *testResults = GetProperty(json, "testResults"); + const grpc_json *testResults = GetProperty(json, kTestResults); if (testResults == nullptr) { env_->LogError("TestResults are null"); status = invalid; } else { - const char *result = GetStringValue(testResults->child, "state"); + const char *result = GetStringValue(testResults->child, kState); if (result == nullptr) { env_->LogInfo("Result state is empty"); status = invalid; @@ -291,11 +310,11 @@ Status AuthzChecker::BuildTestRequestBody(context::RequestContext &context, return status; } - SetProtoValue("token", claims, &token); - SetProtoValue("auth", token, &auth); + SetProtoValue(kToken, claims, &token); + SetProtoValue(kAuth, token, &auth); auto *variables = test_case->mutable_variables(); - (*variables)["request"] = auth; + (*variables)[kRequest] = auth; status = utils::ProtoToJson(request, result_string, utils::JsonOptions::DEFAULT); @@ -327,9 +346,8 @@ void AuthzChecker::HttpFetch( request->set_method(method).set_url(url).set_auth_token(GetAuthToken()); - if (method != "GET") { - request->set_header("Content-Type", "application/json") - .set_body(request_body); + if (method != kHttpGetMethod) { + request->set_header(kContentType, kApplication).set_body(request_body); } env_->RunHTTPRequest(std::move(request)); diff --git a/contrib/endpoints/src/api_manager/context/request_context.h b/contrib/endpoints/src/api_manager/context/request_context.h index 5b8328507a9..c633952a811 100644 --- a/contrib/endpoints/src/api_manager/context/request_context.h +++ b/contrib/endpoints/src/api_manager/context/request_context.h @@ -37,7 +37,9 @@ class RequestContext { std::unique_ptr request); // Get the ApiManagerImpl object. - context::ServiceContext *service_context() { return service_context_.get(); } + context::ServiceContext *service_context() const { + return service_context_.get(); + } // Get the request object. Request *request() { return request_.get(); } diff --git a/contrib/endpoints/src/api_manager/context/service_context.cc b/contrib/endpoints/src/api_manager/context/service_context.cc index d5b6cc5c341..f6dc3d90275 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.cc +++ b/contrib/endpoints/src/api_manager/context/service_context.cc @@ -57,10 +57,12 @@ ServiceContext::ServiceContext(std::unique_ptr env, service_account_token_(env_.get()), service_control_(CreateInterface()), cloud_trace_aggregator_(CreateCloudTraceAggregator()), - is_auth_force_disabled_(config_->server_config() && - config_->server_config() - ->api_authentication_config() - .force_disable()) { + is_auth_force_disabled_( + config_->server_config() && + config_->server_config()->has_api_authentication_config() && + config_->server_config() + ->api_authentication_config() + .force_disable()) { intermediate_report_interval_ = kIntermediateReportInterval; // Check server_config override. diff --git a/contrib/endpoints/src/api_manager/context/service_context.h b/contrib/endpoints/src/api_manager/context/service_context.h index 29de9a508e6..7c285c665b3 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.h +++ b/contrib/endpoints/src/api_manager/context/service_context.h @@ -67,6 +67,7 @@ class ServiceContext { bool IsRulesCheckEnabled() const { return RequireAuth() && config_->server_config() && + config_->server_config()->has_api_authentication_config() && !config_->server_config() ->api_check_security_rules_config() .firebase_server() From 71c0d92fc644286003ce81da96b583db22fb5495 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 2 Feb 2017 10:03:28 -0800 Subject: [PATCH 15/21] Fix a check. --- contrib/endpoints/src/api_manager/context/service_context.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/endpoints/src/api_manager/context/service_context.h b/contrib/endpoints/src/api_manager/context/service_context.h index 7c285c665b3..76b70a81de9 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.h +++ b/contrib/endpoints/src/api_manager/context/service_context.h @@ -67,7 +67,7 @@ class ServiceContext { bool IsRulesCheckEnabled() const { return RequireAuth() && config_->server_config() && - config_->server_config()->has_api_authentication_config() && + config_->server_config()->has_api_check_security_rules_config() && !config_->server_config() ->api_check_security_rules_config() .firebase_server() From 50955e38c05a78b2570d09c09e52cdc13ed33504 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 2 Feb 2017 10:06:16 -0800 Subject: [PATCH 16/21] Delete unwanted constant. --- contrib/endpoints/src/api_manager/check_security_rules.cc | 3 --- 1 file changed, 3 deletions(-) diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index f515bf8f401..e8a76d04b19 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -30,9 +30,6 @@ namespace google { namespace api_manager { namespace { -const char kFirebaseServerStaging[] = - "https://staging-firebaserules.sandbox.googleapis.com/"; - const char kFailedFirebaseReleaseFetch[] = "Failed to fetch Firebase Release"; const char kFailedFirebaseTest[] = "Failed to execute Firebase Test"; const char kInvalidResponse[] = "Invalid JSON response from Firebase Service"; From 35a61dfa1d23660ce9e09f3d6eebb3d56e43012f Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 2 Feb 2017 12:37:47 -0800 Subject: [PATCH 17/21] Address Wayne's comments and add a simple config test. --- .../src/api_manager/check_security_rules.cc | 8 ++------ contrib/endpoints/src/api_manager/config.cc | 4 ++++ contrib/endpoints/src/api_manager/config.h | 3 +++ .../endpoints/src/api_manager/config_test.cc | 17 +++++++++++++++++ .../src/api_manager/context/service_context.h | 3 ++- 5 files changed, 28 insertions(+), 7 deletions(-) diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index e8a76d04b19..1a0d8731a18 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -56,12 +56,8 @@ const char kRequest[] = "request"; const char kContentType[] = "Content-Type"; const char kApplication[] = "application/json"; -const std::string GetFirebaseServer(const context::RequestContext &context) { - return context.service_context() - ->config() - ->server_config() - ->api_check_security_rules_config() - .firebase_server(); +const std::string &GetFirebaseServer(const context::RequestContext &context) { + return context.service_context()->config()->GetFirebaseServer(); } void SetProtoValue(const std::string &key, diff --git a/contrib/endpoints/src/api_manager/config.cc b/contrib/endpoints/src/api_manager/config.cc index b0a1b85d119..f758be15173 100644 --- a/contrib/endpoints/src/api_manager/config.cc +++ b/contrib/endpoints/src/api_manager/config.cc @@ -513,5 +513,9 @@ void Config::SetJwksUri(const string &issuer, const string &jwks_uri, } } +const std::string &Config::GetFirebaseServer() { + return server_config_->api_check_security_rules_config().firebase_server(); +} + } // namespace api_manager } // namespace google diff --git a/contrib/endpoints/src/api_manager/config.h b/contrib/endpoints/src/api_manager/config.h index 9a56d16d745..132312b888f 100644 --- a/contrib/endpoints/src/api_manager/config.h +++ b/contrib/endpoints/src/api_manager/config.h @@ -78,6 +78,9 @@ class Config { void SetJwksUri(const std::string &issuer, const std::string &jwks_uri, bool openid_valid); + // Get the Firebase server from Server config + const std::string &GetFirebaseServer(); + private: GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(Config); diff --git a/contrib/endpoints/src/api_manager/config_test.cc b/contrib/endpoints/src/api_manager/config_test.cc index ace0d2afc49..c0aa2c16940 100644 --- a/contrib/endpoints/src/api_manager/config_test.cc +++ b/contrib/endpoints/src/api_manager/config_test.cc @@ -870,6 +870,23 @@ TEST(Config, TestCorsDisabled) { ASSERT_EQ(nullptr, method1); } +TEST(Config, TestFirebaseServerCheck) { + MockApiManagerEnvironmentWithLog env; + + static const char server_config[] = R"( +api_check_security_rules_config { + firebase_server: "https://myfirebaseserver.com/" +} +)"; + + std::unique_ptr config = + Config::Create(&env, kServiceNameConfig, server_config); + ASSERT_TRUE(config); + + auto server = config->GetFirebaseServer(); + ASSERT_EQ(server, "https://myfirebaseserver.com/"); +} + } // namespace } // namespace api_manager diff --git a/contrib/endpoints/src/api_manager/context/service_context.h b/contrib/endpoints/src/api_manager/context/service_context.h index 76b70a81de9..c0c65626eb2 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.h +++ b/contrib/endpoints/src/api_manager/context/service_context.h @@ -66,7 +66,8 @@ class ServiceContext { } bool IsRulesCheckEnabled() const { - return RequireAuth() && config_->server_config() && + return RequireAuth() && service().apis_size() > 0 && + config_->server_config() && config_->server_config()->has_api_check_security_rules_config() && !config_->server_config() ->api_check_security_rules_config() From 4ef505aa2d4f153b5feb96efc1e30b7217cee6c3 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 2 Feb 2017 13:29:54 -0800 Subject: [PATCH 18/21] Address a review comment. --- .../endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc index abe298a8f74..2df9a843b71 100644 --- a/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc +++ b/contrib/endpoints/src/api_manager/auth/lib/auth_jwt_validator.cc @@ -702,8 +702,8 @@ grpc_jwt_verifier_status JwtValidatorImpl::FillUserInfoAndSetExp( char *json_str = grpc_json_dump_to_string(const_cast<::grpc_json *>(grpc_json), 0); - user_info->claims = json_str == nullptr ? "" : json_str; if (json_str != nullptr) { + user_info->claims = json_str; gpr_free(json_str); } From 9dddcac7fa7ebb6d7b4092cf35ace9fd6528cfeb Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 2 Feb 2017 13:42:25 -0800 Subject: [PATCH 19/21] Add negative test case for config --- .../endpoints/src/api_manager/config_test.cc | 26 +++++++++++++++++-- .../src/api_manager/context/service_context.h | 5 +--- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/contrib/endpoints/src/api_manager/config_test.cc b/contrib/endpoints/src/api_manager/config_test.cc index c0aa2c16940..d9170ddf131 100644 --- a/contrib/endpoints/src/api_manager/config_test.cc +++ b/contrib/endpoints/src/api_manager/config_test.cc @@ -883,10 +883,32 @@ api_check_security_rules_config { Config::Create(&env, kServiceNameConfig, server_config); ASSERT_TRUE(config); - auto server = config->GetFirebaseServer(); - ASSERT_EQ(server, "https://myfirebaseserver.com/"); + ASSERT_EQ(config->GetFirebaseServer(), "https://myfirebaseserver.com/"); } +TEST(Config, TestEmptyFirebaseServerCheck) { + MockApiManagerEnvironmentWithLog env; + + static const char server_config[] = R"( +service_control_config { + check_aggregator_config { + cache_entries: 1000 + flush_interval_ms: 10 + response_expiration_ms: 20 + } + report_aggregator_config { + cache_entries: 1020 + flush_interval_ms: 15 + } +} +)"; + + std::unique_ptr config = + Config::Create(&env, kServiceNameConfig, server_config); + ASSERT_TRUE(config); + + ASSERT_TRUE(config->GetFirebaseServer().empty()); +} } // namespace } // namespace api_manager diff --git a/contrib/endpoints/src/api_manager/context/service_context.h b/contrib/endpoints/src/api_manager/context/service_context.h index c0c65626eb2..18761b52cfd 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.h +++ b/contrib/endpoints/src/api_manager/context/service_context.h @@ -69,10 +69,7 @@ class ServiceContext { return RequireAuth() && service().apis_size() > 0 && config_->server_config() && config_->server_config()->has_api_check_security_rules_config() && - !config_->server_config() - ->api_check_security_rules_config() - .firebase_server() - .empty(); + !config_->GetFirebaseServer().empty(); } auth::Certs &certs() { return certs_; } From c35348d8da1d30aab3ca1a4f0a7c7320c5f0e5a4 Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 2 Feb 2017 14:46:24 -0800 Subject: [PATCH 20/21] Address code review --- .../endpoints/src/api_manager/check_security_rules.cc | 11 ++++------- contrib/endpoints/src/api_manager/config.cc | 6 +++++- contrib/endpoints/src/api_manager/config.h | 2 +- .../src/api_manager/context/service_context.h | 2 -- 4 files changed, 10 insertions(+), 11 deletions(-) diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index 1a0d8731a18..81d5cd84d77 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -56,10 +56,6 @@ const char kRequest[] = "request"; const char kContentType[] = "Content-Type"; const char kApplication[] = "application/json"; -const std::string &GetFirebaseServer(const context::RequestContext &context) { - return context.service_context()->config()->GetFirebaseServer(); -} - void SetProtoValue(const std::string &key, const ::google::protobuf::Value &value, ::google::protobuf::Value *head) { @@ -75,12 +71,13 @@ const std::string GetReleaseName(const context::RequestContext &context) { const std::string GetRulesetTestUri(const context::RequestContext &context, const std::string &ruleset_id) { - return GetFirebaseServer(context) + kV1 + ruleset_id + kTestQuery; + return context.service_context()->config()->GetFirebaseServer() + kV1 + + ruleset_id + kTestQuery; } const std::string GetReleaseUrl(const context::RequestContext &context) { - return GetFirebaseServer(context) + kV1 + kProjects + - context.service_context()->project_id() + kReleases + + return context.service_context()->config()->GetFirebaseServer() + kV1 + + kProjects + context.service_context()->project_id() + kReleases + GetReleaseName(context); } diff --git a/contrib/endpoints/src/api_manager/config.cc b/contrib/endpoints/src/api_manager/config.cc index f758be15173..797aeb03bbb 100644 --- a/contrib/endpoints/src/api_manager/config.cc +++ b/contrib/endpoints/src/api_manager/config.cc @@ -513,7 +513,11 @@ void Config::SetJwksUri(const string &issuer, const string &jwks_uri, } } -const std::string &Config::GetFirebaseServer() { +const std::string Config::GetFirebaseServer() { + if (server_config_ == nullptr) { + return ""; + } + return server_config_->api_check_security_rules_config().firebase_server(); } diff --git a/contrib/endpoints/src/api_manager/config.h b/contrib/endpoints/src/api_manager/config.h index 132312b888f..23877813e81 100644 --- a/contrib/endpoints/src/api_manager/config.h +++ b/contrib/endpoints/src/api_manager/config.h @@ -79,7 +79,7 @@ class Config { bool openid_valid); // Get the Firebase server from Server config - const std::string &GetFirebaseServer(); + const std::string GetFirebaseServer(); private: GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(Config); diff --git a/contrib/endpoints/src/api_manager/context/service_context.h b/contrib/endpoints/src/api_manager/context/service_context.h index 18761b52cfd..61524813157 100644 --- a/contrib/endpoints/src/api_manager/context/service_context.h +++ b/contrib/endpoints/src/api_manager/context/service_context.h @@ -67,8 +67,6 @@ class ServiceContext { bool IsRulesCheckEnabled() const { return RequireAuth() && service().apis_size() > 0 && - config_->server_config() && - config_->server_config()->has_api_check_security_rules_config() && !config_->GetFirebaseServer().empty(); } From 4ccc60603f8532e760bc90184b2a5a3fe825d8ce Mon Sep 17 00:00:00 2001 From: Sarvani Vakkalanka Date: Thu, 2 Feb 2017 15:16:40 -0800 Subject: [PATCH 21/21] Remove unwanted const std::string --- .../endpoints/src/api_manager/check_security_rules.cc | 10 +++++----- contrib/endpoints/src/api_manager/config.cc | 2 +- contrib/endpoints/src/api_manager/config.h | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/contrib/endpoints/src/api_manager/check_security_rules.cc b/contrib/endpoints/src/api_manager/check_security_rules.cc index 81d5cd84d77..45fa30ae7bd 100644 --- a/contrib/endpoints/src/api_manager/check_security_rules.cc +++ b/contrib/endpoints/src/api_manager/check_security_rules.cc @@ -64,24 +64,24 @@ void SetProtoValue(const std::string &key, (*fields)[key] = value; } -const std::string GetReleaseName(const context::RequestContext &context) { +std::string GetReleaseName(const context::RequestContext &context) { return context.service_context()->service_name() + ":" + context.service_context()->service().apis(0).version(); } -const std::string GetRulesetTestUri(const context::RequestContext &context, - const std::string &ruleset_id) { +std::string GetRulesetTestUri(const context::RequestContext &context, + const std::string &ruleset_id) { return context.service_context()->config()->GetFirebaseServer() + kV1 + ruleset_id + kTestQuery; } -const std::string GetReleaseUrl(const context::RequestContext &context) { +std::string GetReleaseUrl(const context::RequestContext &context) { return context.service_context()->config()->GetFirebaseServer() + kV1 + kProjects + context.service_context()->project_id() + kReleases + GetReleaseName(context); } -const std::string GetOperation(const std::string &httpMethod) { +std::string GetOperation(const std::string &httpMethod) { if (httpMethod == kHttpPostMethod) { return kFirebaseCreateMethod; } diff --git a/contrib/endpoints/src/api_manager/config.cc b/contrib/endpoints/src/api_manager/config.cc index 797aeb03bbb..d537ef73e07 100644 --- a/contrib/endpoints/src/api_manager/config.cc +++ b/contrib/endpoints/src/api_manager/config.cc @@ -513,7 +513,7 @@ void Config::SetJwksUri(const string &issuer, const string &jwks_uri, } } -const std::string Config::GetFirebaseServer() { +std::string Config::GetFirebaseServer() { if (server_config_ == nullptr) { return ""; } diff --git a/contrib/endpoints/src/api_manager/config.h b/contrib/endpoints/src/api_manager/config.h index 23877813e81..82586939eb0 100644 --- a/contrib/endpoints/src/api_manager/config.h +++ b/contrib/endpoints/src/api_manager/config.h @@ -79,7 +79,7 @@ class Config { bool openid_valid); // Get the Firebase server from Server config - const std::string GetFirebaseServer(); + std::string GetFirebaseServer(); private: GOOGLE_DISALLOW_EVIL_CONSTRUCTORS(Config);