From da19122f8886662ebd5544d499939fef760c9922 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Tue, 7 May 2019 11:49:45 -0500 Subject: [PATCH 1/4] feat(arborist-sync): change to * permission --- fence/sync/sync_users.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index 7eb198157..f1518d4ab 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -65,7 +65,7 @@ def arborist_role_for_permission(permission): return { "id": permission, "permissions": [ - {"id": permission, "action": {"service": "", "method": permission}} + {"id": permission, "action": {"service": "*", "method": permission}} ], } From b8cc27d7aaee88e7bddfd697fe2e65ff07cb86be Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Tue, 7 May 2019 13:28:29 -0500 Subject: [PATCH 2/4] feat(arborist-sync): support user policy list --- fence/sync/sync_users.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index f1518d4ab..d0060a490 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -1049,6 +1049,9 @@ def _update_arborist(self, session, user_yaml): self.arborist_client.create_user_if_not_exist(username) self.arborist_client.revoke_all_policies_for_user(username) + for policy in user_yaml.policies.get(user.username, []): + self.arborist_client.grant_user_policy(user.username, policy) + for path, permissions in user_resources.iteritems(): for permission in permissions: # "permission" in the dbgap sense, not the arborist sense From c73f70c590d393cc725de21e1635482f0598f2e8 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Tue, 7 May 2019 16:25:52 -0500 Subject: [PATCH 3/4] feat(arborist-sync): leave warning if group exists --- fence/rbac/client.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/fence/rbac/client.py b/fence/rbac/client.py index 63ec4ca9b..49b078574 100644 --- a/fence/rbac/client.py +++ b/fence/rbac/client.py @@ -446,6 +446,11 @@ def create_group(self, name, description="", users=None, policies=None): data["description"] = description response = requests.post(self._group_url, json=data) data = _request_get_json(response) + if response.status_code == 409: + # already exists; this is ok, but leave warning + self.logger.warn( + "group `{}` already exists in arborist".format(name) + ) if response.status_code != 201: msg = data.get("error", "unhelpful response from arborist") self.logger.error("could not create group {}: {}".format(name, msg)) From 3d787811e84655976bf8dce370c6a9187d4790f0 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 10 May 2019 14:49:40 -0500 Subject: [PATCH 4/4] feat(arborist-sync): run formatter --- fence/__init__.py | 6 +++-- fence/blueprints/data/blueprint.py | 6 +---- fence/blueprints/data/indexd.py | 9 ++------ fence/jwt/token.py | 32 +++++++------------------- fence/oidc/endpoints.py | 4 +--- fence/rbac/client.py | 18 ++++----------- fence/resources/admin/admin_users.py | 34 +++++++++++----------------- tests/conftest.py | 1 - tests/data/test_data.py | 22 +++++++++++++----- tests/login/test_fence_login.py | 9 ++------ 10 files changed, 51 insertions(+), 90 deletions(-) diff --git a/fence/__init__.py b/fence/__init__.py index a7b8e4aaa..22a162d86 100644 --- a/fence/__init__.py +++ b/fence/__init__.py @@ -16,7 +16,9 @@ from fence.rbac.client import ArboristClient from fence.resources.aws.boto_manager import BotoManager from fence.resources.openid.google_oauth2 import GoogleOauth2Client as GoogleClient -from fence.resources.openid.microsoft_oauth2 import MicrosoftOauth2Client as MicrosoftClient +from fence.resources.openid.microsoft_oauth2 import ( + MicrosoftOauth2Client as MicrosoftClient +) from fence.resources.openid.orcid_oauth2 import OrcidOauth2Client as ORCIDClient from fence.resources.storage import StorageManager from fence.resources.user.user_session import UserSessionInterface @@ -38,7 +40,7 @@ # Can't read config yet. Just set to debug for now, else no handlers. # Later, in app_config(), will actually set level based on config -logger = get_logger(__name__, log_level='debug') +logger = get_logger(__name__, log_level="debug") app = flask.Flask(__name__) CORS(app=app, headers=["content-type", "accept"], expose_headers="*") diff --git a/fence/blueprints/data/blueprint.py b/fence/blueprints/data/blueprint.py index 75b7f32ad..ddbe71ff2 100644 --- a/fence/blueprints/data/blueprint.py +++ b/fence/blueprints/data/blueprint.py @@ -8,11 +8,7 @@ IndexedFile, get_signed_url_for_file, ) -from fence.errors import ( - Forbidden, - InternalError, - UserError, -) +from fence.errors import Forbidden, InternalError, UserError from fence.utils import is_valid_expiration from fence.rbac import check_arborist_auth diff --git a/fence/blueprints/data/indexd.py b/fence/blueprints/data/indexd.py index c81d08b25..e0f674258 100644 --- a/fence/blueprints/data/indexd.py +++ b/fence/blueprints/data/indexd.py @@ -349,15 +349,10 @@ def check_rbac(self, action): if "rbac" not in self.index_document: raise ValueError("index record missing `rbac`") request = { - "user": { - "jwt": get_jwt(), - }, + "user": {"jwt": get_jwt()}, "request": { "resource": self.index_document["rbac"], - "action": { - "service": "fence", - "method": action, - }, + "action": {"service": "fence", "method": action}, }, } return flask.current_app.arborist.auth_request(request) diff --git a/fence/jwt/token.py b/fence/jwt/token.py index 556b8c34d..f9a4ba8bd 100644 --- a/fence/jwt/token.py +++ b/fence/jwt/token.py @@ -169,9 +169,7 @@ def generate_signed_session_token(kid, private_key, expires_in, context=None): "jti": str(uuid.uuid4()), "context": context, } - logger.debug( - "issuing JWT session token\n" + json.dumps(claims, indent=4) - ) + logger.debug("issuing JWT session token\n" + json.dumps(claims, indent=4)) token = jwt.encode(claims, private_key, headers=headers, algorithm="RS256") token = to_unicode(token, "UTF-8") @@ -271,12 +269,8 @@ def generate_signed_refresh_token( } if flask.current_app: - logger.info( - "issuing JWT refresh token with id [{}] to [{}]".format(jti, sub) - ) - logger.debug( - "issuing JWT refresh token\n" + json.dumps(claims, indent=4) - ) + logger.info("issuing JWT refresh token with id [{}] to [{}]".format(jti, sub)) + logger.debug("issuing JWT refresh token\n" + json.dumps(claims, indent=4)) token = jwt.encode(claims, private_key, headers=headers, algorithm="RS256") token = to_unicode(token, "UTF-8") @@ -313,12 +307,8 @@ def generate_api_key(kid, private_key, user_id, expires_in, scopes, client_id): "jti": jti, "azp": client_id or "", } - logger.info( - "issuing JWT API key with id [{}] to [{}]".format(jti, sub) - ) - logger.debug( - "issuing JWT API key\n" + json.dumps(claims, indent=4) - ) + logger.info("issuing JWT API key with id [{}] to [{}]".format(jti, sub)) + logger.debug("issuing JWT API key\n" + json.dumps(claims, indent=4)) token = jwt.encode(claims, private_key, headers=headers, algorithm="RS256") logger.debug(str(token)) token = to_unicode(token, "UTF-8") @@ -391,12 +381,8 @@ def generate_signed_access_token( ] = linked_google_email if flask.current_app: - logger.info( - "issuing JWT access token with id [{}] to [{}]".format(jti, sub) - ) - logger.debug( - "issuing JWT access token\n" + json.dumps(claims, indent=4) - ) + logger.info("issuing JWT access token with id [{}] to [{}]".format(jti, sub)) + logger.debug("issuing JWT access token\n" + json.dumps(claims, indent=4)) token = jwt.encode(claims, private_key, headers=headers, algorithm="RS256") token = to_unicode(token, "UTF-8") @@ -494,9 +480,7 @@ def generate_id_token( if nonce: claims["nonce"] = nonce - logger.info( - "issuing JWT ID token\n" + json.dumps(claims, indent=4) - ) + logger.info("issuing JWT ID token\n" + json.dumps(claims, indent=4)) token_options = { "iss": {"essential": True, "value": config.get("BASE_URL")}, diff --git a/fence/oidc/endpoints.py b/fence/oidc/endpoints.py index 2d2ce402c..699285b15 100644 --- a/fence/oidc/endpoints.py +++ b/fence/oidc/endpoints.py @@ -59,9 +59,7 @@ def validate_authenticate_client(self): bcrypt.hashpw(client_secret.encode("utf-8"), hashed.encode("utf-8")) != hashed ): - logger.debug( - "client secret hash does not match stored secret hash" - ) + logger.debug("client secret hash does not match stored secret hash") raise InvalidClientError(uri=self.uri) self._client = client diff --git a/fence/rbac/client.py b/fence/rbac/client.py index 49b078574..93dbfcf6d 100644 --- a/fence/rbac/client.py +++ b/fence/rbac/client.py @@ -250,9 +250,7 @@ def create_resource(self, parent_path, resource_json, overwrite=False): msg = data["error"].get("message", msg) resource = resource_json.get("path", "/" + resource_json.get("name")) self.logger.error( - "could not create resource `{}` in arborist: {}".format( - resource, msg - ) + "could not create resource `{}` in arborist: {}".format(resource, msg) ) raise ArboristError(data["error"]) self.logger.info("created resource {}".format(resource_json["name"])) @@ -425,9 +423,7 @@ def revoke_all_policies_for_user(self, username): if response.status_code != 204: msg = data.get("error", "unhelpful response from arborist") self.logger.error( - "could not revoke policies from user `{}`: {}`".format( - username, msg - ) + "could not revoke policies from user `{}`: {}`".format(username, msg) ) return None self.logger.info("revoked all policies from user `{}`".format(username)) @@ -437,20 +433,14 @@ def revoke_all_policies_for_user(self, username): def create_group(self, name, description="", users=None, policies=None): users = users or [] policies = policies or [] - data = { - "name": name, - "users": users, - "policies": policies, - } + data = {"name": name, "users": users, "policies": policies} if description: data["description"] = description response = requests.post(self._group_url, json=data) data = _request_get_json(response) if response.status_code == 409: # already exists; this is ok, but leave warning - self.logger.warn( - "group `{}` already exists in arborist".format(name) - ) + self.logger.warn("group `{}` already exists in arborist".format(name)) if response.status_code != 201: msg = data.get("error", "unhelpful response from arborist") self.logger.error("could not create group {}: {}".format(name, msg)) diff --git a/fence/resources/admin/admin_users.py b/fence/resources/admin/admin_users.py index e14aa50b0..a5c9e2e51 100644 --- a/fence/resources/admin/admin_users.py +++ b/fence/resources/admin/admin_users.py @@ -178,9 +178,7 @@ def delete_google_service_accounts_and_keys(current_session, gcm, gpg_email): def raise_unavailable(sae): raise UnavailableError( - "Error: Google unable to delete service account {}. Aborting".format( - sae - ) + "Error: Google unable to delete service account {}. Aborting".format(sae) ) for sae in service_account_emails: @@ -203,11 +201,11 @@ def raise_unavailable(sae): logger.info( "Google service account with email {} successfully removed " - "from Google, along with all associated service account keys.".format( - sae - ) + "from Google, along with all associated service account keys.".format(sae) + ) + logger.debug( + "Attempting to clear service account records from Fence database..." ) - logger.debug("Attempting to clear service account records from Fence database...") sa = ( current_session.query(GoogleServiceAccount) .filter(GoogleServiceAccount.email == sae) @@ -293,9 +291,8 @@ def raise_unavailable(gpg_email): ) for row in gpg_to_gbag: current_session.delete(row) - logger.debug("Deleting rows in {}...".format( - UserGoogleAccountToProxyGroup.__tablename__ - ) + logger.debug( + "Deleting rows in {}...".format(UserGoogleAccountToProxyGroup.__tablename__) ) uga_to_pg = ( current_session.query(UserGoogleAccountToProxyGroup) @@ -307,10 +304,7 @@ def raise_unavailable(gpg_email): ) for row in uga_to_pg: current_session.delete(row) - logger.debug("Deleting rows in {}...".format( - UserGoogleAccount.__tablename__ - ) - ) + logger.debug("Deleting rows in {}...".format(UserGoogleAccount.__tablename__)) uga = ( current_session.query(UserGoogleAccount) .filter(UserGoogleAccount.user_id == user.id) @@ -323,9 +317,7 @@ def raise_unavailable(gpg_email): current_session.commit() logger.info( "Records for Google proxy group {} successfully cleared from Fence " - "database, along with associated user Google accounts.".format( - gpg_email - ) + "database, along with associated user Google accounts.".format(gpg_email) ) logger.info("Done with Google deletions.") @@ -365,9 +357,7 @@ def delete_user(current_session, username): if google_proxy_group_from_fence_db: gpg_email = google_proxy_group_from_fence_db.email - logger.debug( - "Found Google proxy group in Fence db: {}".format(gpg_email) - ) + logger.debug("Found Google proxy group in Fence db: {}".format(gpg_email)) else: # Construct the proxy group name that would have been used # and check if it exists in cirrus, in case Fence db just @@ -380,7 +370,9 @@ def delete_user(current_session, username): ) google_proxy_group_from_google = gcm.get_group(pgname) gpg_email = ( - google_proxy_group_from_google.get("email") if google_proxy_group_from_google else None + google_proxy_group_from_google.get("email") + if google_proxy_group_from_google + else None ) if not gpg_email: diff --git a/tests/conftest.py b/tests/conftest.py index bfe7ae195..ccec9f3fa 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -182,7 +182,6 @@ def kid_2(): @pytest.fixture(scope="function") def mock_arborist_requests(request): - def do_patch(urls_to_responses=None): urls_to_responses = urls_to_responses or {} defaults = {"arborist/health": {"GET": ("", 200)}} diff --git a/tests/data/test_data.py b/tests/data/test_data.py index 0f44f620a..75c3787b5 100644 --- a/tests/data/test_data.py +++ b/tests/data/test_data.py @@ -561,7 +561,9 @@ def json(self): assert response.status_code == 403, response -def test_initialize_multipart_upload(app, client, auth_client, encoded_creds_jwt, user_client): +def test_initialize_multipart_upload( + app, client, auth_client, encoded_creds_jwt, user_client +): class MockResponse(object): def __init__(self, data, status_code=200): self.data = data @@ -589,7 +591,9 @@ def json(self): data_requests.post.return_value.status_code = 200 arborist_requests.post.return_value = MockResponse({"auth": True}) arborist_requests.post.return_value.status_code = 200 - fence.blueprints.data.indexd.BlankIndex.init_multipart_upload.return_value = "test_uploadId" + fence.blueprints.data.indexd.BlankIndex.init_multipart_upload.return_value = ( + "test_uploadId" + ) headers = { "Authorization": "Bearer " + encoded_creds_jwt.jwt, "Content-Type": "application/json", @@ -610,7 +614,9 @@ def json(self): assert "uploadId" in response.json -def test_multipart_upload_presigned_url(app, client, auth_client, encoded_creds_jwt, user_client): +def test_multipart_upload_presigned_url( + app, client, auth_client, encoded_creds_jwt, user_client +): class MockResponse(object): def __init__(self, data, status_code=200): self.data = data @@ -626,7 +632,9 @@ def json(self): "fence.rbac.client.requests", new_callable=mock.Mock ) - fence.blueprints.data.indexd.BlankIndex.generate_aws_presigned_url_for_part = MagicMock() + fence.blueprints.data.indexd.BlankIndex.generate_aws_presigned_url_for_part = ( + MagicMock() + ) with data_requests_mocker as data_requests, arborist_requests_mocker as arborist_requests: data_requests.post.return_value = MockResponse( { @@ -638,7 +646,9 @@ def json(self): data_requests.post.return_value.status_code = 200 arborist_requests.post.return_value = MockResponse({"auth": True}) arborist_requests.post.return_value.status_code = 200 - fence.blueprints.data.indexd.BlankIndex.generate_aws_presigned_url_for_part.return_value = "test_presigned" + fence.blueprints.data.indexd.BlankIndex.generate_aws_presigned_url_for_part.return_value = ( + "test_presigned" + ) headers = { "Authorization": "Bearer " + encoded_creds_jwt.jwt, "Content-Type": "application/json", @@ -650,4 +660,4 @@ def json(self): response = client.post("/data/multipart/upload", headers=headers, data=data) assert response.status_code == 200, response - assert "presigned_url" in response.json \ No newline at end of file + assert "presigned_url" in response.json diff --git a/tests/login/test_fence_login.py b/tests/login/test_fence_login.py index 72cfbd779..c36b44654 100644 --- a/tests/login/test_fence_login.py +++ b/tests/login/test_fence_login.py @@ -10,12 +10,7 @@ @pytest.fixture(scope="function") def config_idp_in_client( - app, - db_session, - kid_2, - rsa_private_key_2, - rsa_public_key_2, - restore_config, + app, db_session, kid_2, rsa_private_key_2, rsa_public_key_2, restore_config ): """ Set info about this fence's (client fence's) IDP in config. @@ -46,7 +41,7 @@ def config_idp_in_client( "api_base_url": "http://other-fence", "authorize_url": "http://other-fence/oauth2/authorize", } - } + }, } ) app.fence_client = OAuthClient(**config["OPENID_CONNECT"]["fence"])