Allow for HTTP based function from Firebase rules #202
Conversation
|
Jenkins job proxy/presubmit passed |
4 similar comments
|
Jenkins job proxy/presubmit passed |
|
Jenkins job proxy/presubmit passed |
|
Jenkins job proxy/presubmit passed |
|
Jenkins job proxy/presubmit passed |
|
Jenkins job proxy/presubmit passed |
1 similar comment
|
Jenkins job proxy/presubmit passed |
| return func_call_iter_ == response_.test_results(0).function_calls().end(); | ||
| } | ||
|
|
||
| Status FirebaseRequest::AddFunctionMock(proto::TestRulesetRequest *request, |
There was a problem hiding this comment.
Is this for testing? If so, it should be in a separate file for testing. We don't want to mix testing code with real code.
There was a problem hiding this comment.
No This is not for testing. These are private methods in the class. The FunctionMock is the name of a message in the TestRulesetRequest protobuf.
| TestRulesetResponse response_; | ||
| HttpRequest *next_request_; | ||
| HttpRequest firebase_http_request_; | ||
| HttpRequest external_http_request_; |
There was a problem hiding this comment.
what is the different between external_http_request and firebase_http_request? can we share them? Are they run parallel?
Need more comment.
There was a problem hiding this comment.
They are not run in parallel. I will add more comments.
|
Jenkins job proxy/presubmit passed |
1 similar comment
|
Jenkins job proxy/presubmit passed |
|
Jenkins job proxy/presubmit passed |
| @@ -65,6 +65,7 @@ class ServiceAccountToken { | |||
| JWT_TOKEN_FOR_SERVICE_CONTROL = 0, | |||
| JWT_TOKEN_FOR_CLOUD_TRACING, | |||
| JWT_TOKEN_FOR_FIREBASE, | |||
| JWT_TOKEN_FOR_AUTHORIZATION_SERVICE, | |||
There was a problem hiding this comment.
Let's add a comment for this one, like "JWT token for accessing the http endpoints that user defines in Firebase Rules".
| Status status = BuildTestRequestBody(*context.get(), &body); | ||
| if (!status.ok()) { | ||
| continuation(status); | ||
| void AuthzChecker::CallNextRequest( |
There was a problem hiding this comment.
Can you add a comment for what the function is doing?
There was a problem hiding this comment.
There is a comment that already exists in the class declaration. look at line 69. Let me know if you need me to add anything more to it.
| .authorization_service_audience() | ||
| .empty()) { | ||
| service_account_token_.SetAudience( | ||
| auth::ServiceAccountToken::JWT_TOKEN_FOR_AUTHORIZATION_SERVICE, |
There was a problem hiding this comment.
As we discussed, I think it makes sense to ask API producer to enter the accepted audience as a http.invoke() function parameter, instead of having it statically in ESP config. There are two advantages. First, the rules are allowed to call multiple http endpoints, each of which accepts different JWT audience. Second, ESP service config is not bound to rules logic. API producer can change the rules to call a different http endpoint without the need to change ESP config and redeploy.
There was a problem hiding this comment.
While that was what I had initially suggested we do about the audiences, I started having second thoughts once I started writing the firebase rules. For example, the fact that we are using something called "audiences" to generated an auth token is ESP specific and we are only generating these for Google based services. I am not sure how this will translate to custom auth cases. I think we should think more about this and ask experts on the best way to do this. But this change should not affect this PR. We can address this as a part of another PR .
There was a problem hiding this comment.
"aud" is a standard JWT claim. It's not ESP specific. Any service that accepts JWT defines what audience it accepts. Google services accept JWT audience that matches the service name. Not all services do the same, and the service name can have different formats. That's why we need to ask API producers to specify the accepted audience in the function specification when they write rules.
I am ok with having this change in a separate PR.
There was a problem hiding this comment.
Yes. aud is a standard JWT claim. My argument is not that the users don't understand JWTs except that having to provide "aud" here is more confusing. Here is the reason why I think it is more confusing. As a firebase rules author, my thought would be that the user is already authenticated since the user has already provided a valid JWT. Now, the rules file will used the JWT claims to authroize the user. But instead of doing just that, we are also forcing the user to think more about what ESP is going to do which is generate more JWTs. I also don't think it is limiting the user from using multiple authroization sources like cloud datastore AND cloud SQL. Actually, we should encourage our users to not have their firebase rules tied to their datastore schema. This is really painful for the user in the long term for both providing the rules and well as schema changes. Instead, the rules should access some endpoint that is executed as a part of app running in say AppEngine. This allows them to use the provided client libraries that allow for better testing and maintenance. Lets discuss more with Wencheng and Wayne.
| @@ -0,0 +1,41 @@ | |||
| # Copyright 2016 Google Inc. All Rights Reserved. | |||
| } | ||
|
|
||
| // Convert HTTP method to Firebase specific method. | ||
| std::string GetOperation(const std::string &httpMethod) { |
There was a problem hiding this comment.
nit return const std::string&
There was a problem hiding this comment.
As I thought. This gives a bunch of warnings. Looks like the compiler is not liking this even if I am returing on const strings that are already declared ...
contrib/endpoints/src/api_manager/firebase_rules/firebase_request.cc:61:12: warning: returning reference to temporary [-Wreturn-local-addr]
return kFirebaseCreateMethod;
^
contrib/endpoints/src/api_manager/firebase_rules/firebase_request.cc:66:12: warning: returning reference to temporary [-Wreturn-local-addr]
return kFirebaseGetMethod;
^
contrib/endpoints/src/api_manager/firebase_rules/firebase_request.cc:70:12: warning: returning reference to temporary [-Wreturn-local-addr]
return kFirebaseDeleteMethod;
^
contrib/endpoints/src/api_manager/firebase_rules/firebase_request.cc:73:10: warning: returning reference to temporary [-Wreturn-local-addr]
return kFirebaseUpdateMethod;
There was a problem hiding this comment.
I think if you define your const as
const std::string kFirebaseCreateMethod, then you can return const std::string&. It will not do data copy.
There was a problem hiding this comment.
Here is Google's c++ style guide's decision on global or static std::string objects. To my understanding, this seems conflicting advice. Let me know if we have decided not to follow this guideline and I am also curious as to why we have decided this is not the best thing to do ...
There was a problem hiding this comment.
I don't like it either. I think we have code using std::string as static const variables.
| next_request_ = &firebase_http_request_; | ||
| } | ||
|
|
||
| bool FirebaseRequest::IsDone() { return is_done_; } |
There was a problem hiding this comment.
nit: is_done() for accessors.
| const ::google::protobuf::Value &value, | ||
| ::google::protobuf::Value *head) { | ||
| ::google::protobuf::Struct *s = head->mutable_struct_value(); | ||
| Map<std::string, google::protobuf::Value> *fields = s->mutable_fields(); |
There was a problem hiding this comment.
nit: auto fields = head->mutable_struct_value()->mutable_fields()
|
Jenkins job proxy/presubmit passed |
| @@ -17,14 +17,12 @@ | |||
| #include <iostream> | |||
| #include <sstream> | |||
| #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/firebase_rules/firebase_request.h" | |||
There was a problem hiding this comment.
why does the new file have to be on its own folder? Can it be part of auth folder? Or should we create a new folder "authz" for all future authorization files?
There was a problem hiding this comment.
I initially started off with authz but then decided to firebase_rules since this is very explicit as to what is being used.
- I don't think auth is the right place for this since authentication and authorization are two different things.
- Here are few ways things can go:
a) If we continue using firebase rules for authorization, then this is directory is still good.
b) If we decide to support another form of authz along with firebase rules, then I think we can create a directory called authz and move the firebase_rules under that directory and add another directory that appropriately names the authorization mechanism we will support.
c) We decide to discontinue using firebase rules, then we delete this directory.
I think keeping this as firebase_rules makes a lot of sense since that is what we are doing.
| auth::ServiceAccountToken::JWT_TOKEN_FOR_FIREBASE); | ||
| const std::string &GetAuthToken( | ||
| auth::ServiceAccountToken::JWT_TOKEN_TYPE token_type) { | ||
| return sa_token_->GetAuthToken(token_type); |
There was a problem hiding this comment.
is this function necessary? it did not do anything.
| } | ||
|
|
||
| std::shared_ptr<AuthzChecker> GetPtr() { return shared_from_this(); } | ||
|
|
||
| ApiManagerEnvInterface *env_; | ||
| auth::ServiceAccountToken *sa_token_; | ||
| std::unique_ptr<FirebaseRequest> request_; |
There was a problem hiding this comment.
Does this object handle single request? or it is a state machine, handling a complex logic. If the latter, should it be call FirebaseHandler?
| HttpFetch(GetRulesetTestUri(*context, ruleset_id), kHttpPostMethod, body, | ||
| [context, continuation, checker, ruleset_id](Status status, | ||
| std::string &&body) { | ||
| auto httpRequest = request_->GetHttpRequest(); |
There was a problem hiding this comment.
style, it should be http_request.
prefer to list the type here instead of auto. I like to know which http request type is used since there are so many http request type now.
| // then the reponse provided via UpdateResponse is converted into a | ||
| // protobuf::Value. This value is initialized to nullptr and will be nullptr | ||
| // once is_done_ is set to true. | ||
| HttpRequest *next_request_; |
There was a problem hiding this comment.
this implies state. Should we define a state variable for that?
There was a problem hiding this comment.
We don't need the state variable. I have updated the comments so they refer to the exact method and no reference to a State Machine.
| } | ||
|
|
||
| // Convert HTTP method to Firebase specific method. | ||
| std::string GetOperation(const std::string &httpMethod) { |
There was a problem hiding this comment.
I think if you define your const as
const std::string kFirebaseCreateMethod, then you can return const std::string&. It will not do data copy.
| using ::google::api_manager::utils::Status; | ||
| using TestRulesetResponse = ::google::api_manager::proto::TestRulesetResponse; | ||
| using FunctionCall = TestRulesetResponse::TestResult::FunctionCall; | ||
| using ::google::protobuf::RepeatedPtrField; |
There was a problem hiding this comment.
per c++ style guide, name space alias should not be used in the header file. It can only be used in the cc file. Otherwise, it will easily cause confusion.
|
|
||
| // Update the first request to be sent which is the TestRulesetRequest | ||
| // request. | ||
| SetStatus(UpdateRulesetRequestBody(RepeatedPtrField<FunctionCall>())); |
There was a problem hiding this comment.
Here I am confused now. It will help if you can add comment about the data flow:
What ESP should do first, what Firebase will response, and how ESP will handle the response.
There was a problem hiding this comment.
I will add the comments. So far as const std::string goes, let me know about why we are not following the google style guide in this case as described here:
There was a problem hiding this comment.
I have added the full message flow sequence in the header file. Let me know if you think this makes it better to understand the flow.
|
Jenkins job proxy/presubmit passed |
| std::function<void(Status status)> continuation); | ||
| // This method invokes the Firebase TestRuleset API endpoint as well as user | ||
| // defined endpoints provided by the TestRulesetResponse. | ||
| void CallNextRequest(std::function<void(Status status)> continuation); | ||
|
|
||
| // Parse the respose for GET RELEASE API call |
| @@ -182,39 +129,38 @@ void AuthzChecker::Check( | |||
| // If the parsing of the release body is successful, then call the | |||
| // Test Api for firebase rules service. | |||
| if (status.ok()) { | |||
There was a problem hiding this comment.
This if-else clause can be combined with the if-else clause above.
There was a problem hiding this comment.
That is another way but here is how that code will look.
if (status.ok()) {
checker->env_->LogDebug(
std::string("GetReleasName succeeded with ") + body);
status = checker->ParseReleaseResponse(body, &ruleset_id);
if (status.ok()) {
checker->request_handler_ = std::unique_ptr(
new FirebaseRequest(ruleset_id, checker->env_, context));
checker->CallNextRequest(final_continuation);
} else { // I cannot remove this else statement since this will not go into the else part below
final_continuation(status)
}
} else {
checker->env_->LogError(std::string("GetReleaseName for ") +
GetReleaseUrl(*context.get()) +
" with status " + status.ToString());
status = Status(Code::INTERNAL, kFailedFirebaseReleaseFetch);
}
The code does not look as good with status checks within status checks. Which is why I have written code in the way it is.
| std::function<void(Status status)> continuation); | ||
| // This method invokes the Firebase TestRuleset API endpoint as well as user | ||
| // defined endpoints provided by the TestRulesetResponse. | ||
| void CallNextRequest(std::function<void(Status status)> continuation); | ||
|
|
||
| // Parse the respose for GET RELEASE API call | ||
| Status ParseReleaseResponse(const std::string &json_str, | ||
| std::string *ruleset_id); |
There was a problem hiding this comment.
use std::string & instead of std::string *.
There was a problem hiding this comment.
Here is the Google syle guide link that says output variables should be pointers. The ParseReleaseResponse will set the ruleset_id and making it a pointer is a way of saying that it will be modified. It should also be the last parameter.
| auto *fields = variables->mutable_fields(); | ||
|
|
||
| path.set_string_value(context_->request()->GetRequestPath()); | ||
| (*fields)[kPath] = path; |
There was a problem hiding this comment.
You can also use SetProtoValue() function here.
There was a problem hiding this comment.
No. SetProtoValue will not work here. This is because of the the way mutable_* works. Since I need all the kPath and kAuth in the same map, I cannot use SetProtoValue.
| (*fields)[kMethod] = method; | ||
|
|
||
| SetProtoValue(kToken, claims, &token); | ||
| (*fields)[kAuth] = token; |
There was a problem hiding this comment.
Same reasoning as above.
|
Jenkins job proxy/presubmit passed |
| FirebaseRequest::Find(const FunctionCall &func_call) { | ||
| return std::find_if(funcs_with_result_.begin(), funcs_with_result_.end(), | ||
| [func_call](std::tuple<FunctionCall, std::string> item) { | ||
| return MessageDifferencer::Equals(std::get<0>(item), |
There was a problem hiding this comment.
Can we use a map with string key as serialized protobuf? Compare string should be much faster than this equal.
There was a problem hiding this comment.
As we discussed, creating an string and then comparing is better than just comparing. When ever we get a TestRulesetResponse, creating a serialized String to generate the string key and them trying to do the comparison will be more slower.
| } | ||
|
|
||
| // Convert HTTP method to Firebase specific method. | ||
| std::string GetOperation(const std::string &httpMethod) { |
There was a problem hiding this comment.
I don't like it either. I think we have code using std::string as static const variables.
| return; | ||
| } | ||
|
|
||
| if (next_request_ == nullptr) { |
There was a problem hiding this comment.
we should use GOOGLE_DCHECK() here then.
| Status FirebaseRequest::RequestStatus() { return current_status_; } | ||
|
|
||
| void FirebaseRequest::UpdateResponse(const std::string &body) { | ||
| if (is_done()) { |
There was a problem hiding this comment.
is it is code bug? it should be GOOGLE_DCHECK()
|
Jenkins job proxy/presubmit passed |
* Created check security rules file and a few dummy/helper functions. (#40) * Created check security rules file and a few dummy/helper functions. And added it to check work flow. * Fix format. * Firebase: Merge from master. (#53) * Simple TCP server to show how to retrieve original dest IP:port after an iptables redirect (#38) * Simple TCP server to show how to retrieve original dest IP:port after an iptables redirect * Fixed style. * Rebase Envoy (#41) * Update prototype to use iptables (#42) * Rebase to fixed Envoy (#43) * Handle HEAD request. (#34) * Handle HEAD request. * Try with GET if HEAD fails. * Address comments. * Format file. * Expose bazel target (#48) * Try again (#49) * Enable ESP to invoke Firebase Security rules. (#54) * Enable ESP to invoke Firebase Security rules. * Address code review comments. * Remove some debug logs * Add proto file to capture TestRulesetRequest. * clang-format files * Resolve a merge issue with previous commit * Allow security rules to disabled via serverconfig * format file * Addressed Wayne's review comments. * Add firebase server to Server Config. * Address Lizan's review comments * Address review comments. * Disable check rules service by default. * Address more review comments. * Fix a check. * Delete unwanted constant. * Address Wayne's comments and add a simple config test. * Address a review comment. * Add negative test case for config * Address code review * Remove unwanted const std::string * Merge from master into firebase (#65) * Simple TCP server to show how to retrieve original dest IP:port after an iptables redirect (#38) * Simple TCP server to show how to retrieve original dest IP:port after an iptables redirect * Fixed style. * Rebase Envoy (#41) * Update prototype to use iptables (#42) * Rebase to fixed Envoy (#43) * Handle HEAD request. (#34) * Handle HEAD request. * Try with GET if HEAD fails. * Address comments. * Format file. * Expose bazel target (#48) * Try again (#49) * Integrate with mixer client. (#55) * Integrate with mixer client. * Restore repositories.bzl back. * Add originIp and originHost attributes. (#56) * Add uuid-dev dependency in README.md (#45) * Extract originIp and OriginHost. (#57) * Extract originIp and OriginHost. * Make header x-forwarded-host const. * Update buckets for UI. (#58) * Update buckets for UI. * Only update time_distribution. * Add targetService attribute. (#59) * Use envoy new access_log handler for sending Report. (#60) * use access_log handler. * Not to use Loggable base class. * Update to the latest envoy with #396. (#61) * Fix tclap dependency fetching error (#62) * Update the auth checke to use service.experimental.authorization.providerwq! * Update the auth check to use service.experimental.authorization.provider * Update the auth check to use service.experimental.authorization.provider (#67) * Update the auth check to use service.experimental.authorization.provider * Address comments and revert accidental change. * Remove unnecessary added accidentally. * Another patch * fix the logic * fix lint * Fix broken test and add unit tests * Fix comments * Fix style check * revert style for raw string * fix small lint * fix small lint * fix small lint * Unit tests for check security rules. (#75) * Unit tests for check security rules. * format * Address review comments. * Fix typos * Merge from master to firebase (#143) * Simple TCP server to show how to retrieve original dest IP:port after an iptables redirect (#38) * Simple TCP server to show how to retrieve original dest IP:port after an iptables redirect * Fixed style. * Rebase Envoy (#41) * Update prototype to use iptables (#42) * Rebase to fixed Envoy (#43) * Handle HEAD request. (#34) * Handle HEAD request. * Try with GET if HEAD fails. * Address comments. * Format file. * Expose bazel target (#48) * Try again (#49) * Integrate with mixer client. (#55) * Integrate with mixer client. * Restore repositories.bzl back. * Add originIp and originHost attributes. (#56) * Add uuid-dev dependency in README.md (#45) * Extract originIp and OriginHost. (#57) * Extract originIp and OriginHost. * Make header x-forwarded-host const. * Update buckets for UI. (#58) * Update buckets for UI. * Only update time_distribution. * Add targetService attribute. (#59) * Use envoy new access_log handler for sending Report. (#60) * use access_log handler. * Not to use Loggable base class. * Update to the latest envoy with #396. (#61) * Fix tclap dependency fetching error (#62) * Integrate mixer client directly with envoy. (#66) * Integrate mixer client directly with envoy. * Send response header in Report. * rename filter name from esp to mixer. * add README. * Add release binary script. (#68) * Push tar.gz to GCS (#69) * Push tar.gz to GCS * Rename envoy_esp * Remove mixer_client from api_manager. (#72) * Update mixer client SHA. (#74) * Update readme. (#73) * Adds Jenkinsfile and updates release-binary to create a SHA. (#71) * Adds Jenkinsfile and update release-binary * Update Jenkinsfile and gitignore * Fixes typo and use normal build Node * Uses default bazel config * Using batch mode * Update bazel memory settings * Do not use Jenkins bazel env * Set .bazelrc for postsubmit * Update grpc and protobuf (#70) * protobuf v3.2.0 * grpc v1.1.1 * Align auth lib with grpc 1.1.1 * Add sourceService. (#78) * Add script to build docker image. (#77) * Add script to build docker image. * Add start_envoy for docker image. * Use official attribute names (#80) * Use official attribute names * fix format * Creates a KEY for mixer client dep. Updates release-binary (#79) * Updated mixer repo to use a key for commit * release-binary skip build if file exists. * Update src/envoy/mixer/README. (#82) * Fix src/envoy/mixer/README.md (#85) * Get attributes from envoy config. (#87) * Send all attributes. * Remove unused const strings. * Address comment. * updated SHA to point to newer envoy with RDS API feature (#94) * Disable travis on stable branches (#96) * Publish debug binaries (no release yet) (#98) * Copies the binary instead of linking for release (#102) * Not to use api_key if its service is not actived. (#109) * Update envoy and add c-ares (#107) * Update envoy and add c-ares depedencies * Update release script with debug and normal binary * remove debug ls * formatting * Send StatusCode Attributes to Mixer. (#110) * Add send_attribute filter. (#115) * Add send_attribute filter. * Fix format * rename variable serialized_attributes_ * Address the comments. * Fail request if api_key is not valid. (#116) * Fail request if api_key is not valid. * Format code. * Update comments. * Address comment. * Rename response.http.code (#125) * Send headers as string map. (#129) * Send headers as string map. * Remove origin.ip and origin.host. * Fix format * unify bazel's docker build targets with other istio repos (#127) * update base debug docker image reference (#133) * Update postsubmit to create docker images (#132) * Adding config release for bazel build (#135) * Fix mixer client crash. (#136) * Get mixerclient with response parsing. (#138) * Update nghttp2 to sync with envoy (#140) * Fix src/envoy/mixer/README.md * Update nghttp2 to sync with envoy * update * fix typo * Merge from master to firebase (#159) * Simple TCP server to show how to retrieve original dest IP:port after an iptables redirect (#38) * Simple TCP server to show how to retrieve original dest IP:port after an iptables redirect * Fixed style. * Rebase Envoy (#41) * Update prototype to use iptables (#42) * Rebase to fixed Envoy (#43) * Handle HEAD request. (#34) * Handle HEAD request. * Try with GET if HEAD fails. * Address comments. * Format file. * Expose bazel target (#48) * Try again (#49) * Integrate with mixer client. (#55) * Integrate with mixer client. * Restore repositories.bzl back. * Add originIp and originHost attributes. (#56) * Add uuid-dev dependency in README.md (#45) * Extract originIp and OriginHost. (#57) * Extract originIp and OriginHost. * Make header x-forwarded-host const. * Update buckets for UI. (#58) * Update buckets for UI. * Only update time_distribution. * Add targetService attribute. (#59) * Use envoy new access_log handler for sending Report. (#60) * use access_log handler. * Not to use Loggable base class. * Update to the latest envoy with #396. (#61) * Fix tclap dependency fetching error (#62) * Integrate mixer client directly with envoy. (#66) * Integrate mixer client directly with envoy. * Send response header in Report. * rename filter name from esp to mixer. * add README. * Add release binary script. (#68) * Push tar.gz to GCS (#69) * Push tar.gz to GCS * Rename envoy_esp * Remove mixer_client from api_manager. (#72) * Update mixer client SHA. (#74) * Update readme. (#73) * Adds Jenkinsfile and updates release-binary to create a SHA. (#71) * Adds Jenkinsfile and update release-binary * Update Jenkinsfile and gitignore * Fixes typo and use normal build Node * Uses default bazel config * Using batch mode * Update bazel memory settings * Do not use Jenkins bazel env * Set .bazelrc for postsubmit * Update grpc and protobuf (#70) * protobuf v3.2.0 * grpc v1.1.1 * Align auth lib with grpc 1.1.1 * Add sourceService. (#78) * Add script to build docker image. (#77) * Add script to build docker image. * Add start_envoy for docker image. * Use official attribute names (#80) * Use official attribute names * fix format * Creates a KEY for mixer client dep. Updates release-binary (#79) * Updated mixer repo to use a key for commit * release-binary skip build if file exists. * Update src/envoy/mixer/README. (#82) * Fix src/envoy/mixer/README.md (#85) * Get attributes from envoy config. (#87) * Send all attributes. * Remove unused const strings. * Address comment. * updated SHA to point to newer envoy with RDS API feature (#94) * Disable travis on stable branches (#96) * Publish debug binaries (no release yet) (#98) * Copies the binary instead of linking for release (#102) * Not to use api_key if its service is not actived. (#109) * Update envoy and add c-ares (#107) * Update envoy and add c-ares depedencies * Update release script with debug and normal binary * remove debug ls * formatting * Send StatusCode Attributes to Mixer. (#110) * Add send_attribute filter. (#115) * Add send_attribute filter. * Fix format * rename variable serialized_attributes_ * Address the comments. * Fail request if api_key is not valid. (#116) * Fail request if api_key is not valid. * Format code. * Update comments. * Address comment. * Rename response.http.code (#125) * Send headers as string map. (#129) * Send headers as string map. * Remove origin.ip and origin.host. * Fix format * unify bazel's docker build targets with other istio repos (#127) * update base debug docker image reference (#133) * Update postsubmit to create docker images (#132) * Adding config release for bazel build (#135) * Fix mixer client crash. (#136) * Get mixerclient with response parsing. (#138) * Update nghttp2 to sync with envoy (#140) * Fix src/envoy/mixer/README.md * Update nghttp2 to sync with envoy * update * fix typo * Populate origin.user attribute from the SAN field of client cert (#142) * Test * test * test * revert file * address comments * test * fix typo * fix format * fix format * Update to latest mixer_client. (#145) * Update to latest mixer_client. * Updated the sha. * Not call report if decodeHeaders is not called. (#150) * Update mixerclient with sync-ed grpc write and fail-fast. (#155) * Update mixerclient with sync-ed write and fail-fast. * Update to latest test. * Update again * Update envoy to PR553 (#156) * Update envoy to PR553 * Update libevent to 2.1.8 * Update the Commit id for envoy * Allow for HTTP based function from Firebase rules (#202) * Allow for HTTP based function from Firebase rules * Fix code style check * Added more comments. * Fix style issues. * Address code review comments from Limin and Lizan. * Add more comments and address CR comments. * Fix a typo. * Address Wayne's CR comments. * Merge from master to firebase (#237) * Simple TCP server to show how to retrieve original dest IP:port after an iptables redirect (#38) * Simple TCP server to show how to retrieve original dest IP:port after an iptables redirect * Fixed style. * Rebase Envoy (#41) * Update prototype to use iptables (#42) * Rebase to fixed Envoy (#43) * Handle HEAD request. (#34) * Handle HEAD request. * Try with GET if HEAD fails. * Address comments. * Format file. * Expose bazel target (#48) * Try again (#49) * Integrate with mixer client. (#55) * Integrate with mixer client. * Restore repositories.bzl back. * Add originIp and originHost attributes. (#56) * Add uuid-dev dependency in README.md (#45) * Extract originIp and OriginHost. (#57) * Extract originIp and OriginHost. * Make header x-forwarded-host const. * Update buckets for UI. (#58) * Update buckets for UI. * Only update time_distribution. * Add targetService attribute. (#59) * Use envoy new access_log handler for sending Report. (#60) * use access_log handler. * Not to use Loggable base class. * Update to the latest envoy with #396. (#61) * Fix tclap dependency fetching error (#62) * Integrate mixer client directly with envoy. (#66) * Integrate mixer client directly with envoy. * Send response header in Report. * rename filter name from esp to mixer. * add README. * Add release binary script. (#68) * Push tar.gz to GCS (#69) * Push tar.gz to GCS * Rename envoy_esp * Remove mixer_client from api_manager. (#72) * Update mixer client SHA. (#74) * Update readme. (#73) * Adds Jenkinsfile and updates release-binary to create a SHA. (#71) * Adds Jenkinsfile and update release-binary * Update Jenkinsfile and gitignore * Fixes typo and use normal build Node * Uses default bazel config * Using batch mode * Update bazel memory settings * Do not use Jenkins bazel env * Set .bazelrc for postsubmit * Update grpc and protobuf (#70) * protobuf v3.2.0 * grpc v1.1.1 * Align auth lib with grpc 1.1.1 * Add sourceService. (#78) * Add script to build docker image. (#77) * Add script to build docker image. * Add start_envoy for docker image. * Use official attribute names (#80) * Use official attribute names * fix format * Creates a KEY for mixer client dep. Updates release-binary (#79) * Updated mixer repo to use a key for commit * release-binary skip build if file exists. * Update src/envoy/mixer/README. (#82) * Fix src/envoy/mixer/README.md (#85) * Get attributes from envoy config. (#87) * Send all attributes. * Remove unused const strings. * Address comment. * updated SHA to point to newer envoy with RDS API feature (#94) * Disable travis on stable branches (#96) * Publish debug binaries (no release yet) (#98) * Copies the binary instead of linking for release (#102) * Not to use api_key if its service is not actived. (#109) * Update envoy and add c-ares (#107) * Update envoy and add c-ares depedencies * Update release script with debug and normal binary * remove debug ls * formatting * Send StatusCode Attributes to Mixer. (#110) * Add send_attribute filter. (#115) * Add send_attribute filter. * Fix format * rename variable serialized_attributes_ * Address the comments. * Fail request if api_key is not valid. (#116) * Fail request if api_key is not valid. * Format code. * Update comments. * Address comment. * Rename response.http.code (#125) * Send headers as string map. (#129) * Send headers as string map. * Remove origin.ip and origin.host. * Fix format * unify bazel's docker build targets with other istio repos (#127) * update base debug docker image reference (#133) * Update postsubmit to create docker images (#132) * Adding config release for bazel build (#135) * Fix mixer client crash. (#136) * Get mixerclient with response parsing. (#138) * Update nghttp2 to sync with envoy (#140) * Fix src/envoy/mixer/README.md * Update nghttp2 to sync with envoy * update * fix typo * Populate origin.user attribute from the SAN field of client cert (#142) * Test * test * test * revert file * address comments * test * fix typo * fix format * fix format * Update to latest mixer_client. (#145) * Update to latest mixer_client. * Updated the sha. * Not call report if decodeHeaders is not called. (#150) * Update mixerclient with sync-ed grpc write and fail-fast. (#155) * Update mixerclient with sync-ed write and fail-fast. * Update to latest test. * Update again * Update envoy to PR553 (#156) * Update envoy to PR553 * Update libevent to 2.1.8 * Uses a specific version of the Shared Pipeline lib (#158) * Update lyft/envoy commit Id to latest. (#161) * Update lyft/envoy commit Id to latest. * Remove the comment about pull request * Add new line - will delete in next commit. * Update repositories.bzl (#169) * Always set response latency (#172) * Update mixerclient to sync_transport change. (#178) * Use opaque config to turn on/off forward attribute and mixer filter (#179) * Modify mixer filter * Swap defaults * Make the filter decoder only * cache mixer disabled decision * Fix a bug in opaque config change and test it out (#182) * Fix a bug and test it out * Update filter type * Update README.md * Update mixer client to mixer api with gogoproto. (#184) * Move .bazelrc to tools/bazel.rc (#186) * Move .bazelrc to tools/bazel.rc * Update Jenkinsfile with latest version of pipeline * Support apikey based traffic restriction (#189) * b/36368559 support apikey based traffic restriction * Fixed code formatting * Fix crash in unreachable/overloaded RDS (#190) * Add mixer client end to end integration test. (#177) * Add mixer client end to end integration test. * Split some repositories into a separate file. * use real mixer for fake mixer_server. * Test repository * use mixer bzl file. * Use mixer repositories * Not to use mixer repository. * Add return line at the end of WORKSPACE. * Fix broken link (#193) * Make quota call (#192) * hookup quota call * Make quota call. * Update indent. * Update envoy and update configs (#195) * Update envoy and update configs * Use gcc-4.9 for travis * Use bazel 0.4.5 * Fix SHA of lightstep-tracer-common * Enable check cache and refactory mixer config loading (#197) * Refactory the mixer config loading. * fix format * Add integration test. * updated README.md * s/send/sent/ * Split into separate tests. (#201) * Update README on how to enable check cache. (#204) * Update README on how to enable check cache. * Update the comment. * build: support Envoy native Bazel build. (#210) * build: support Envoy native Bazel build. This patch switches the Envoy build from src/envoy/repositories.bzl to using the upstream native build. See envoyproxy/envoy#663 for the corresponding changes on the Envoy side. * Use Envoy master with BUILD.wip rename merged. * Fix clang-format issues. * Fixes bazel.rc issues (#212) * Fixes bazel rc issues * Update Jenkins to latest pipeline version * Fix go build (#224) * Use TranscoderInputStream to reduce confusion around ByteCount() (#225) * Add TranscoderInputStream to reduce confusion * fix_format * Merge latest changes from rate_limiting to master (#221) * Point to googleapi in service control client. (#91) * Point to googleapi in service control client. * Use git repository for service-control-client. * Merge latest changes from master (#104) * Get attributes from envoy config. (#87) * Send all attributes. * Remove unused const strings. * Address comment. * updated SHA to point to newer envoy with RDS API feature (#94) * Disable travis on stable branches (#96) * Publish debug binaries (no release yet) (#98) * Copies the binary instead of linking for release (#102) * Extract quota config from service config. (#101) * Add metric_cost in config. * Remove group rules. * Call loadQuotaConfig in config::create. * Update latest update from master branch (#106) * Get attributes from envoy config. (#87) * Send all attributes. * Remove unused const strings. * Address comment. * updated SHA to point to newer envoy with RDS API feature (#94) * Disable travis on stable branches (#96) * Publish debug binaries (no release yet) (#98) * Copies the binary instead of linking for release (#102) * Added quota contoll without the service control client library (#93) * Added quota contoll without the service control client library * Applied code review * Applied code review * Resolve conflicts * Resolve conflicts * Fixed format error reported by script/check-style * Fixed a bug at Aggregated::GetAuthToken that causes Segmentation Fault * Changed usage of template funcion * Applied latest changes from the repo * Applied latest changes from the repo * Applied latest changes from the repo * Adde comments * Updated log information * Applied #101 * Changed metric_cost_map to metric_cost_vector * Fixed test case compilation error * Fixed test case compilation error * Add unit test for quota config. (#108) * Add unit test for quota config. * Add comments. * Update test specifics. * Merge latest changes from master branch (#112) * Get attributes from envoy config. (#87) * Send all attributes. * Remove unused const strings. * Address comment. * updated SHA to point to newer envoy with RDS API feature (#94) * Disable travis on stable branches (#96) * Publish debug binaries (no release yet) (#98) * Copies the binary instead of linking for release (#102) * Not to use api_key if its service is not actived. (#109) * If QuotaControl service is not available, return utils::Status::OK (#113) * If QuotaControl service is not available, return utils::Status::OK * Updated comment * Return HTTP status code 429 on google.rpc.Code.RESOURCE_EXHAUSTED (#119) * Fixed incorrectly resolved conflicts (#123) * Added unit test cases for rate limiting (#124) * Fixed incorrectly resolved conflicts * Added unit test cases for rate limiting * Added unit test cases for rate limiting * Added unit test cases for rate limiting * Added unit test cases for rate limiting * Added unit test cases for rate limiting * Added unit test cases for rate limiting * Rename response.http.code (#125) (#128) * Added handling of error code QUOTA_SYSTEM_UNAVAILABLE (#148) * Integrated service control client library with quota cache aggregation (#149) * Fixed error on merge (#151) * Integrated service control client library with quota cache aggregation * Fixed error on merge * Fixed the compatibility issue with the latest update on esp (#152) * Removed copied proto files (#208) * Set default allocate quota request timeout to 1sec and applied latest service control client library change (#211) * Merged key_restriction related changes from master (#213) * Merge latest changes from master branch (#217) * Not call report if decodeHeaders is not called. (#150) * Update mixerclient with sync-ed grpc write and fail-fast. (#155) * Update mixerclient with sync-ed write and fail-fast. * Update to latest test. * Update again * Update envoy to PR553 (#156) * Update envoy to PR553 * Update libevent to 2.1.8 * Uses a specific version of the Shared Pipeline lib (#158) * Update lyft/envoy commit Id to latest. (#161) * Update lyft/envoy commit Id to latest. * Remove the comment about pull request * Add new line - will delete in next commit. * Update repositories.bzl (#169) * Always set response latency (#172) * Update mixerclient to sync_transport change. (#178) * Use opaque config to turn on/off forward attribute and mixer filter (#179) * Modify mixer filter * Swap defaults * Make the filter decoder only * cache mixer disabled decision * Fix a bug in opaque config change and test it out (#182) * Fix a bug and test it out * Update filter type * Update README.md * Update mixer client to mixer api with gogoproto. (#184) * Move .bazelrc to tools/bazel.rc (#186) * Move .bazelrc to tools/bazel.rc * Update Jenkinsfile with latest version of pipeline * Support apikey based traffic restriction (#189) * b/36368559 support apikey based traffic restriction * Fixed code formatting * Fix crash in unreachable/overloaded RDS (#190) * Add mixer client end to end integration test. (#177) * Add mixer client end to end integration test. * Split some repositories into a separate file. * use real mixer for fake mixer_server. * Test repository * use mixer bzl file. * Use mixer repositories * Not to use mixer repository. * Add return line at the end of WORKSPACE. * Fix broken link (#193) * Make quota call (#192) * hookup quota call * Make quota call. * Update indent. * Update envoy and update configs (#195) * Update envoy and update configs * Use gcc-4.9 for travis * Use bazel 0.4.5 * Fix SHA of lightstep-tracer-common * Enable check cache and refactory mixer config loading (#197) * Refactory the mixer config loading. * fix format * Add integration test. * updated README.md * s/send/sent/ * Split into separate tests. (#201) * Update README on how to enable check cache. (#204) * Update README on how to enable check cache. * Update the comment. * build: support Envoy native Bazel build. (#210) * build: support Envoy native Bazel build. This patch switches the Envoy build from src/envoy/repositories.bzl to using the upstream native build. See envoyproxy/envoy#663 for the corresponding changes on the Envoy side. * Use Envoy master with BUILD.wip rename merged. * Fix clang-format issues. * Fixes bazel.rc issues (#212) * Fixes bazel rc issues * Update Jenkins to latest pipeline version * Updated the commit id of cloudendpoints/service-control-client-cxx (#218) * Update commitid of cloudendpoints/service-control-client-cxx repo (#220) * Send delta metrics for intermediate reports. (#219) * Send delta metrics for intermediate reports. * Move last_request_bytes/last_response_bytes to RequestContext. * Handle final report. * Address comment. * Update attributes to match the canonical attribute list. (#232) * Update response.http.code to response.code and response.latency to response.duration to line up with the canonical attributes in istio/istio.github.io/docs/concepts/attributes.md * Format according to clang-format * Add envoy Buffer based TranscoderInputStream (#231) * Add envoy Buffer based TranscoderInputStream * fix format * A few doc changes for consistency across repos. (#235) * Add repositories.bzl * Added missing export setting in bazel configuration (#236) * Added export missing in bazel configuration * Added export missing in bazel configuration * Allow HTTP functions in firebase rules to specify audience (#244) * Allow HTTP functions in firebase rules to specify audience * Allow GetAuthToken to ignore cache and fix style checks. * Fix GetAuthToken * Address Wayne's comment * Check for empty response body * Remove .bazelrc.jenkins file not present in the master branch. * Remove forward_attribute_filter.cc not present in master.
The changes in this PR will allow firebase rules to fetch the required information to do authorization from a REST endpoint. This endpoint could be a Google Datastore or Cloud SQL or spanner endpoint. ESP will invoke the HTTP request on behalf of Firebase rules and provide the response back to the Firebase service as a JSON value. This value can directly used as a Map in firebase rules to check for various attributes or policies to make authorization decisions.
Testing:
Integration testing via Cloud Datastore works.
Added more Unit tests to cover the new implementation.
Note that the old implementation of basic firebase rules should still work and the unit tests for those remain.