-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Allow HTTP functions in firebase rules to specify audience #244
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
1c472a9
f78d039
46bb6cc
e47d0f2
aa1ddc8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,8 +77,17 @@ class ServiceAccountToken { | |
| // Gets the auth token to access Google services. | ||
| // If client auth secret is specified, use it to calcualte JWT token. | ||
| // Otherwise, use the access token fetched from metadata server. | ||
| // If ignore_cache is true then a JWT token is regenerated even if the current | ||
| // cached JWT token is valid. | ||
| const std::string& GetAuthToken(JWT_TOKEN_TYPE type); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The comments need to be updated. There is no "ignore_cache" parameter.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes. Will do. |
||
|
|
||
| // Gets the auth token to access Google services. This method accepts an | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The target service does not have to be Google services. Correct? |
||
| // audience parameter to set when generating JWT token. | ||
| // If client auth secret is specified, use it to calcualte JWT token. | ||
| // Otherwise, use the access token fetched from metadata server. | ||
| const std::string& GetAuthToken(JWT_TOKEN_TYPE type, | ||
| const std::string& audience); | ||
|
|
||
| private: | ||
| // Stores base token info. Used for both OAuth and JWT tokens. | ||
| class TokenInfo { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,6 +49,9 @@ const std::string kFirebaseDeleteMethod = "delete"; | |
| const std::string kFirebaseUpdateMethod = "update"; | ||
| const std::string kV1 = "/v1"; | ||
| const std::string kTestQuery = ":test?alt=json"; | ||
| const char kFirebaseAudience[] = | ||
| "https://staging-firebaserules.sandbox.googleapis.com/" | ||
| "google.firebase.rules.v1.FirebaseRulesService"; | ||
|
|
||
| void SetProtoValue(const std::string &key, | ||
| const ::google::protobuf::Value &value, | ||
|
|
@@ -94,6 +97,8 @@ FirebaseRequest::FirebaseRequest( | |
| firebase_http_request_.method = kHttpPostMethod; | ||
| firebase_http_request_.token_type = | ||
| auth::ServiceAccountToken::JWT_TOKEN_FOR_FIREBASE; | ||
| firebase_http_request_.audience = kFirebaseAudience; | ||
|
|
||
| external_http_request_.token_type = | ||
| auth::ServiceAccountToken::JWT_TOKEN_FOR_AUTHORIZATION_SERVICE; | ||
|
|
||
|
|
@@ -305,6 +310,8 @@ Status FirebaseRequest::SetNextRequest() { | |
| auto call = *func_call_iter_; | ||
| external_http_request_.url = call.args(0).string_value(); | ||
| external_http_request_.method = call.args(1).string_value(); | ||
| external_http_request_.audience = | ||
| call.args(call.args_size() - 1).string_value(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "audience" is an optional field here. For datastore, we provide a default audience if not specified. How do you handle the case that "audience" field is provided by in the rules for datastore? Do you overwrite the audience field with the default datastore audience? How do you handle the case that "audience" is not provided? What is the default audience?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is not optional right now since there is no default behavior implemented right now. There is no default audience being specified at all. |
||
| std::string body; | ||
| status = | ||
| utils::ProtoToJson(call.args(2), &body, utils::JsonOptions::DEFAULT); | ||
|
|
@@ -334,9 +341,9 @@ Status FirebaseRequest::CheckFuncCallArgs(const FunctionCall &func) { | |
|
|
||
| // We only support functions that call with three argument: HTTP URL, HTTP | ||
| // method and body. The body can be empty | ||
| if (func.args_size() < 2 || func.args_size() > 3) { | ||
| if (func.args_size() < 3 || func.args_size() > 4) { | ||
| std::ostringstream os; | ||
| os << func.function() << " Require 2 or 3 arguments. But has " | ||
| os << func.function() << " Require 3 or 4 arguments. But has " | ||
| << func.args_size(); | ||
| return Status(Code::INVALID_ARGUMENT, os.str()); | ||
| } | ||
|
|
@@ -348,6 +355,14 @@ Status FirebaseRequest::CheckFuncCallArgs(const FunctionCall &func) { | |
| std::string(func.function() + " Arguments 1 and 2 should be strings")); | ||
| } | ||
|
|
||
| if (func.args(func.args_size() - 1).kind_case() != | ||
| google::protobuf::Value::kStringValue) { | ||
| return Status( | ||
| Code::INVALID_ARGUMENT, | ||
| std::string(func.function() + "The last argument should be a string" | ||
| "that specifies audience")); | ||
| } | ||
|
|
||
| if (!utils::IsHttpRequest(func.args(0).string_value())) { | ||
| return Status( | ||
| Code::INVALID_ARGUMENT, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is better to compare the new audience with old audience, if diff, wipe out the cache.