From 1027694e54e30e586978708cdeba8e8a9b5e8eba Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Tue, 23 Jul 2019 15:24:12 -0500 Subject: [PATCH 01/22] feat(google): support requester pays for signed urls and google temp creds --- fence/blueprints/data/indexd.py | 55 ++++++++++++++--- fence/blueprints/storage_creds/google.py | 5 ++ fence/config-default.yaml | 18 +++++- fence/resources/google/utils.py | 76 ++++++++++++++++++++++++ requirements.txt | 2 +- 5 files changed, 145 insertions(+), 11 deletions(-) diff --git a/fence/blueprints/data/indexd.py b/fence/blueprints/data/indexd.py index 9e335a611..db907a887 100644 --- a/fence/blueprints/data/indexd.py +++ b/fence/blueprints/data/indexd.py @@ -4,6 +4,7 @@ from cached_property import cached_property import cirrus +from cirrus import GoogleCloudManager from cdislogging import get_logger from cdispyutils.config import get_value from cdispyutils.hmac4 import generate_aws_presigned_url @@ -32,6 +33,7 @@ create_primary_service_account_key, get_or_create_proxy_group_id, get_google_app_creds, + give_service_account_billing_access_if_necessary, ) from fence.utils import get_valid_expiration_from_request from . import multipart_upload @@ -52,6 +54,13 @@ def get_signed_url_for_file(action, file_id): requested_protocol = flask.request.args.get("protocol", None) + r_pays_project = flask.request.args.get("userProject", None) + + # TODO maybe we want an arg to ask NOT to setup the user project billing in case + # it is provided out of the knowledge of this fence instance? e.g. someone + # manually will provide the SA access to the given project + setup_billing = flask.request.args.get("setupUserProjectBilling", None) + # TODO ^^^^ do we need this?? # default to signing the url even if it's a public object # this will work so long as we're provided a user token @@ -66,7 +75,11 @@ def get_signed_url_for_file(action, file_id): expires_in = min(requested_expires_in, expires_in) signed_url = indexed_file.get_signed_url( - requested_protocol, action, expires_in, force_signed_url=force_signed_url + requested_protocol, + action, + expires_in, + force_signed_url=force_signed_url, + r_pays_project=r_pays_project, ) return {"url": signed_url} @@ -295,7 +308,9 @@ def indexed_file_locations(self): urls = self.index_document.get("urls", []) return list(map(IndexedFileLocation.from_url, urls)) - def get_signed_url(self, protocol, action, expires_in, force_signed_url=True): + def get_signed_url( + self, protocol, action, expires_in, force_signed_url=True, r_pays_project=None + ): if self.public and action == "upload": raise Unauthorized("Cannot upload on public files") # don't check the authorization if the file is public @@ -306,9 +321,13 @@ def get_signed_url(self, protocol, action, expires_in, force_signed_url=True): ) if action is not None and action not in SUPPORTED_ACTIONS: raise NotSupported("action {} is not supported".format(action)) - return self._get_signed_url(protocol, action, expires_in, force_signed_url) + return self._get_signed_url( + protocol, action, expires_in, force_signed_url, r_pays_project + ) - def _get_signed_url(self, protocol, action, expires_in, force_signed_url): + def _get_signed_url( + self, protocol, action, expires_in, force_signed_url, r_pays_project + ): if not protocol: # no protocol specified, return first location as signed url try: @@ -317,6 +336,7 @@ def _get_signed_url(self, protocol, action, expires_in, force_signed_url): expires_in, public_data=self.public, force_signed_url=force_signed_url, + r_pays_project=r_pays_project, ) except IndexError: raise NotFound("Can't find any file locations.") @@ -331,6 +351,7 @@ def _get_signed_url(self, protocol, action, expires_in, force_signed_url): expires_in, public_data=self.public, force_signed_url=force_signed_url, + r_pays_project=r_pays_project, ) raise NotFound( @@ -469,7 +490,7 @@ def from_url(url): return IndexedFileLocation(url) def get_signed_url( - self, action, expires_in, public_data=False, force_signed_url=True + self, action, expires_in, public_data=False, force_signed_url=True, **kwargs ): return self.url @@ -579,7 +600,7 @@ def get_bucket_region(self): return bucket_cred["region"] def get_signed_url( - self, action, expires_in, public_data=False, force_signed_url=True + self, action, expires_in, public_data=False, force_signed_url=True, **kwargs ): aws_creds = get_value( config, "AWS_CREDENTIALS", InternalError("credentials not configured") @@ -714,7 +735,12 @@ class GoogleStorageIndexedFileLocation(IndexedFileLocation): """ def get_signed_url( - self, action, expires_in, public_data=False, force_signed_url=True + self, + action, + expires_in, + public_data=False, + force_signed_url=True, + r_pays_project=None, ): resource_path = ( self.parsed_url.netloc.strip("/") + "/" + self.parsed_url.path.strip("/") @@ -737,12 +763,13 @@ def get_signed_url( expiration_time, user_info.get("user_id"), user_info.get("username"), + r_pays_project=r_pays_project, ) return url def _generate_anonymous_google_storage_signed_url( - self, http_verb, resource_path, expiration_time + self, http_verb, resource_path, expiration_time, r_pays_project=None ): # we will use the main fence SA service account to sign anonymous requests private_key = get_google_app_creds() @@ -754,11 +781,18 @@ def _generate_anonymous_google_storage_signed_url( content_type="", md5_value="", service_account_creds=private_key, + requester_pays_user_project=r_pays_project, ) return final_url def _generate_google_storage_signed_url( - self, http_verb, resource_path, expiration_time, user_id, username + self, + http_verb, + resource_path, + expiration_time, + user_id, + username, + r_pays_project=None, ): proxy_group_id = get_or_create_proxy_group_id() @@ -781,6 +815,8 @@ def _generate_google_storage_signed_url( user_id=user_id, username=username, proxy_group_id=proxy_group_id ) + give_service_account_billing_access_if_necessary(private_key, r_pays_project) + final_url = cirrus.google_cloud.utils.get_signed_url( resource_path, http_verb, @@ -789,6 +825,7 @@ def _generate_google_storage_signed_url( content_type="", md5_value="", service_account_creds=private_key, + requester_pays_user_project=r_pays_project, ) return final_url diff --git a/fence/blueprints/storage_creds/google.py b/fence/blueprints/storage_creds/google.py index a5a07e425..6e00d16bc 100644 --- a/fence/blueprints/storage_creds/google.py +++ b/fence/blueprints/storage_creds/google.py @@ -19,6 +19,7 @@ get_service_account, get_or_create_service_account, get_or_create_proxy_group_id, + give_service_account_billing_access_if_necessary, ) from fence.utils import get_valid_expiration_from_request @@ -149,10 +150,14 @@ def post(self): proxy_group_id = get_or_create_proxy_group_id() username = current_token.get("context", {}).get("user", {}).get("name") + r_pays_project = flask.request.args.get("userProject", None) + key, service_account = create_google_access_key( client_id, user_id, username, proxy_group_id ) + give_service_account_billing_access_if_necessary(key, r_pays_project) + if client_id is None: self.handle_user_service_account_creds(key, service_account) diff --git a/fence/config-default.yaml b/fence/config-default.yaml index a62fb9156..016c6e3ab 100644 --- a/fence/config-default.yaml +++ b/fence/config-default.yaml @@ -454,7 +454,7 @@ INDEXD_PASSWORD: '' ARBORIST: null # ////////////////////////////////////////////////////////////////////////////////////// -# CLOUD API LIBRARY (CIRRUS) CONFIGURATION +# CLOUD API LIBRARY (CIRRUS) AND GOOGLE CONFIGURATION # - Support Google Data Access Methods # ////////////////////////////////////////////////////////////////////////////////////// # Setting this up allows fence to create buckets, manage Google groups, etc. @@ -483,6 +483,22 @@ GOOGLE_GROUP_PREFIX: '' # length restrictions on service account names. GOOGLE_SERVICE_ACCOUNT_PREFIX: '' +# A Google Project identitifier representing the default project to bill to for +# accessing Google Requester Pays buckets. If this is provided and the API call for +# Google access does not include a `userProject`, this will be used instead. Fence +# will automatically attempt to create a Custom Role in the project and give the +# necessary Google Service Account that role (which will allow it to bill to the project) +# +# NOTE: The Fence SA will need the necessary permissions in the specified project to +# both create a custom role and update the project's IAM Policy to include the +# necessary SA. For more details on required permissions, check: +# https://github.com/uc-cdis/cirrus#credentials +# +# NOTE2: It may be possible to further restrict the permissions in the future to +# be more fine-grained. +# +GOOGLE_REQUESTER_PAYS_BILLING_PROJECT: + # ////////////////////////////////////////////////////////////////////////////////////// # EMAIL # - Support for sending emails from fence. Used for user certificates diff --git a/fence/resources/google/utils.py b/fence/resources/google/utils.py index e95cfeaad..00c3b39fd 100644 --- a/fence/resources/google/utils.py +++ b/fence/resources/google/utils.py @@ -154,6 +154,82 @@ def create_primary_service_account_key(user_id, username, proxy_group_id, expire return sa_private_key +def get_sa_email_from_private_key(sa_private_key): + """ + Return the service account email given the a private service account key. + + Args: + sa_private_key (dict): + + JSON key in Google Credentials File format: + + .. code-block:: JavaScript + + { + "type": "service_account", + "project_id": "project-id", + "private_key_id": "some_number", + "private_key": "-----BEGIN PRIVATE KEY-----\n.... + =\n-----END PRIVATE KEY-----\n", + "client_email": "api@project-id.iam.gserviceaccount.com", + "client_id": "...", + "auth_uri": "https://accounts.google.com/o/oauth2/auth", + "token_uri": "https://accounts.google.com/o/oauth2/token", + "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs", + "client_x509_cert_url": "https://www.googleapis.com/...api%40project-id.iam.gserviceaccount.com" + } + + Returns: + str: Service account email + """ + return sa_private_key.get("client_email") + + +def give_service_account_billing_access_if_necessary( + sa_private_key, r_pays_project=None +): + """ + Give the Service Account (whose key is provided) the privilege to bill to the + given project. If a project is not provided and there is a configured Google project + to bill to, we will use that. + + Args: + sa_private_key (dict): + JSON key in Google Credentials File format: + + .. code-block:: JavaScript + + { + "type": "service_account", + "project_id": "project-id", + "private_key_id": "some_number", + "private_key": "-----BEGIN PRIVATE KEY-----\n.... + =\n-----END PRIVATE KEY-----\n", + "client_email": "api@project-id.iam.gserviceaccount.com", + "client_id": "...", + "auth_uri": "https://accounts.google.com/o/oauth2/auth", + "token_uri": "https://accounts.google.com/o/oauth2/token", + "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs", + "client_x509_cert_url": "https://www.googleapis.com/...api%40project-id.iam.gserviceaccount.com" + } + r_pays_project (str, optional): The Google Project identifier to bill to + """ + # TODO use configured project if it exists and no user project was given + if config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"] and not r_pays_project: + r_pays_project = config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"] + + if r_pays_project: + sa_account_id = get_sa_email_from_private_key(sa_private_key) + # if a project is provided, attempt to create custom role that gives + # the SA access to bill the project provided + # TODO this may fail if our fence SA doesn't have the right permissions + # to add this role and update the project policy + with GoogleCloudManager(project_id=r_pays_project) as g_cloud_manager: + g_cloud_manager.give_service_account_billing_access( + sa_account_id, project_id=r_pays_project + ) + + def create_google_access_key(client_id, user_id, username, proxy_group_id): """ Return an access key for current user and client. diff --git a/requirements.txt b/requirements.txt index f5fc92f63..6b98fd24d 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,7 +16,6 @@ Flask_OAuthlib==0.9.4 flask-restful==0.3.6 Flask_SQLAlchemy_Session==1.1 gen3config==0.1.7 -gen3cirrus==1.0.0 gen3users httplib2==0.10.3 markdown==3.1.1 @@ -39,3 +38,4 @@ Werkzeug==0.12.2 pyyaml==5.1 retry==0.9.2 git+https://github.com/uc-cdis/storage-client.git@1.0.0#egg=storageclient +-e git+https://git@github.com/uc-cdis/cirrus.git@feat/requester-pays#egg=cirrus From 6bf88d1026c436e92073ac6e81566c7e907710c1 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Tue, 23 Jul 2019 15:35:20 -0500 Subject: [PATCH 02/22] fix(deps): force branch of cirrus for testing --- setup.py | 1 - 1 file changed, 1 deletion(-) diff --git a/setup.py b/setup.py index ccc4731f0..7d9a13638 100644 --- a/setup.py +++ b/setup.py @@ -17,7 +17,6 @@ "Flask-CORS>=3.0.3,<4.0.0", "Flask_OAuthlib>=0.9.4,<1.0.0", "Flask_SQLAlchemy_Session>=1.1,<2.0", - "gen3cirrus>=1.0.0,<2.0", "gen3config>=0.1.6,<1.0.0", "google_api_python_client>=1.6.4,<2.0.0", "httplib2>=0.10.3,<1.0.0", From 225f39d3ca2284335e0d3f72595655fb80aef8a6 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Tue, 23 Jul 2019 15:54:00 -0500 Subject: [PATCH 03/22] fix(deps): install cirrus from branch --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index 6b98fd24d..acd2d23f4 100644 --- a/requirements.txt +++ b/requirements.txt @@ -38,4 +38,4 @@ Werkzeug==0.12.2 pyyaml==5.1 retry==0.9.2 git+https://github.com/uc-cdis/storage-client.git@1.0.0#egg=storageclient --e git+https://git@github.com/uc-cdis/cirrus.git@feat/requester-pays#egg=cirrus +git+https://git@github.com/uc-cdis/cirrus.git@feat/requester-pays#egg=cirrus From ee2f9471be114faaea59c0427ea5beb3d33849d3 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Tue, 23 Jul 2019 17:29:36 -0500 Subject: [PATCH 04/22] feat(google): handle requester pays errors from cirrus --- fence/resources/google/utils.py | 35 ++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) diff --git a/fence/resources/google/utils.py b/fence/resources/google/utils.py index 00c3b39fd..c6b28c4aa 100644 --- a/fence/resources/google/utils.py +++ b/fence/resources/google/utils.py @@ -19,6 +19,7 @@ from fence.auth import current_token from fence.config import config +from fence.errors import NotSupported from fence.models import ( GoogleServiceAccount, GoogleServiceAccountKey, @@ -220,13 +221,33 @@ def give_service_account_billing_access_if_necessary( if r_pays_project: sa_account_id = get_sa_email_from_private_key(sa_private_key) - # if a project is provided, attempt to create custom role that gives - # the SA access to bill the project provided - # TODO this may fail if our fence SA doesn't have the right permissions - # to add this role and update the project policy - with GoogleCloudManager(project_id=r_pays_project) as g_cloud_manager: - g_cloud_manager.give_service_account_billing_access( - sa_account_id, project_id=r_pays_project + + try: + # if a project is provided, attempt to create custom role that gives + # the SA access to bill the project provided + # TODO this may fail if our fence SA doesn't have the right permissions + # to add this role and update the project policy + with GoogleCloudManager(project_id=r_pays_project) as g_cloud_manager: + g_cloud_manager.give_service_account_billing_access( + sa_account_id, project_id=r_pays_project + ) + except Exception as exc: + logger.error( + "Unable to create a custom role in Google Project {} to " + "give Google service account {} rights to bill the project. Error: {}".format( + r_pays_project, sa_account_id, exc + ) + ) + raise NotSupported( + "You provided {} as a `userProject` for requester pays billing, " + "but we could not create a custom role in the project to provide " + "the necessary service account ({}) billing permission. It could be that " + "our main service account {} does not have valid permissions in the " + "project you supplied.".format( + r_pays_project, + sa_account_id, + config["CIRRUS_CFG"].get("GOOGLE_ADMIN_EMAIL"), + ) ) From 25a126457ddd39641e8db0f0af1101453631c263 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Tue, 23 Jul 2019 19:01:28 -0500 Subject: [PATCH 05/22] feat(google): endpoint to expose configured default billing project, documentation about requester pays support --- docs/google_architecture.md | 67 ++++++++++++++++++++++++++++++++- fence/blueprints/google.py | 21 ++++++++++- fence/resources/google/utils.py | 10 ++--- 3 files changed, 90 insertions(+), 8 deletions(-) diff --git a/docs/google_architecture.md b/docs/google_architecture.md index a5ba74e48..654c35b9a 100644 --- a/docs/google_architecture.md +++ b/docs/google_architecture.md @@ -68,6 +68,71 @@ The scopes that allow access to these methods are also available for individual > NOTE: When these docs talk about `clients` accessing data, it's reasonable to assume that this means both `clients` and `users` themselves. Much of the algorithms behave similarly if the user themselves were to request access without going through an outside application. +### Handling Requester Pays Storage Buckets in Google (Optional) + +Google supports a bucket-level configuration called ["Requester Pays"](https://cloud.google.com/storage/docs/requester-pays) which effectively pushes the cost of data access in a Google Storage bucket to the entity accessing the data. + +> By default, the Google project the Google Bucket is in gets billed. + +The Data Access methods [Signed URLs](#signed-urls) and [Temporary Service Account Credentials](#temporary-service-account-credentials) support accessing requester pays buckets with some additional configuration and considerations (noteably: how it affects end-users). + +In order to fully understand the options for requester pays support, it's important to first understand the technical steps to get any of these Google Data Access methods to work, as detailed in the library Fence uses for Google API interactions, [cirrus](https://github.com/uc-cdis/cirrus). It would also be useful to read through the access method details for [Signed URLs](#signed-urls) and [Temporary Service Account Credentials](#temporary-service-account-credentials) first. + +#### Options for Billing Project(s) + +The easiest option for supporting requester pays is to simply bill a Google Project you already own for all access to the bucket instead of requiring end-users to supply a project to bill. This essentially makes the requester pays bucket a non-requester pays bucket, since you'll be paying for all the access. But this may be a necessary solution in cases where: + +1) you want to serve data from a bucket you don't fully control +2) you don't want end-users to have to do manual configuration in Google Cloud Platform to enable billing their project +3) you/end-users don't want to have to give your application IAM permissions in a project the end-user owns + +> NOTE: It is theoretically possible to eliminate `3` by having Fence assume that `2` has happened. However, the current implementation does **not** do that. It programatically attempts to create a custom role in the billing project and provide the necessary access to reduce the manual configuration required. This is, of course, at the expense of requiring that end-users trust your application enough to give it IAM permissions in their project. + +#### Billing Your Own Project + +To bill a project you own, Fence offers an optional configuration for a "default billing project" which you can set to be a project that you manage and can provide the necessary permissions to the Fence admin service account. + +> NOTE: At the time of writing, the configuration variable for the "default billing project" is `GOOGLE_REQUESTER_PAYS_BILLING_PROJECT`. The configuration for the Fence admin service account is `CIRRUS_CFG/GOOGLE_ADMIN_EMAIL`. + +For the [Temporary Service Account Credentials](#temporary-service-account-credentials) access methods, clients need to know what the "default billing project" is (to include in their direct requests to Google). The configured "default billing project" is exposed through an API endpoint [detailed here](http://petstore.swagger.io/?url=https://raw.githubusercontent.com/uc-cdis/fence/master/openapis/swagger.yaml#/google). + +#### End-users Specifying a Billing Project + +API requests to create a signed url and get temporary service account credentials also support the end-user providing a project to bill, in the form of a query param `userProject`. See the [API Documentation](#API-documentation) for more details. + +If you do **not** want to bill a project you own and actually require end-users to pay for access to requester pays buckets, it will require manual configuration by the end-users. The configuration necessary for billing a project is the same whether you or an end-user has to enable it, as detailed below. + +#### Required Google Cloud Platform (GCP) Configuration for Billing Project + +Whether you bill your own project, or require end-users to specify a billing project, the required configuration in GCP is the same. + +The Fence admin service account needs a couple pre-defined Google roles (through their Cloud IAM) on whatever project is provided for billing (be that in a request to Fence or whatever is set as the "default billing project"): + +* `Project IAM Admin`: to update the project's policy to give the necessary service account(s) billing permission +* `Role Administrator`: for creating a custom role that only provides billing permission to the project + +> NOTE: The custom role that Fence creates contains a single permission in Google `serviceusage.services.use`. "All actions that include a billing project in the request require serviceusage.services.use permission for the project that's specified" [according to Google's docs](https://cloud.google.com/storage/docs/access-control/iam-console). + +So once you configure a "default billing project" and give the Fence admin service account the roles mentioned above, access to any requester pays buckets will be billed to that project. This is accomplished in different ways depending on the access method: + +1) For [Signed URLs](#signed-urls): a `userProject=` query parameter will be appended to the signed url + * will only be appended if a valid `userProject` is provided in the request **or** Fence is configured with a "default billing project" +2) For [Temporary Service Account Credentials](#temporary-service-account-credentials): the service account key provided will have the necessary permissions on the `userProject` provided so that subsequent requests to Google using these service account credentials will allow specifying that `userProject` to bill + * depending on how the creds are used, this may involve adding additional query params or args to Google SDKs/services to provide the `userProject` + +Example for Google's Cloud Storage SDK `gsutil`: + +```bash +# activate the temporary service account credentials recieved from Fence +# this assumes the creds are saved in a file named `creds.json` +gcloud auth activate-service-account --key-file ./creds.json + +# copy a file from the requester pays bucket locally +gsutil -u google-project-to-bill cp gs://some-requester-pays-bucket/file.txt . +``` + +In the above script, `google-project-to-bill` is either the `userProject` provided in the request to Fence, or Fence's "default billing project". `some-requester-pays-bucket` is a Google Storage Bucket with requester pays enabled. + ## Data Access Methods ### Signed URLs @@ -179,7 +244,7 @@ Projects are always validated against the following checks: * Google Project has valid service accounts * Key: `service_accounts` * Checks if the Service Account members on the project pass the Service Account validity checks detailed below. - + Service Accounts on the project, as well as the Service Account being registered, are validated against some combination of the following checks (which checks occur ultimately depend on the type of Service Account and whether or not the Service Account is currently being registered or not). * Service Account is owned by Google Project identified in the request diff --git a/fence/blueprints/google.py b/fence/blueprints/google.py index f977bb38a..ea1659349 100644 --- a/fence/blueprints/google.py +++ b/fence/blueprints/google.py @@ -60,6 +60,10 @@ def make_google_blueprint(): GoogleServiceAccountRoot, "/service_accounts", strict_slashes=False ) + blueprint_api.add_resource( + GoogleBillingAccount, "/billing_account", strict_slashes=False + ) + blueprint_api.add_resource( GoogleServiceAccountDryRun, "/service_accounts/_dry_run/", @@ -100,6 +104,20 @@ def __init__(self, email, project_access, google_project_id, user_id=None): self.user_id = user_id +class GoogleBillingAccount(Resource): + def get(self): + """ + Get the configured default Google billing project if it exists. + """ + if not config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"]: + return ( + "No configured default Google billing project for accessing requester pays data", + 404, + ) + + return {"project_id": config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"]} + + class GoogleServiceAccountRoot(Resource): @require_auth_header({"google_service_account"}) def post(self): @@ -455,7 +473,6 @@ def _update_service_account_permissions(self, sa): patch_user_service_account( sa.google_project_id, sa.email, sa.project_access ) - except CirrusNotFound as exc: return ( "Can not update the service accout {}. Detail {}".format(sa.email, exc), @@ -467,7 +484,7 @@ def _update_service_account_permissions(self, sa): 400, ) except Exception: - return (" Can not delete the service account {}".format(sa.email), 500) + return ("Can not update the service account {}".format(sa.email), 500) return ("Successfully update service account {}".format(sa.email), 200) diff --git a/fence/resources/google/utils.py b/fence/resources/google/utils.py index c6b28c4aa..689ee6f23 100644 --- a/fence/resources/google/utils.py +++ b/fence/resources/google/utils.py @@ -215,7 +215,7 @@ def give_service_account_billing_access_if_necessary( } r_pays_project (str, optional): The Google Project identifier to bill to """ - # TODO use configured project if it exists and no user project was given + # use configured project if it exists and no user project was given if config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"] and not r_pays_project: r_pays_project = config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"] @@ -225,8 +225,8 @@ def give_service_account_billing_access_if_necessary( try: # if a project is provided, attempt to create custom role that gives # the SA access to bill the project provided - # TODO this may fail if our fence SA doesn't have the right permissions - # to add this role and update the project policy + # NOTE: this may fail if our fence SA doesn't have the right permissions + # to add this role and update the project policy with GoogleCloudManager(project_id=r_pays_project) as g_cloud_manager: g_cloud_manager.give_service_account_billing_access( sa_account_id, project_id=r_pays_project @@ -240,9 +240,9 @@ def give_service_account_billing_access_if_necessary( ) raise NotSupported( "You provided {} as a `userProject` for requester pays billing, " - "but we could not create a custom role in the project to provide " + "but we could not create a custom role in that project to provide " "the necessary service account ({}) billing permission. It could be that " - "our main service account {} does not have valid permissions in the " + "our main service account ({}) does not have valid permissions in the " "project you supplied.".format( r_pays_project, sa_account_id, From 107cdda0655095a0015b17e298adcde5bd1cacd9 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Wed, 24 Jul 2019 09:34:59 -0500 Subject: [PATCH 06/22] chore(config): clarify comments about required roles --- fence/config-default.yaml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/fence/config-default.yaml b/fence/config-default.yaml index 016c6e3ab..d4e504686 100644 --- a/fence/config-default.yaml +++ b/fence/config-default.yaml @@ -490,9 +490,10 @@ GOOGLE_SERVICE_ACCOUNT_PREFIX: '' # necessary Google Service Account that role (which will allow it to bill to the project) # # NOTE: The Fence SA will need the necessary permissions in the specified project to -# both create a custom role and update the project's IAM Policy to include the -# necessary SA. For more details on required permissions, check: -# https://github.com/uc-cdis/cirrus#credentials +# both create a custom role and update the Project's IAM Policy to include the +# necessary SA. At the time of writing, there are pre-defined roles in Google's +# IAM that provide the necessary permissions. Those are "Project IAM Admin" and +# "Role Administrator" # # NOTE2: It may be possible to further restrict the permissions in the future to # be more fine-grained. From c15fe800fff04222184770e7b784e520dcc36ae2 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Wed, 24 Jul 2019 10:41:19 -0500 Subject: [PATCH 07/22] fix(google): add missing import --- fence/blueprints/google.py | 1 + 1 file changed, 1 insertion(+) diff --git a/fence/blueprints/google.py b/fence/blueprints/google.py index ea1659349..d2134e9aa 100644 --- a/fence/blueprints/google.py +++ b/fence/blueprints/google.py @@ -13,6 +13,7 @@ from fence.auth import current_token, require_auth_header from fence.restful import RestfulApi +from fence.config import config from fence.errors import UserError, NotFound, Unauthorized, Forbidden from fence.resources.google.validity import GoogleProjectValidity from fence.resources.google.access_utils import ( From dbfb5ed3a5db96ac391a1fc4472ff472312f25f6 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Wed, 24 Jul 2019 12:32:47 -0500 Subject: [PATCH 08/22] feat(google): handle errors differently depending on whether billing project is configured or provided by user --- fence/resources/google/utils.py | 38 +++++++++++++++++++++++---------- 1 file changed, 27 insertions(+), 11 deletions(-) diff --git a/fence/resources/google/utils.py b/fence/resources/google/utils.py index 689ee6f23..0cf70def3 100644 --- a/fence/resources/google/utils.py +++ b/fence/resources/google/utils.py @@ -19,7 +19,7 @@ from fence.auth import current_token from fence.config import config -from fence.errors import NotSupported +from fence.errors import NotSupported, InternalError from fence.models import ( GoogleServiceAccount, GoogleServiceAccountKey, @@ -215,9 +215,12 @@ def give_service_account_billing_access_if_necessary( } r_pays_project (str, optional): The Google Project identifier to bill to """ + is_default_billing = False + # use configured project if it exists and no user project was given if config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"] and not r_pays_project: r_pays_project = config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"] + is_default_billing = True if r_pays_project: sa_account_id = get_sa_email_from_private_key(sa_private_key) @@ -238,17 +241,30 @@ def give_service_account_billing_access_if_necessary( r_pays_project, sa_account_id, exc ) ) - raise NotSupported( - "You provided {} as a `userProject` for requester pays billing, " - "but we could not create a custom role in that project to provide " - "the necessary service account ({}) billing permission. It could be that " - "our main service account ({}) does not have valid permissions in the " - "project you supplied.".format( - r_pays_project, - sa_account_id, - config["CIRRUS_CFG"].get("GOOGLE_ADMIN_EMAIL"), + if is_default_billing: + raise InternalError( + "Fence has a configured Google Project for requester pays billing ({}), " + "but could not create a custom role in that project to provide " + "the necessary service account ({}) billing permission. It could be that " + "the Fence admin service account ({}) does not have valid permissions in the " + "project.".format( + r_pays_project, + sa_account_id, + config["CIRRUS_CFG"].get("GOOGLE_ADMIN_EMAIL"), + ) + ) + else: + raise NotSupported( + "You provided {} as a `userProject` for requester pays billing, " + "but we could not create a custom role in that project to provide " + "the necessary service account ({}) billing permission. It could be that " + "our main service account ({}) does not have valid permissions in the " + "project you supplied to create a custom role and change the project IAM policy.".format( + r_pays_project, + sa_account_id, + config["CIRRUS_CFG"].get("GOOGLE_ADMIN_EMAIL"), + ) ) - ) def create_google_access_key(client_id, user_id, username, proxy_group_id): From fcba030055c8f8e48d4df6ad24fb2a26a411d428 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Wed, 24 Jul 2019 13:52:28 -0500 Subject: [PATCH 09/22] feat(google): handle unsafe configuration, change endpoint name --- fence/blueprints/google.py | 2 +- fence/config.py | 46 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) diff --git a/fence/blueprints/google.py b/fence/blueprints/google.py index d2134e9aa..225a7ae2c 100644 --- a/fence/blueprints/google.py +++ b/fence/blueprints/google.py @@ -62,7 +62,7 @@ def make_google_blueprint(): ) blueprint_api.add_resource( - GoogleBillingAccount, "/billing_account", strict_slashes=False + GoogleBillingAccount, "/billing_project", strict_slashes=False ) blueprint_api.add_resource( diff --git a/fence/config.py b/fence/config.py index 5c771d4ba..67c9a7faf 100644 --- a/fence/config.py +++ b/fence/config.py @@ -5,6 +5,9 @@ import cirrus from gen3config import Config +from cdislogging import get_logger + +logger = get_logger(__name__) DEFAULT_CFG_PATH = os.path.join( os.path.dirname(os.path.abspath(__file__)), "config-default.yaml" @@ -66,5 +69,48 @@ def post_process(self): cirrus.config.config.update(**self._configs.get("CIRRUS_CFG", {})) + # if we have a default google project for billing requester pays, we should + # NOT allow end-users to have permission to create Temporary Google Service + # Account credentials, as they could use the default project to bill non-Fence + # aware Google Buckets + # + # NOTE: This does NOT restrict clients from generating temporary service account + # credentials under the assumption that the clients are trusted 1) not + # to share the credentials directly with end-users and 2) will not mis-use + # billing rights (in other words, only use it when interacting with buckets + # Fence is aware of) + if self._configs.get("GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"): + if ( + "USER_ALLOWED_SCOPES" in self._configs + and "google_credentials" in self._configs["USER_ALLOWED_SCOPES"] + ): + logger.warning( + "Configuration does not restrict end-user access to billing. Correcting. " + "GOOGLE_REQUESTER_PAYS_BILLING_PROJECT is set to a non-None value, {}. " + "USER_ALLOWED_SCOPES includes `google_credentials`. Removing " + "`google_credentials` from USER_ALLOWED_SCOPES as this would allow " + "end-users to indescriminently bill our default project. Clients are inheritently " + "trusted, so we do not restrict this scope for clients.".format( + self._configs.get("GOOGLE_REQUESTER_PAYS_BILLING_PROJECT") + ) + ) + self._configs["USER_ALLOWED_SCOPES"].remove("google_credentials") + + if ( + "SESSION_ALLOWED_SCOPES" in self._configs + and "google_credentials" in self._configs["SESSION_ALLOWED_SCOPES"] + ): + logger.warning( + "Configuration does not restrict end-user access to billing. Correcting. " + "GOOGLE_REQUESTER_PAYS_BILLING_PROJECT is set to a non-None value, {}. " + "SESSION_ALLOWED_SCOPES includes `google_credentials`. Removing " + "`google_credentials` from SESSION_ALLOWED_SCOPES as this would allow " + "end-users to indescriminently bill our default project. Clients are inheritently " + "trusted, so we do not restrict this scope for clients.".format( + self._configs.get("GOOGLE_REQUESTER_PAYS_BILLING_PROJECT") + ) + ) + self._configs["SESSION_ALLOWED_SCOPES"].remove("google_credentials") + config = FenceConfig(DEFAULT_CFG_PATH) From c317016b5e3b2d32ee3568e1303e7559432741b5 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Wed, 24 Jul 2019 16:33:18 -0500 Subject: [PATCH 10/22] feat(google): additional logging, openapi updates --- fence/resources/google/utils.py | 7 ++++ openapis/swagger.yaml | 57 +++++++++++++++++++++++++++++---- 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/fence/resources/google/utils.py b/fence/resources/google/utils.py index 0cf70def3..53d5271f7 100644 --- a/fence/resources/google/utils.py +++ b/fence/resources/google/utils.py @@ -266,6 +266,13 @@ def give_service_account_billing_access_if_necessary( ) ) + logger.info( + "Created a custom role in Google Project {} to " + "give Google service account {} rights to bill the project.".format( + r_pays_project, sa_account_id + ) + ) + def create_google_access_key(client_id, user_id, username, proxy_group_id): """ diff --git a/openapis/swagger.yaml b/openapis/swagger.yaml index e91a4805b..01dc02125 100644 --- a/openapis/swagger.yaml +++ b/openapis/swagger.yaml @@ -324,6 +324,12 @@ paths: public url without using anonymous signing creds). schema: type: boolean + - name: userProject + required: false + in: query + description: a Google Project to bill for accessing data in requester pays buckets in Google Storage + schema: + type: string responses: '200': description: successful operation @@ -433,7 +439,7 @@ paths: description: >- For uploading the big file with the size is larger than 5GB in data upload flow, Fence needs to provide a list of endpoints for supporting multipart upload presigned url - This is the first step on the API side for the multipart upload presigned url. This endpoint + This is the first step on the API side for the multipart upload presigned url. This endpoint causes fence to make a request to indexd to create a new, blank index record, and returns the GUID for this new record and an uploadId for multipart upload presigned url security: @@ -461,7 +467,7 @@ paths: uploadId: type: string description: the uploadId for multipart upload presigned URL usable for data upload - + '/multipart/upload': post: tags: @@ -502,7 +508,7 @@ paths: summary: >- Complete aws multipart upload description: >- - This is the last step for the multipart upload presigned url. All the parts which were submitted + This is the last step for the multipart upload presigned url. All the parts which were submitted need to be combined together. This enpoint takes a list of the part info (partNumber, Etag) and uploadId in order to finish the upload security: @@ -571,6 +577,12 @@ paths: the configured maximum will be used. schema: type: integer + - name: userProject + required: false + in: query + description: a Google Project to bill for accessing data in requester pays buckets in Google Storage + schema: + type: string responses: '200': description: Temporary private Google key in Google Credentials File format @@ -586,7 +598,7 @@ paths: security: - OAuth2: - user - operationId: deleteGoogleKeypair + operationId: deleteAllGoogleKeypair parameters: - name: all required: true @@ -1007,6 +1019,29 @@ paths: responses: '302': description: redirect to session-stored redirect value + /google/billing_project/: + get: + tags: + - google + summary: Get the configured default Google project for billing + description: >- + Get the configured default Google project identifier that will be used for + billing access to requester pays Google buckets. This effectively bills all + access to this Project and *not* the end-user. If there is no configured default + billing project, requests to Data Access Method endpoints will rely on + a query parameter `userProject` for the end-user to specify a valid Google + project that Fence has the necessary roles in to provide relevant service + accounts the necessary access. Please see the README for more details. + operationId: getGoogleBillingProject + responses: + '200': + description: 'there is a default billing project, id is in response' + content: + '*/*': + schema: + $ref: '#/components/schemas/GoogleBillingProject' + '404': + description: 'there is no configured default billing project' /google/service_accounts/: post: tags: @@ -1358,7 +1393,7 @@ components: the key of the object in the format of GUID/filename uploadId: type: string - description: the uploadId for multipart presigned URL upload usable for data upload + description: the uploadId for multipart presigned URL upload usable for data upload partNumber: type: integer description: the part number of the part (start from 1) @@ -1375,7 +1410,7 @@ components: the key of the object in the format of GUID/filename uploadId: type: string - description: the uploadId for multipart presigned URL upload usable for data upload + description: the uploadId for multipart presigned URL upload usable for data upload parts: type: array items: @@ -1460,6 +1495,16 @@ components: service_account_email: type: string description: Service account's email + GoogleBillingProject: + type: object + required: + - project_id + properties: + project_id: + type: string + description: >- + Google Project identifier that serves as the default billing project + for all requester pays access GoogleServiceAccounts: type: object properties: From c37cb89c33718f1a1204b5f3854ab17aac2ec25b Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Thu, 25 Jul 2019 09:33:30 -0500 Subject: [PATCH 11/22] fix(google): get default account for signed url requester pays --- fence/blueprints/data/indexd.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fence/blueprints/data/indexd.py b/fence/blueprints/data/indexd.py index db907a887..ee4dd2318 100644 --- a/fence/blueprints/data/indexd.py +++ b/fence/blueprints/data/indexd.py @@ -817,6 +817,10 @@ def _generate_google_storage_signed_url( give_service_account_billing_access_if_necessary(private_key, r_pays_project) + # use configured project if it exists and no user project was given + if config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"] and not r_pays_project: + r_pays_project = config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"] + final_url = cirrus.google_cloud.utils.get_signed_url( resource_path, http_verb, From 2f8ef33e9f36485a034ca14a52db3503f184b915 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Thu, 25 Jul 2019 09:40:02 -0500 Subject: [PATCH 12/22] chore(google): additional logging --- fence/resources/google/access_utils.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/fence/resources/google/access_utils.py b/fence/resources/google/access_utils.py index 6e4e6ba05..c48cedc00 100644 --- a/fence/resources/google/access_utils.py +++ b/fence/resources/google/access_utils.py @@ -696,8 +696,16 @@ def add_user_service_account_to_google( service_account (UserServiceAccount): user service account """ + logger.debug( + "attempting to add {} to groups for projects: {}".format( + service_account, to_add_project_ids + ) + ) for project_id in to_add_project_ids: access_groups = _get_google_access_groups(session, project_id) + logger.debug( + "google group(s) for project {}: {}".format(project_id, access_groups) + ) for access_group in access_groups: try: # TODO: Need to remove try/catch after major refactor From 3e19679a695b90b8874650dcb90f3fb2d19762fc Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Fri, 26 Jul 2019 12:41:58 -0500 Subject: [PATCH 13/22] feat(google): separate logic for automatic billing setup and default billing project for signed urls and temp creds --- docs/google_architecture.md | 36 +++++++++----- fence/blueprints/data/indexd.py | 11 +++-- fence/blueprints/google.py | 17 ++++--- fence/blueprints/storage_creds/google.py | 7 ++- fence/config-default.yaml | 21 ++++++-- fence/config.py | 10 ++-- fence/resources/google/utils.py | 20 ++++++-- openapis/swagger.yaml | 61 +++++++++++++++++------- 8 files changed, 125 insertions(+), 58 deletions(-) diff --git a/docs/google_architecture.md b/docs/google_architecture.md index 654c35b9a..51b829744 100644 --- a/docs/google_architecture.md +++ b/docs/google_architecture.md @@ -72,27 +72,29 @@ The scopes that allow access to these methods are also available for individual Google supports a bucket-level configuration called ["Requester Pays"](https://cloud.google.com/storage/docs/requester-pays) which effectively pushes the cost of data access in a Google Storage bucket to the entity accessing the data. -> By default, the Google project the Google Bucket is in gets billed. +> Without requester pays enabled, the Google project the Google Bucket is in gets billed. The Data Access methods [Signed URLs](#signed-urls) and [Temporary Service Account Credentials](#temporary-service-account-credentials) support accessing requester pays buckets with some additional configuration and considerations (noteably: how it affects end-users). -In order to fully understand the options for requester pays support, it's important to first understand the technical steps to get any of these Google Data Access methods to work, as detailed in the library Fence uses for Google API interactions, [cirrus](https://github.com/uc-cdis/cirrus). It would also be useful to read through the access method details for [Signed URLs](#signed-urls) and [Temporary Service Account Credentials](#temporary-service-account-credentials) first. +In order to fully understand the options for requester pays support, it's important to first understand the technical steps to get any of these Google Data Access methods to work, as detailed in the library Fence uses for Google API interactions, [cirrus](https://github.com/uc-cdis/cirrus). It would also be useful to read through the access method details for [Signed URLs](#signed-urls) and [Temporary Service Account Credentials](#temporary-service-account-credentials). #### Options for Billing Project(s) -The easiest option for supporting requester pays is to simply bill a Google Project you already own for all access to the bucket instead of requiring end-users to supply a project to bill. This essentially makes the requester pays bucket a non-requester pays bucket, since you'll be paying for all the access. But this may be a necessary solution in cases where: +The easiest option for supporting requester pays is to simply bill a Google Project you already own for all access to the bucket instead of requiring end-users to supply a project to bill. This essentially makes the requester pays bucket a non-requester pays bucket, since you'll be paying for all the access. This may be a necessary solution in cases where: -1) you want to serve data from a bucket you don't fully control +1) you want to serve data from a bucket you don't fully control (in other words, can't just turn "requester pays" off) 2) you don't want end-users to have to do manual configuration in Google Cloud Platform to enable billing their project -3) you/end-users don't want to have to give your application IAM permissions in a project the end-user owns +3) you/end-users don't want to have to give your application IAM permissions in a project the end-user owns to automatically enable billing -> NOTE: It is theoretically possible to eliminate `3` by having Fence assume that `2` has happened. However, the current implementation does **not** do that. It programatically attempts to create a custom role in the billing project and provide the necessary access to reduce the manual configuration required. This is, of course, at the expense of requiring that end-users trust your application enough to give it IAM permissions in their project. +**NOTE:** If you do _not_ want to bill yourself for access, it is possible to require end-users to provide the project to bill OR configure a default billing project other than one you own. _However_, this will require more work for end-users that you need to consider. + +To bill a project you do _not_ own, users either need to do `2` from above (manually give the necessary service account(s) (that Fence uses) access to bill the project they specify) OR they can agree to `3` (let Fence automatically assign the necessary billing permission in the project they specify). `3` requires that the Fence admin service account have the necessary roles in the users project(s) though. More details about that further down. #### Billing Your Own Project To bill a project you own, Fence offers an optional configuration for a "default billing project" which you can set to be a project that you manage and can provide the necessary permissions to the Fence admin service account. -> NOTE: At the time of writing, the configuration variable for the "default billing project" is `GOOGLE_REQUESTER_PAYS_BILLING_PROJECT`. The configuration for the Fence admin service account is `CIRRUS_CFG/GOOGLE_ADMIN_EMAIL`. +> NOTE: At the time of writing, the configuration variable for the "default billing project" for signed urls is `BILLING_PROJECT_FOR_SIGNED_URLS`. The "default billing project" for temporary service account credentials is `BILLING_PROJECT_FOR_SA_CREDS`. The configuration for the Fence admin service account is `CIRRUS_CFG/GOOGLE_ADMIN_EMAIL` and is available through the API. For the [Temporary Service Account Credentials](#temporary-service-account-credentials) access methods, clients need to know what the "default billing project" is (to include in their direct requests to Google). The configured "default billing project" is exposed through an API endpoint [detailed here](http://petstore.swagger.io/?url=https://raw.githubusercontent.com/uc-cdis/fence/master/openapis/swagger.yaml#/google). @@ -104,20 +106,28 @@ If you do **not** want to bill a project you own and actually require end-users #### Required Google Cloud Platform (GCP) Configuration for Billing Project -Whether you bill your own project, or require end-users to specify a billing project, the required configuration in GCP is the same. +Whether you bill your own project, or require end-users to specify a billing project, the required configuration in GCP is the same. The service account used to sign the URL and/or the service account used for Temporary Service Account Credentials needs the GCP permission `serviceusage.services.use` in the Google Project specified to bill to. + +> "All actions that include a billing project in the request require serviceusage.services.use permission for the project that's specified" [according to Google's docs](https://cloud.google.com/storage/docs/access-control/iam-console). + +You have 2 options to achieve the above: + +1) assume end-users will provide the necessary permission for billing +2) configure Fence to automatically attempt to provide the necessary permission for billing -The Fence admin service account needs a couple pre-defined Google roles (through their Cloud IAM) on whatever project is provided for billing (be that in a request to Fence or whatever is set as the "default billing project"): +If you want Fence to automatically attempt to provide the necessary permissions to the relevant service accounts for data access, the Fence admin service account needs a couple pre-defined Google roles (through their Cloud IAM) on whatever project is provided for billing (be that in a request to Fence or whatever is configured as the "default billing project"): * `Project IAM Admin`: to update the project's policy to give the necessary service account(s) billing permission * `Role Administrator`: for creating a custom role that only provides billing permission to the project -> NOTE: The custom role that Fence creates contains a single permission in Google `serviceusage.services.use`. "All actions that include a billing project in the request require serviceusage.services.use permission for the project that's specified" [according to Google's docs](https://cloud.google.com/storage/docs/access-control/iam-console). +> NOTE: The custom role that Fence creates contains the single permission in Google `serviceusage.services.use`. -So once you configure a "default billing project" and give the Fence admin service account the roles mentioned above, access to any requester pays buckets will be billed to that project. This is accomplished in different ways depending on the access method: +#### Requester Pays Signed URLs and Temporary Service Account Credentials 1) For [Signed URLs](#signed-urls): a `userProject=` query parameter will be appended to the signed url - * will only be appended if a valid `userProject` is provided in the request **or** Fence is configured with a "default billing project" -2) For [Temporary Service Account Credentials](#temporary-service-account-credentials): the service account key provided will have the necessary permissions on the `userProject` provided so that subsequent requests to Google using these service account credentials will allow specifying that `userProject` to bill + * will only be appended if a valid `userProject` is provided in the request **or** Fence is configured with a "default billing project" for signed URLs + * if Fence is configured to automatically enable billing permission, it will do that for the service account used to sign the URL +2) For [Temporary Service Account Credentials](#temporary-service-account-credentials): iff Fence was configured to automatically enable billing permission, the service account key provided will have the necessary permissions on the `userProject` provided (in request or configured "default billing project") so that subsequent requests to Google using these service account credentials will allow specifying that `userProject` to bill * depending on how the creds are used, this may involve adding additional query params or args to Google SDKs/services to provide the `userProject` Example for Google's Cloud Storage SDK `gsutil`: diff --git a/fence/blueprints/data/indexd.py b/fence/blueprints/data/indexd.py index ee4dd2318..7dec43714 100644 --- a/fence/blueprints/data/indexd.py +++ b/fence/blueprints/data/indexd.py @@ -815,11 +815,16 @@ def _generate_google_storage_signed_url( user_id=user_id, username=username, proxy_group_id=proxy_group_id ) - give_service_account_billing_access_if_necessary(private_key, r_pays_project) + if config["ENABLE_AUTOMATIC_BILLING_PERMISSION_SIGNED_URLS"]: + give_service_account_billing_access_if_necessary( + private_key, + r_pays_project, + default_billing_project=config["BILLING_PROJECT_FOR_SIGNED_URLS"], + ) # use configured project if it exists and no user project was given - if config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"] and not r_pays_project: - r_pays_project = config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"] + if config["BILLING_PROJECT_FOR_SIGNED_URLS"] and not r_pays_project: + r_pays_project = config["BILLING_PROJECT_FOR_SIGNED_URLS"] final_url = cirrus.google_cloud.utils.get_signed_url( resource_path, diff --git a/fence/blueprints/google.py b/fence/blueprints/google.py index 225a7ae2c..e220f7f94 100644 --- a/fence/blueprints/google.py +++ b/fence/blueprints/google.py @@ -62,7 +62,7 @@ def make_google_blueprint(): ) blueprint_api.add_resource( - GoogleBillingAccount, "/billing_project", strict_slashes=False + GoogleBillingAccount, "/billing_projects", strict_slashes=False ) blueprint_api.add_resource( @@ -108,15 +108,14 @@ def __init__(self, email, project_access, google_project_id, user_id=None): class GoogleBillingAccount(Resource): def get(self): """ - Get the configured default Google billing project if it exists. + Get the configured default Google billing projects if it exists. """ - if not config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"]: - return ( - "No configured default Google billing project for accessing requester pays data", - 404, - ) - - return {"project_id": config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"]} + return { + "signed_urls": {"project_id": config["BILLING_PROJECT_FOR_SIGNED_URLS"]}, + "temporary_service_account_credentials": { + "project_id": config["BILLING_PROJECT_FOR_SA_CREDS"] + }, + } class GoogleServiceAccountRoot(Resource): diff --git a/fence/blueprints/storage_creds/google.py b/fence/blueprints/storage_creds/google.py index 6e00d16bc..3abebf2f1 100644 --- a/fence/blueprints/storage_creds/google.py +++ b/fence/blueprints/storage_creds/google.py @@ -156,7 +156,12 @@ def post(self): client_id, user_id, username, proxy_group_id ) - give_service_account_billing_access_if_necessary(key, r_pays_project) + if config["ENABLE_AUTOMATIC_BILLING_PERMISSION_SA_CREDS"]: + give_service_account_billing_access_if_necessary( + key, + r_pays_project, + default_billing_project=config["BILLING_PROJECT_FOR_SA_CREDS"], + ) if client_id is None: self.handle_user_service_account_creds(key, service_account) diff --git a/fence/config-default.yaml b/fence/config-default.yaml index d4e504686..759e31640 100644 --- a/fence/config-default.yaml +++ b/fence/config-default.yaml @@ -484,10 +484,20 @@ GOOGLE_GROUP_PREFIX: '' GOOGLE_SERVICE_ACCOUNT_PREFIX: '' # A Google Project identitifier representing the default project to bill to for -# accessing Google Requester Pays buckets. If this is provided and the API call for -# Google access does not include a `userProject`, this will be used instead. Fence -# will automatically attempt to create a Custom Role in the project and give the -# necessary Google Service Account that role (which will allow it to bill to the project) +# accessing Google Requester Pays buckets (for signed urls and/or temporary service account +# credentials). If this is provided and the API call for +# Google access does not include a `userProject`, this will be used instead. +# +# WARNING: Setting this WITHOUT setting "ENABLE_AUTOMATIC_BILLING_*" to `true` below, +# means that clients and end-users will be responsible for making sure that +# the service account used in either of these methods actually has billing +# permission in the specified project. +BILLING_PROJECT_FOR_SIGNED_URLS: +BILLING_PROJECT_FOR_SA_CREDS: + +# Setting this to `true` will make Fence automatically attempt to create a Custom Role +# in the billing project and give the necessary Google Service Account that role +# (which will allow it to bill to the project). # # NOTE: The Fence SA will need the necessary permissions in the specified project to # both create a custom role and update the Project's IAM Policy to include the @@ -498,7 +508,8 @@ GOOGLE_SERVICE_ACCOUNT_PREFIX: '' # NOTE2: It may be possible to further restrict the permissions in the future to # be more fine-grained. # -GOOGLE_REQUESTER_PAYS_BILLING_PROJECT: +ENABLE_AUTOMATIC_BILLING_PERMISSION_SIGNED_URLS: false +ENABLE_AUTOMATIC_BILLING_PERMISSION_SA_CREDS: false # ////////////////////////////////////////////////////////////////////////////////////// # EMAIL diff --git a/fence/config.py b/fence/config.py index 67c9a7faf..16db9b58e 100644 --- a/fence/config.py +++ b/fence/config.py @@ -79,19 +79,19 @@ def post_process(self): # to share the credentials directly with end-users and 2) will not mis-use # billing rights (in other words, only use it when interacting with buckets # Fence is aware of) - if self._configs.get("GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"): + if self._configs.get("BILLING_PROJECT_FOR_SA_CREDS"): if ( "USER_ALLOWED_SCOPES" in self._configs and "google_credentials" in self._configs["USER_ALLOWED_SCOPES"] ): logger.warning( "Configuration does not restrict end-user access to billing. Correcting. " - "GOOGLE_REQUESTER_PAYS_BILLING_PROJECT is set to a non-None value, {}. " + "BILLING_PROJECT_FOR_SA_CREDS is set to a non-None value, {}. " "USER_ALLOWED_SCOPES includes `google_credentials`. Removing " "`google_credentials` from USER_ALLOWED_SCOPES as this would allow " "end-users to indescriminently bill our default project. Clients are inheritently " "trusted, so we do not restrict this scope for clients.".format( - self._configs.get("GOOGLE_REQUESTER_PAYS_BILLING_PROJECT") + self._configs.get("BILLING_PROJECT_FOR_SA_CREDS") ) ) self._configs["USER_ALLOWED_SCOPES"].remove("google_credentials") @@ -102,12 +102,12 @@ def post_process(self): ): logger.warning( "Configuration does not restrict end-user access to billing. Correcting. " - "GOOGLE_REQUESTER_PAYS_BILLING_PROJECT is set to a non-None value, {}. " + "BILLING_PROJECT_FOR_SA_CREDS is set to a non-None value, {}. " "SESSION_ALLOWED_SCOPES includes `google_credentials`. Removing " "`google_credentials` from SESSION_ALLOWED_SCOPES as this would allow " "end-users to indescriminently bill our default project. Clients are inheritently " "trusted, so we do not restrict this scope for clients.".format( - self._configs.get("GOOGLE_REQUESTER_PAYS_BILLING_PROJECT") + self._configs.get("BILLING_PROJECT_FOR_SA_CREDS") ) ) self._configs["SESSION_ALLOWED_SCOPES"].remove("google_credentials") diff --git a/fence/resources/google/utils.py b/fence/resources/google/utils.py index 53d5271f7..a331eb5ef 100644 --- a/fence/resources/google/utils.py +++ b/fence/resources/google/utils.py @@ -19,7 +19,7 @@ from fence.auth import current_token from fence.config import config -from fence.errors import NotSupported, InternalError +from fence.errors import NotSupported, InternalError, UserError from fence.models import ( GoogleServiceAccount, GoogleServiceAccountKey, @@ -187,7 +187,7 @@ def get_sa_email_from_private_key(sa_private_key): def give_service_account_billing_access_if_necessary( - sa_private_key, r_pays_project=None + sa_private_key, r_pays_project=None, default_billing_project=None ): """ Give the Service Account (whose key is provided) the privilege to bill to the @@ -215,11 +215,21 @@ def give_service_account_billing_access_if_necessary( } r_pays_project (str, optional): The Google Project identifier to bill to """ - is_default_billing = False + if not r_pays_project and not default_billing_project: + raise UserError( + "You did NOT provide a `userProject` for requester pays billing, " + "so we could not create a custom role in that project to provide " + "the necessary service account ({}) billing permission. " + "Our main service account ({}) will need valid permissions in the " + "project you supplied to create a custom role and change the project IAM policy.".format( + sa_account_id, config["CIRRUS_CFG"].get("GOOGLE_ADMIN_EMAIL") + ) + ) # use configured project if it exists and no user project was given - if config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"] and not r_pays_project: - r_pays_project = config["GOOGLE_REQUESTER_PAYS_BILLING_PROJECT"] + is_default_billing = False + if default_billing_project and not r_pays_project: + r_pays_project = default_billing_project is_default_billing = True if r_pays_project: diff --git a/openapis/swagger.yaml b/openapis/swagger.yaml index 01dc02125..a0e2a5076 100644 --- a/openapis/swagger.yaml +++ b/openapis/swagger.yaml @@ -327,7 +327,13 @@ paths: - name: userProject required: false in: query - description: a Google Project to bill for accessing data in requester pays buckets in Google Storage + description: >- + a Google Project to bill for accessing data in requester pays buckets in Google Storage. + Will override any configured default billing projects. If Fence is configured + to automatically attempt to provide the necessary service account(s) billing permission, + the provided Google Project will need to have given the Fence admin service account + necessary permissions to create custom roles and set Project IAM policies. Please see + README for more information. schema: type: string responses: @@ -580,7 +586,13 @@ paths: - name: userProject required: false in: query - description: a Google Project to bill for accessing data in requester pays buckets in Google Storage + description: >- + a Google Project to bill for accessing data in requester pays buckets in Google Storage. + Will override any configured default billing projects. If Fence is configured + to automatically attempt to provide the necessary service account(s) billing permission, + the provided Google Project will need to have given the Fence admin service account + necessary permissions to create custom roles and set Project IAM policies. Please see + README for more information. schema: type: string responses: @@ -1019,29 +1031,28 @@ paths: responses: '302': description: redirect to session-stored redirect value - /google/billing_project/: + /google/billing_projects/: get: tags: - google summary: Get the configured default Google project for billing description: >- - Get the configured default Google project identifier that will be used for + Get the configured default Google project identifiers that will be used for billing access to requester pays Google buckets. This effectively bills all - access to this Project and *not* the end-user. If there is no configured default + access to the specified Project and *not* the end-user (may be a different project + for different data access methods). If there is no configured default billing project, requests to Data Access Method endpoints will rely on a query parameter `userProject` for the end-user to specify a valid Google project that Fence has the necessary roles in to provide relevant service accounts the necessary access. Please see the README for more details. - operationId: getGoogleBillingProject + operationId: getGoogleBillingProjects responses: '200': - description: 'there is a default billing project, id is in response' + description: 'billing projects in response.' content: '*/*': schema: - $ref: '#/components/schemas/GoogleBillingProject' - '404': - description: 'there is no configured default billing project' + $ref: '#/components/schemas/GoogleBillingProjects' /google/service_accounts/: post: tags: @@ -1495,16 +1506,32 @@ components: service_account_email: type: string description: Service account's email - GoogleBillingProject: + GoogleBillingProjects: type: object required: - - project_id + - signed_urls + - temporary_service_account_credentials properties: - project_id: - type: string - description: >- - Google Project identifier that serves as the default billing project - for all requester pays access + signed_urls: + type: object + required: + - project_id + properties: + project_id: + type: string + description: >- + Google Project identifier that serves as the default billing project + for all requester pays access + temporary_service_account_credentials: + type: object + required: + - project_id + properties: + project_id: + type: string + description: >- + Google Project identifier that serves as the default billing project + for all requester pays access GoogleServiceAccounts: type: object properties: From 318737b21ddddb8b6a2cb75813c7fe4e4217751e Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Mon, 29 Jul 2019 16:14:20 -0500 Subject: [PATCH 14/22] fix(scopes): don't allow SA temp creds for end-users if any default billing account is set --- fence/config.py | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/fence/config.py b/fence/config.py index f192394dd..55b673d10 100644 --- a/fence/config.py +++ b/fence/config.py @@ -80,20 +80,20 @@ def post_process(self): # to share the credentials directly with end-users and 2) will not mis-use # billing rights (in other words, only use it when interacting with buckets # Fence is aware of) - if self._configs.get("BILLING_PROJECT_FOR_SA_CREDS"): + if self._configs.get("BILLING_PROJECT_FOR_SA_CREDS") or self._configs.get( + "BILLING_PROJECT_FOR_SIGNED_URLS" + ): if ( "USER_ALLOWED_SCOPES" in self._configs and "google_credentials" in self._configs["USER_ALLOWED_SCOPES"] ): logger.warning( "Configuration does not restrict end-user access to billing. Correcting. " - "BILLING_PROJECT_FOR_SA_CREDS is set to a non-None value, {}. " + "BILLING_PROJECT_FOR_SA_CREDS or BILLING_PROJECT_FOR_SIGNED_URLS is set to a non-None value. " "USER_ALLOWED_SCOPES includes `google_credentials`. Removing " - "`google_credentials` from USER_ALLOWED_SCOPES as this would allow " + "`google_credentials` from USER_ALLOWED_SCOPES as this could allow " "end-users to indescriminently bill our default project. Clients are inheritently " - "trusted, so we do not restrict this scope for clients.".format( - self._configs.get("BILLING_PROJECT_FOR_SA_CREDS") - ) + "trusted, so we do not restrict this scope for clients." ) self._configs["USER_ALLOWED_SCOPES"].remove("google_credentials") @@ -103,13 +103,11 @@ def post_process(self): ): logger.warning( "Configuration does not restrict end-user access to billing. Correcting. " - "BILLING_PROJECT_FOR_SA_CREDS is set to a non-None value, {}. " - "SESSION_ALLOWED_SCOPES includes `google_credentials`. Removing " - "`google_credentials` from SESSION_ALLOWED_SCOPES as this would allow " + "BILLING_PROJECT_FOR_SA_CREDS or BILLING_PROJECT_FOR_SIGNED_URLS is set to a non-None value. " + "USER_ALLOWED_SCOPES includes `google_credentials`. Removing " + "`google_credentials` from USER_ALLOWED_SCOPES as this could allow " "end-users to indescriminently bill our default project. Clients are inheritently " - "trusted, so we do not restrict this scope for clients.".format( - self._configs.get("BILLING_PROJECT_FOR_SA_CREDS") - ) + "trusted, so we do not restrict this scope for clients." ) self._configs["SESSION_ALLOWED_SCOPES"].remove("google_credentials") From 329eaa5d120c0263176b378ab2e85fe895bc7d02 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Tue, 30 Jul 2019 10:13:25 -0500 Subject: [PATCH 15/22] fix(import): add missing config import --- fence/blueprints/storage_creds/google.py | 1 + 1 file changed, 1 insertion(+) diff --git a/fence/blueprints/storage_creds/google.py b/fence/blueprints/storage_creds/google.py index 3abebf2f1..d6c16cf1b 100644 --- a/fence/blueprints/storage_creds/google.py +++ b/fence/blueprints/storage_creds/google.py @@ -9,6 +9,7 @@ from cirrus import GoogleCloudManager from cirrus.config import config as cirrus_config +from fence.config import config from fence.auth import require_auth_header from fence.auth import current_token from fence.errors import UserError From cce365996d1b042e6b4d5083feac269bf3a72b93 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Tue, 30 Jul 2019 10:21:45 -0500 Subject: [PATCH 16/22] chore(cleanup): use cirrus tag, cleanup unneeded code --- fence/blueprints/data/indexd.py | 6 ------ requirements.txt | 2 +- setup.py | 1 + 3 files changed, 2 insertions(+), 7 deletions(-) diff --git a/fence/blueprints/data/indexd.py b/fence/blueprints/data/indexd.py index 7dec43714..5393f22c4 100644 --- a/fence/blueprints/data/indexd.py +++ b/fence/blueprints/data/indexd.py @@ -56,12 +56,6 @@ def get_signed_url_for_file(action, file_id): requested_protocol = flask.request.args.get("protocol", None) r_pays_project = flask.request.args.get("userProject", None) - # TODO maybe we want an arg to ask NOT to setup the user project billing in case - # it is provided out of the knowledge of this fence instance? e.g. someone - # manually will provide the SA access to the given project - setup_billing = flask.request.args.get("setupUserProjectBilling", None) - # TODO ^^^^ do we need this?? - # default to signing the url even if it's a public object # this will work so long as we're provided a user token force_signed_url = True diff --git a/requirements.txt b/requirements.txt index acd2d23f4..1f073d509 100644 --- a/requirements.txt +++ b/requirements.txt @@ -16,6 +16,7 @@ Flask_OAuthlib==0.9.4 flask-restful==0.3.6 Flask_SQLAlchemy_Session==1.1 gen3config==0.1.7 +gen3cirrus==1.1.0 gen3users httplib2==0.10.3 markdown==3.1.1 @@ -38,4 +39,3 @@ Werkzeug==0.12.2 pyyaml==5.1 retry==0.9.2 git+https://github.com/uc-cdis/storage-client.git@1.0.0#egg=storageclient -git+https://git@github.com/uc-cdis/cirrus.git@feat/requester-pays#egg=cirrus diff --git a/setup.py b/setup.py index 7d9a13638..3b8bbf493 100644 --- a/setup.py +++ b/setup.py @@ -18,6 +18,7 @@ "Flask_OAuthlib>=0.9.4,<1.0.0", "Flask_SQLAlchemy_Session>=1.1,<2.0", "gen3config>=0.1.6,<1.0.0", + "gen3cirrus>=1.1.0,<2.0", "google_api_python_client>=1.6.4,<2.0.0", "httplib2>=0.10.3,<1.0.0", "markdown>=3.1.1,<4.0.0", From e94fe0ac9246cc9ad6ac5008f220fc955fbd06b0 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Tue, 30 Jul 2019 15:24:51 -0500 Subject: [PATCH 17/22] Update docs/google_architecture.md Co-Authored-By: Pauline Ribeyre --- docs/google_architecture.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/google_architecture.md b/docs/google_architecture.md index 5fab11028..0a2aee73a 100644 --- a/docs/google_architecture.md +++ b/docs/google_architecture.md @@ -96,7 +96,7 @@ To bill a project you own, Fence offers an optional configuration for a "default > NOTE: At the time of writing, the configuration variable for the "default billing project" for signed urls is `BILLING_PROJECT_FOR_SIGNED_URLS`. The "default billing project" for temporary service account credentials is `BILLING_PROJECT_FOR_SA_CREDS`. The configuration for the Fence admin service account is `CIRRUS_CFG/GOOGLE_ADMIN_EMAIL` and is available through the API. -For the [Temporary Service Account Credentials](#temporary-service-account-credentials) access methods, clients need to know what the "default billing project" is (to include in their direct requests to Google). The configured "default billing project" is exposed through an API endpoint [detailed here](http://petstore.swagger.io/?url=https://raw.githubusercontent.com/uc-cdis/fence/master/openapis/swagger.yaml#/google). +For the [Temporary Service Account Credentials](#temporary-service-account-credentials) access methods, clients need to know what the "default billing project" is (to include in their direct requests to Google). The configured "default billing project" is exposed through an API endpoint [detailed here](http://petstore.swagger.io/?url=https://raw.githubusercontent.com/uc-cdis/fence/master/openapis/swagger.yaml#/google/getGoogleBillingProjects). #### End-users Specifying a Billing Project From 69aede92bca60569f43abdfbb4822a6871afcf06 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Tue, 30 Jul 2019 15:26:39 -0500 Subject: [PATCH 18/22] Update docs/google_architecture.md Co-Authored-By: Pauline Ribeyre --- docs/google_architecture.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/google_architecture.md b/docs/google_architecture.md index 0a2aee73a..f6aec40cf 100644 --- a/docs/google_architecture.md +++ b/docs/google_architecture.md @@ -100,7 +100,7 @@ For the [Temporary Service Account Credentials](#temporary-service-account-crede #### End-users Specifying a Billing Project -API requests to create a signed url and get temporary service account credentials also support the end-user providing a project to bill, in the form of a query param `userProject`. See the [API Documentation](#API-documentation) for more details. +API requests to create a signed url and get temporary service account credentials also support the end-user providing a project to bill, in the form of a query param `userProject`. See the [API Documentation](https://github.com/uc-cdis/fence/tree/master#API-documentation) for more details. If you do **not** want to bill a project you own and actually require end-users to pay for access to requester pays buckets, it will require manual configuration by the end-users. The configuration necessary for billing a project is the same whether you or an end-user has to enable it, as detailed below. From f41c241ced75a4884033a39007ad1a2bd6652c53 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Wed, 31 Jul 2019 11:50:36 -0500 Subject: [PATCH 19/22] Update docs/google_architecture.md Co-Authored-By: Pauline Ribeyre --- docs/google_architecture.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/google_architecture.md b/docs/google_architecture.md index f6aec40cf..a5c7ff49e 100644 --- a/docs/google_architecture.md +++ b/docs/google_architecture.md @@ -127,7 +127,7 @@ If you want Fence to automatically attempt to provide the necessary permissions 1) For [Signed URLs](#signed-urls): a `userProject=` query parameter will be appended to the signed url * will only be appended if a valid `userProject` is provided in the request **or** Fence is configured with a "default billing project" for signed URLs * if Fence is configured to automatically enable billing permission, it will do that for the service account used to sign the URL -2) For [Temporary Service Account Credentials](#temporary-service-account-credentials): iff Fence was configured to automatically enable billing permission, the service account key provided will have the necessary permissions on the `userProject` provided (in request or configured "default billing project") so that subsequent requests to Google using these service account credentials will allow specifying that `userProject` to bill +2) For [Temporary Service Account Credentials](#temporary-service-account-credentials): if Fence was configured to automatically enable billing permission, the service account key provided will have the necessary permissions on the `userProject` provided (in request or configured "default billing project") so that subsequent requests to Google using these service account credentials will allow specifying that `userProject` to bill * depending on how the creds are used, this may involve adding additional query params or args to Google SDKs/services to provide the `userProject` Example for Google's Cloud Storage SDK `gsutil`: From 6a2503da6e34f30d8136ebd60b0a71ba8e3dba6c Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Wed, 31 Jul 2019 11:51:00 -0500 Subject: [PATCH 20/22] Update fence/config.py Co-Authored-By: Pauline Ribeyre --- fence/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fence/config.py b/fence/config.py index 55b673d10..b383bbd73 100644 --- a/fence/config.py +++ b/fence/config.py @@ -104,7 +104,7 @@ def post_process(self): logger.warning( "Configuration does not restrict end-user access to billing. Correcting. " "BILLING_PROJECT_FOR_SA_CREDS or BILLING_PROJECT_FOR_SIGNED_URLS is set to a non-None value. " - "USER_ALLOWED_SCOPES includes `google_credentials`. Removing " + "SESSION_ALLOWED_SCOPES includes `google_credentials`. Removing " "`google_credentials` from USER_ALLOWED_SCOPES as this could allow " "end-users to indescriminently bill our default project. Clients are inheritently " "trusted, so we do not restrict this scope for clients." From 67a9a37cc821762220a40f949940c2d938ed94d7 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Wed, 31 Jul 2019 11:55:36 -0500 Subject: [PATCH 21/22] Update fence/resources/google/utils.py Co-Authored-By: Pauline Ribeyre --- fence/resources/google/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fence/resources/google/utils.py b/fence/resources/google/utils.py index a331eb5ef..99deb6ee5 100644 --- a/fence/resources/google/utils.py +++ b/fence/resources/google/utils.py @@ -236,7 +236,7 @@ def give_service_account_billing_access_if_necessary( sa_account_id = get_sa_email_from_private_key(sa_private_key) try: - # if a project is provided, attempt to create custom role that gives + # attempt to create custom role that gives # the SA access to bill the project provided # NOTE: this may fail if our fence SA doesn't have the right permissions # to add this role and update the project policy From 54b163a6bc643eaf2ab3a7741118dcfda826aa18 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Wed, 31 Jul 2019 11:57:32 -0500 Subject: [PATCH 22/22] chore(review): address review comments, refactoring --- fence/resources/google/utils.py | 43 ++++++--------------------------- setup.py | 2 +- 2 files changed, 9 insertions(+), 36 deletions(-) diff --git a/fence/resources/google/utils.py b/fence/resources/google/utils.py index a331eb5ef..a0279bcc2 100644 --- a/fence/resources/google/utils.py +++ b/fence/resources/google/utils.py @@ -155,37 +155,6 @@ def create_primary_service_account_key(user_id, username, proxy_group_id, expire return sa_private_key -def get_sa_email_from_private_key(sa_private_key): - """ - Return the service account email given the a private service account key. - - Args: - sa_private_key (dict): - - JSON key in Google Credentials File format: - - .. code-block:: JavaScript - - { - "type": "service_account", - "project_id": "project-id", - "private_key_id": "some_number", - "private_key": "-----BEGIN PRIVATE KEY-----\n.... - =\n-----END PRIVATE KEY-----\n", - "client_email": "api@project-id.iam.gserviceaccount.com", - "client_id": "...", - "auth_uri": "https://accounts.google.com/o/oauth2/auth", - "token_uri": "https://accounts.google.com/o/oauth2/token", - "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs", - "client_x509_cert_url": "https://www.googleapis.com/...api%40project-id.iam.gserviceaccount.com" - } - - Returns: - str: Service account email - """ - return sa_private_key.get("client_email") - - def give_service_account_billing_access_if_necessary( sa_private_key, r_pays_project=None, default_billing_project=None ): @@ -195,8 +164,7 @@ def give_service_account_billing_access_if_necessary( to bill to, we will use that. Args: - sa_private_key (dict): - JSON key in Google Credentials File format: + sa_private_key (dict): JSON key in Google Credentials File format: .. code-block:: JavaScript @@ -214,14 +182,19 @@ def give_service_account_billing_access_if_necessary( "client_x509_cert_url": "https://www.googleapis.com/...api%40project-id.iam.gserviceaccount.com" } r_pays_project (str, optional): The Google Project identifier to bill to + default_billing_project (str, optional): the default The Google Project + identifier to bill to if r_pays_project is None """ if not r_pays_project and not default_billing_project: + sa_account_id = sa_private_key.get("client_email") raise UserError( "You did NOT provide a `userProject` for requester pays billing, " "so we could not create a custom role in that project to provide " "the necessary service account ({}) billing permission. " "Our main service account ({}) will need valid permissions in the " - "project you supplied to create a custom role and change the project IAM policy.".format( + "project you supplied to create a custom role and change the project " + "IAM policy. There is no configured default billing project so you must " + "provide a `userProject` query parameter.".format( sa_account_id, config["CIRRUS_CFG"].get("GOOGLE_ADMIN_EMAIL") ) ) @@ -233,7 +206,7 @@ def give_service_account_billing_access_if_necessary( is_default_billing = True if r_pays_project: - sa_account_id = get_sa_email_from_private_key(sa_private_key) + sa_account_id = sa_private_key.get("client_email") try: # if a project is provided, attempt to create custom role that gives diff --git a/setup.py b/setup.py index 3b8bbf493..c8fd66e5f 100644 --- a/setup.py +++ b/setup.py @@ -17,8 +17,8 @@ "Flask-CORS>=3.0.3,<4.0.0", "Flask_OAuthlib>=0.9.4,<1.0.0", "Flask_SQLAlchemy_Session>=1.1,<2.0", - "gen3config>=0.1.6,<1.0.0", "gen3cirrus>=1.1.0,<2.0", + "gen3config>=0.1.6,<1.0.0", "google_api_python_client>=1.6.4,<2.0.0", "httplib2>=0.10.3,<1.0.0", "markdown>=3.1.1,<4.0.0",