From 7fa5a3faee7d3c913bdf86614b708b6b7ea00c57 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Tue, 2 Apr 2019 16:10:57 -0500 Subject: [PATCH 01/30] feat(sleek-jwts): remove policies --- fence/jwt/token.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/fence/jwt/token.py b/fence/jwt/token.py index 13e459f7f..86a89a537 100644 --- a/fence/jwt/token.py +++ b/fence/jwt/token.py @@ -393,7 +393,6 @@ def generate_signed_access_token( "must provide value for `iss` (issuer) field if" " running outside of flask application" ) - policies = [policy.id for policy in user.policies] claims = { "pur": "access", @@ -408,7 +407,6 @@ def generate_signed_access_token( "name": user.username, "is_admin": user.is_admin, "projects": dict(user.project_access), - "policies": policies, "google": {"proxy_group": user.google_proxy_group_id}, } }, @@ -481,7 +479,6 @@ def generate_id_token( # If not provided, assume auth time is time this ID token is issued auth_time = auth_time or iat - policies = [policy.id for policy in user.policies] # NOTE: if the claims here are modified, be sure to update the # `claims_supported` field returned from the OIDC configuration endpoint @@ -502,7 +499,6 @@ def generate_id_token( "name": user.username, "is_admin": user.is_admin, "projects": dict(user.project_access), - "policies": policies, "email": user.email, "display_name": user.display_name, "phone_number": user.phone_number, From 6fca9595f56c4acb2bb3751b34d53772020d3342 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Tue, 2 Apr 2019 16:42:43 -0500 Subject: [PATCH 02/30] feat(sleek-jwts): add migration to drop policies --- fence/models.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/fence/models.py b/fence/models.py index e07424614..acbb386b8 100644 --- a/fence/models.py +++ b/fence/models.py @@ -563,6 +563,8 @@ def migrate(driver): _update_for_authlib(driver, md) + _remove_policy(driver, md) + def add_foreign_key_column_if_not_exist( table_name, @@ -854,3 +856,10 @@ def _update_for_authlib(driver, md): ) ) session.commit() + + +def _remove_policy(driver, md): + with driver.session as session: + session.execute("DROP TABLE users_to_policies;") + session.execute("DROP TABLE policy;") + session.commit() From 45f9b95ab2978054c41cd9645e68c38609055792 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Wed, 3 Apr 2019 11:43:10 -0500 Subject: [PATCH 03/30] feat(download-auth): use arborist for data download check --- fence/auth.py | 19 +++++++++++++++++++ fence/blueprints/data/indexd.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+) diff --git a/fence/auth.py b/fence/auth.py index 263032291..6afbc74ad 100644 --- a/fence/auth.py +++ b/fence/auth.py @@ -19,6 +19,25 @@ from fence.config import config +def get_jwt(): + """ + Return the user's JWT from authorization header. Requires flask application context. + + Raises: + - Unauthorized, if header is missing or not in the correct format + """ + header = flask.request.headers.get("Authorization") + if not header: + raise Unauthorized("missing authorization header") + try: + bearer, token = header.split(" ") + except ValueError: + raise Unauthorized("authorization header not in expected format") + if bearer.lower() != "bearer": + raise Unauthorized("expected bearer token in auth header") + return token + + def build_redirect_url(hostname, path): """ Compute a redirect given a hostname and next path where diff --git a/fence/blueprints/data/indexd.py b/fence/blueprints/data/indexd.py index 4744a962d..dfb744540 100644 --- a/fence/blueprints/data/indexd.py +++ b/fence/blueprints/data/indexd.py @@ -10,6 +10,8 @@ import requests from fence.auth import ( + get_jwt, + has_oauth, current_token, login_required, set_current_token, @@ -270,6 +272,23 @@ def set_acls(self): else: raise Unauthorized("This file is not accessible") + def check_rbac(self, action): + if "rbac" not in self.index_document: + raise ValueError("index record missing `rbac`") + request = { + "user": { + "jwt": get_jwt(), + }, + "request": { + "resource": self.index_document["rbac"], + "action": { + "service": "fence", + "method": action, + }, + }, + } + return flask.current_app.arborist.auth_request(request) + @cached_property def metadata(self): return self.index_document.get("metadata", {}) @@ -291,6 +310,15 @@ def check_authorization(self, action): username = flask.g.user.username return self.index_document.get("uploader") == username + try: + # action should be upload or download + # return bool for authorization + return self.check_rbac(action) + except ValueError: + # this is ok; we'll default to ACL field (previous behavior) + # may want to deprecate in future + pass + if flask.g.token is None: given_acls = set(filter_auth_ids(action, flask.g.user.project_access)) else: From b3734dc6093093127ec5ca3ab15d220e19fd974b Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Wed, 3 Apr 2019 12:38:31 -0500 Subject: [PATCH 04/30] feat(download-auth): userdatamodel pin --- requirements.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index b6c98bb92..0c06b4e8a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,7 +29,8 @@ SQLAlchemy==0.9.9 temps==0.3.0 Werkzeug==0.12.2 pyyaml -userdatamodel==1.2.0 +#userdatamodel==1.2.0 +-e git+https://git@github.com/uc-cdis/userdatamodel.git@fix/remove-policy#egg=userdatamodel -e git+https://github.com/uc-cdis/flask-postgres-session.git@68bf5a9723a351729855c429eca8a0f4bbb830c7#egg=flask_postgres_session-0.1.3 -e git+https://git@github.com/uc-cdis/cdiserrors.git@0.1.0#egg=cdiserrors -e git+https://git@github.com/uc-cdis/cdislogging.git@master#egg=cdislogging From bbbfdb1db4271c9a8803ff3b56bf5a611aa34a7e Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Wed, 3 Apr 2019 13:02:47 -0500 Subject: [PATCH 05/30] feat(sleek-jwts): remove policy from models --- fence/models.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/fence/models.py b/fence/models.py index acbb386b8..93b40c60d 100644 --- a/fence/models.py +++ b/fence/models.py @@ -43,7 +43,6 @@ HMACKeyPair, HMACKeyPairArchive, IdentityProvider, - Policy, Project, ProjectToBucket, S3Credential, @@ -52,7 +51,6 @@ User, UserToBucket, UserToGroup, - users_to_policies, ) From d21254aeada862a451c56228329a278dade87bcd Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 5 Apr 2019 13:48:59 -0500 Subject: [PATCH 06/30] feat(sleek-jwts): remove unused policies endpoints --- fence/__init__.py | 4 - fence/blueprints/rbac.py | 239 ------------------------------------ fence/rbac/client.py | 37 ++++++ fence/sync/sync_users.py | 38 ++---- tests/rbac/conftest.py | 17 --- tests/rbac/test_policies.py | 158 ------------------------ 6 files changed, 47 insertions(+), 446 deletions(-) delete mode 100644 fence/blueprints/rbac.py delete mode 100644 tests/rbac/test_policies.py diff --git a/fence/__init__.py b/fence/__init__.py index 8740e151e..2eeaa04ea 100644 --- a/fence/__init__.py +++ b/fence/__init__.py @@ -27,7 +27,6 @@ import fence.blueprints.data import fence.blueprints.login import fence.blueprints.oauth2 -import fence.blueprints.rbac import fence.blueprints.misc import fence.blueprints.storage_creds import fence.blueprints.user @@ -91,9 +90,6 @@ def app_register_blueprints(app): google_blueprint = fence.blueprints.google.make_google_blueprint() app.register_blueprint(google_blueprint, url_prefix="/google") - if config.get("ARBORIST"): - app.register_blueprint(fence.blueprints.rbac.blueprint, url_prefix="/rbac") - fence.blueprints.misc.register_misc(app) @app.route("/") diff --git a/fence/blueprints/rbac.py b/fence/blueprints/rbac.py deleted file mode 100644 index 6adb6ea59..000000000 --- a/fence/blueprints/rbac.py +++ /dev/null @@ -1,239 +0,0 @@ -""" -Provide an interface in front of the engine for role-based access control -(RBAC). - -TODO (rudyardrichter): -instead of ``login_required``, these routes should check with arborist to see -if the user has roles allowing them to use these endpoints. -""" - -import flask - -from fence.auth import login_required -from fence.errors import NotFound, UserError -from fence.models import Policy, User - - -blueprint = flask.Blueprint("rbac", __name__) - - -@blueprint.route("/policies/", methods=["GET"]) -@login_required({"admin"}) -def list_policies(): - """ - List all the existing policies. (In fence, not arborist.) - - Example output JSON: - - { - "policies": [ - "policy-abc", - "policy-xyz" - ] - } - """ - with flask.current_app.db.session as session: - policies = _list_all_policies(session) - return flask.jsonify({"policies": [policy.id for policy in policies]}) - - -@blueprint.route("/policies/", methods=["POST"]) -@login_required({"admin"}) -def create_policy(): - """ - Create new policies in the fence database, *without* granting it to any - users. The policy *must* already exist in arborist, otherwise this - operation will fail. - """ - policy_ids = flask.request.get_json().get("policies", []) - missing = flask.current_app.arborist.policies_not_exist(policy_ids) - if missing: - raise ValueError( - "the following policies do not exist in arborist: " + ", ".join(missing) - ) - with flask.current_app.db.session as session: - for policy_id in policy_ids: - session.add(Policy(id=policy_id)) - return flask.jsonify({"created": policy_ids}), 201 - - -@blueprint.route("/policies/", methods=["DELETE"]) -@login_required({"admin"}) -def delete_policy(policy_id): - """ - Delete a policy from the database. - - Fence should not include policies in tokens which are not understood by - arborist, so when policies are removed from arborist, arborist (or arborist - maintainers) can use this endpoint to remove the same policy from fence. - """ - with flask.current_app.db.session as session: - policy_to_delete = session.query(Policy).filter(Policy.id == policy_id).first() - session.delete(policy_to_delete) - return "", 204 - - -@blueprint.route("/user//policies/", methods=["GET"]) -@login_required({"admin"}) -def list_user_policies(user_id): - """ - List the policies that this user has access to. - - Output will be in the same format as the ``/policy/`` endpoint, but - only containing policies this user has access to. - """ - return flask.jsonify({"policies": _get_user_policy_ids(user_id)}) - - -@blueprint.route("/user//policies/", methods=["POST"]) -@login_required({"admin"}) -def grant_policy_to_user(user_id): - """ - Grant additional policies to a user. - """ - policy_ids = _validate_policy_ids(flask.request.get_json().get("policies")) - - with flask.current_app.db.session as session: - policies = lookup_policies(policy_ids) - user = session.query(User).filter(User.id == user_id).first() - if not user: - raise ValueError("no user exists with ID: {}".format(user_id)) - user.policies.extend(policies) - session.commit() - - return "", 204 - - -@blueprint.route("/user//policies/", methods=["PUT"]) -@login_required({"admin"}) -def replace_user_policies(user_id): - """ - Overwrite the user's existing policies and replace them with the ones - provided in the request. - """ - policy_ids = _validate_policy_ids(flask.request.get_json().get("policies")) - - with flask.current_app.db.session as session: - policies = lookup_policies(policy_ids) - user = session.query(User).filter_by(id=user_id).first() - if not user: - raise NotFound("no user exists with ID: {}".format(user_id)) - user.policies = policies - session.commit() - - return "", 204 - - -@blueprint.route("/user//policies/", methods=["DELETE"]) -@login_required({"admin"}) -def revoke_user_policies(user_id): - """ - Revoke all the policies which this user has access to. - """ - with flask.current_app.db.session as session: - user = session.query(User).filter_by(id=user_id).first() - if not user: - raise NotFound("no user exists with ID: {}".format(user_id)) - # Set user's policies to empty list. - user.policies = [] - session.commit() - return "", 204 - - -@blueprint.route("/user//policies/", methods=["DELETE"]) -@login_required({"admin"}) -def revoke_user_policy(user_id, policy_id): - """ - Revoke a specific policy (the policy ID in the path) granted to a user - (specified by user ID in the path). - """ - with flask.current_app.db.session as session: - user = session.query(User).filter_by(User.id == user_id).first() - if not user: - raise NotFound("no user exists with ID: {}".format(user_id)) - user.policies.remove(policy_id) - session.flush() - return "", 204 - - -def lookup_policies(policy_ids): - """ - Look up the list of policies from the database. - - Requires flask application context. - - Args: - policy_ids (List[str]): list of IDs for the policies to return - - Return: - List[fence.model.Policy]: list of policy models - - Raises: - - ValueError: if any of the policy IDs do not correspond to an existing - policy - """ - policies = [] - with flask.current_app.db.session as session: - for policy_id in policy_ids: - policy = session.query(Policy).filter_by(id=policy_id).first() - if not policy: - raise ValueError("policy not registered in fence: {}".format(policy_id)) - policies.append(policy) - return policies - - -def _list_all_policies(session): - """ - List all the policies existing in a database session. - - Args: - session (sqlalchemy.orm.session.Session) - - Return: - List[fence.models.Policy] - """ - return session.query(Policy).all() - - -def _get_user_policy_ids(user_id): - """ - List the IDs for policies which are granted to this user, according to the - fence database. - - Args: - user_id (str): the id for a user - - Return: - List[str]: list of policies granted to the user - """ - with flask.current_app.db.session as session: - user = session.query(User).filter(User.id == user_id).first() - if not user: - raise NotFound("no user exists with ID: {}".format(user_id)) - return [policy.id for policy in user.policies] - - -def _validate_policy_ids(policy_ids): - """ - Check some user-inputted policy IDs which should correspond to policies in - arborist. - - Args: - policy_ids (List[str]): list of policy IDs - - Return: - List[str]: the same policy_ids, if they validated - - Raises: - UserError: if the policy ID list fails to validate - """ - if not policy_ids: - raise UserError("JSON missing required value `policies`") - missing_policies = flask.current_app.arborist.policies_not_exist(policy_ids) - if any(missing_policies): - raise UserError( - "policies with these IDs do not exist in arborist: {}".format( - missing_policies - ) - ) - return policy_ids diff --git a/fence/rbac/client.py b/fence/rbac/client.py index 0e709a6ad..b5b1b0785 100644 --- a/fence/rbac/client.py +++ b/fence/rbac/client.py @@ -72,6 +72,7 @@ def __init__(self, logger=None, arborist_base_url="http://arborist-service/"): self._policy_url = self._base_url + "/policy/" self._resource_url = self._base_url + "/resource" self._role_url = self._base_url + "/role/" + self._user_url = self._base_url + "/user" def healthy(self): """ @@ -348,3 +349,39 @@ def create_policy(self, policy_json, skip_if_exists=True): raise ArboristError(data["error"]) self.logger.info("created policy {}".format(policy_json["id"])) return data + + @_arborist_retry() + def grant_user_policy(self, username, policy_id): + """ + MUST be user name, and not serial user ID + """ + url = self._user_url + "/{}/policy".format(username) + request = {"policy": policy_id} + response = requests.post(url, json=request) + data = _request_get_json(response) + if response.status_code != 204: + msg = data.get("error", "unhelpful response from arborist") + self.logger.error( + "could not grant policy `{}` to user `{}`: {}".format( + policy_id, username, msg + ) + ) + return None + self.logger.info("granted policy `{}` to user `{}`".format(policy_id, username)) + return data + + @_arborist_retry() + def revoke_all_policies_for_user(self, username): + url = self._url_user + "/{}/policy".format(username) + response = requests.delete(url) + data = _request_get_json(response) + 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 + ) + ) + return None + self.logger.info("revoked all policies from user `{}`".format(username)) + return data diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index e0a5ed51d..018a7008a 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -21,7 +21,6 @@ from fence.models import ( AccessPrivilege, AuthorizationProvider, - Policy, Project, Tag, User, @@ -632,21 +631,6 @@ def sync_to_db_and_storage_backend( self._validate_and_update_user_admin(sess, user_info_lowercase) - # Add policies to user models in the database. These will show up in users' - # JWTs; services can send the JWTs to arborist. - if user_policies: - self.logger.info("populating RBAC information from YAML file") - for username, policies in user_policies.iteritems(): - user = query_for_user(session=sess, username=username) - for policy_id in policies: - policy = self._get_or_create_policy(sess, policy_id) - if policy not in user.policies: - user.policies.append(policy) - self.logger.info( - "granted policy `{}` to user `{}` ({})".format( - policy_id, username, user.id - ) - ) sess.commit() def _revoke_from_db(self, sess, to_delete): @@ -997,8 +981,14 @@ def _sync(self, sess): else: self.logger.info("No resources specified; skipping arborist sync") - @staticmethod - def _reset_user_access(session): + def _reset_user_access(self, session): + all_users = session.query(User).all() + + if self.arborist_client: + usernames = {user.username for user in all_users} + for username in usernames: + self.arborist_client.revoke_all_policies_for_user(username) + session.execute(users_to_policies.delete()) # TODO (rudyardrichter 2018-09-10): revoke admin access etc @@ -1100,8 +1090,8 @@ def _update_arborist(self, session, user_yaml): "not creating policy in arborist; {}".format(str(e)) ) created_policies.add(policy_id) - policy = self._get_or_create_policy(session, policy_id) - user.policies.append(policy) + + self.arborist_client.grant_user_policy(user.username, policy_id) self.logger.info( "granted policy `{}` to user `{}`".format( policy_id, user.username @@ -1109,11 +1099,3 @@ def _update_arborist(self, session, user_yaml): ) return True - - def _get_or_create_policy(self, session, policy_id): - policy = session.query(Policy).filter_by(id=policy_id).first() - if not policy: - policy = Policy(id=policy_id) - session.add(policy) - self.logger.info("created policy `{}`".format(policy_id)) - return policy diff --git a/tests/rbac/conftest.py b/tests/rbac/conftest.py index fc75da3ae..b35adbab5 100644 --- a/tests/rbac/conftest.py +++ b/tests/rbac/conftest.py @@ -32,20 +32,3 @@ def mock_arborist_client(app, monkeypatch): @pytest.fixture def arborist_client(): return ArboristClient() - - -@pytest.fixture -def example_policies(): - """ - Create some example policies and also add them to the database. - - Return: - List[fence.models.Policy]: list of policies added - """ - policies = [ - Policy(id="apple"), - Policy(id="banana"), - Policy(id="canteloupe"), - Policy(id="durian"), - ] - return policies diff --git a/tests/rbac/test_policies.py b/tests/rbac/test_policies.py deleted file mode 100644 index ff77cf7a8..000000000 --- a/tests/rbac/test_policies.py +++ /dev/null @@ -1,158 +0,0 @@ -# pylint: disable=unused-argument -""" -Run tests for the policy endpoints in the RBAC blueprint. - -Note that any test which will cause a call to -``fence.blueprints.rbac.lookup_policies`` must add the test policies to the -database, otherwise fence will raise an error from not finding a policy. Use -something like this: - - # Put the example policies in the database first. - for policy in example_policies: - db_session.add(policy) -""" - -import json - -try: - import mock -except ImportError: - from unittest import mock - -from fence.blueprints.rbac import _get_user_policy_ids -from fence.rbac.client import ArboristClient -from fence.models import Policy, User - - -def test_list_policies(db_session, client, example_policies): - """ - Test the ``/rbac/policy`` endpoint for listing existing policies. - """ - # Put the example policies in the database first. - for policy in example_policies: - db_session.add(policy) - - policies_response = client.get("/rbac/policies/").json - assert "policies" in policies_response - policy_ids = policies_response["policies"] - assert set(policy_ids) == set(policy.id for policy in example_policies) - - -def test_list_user_policies(db_session, client, user_client, example_policies): - """ - Test listing the policies granted to the user using the - ``/rbac/user//policies`` endpoint. - """ - # Set up the user to have the example policies. - user = db_session.query(User).filter_by(id=user_client.user_id).first() - user.policies = example_policies - - path = "/rbac/user/{}/policies/".format(user_client.user_id) - policies_response = client.get(path) - assert "policies" in policies_response.json - policies_from_db = _get_user_policy_ids(user_client.user_id) - assert set(policies_from_db) == set(policies_response.json["policies"]) - - -def test_grant_policy_to_user( - client, db_session, user_client, example_policies, mock_arborist_client -): - """ - Test granting an additional policy to a user and check in the policy - listing endpoint and the database that the change goes through correctly. - """ - # Put the example policies in the database first. - for policy in example_policies: - db_session.add(policy) - - # Get the list of policies before adding a new one - path = "/rbac/user/{}/policies/".format(user_client.user_id) - policies_before = client.get(path).json["policies"] - - # Grant user one additional policy for example - policies = {"policies": [example_policies[0].id]} - response = client.post( - path, data=json.dumps(policies), content_type="application/json" - ) - assert response.status_code == 204 - - # Check that the new one was added correctly (shows up in endpoint). - policies_after = client.get(path).json["policies"] - assert len(policies_after) == len(policies_before) + 1 - assert example_policies[0].id in policies_after - # Check new policy is in database. - db_policies = _get_user_policy_ids(user_client.user_id) - assert set(policies_after) == set(db_policies) - - -def test_replace_user_policies( - client, db_session, user_client, example_policies, mock_arborist_client -): - """ - Test overwriting the policies granted to a user and check in the policy - listing endpoint and the database that the change goes through correctly. - """ - # Put the example policies in the database first. - for policy in example_policies: - db_session.add(policy) - - policies_even = example_policies[::2] - policies_odd = example_policies[1::2] - - # Set up the user to have every odd example policy in the test list. - user = db_session.query(User).filter_by(id=user_client.user_id).first() - user.policies = policies_odd - - # Hit the endpoint and change the user's policies to be every even test - # policy. - path = "/rbac/user/{}/policies/".format(user_client.user_id) - policies = {"policies": [policy.id for policy in policies_even]} - response = client.put( - path, data=json.dumps(policies), content_type="application/json" - ) - assert response.status_code == 204 - - # Check policies from endpoint. - expected_policy_ids = [policy.id for policy in policies_even] - policies_after = client.get(path).json["policies"] - assert set(policies_after) == set(expected_policy_ids) - # Check policies in database. - user_policies_from_db = _get_user_policy_ids(user_client.user_id) - assert set(user_policies_from_db) == set(expected_policy_ids) - - -def test_revoke_user_policies(client, user_client): - """ - Test revoking all the policies granted to a user using the - ``/rbac/user/policies/`` endpoint with a ``DELETE`` call. - """ - path = "/rbac/user/{}/policies/".format(user_client.user_id) - response = client.delete(path) - assert response.status_code == 204 - - # Check policies response for the user is empty. - policies_after = client.get(path).json["policies"] - assert policies_after == [] - # Check policies in database are empty. - db_policies = _get_user_policy_ids(user_client.user_id) - assert db_policies == [] - - -def test_create_policy(client, db_session): - """ - Test creating a policy using the ``/rbac/policies/`` endpoint, adding the - policy in the fence database and also registering it in arborist. - """ - policies = {"policies": ["test-policy-1", "test-policy-2"]} - with ( - mock.patch.object(ArboristClient, "policies_not_exist", return_value=[]) - ) as mock_policies_not_exist: - response = client.post( - "/rbac/policies/", - data=json.dumps(policies), - content_type="application/json", - ) - mock_policies_not_exist.assert_called_once() - assert response.status_code == 201 - policy = db_session.query(Policy).filter(Policy.id == "test-policy-1").first() - assert policy From 644910cebc37761252598957e323c0e535e996e7 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Wed, 3 Apr 2019 12:38:31 -0500 Subject: [PATCH 07/30] feat(download-auth): userdatamodel pin --- fence/models.py | 2 -- requirements.txt | 3 ++- setup.py | 1 - 3 files changed, 2 insertions(+), 4 deletions(-) diff --git a/fence/models.py b/fence/models.py index e07424614..60593a301 100644 --- a/fence/models.py +++ b/fence/models.py @@ -43,7 +43,6 @@ HMACKeyPair, HMACKeyPairArchive, IdentityProvider, - Policy, Project, ProjectToBucket, S3Credential, @@ -52,7 +51,6 @@ User, UserToBucket, UserToGroup, - users_to_policies, ) diff --git a/requirements.txt b/requirements.txt index b6c98bb92..0c06b4e8a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,7 +29,8 @@ SQLAlchemy==0.9.9 temps==0.3.0 Werkzeug==0.12.2 pyyaml -userdatamodel==1.2.0 +#userdatamodel==1.2.0 +-e git+https://git@github.com/uc-cdis/userdatamodel.git@fix/remove-policy#egg=userdatamodel -e git+https://github.com/uc-cdis/flask-postgres-session.git@68bf5a9723a351729855c429eca8a0f4bbb830c7#egg=flask_postgres_session-0.1.3 -e git+https://git@github.com/uc-cdis/cdiserrors.git@0.1.0#egg=cdiserrors -e git+https://git@github.com/uc-cdis/cdislogging.git@master#egg=cdislogging diff --git a/setup.py b/setup.py index a59dc3023..7a13c6364 100644 --- a/setup.py +++ b/setup.py @@ -34,7 +34,6 @@ "six>=1.11.0,<2.0.0", "SQLAlchemy>=0.9.9,<1.0.0", "temps>=0.3.0,<1.0.0", - "userdatamodel", "Werkzeug>=0.12.2,<1.0.0", "storageclient", "pyyaml", From c96f8e3ad1d56c413f84e67ad0938a5ee1eed9ed Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 5 Apr 2019 14:58:23 -0500 Subject: [PATCH 08/30] feat(download-auth): remove unused imports --- fence/sync/sync_users.py | 2 -- tests/dbgap_sync/test_user_sync.py | 19 ++----------------- tests/rbac/conftest.py | 1 - 3 files changed, 2 insertions(+), 20 deletions(-) diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index 018a7008a..9c911e086 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -24,7 +24,6 @@ Project, Tag, User, - users_to_policies, query_for_user, ) from fence.rbac.client import ArboristClient, ArboristError @@ -989,7 +988,6 @@ def _reset_user_access(self, session): for username in usernames: self.arborist_client.revoke_all_policies_for_user(username) - session.execute(users_to_policies.delete()) # TODO (rudyardrichter 2018-09-10): revoke admin access etc def _update_arborist(self, session, user_yaml): diff --git a/tests/dbgap_sync/test_user_sync.py b/tests/dbgap_sync/test_user_sync.py index 6b6edf002..f1d9b5638 100644 --- a/tests/dbgap_sync/test_user_sync.py +++ b/tests/dbgap_sync/test_user_sync.py @@ -321,20 +321,5 @@ def test_update_arborist(syncer, db_session): with open(LOCAL_YAML_DIR, "r") as f: user_data = yaml.safe_load(f) - policies = db_session.query(models.Policy).all() - policy_ids = [policy.id for policy in policies] - - # For every user in the user data, check that the matching policies were - # created, and also granted to this user, i.e. the entry in the database - # for this user has policies for everything in the original user data. - for username, data in user_data["users"].items(): - if "projects" not in data: - continue - for project in data["projects"]: - for privilege in project["privilege"]: - policy_id = _format_policy_id(project["resource"], privilege) - assert policy_id in policy_ids - user = models.query_for_user(session=db_session, username=username) - user_policies = user.policies - user_policy_ids = [policy.id for policy in user_policies] - assert policy_id in user_policy_ids + # TODO: update since policies are moved over to arborist now + # should be part of user sync changes probably diff --git a/tests/rbac/conftest.py b/tests/rbac/conftest.py index b35adbab5..d014cf3d1 100644 --- a/tests/rbac/conftest.py +++ b/tests/rbac/conftest.py @@ -12,7 +12,6 @@ import pytest -from fence.models import Policy from fence.rbac.client import ArboristClient From aafa11aedc73cc2852a6de603e296f417b1a39c2 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 5 Apr 2019 15:24:05 -0500 Subject: [PATCH 09/30] feat(download-auth): fix drop commands --- fence/models.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fence/models.py b/fence/models.py index 93b40c60d..74304d846 100644 --- a/fence/models.py +++ b/fence/models.py @@ -858,6 +858,6 @@ def _update_for_authlib(driver, md): def _remove_policy(driver, md): with driver.session as session: - session.execute("DROP TABLE users_to_policies;") - session.execute("DROP TABLE policy;") + session.execute("DROP TABLE IF EXISTS users_to_policies;") + session.execute("DROP TABLE IF EXISTS policy;") session.commit() From c7424ab01462cc321fa907df3c50b07ce3ff40d3 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 5 Apr 2019 15:32:25 -0500 Subject: [PATCH 10/30] feat(download-auth): remove more policies references --- tests/dbgap_sync/test_user_sync.py | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/tests/dbgap_sync/test_user_sync.py b/tests/dbgap_sync/test_user_sync.py index f1d9b5638..1ef6190b2 100644 --- a/tests/dbgap_sync/test_user_sync.py +++ b/tests/dbgap_sync/test_user_sync.py @@ -68,17 +68,8 @@ def test_sync(syncer, db_session, storage_client): "upload", } assert len(user_access) == 1 - user_policy_ids = {policy.id for policy in user.policies} - expect_policies = { - "test-policy-1", - "test-policy-3", - "programs.test.projects.test-read", - "programs.test.projects.test-create", - "programs.test.projects.test-upload", - "programs.test.projects.test-update", - "programs.test.projects.test-delete", - } - assert user_policy_ids == expect_policies + + # TODO: check user policy access (add in user sync changes) user = models.query_for_user(session=db_session, username="deleted_user@gmail.com") assert not user.is_admin From 8e656d6677b6664fadb25816a25d50f4a48a12a4 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Thu, 11 Apr 2019 14:29:47 -0500 Subject: [PATCH 11/30] feat(download-auth): add test for download endpoint using rbac --- tests/conftest.py | 135 +++++++++++++++++++++++++++++++++++++--- tests/data/test_data.py | 50 ++++++++++++++- 2 files changed, 177 insertions(+), 8 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 5508108a1..e9184db40 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -182,6 +182,20 @@ def kid_2(): @pytest.fixture(scope="function") def mock_arborist_requests(request): + """ + This fixture returns a function which you call to mock out arborist endopints. + + Give it an argument in this format: + + { + "arborist/health": { + "GET": ("", 200) + }, + "arborist/auth/request": { + "POST": ('{"auth": "false"}', 403) + } + } + """ def do_patch(urls_to_responses=None): urls_to_responses = urls_to_responses or {} @@ -190,14 +204,19 @@ def do_patch(urls_to_responses=None): urls_to_responses = defaults def make_mock_response(method): - def response(url): + + def response(url, *args, **kwargs): mocked_response = MagicMock(requests.Response) - if url in urls_to_responses: - if method in urls_to_responses[url]: - content, code = urls_to_responses[url][method] - mocked_response.status_code = code - if isinstance(content, dict): - mocked_response.json.return_value = content + if url not in urls_to_responses: + mocked_response.status_code = 404 + return mocked_response + if method not in urls_to_responses[url]: + mocked_response.status_code = 405 + return mocked_response + content, code = urls_to_responses[url][method] + mocked_response.status_code = code + if isinstance(content, dict): + mocked_response.json.return_value = content return mocked_response return response @@ -479,6 +498,108 @@ def indexd_client(app, request): return output + +@pytest.fixture(scope="function") +def indexd_client_with_arborist(app, request): + record = {} + + def do_patch(rbac): + if request.param == "gs": + record = { + "did": "", + "baseid": "", + "rev": "", + "size": 10, + "file_name": "file1", + "urls": ["gs://bucket1/key"], + "rbac": rbac, + "hashes": {}, + "metadata": {"acls": "phs000178,phs000218"}, + "form": "", + "created_date": "", + "updated_date": "", + } + elif request.param == "gs_acl": + record = { + "did": "", + "baseid": "", + "rev": "", + "size": 10, + "file_name": "file1", + "urls": ["gs://bucket1/key"], + "hashes": {}, + "acl": ["phs000178", "phs000218"], + "rbac": rbac, + "form": "", + "created_date": "", + "updated_date": "", + } + elif request.param == "s3_acl": + record = { + "did": "", + "baseid": "", + "rev": "", + "size": 10, + "file_name": "file1", + "urls": ["s3://bucket1/key"], + "hashes": {}, + "acl": ["phs000178", "phs000218"], + "rbac": rbac, + "form": "", + "created_date": "", + "updated_date": "", + } + elif request.param == "s3_external": + record = { + "did": "", + "baseid": "", + "rev": "", + "size": 10, + "file_name": "file1", + "urls": ["s3://bucket1/key"], + "hashes": {}, + "acl": ["phs000178", "phs000218"], + "rbac": rbac, + "form": "", + "created_date": "", + "updated_date": "", + } + else: + record = { + "did": "", + "baseid": "", + "rev": "", + "size": 10, + "file_name": "file1", + "urls": ["s3://bucket1/key"], + "hashes": {}, + "metadata": {"acls": "phs000178,phs000218"}, + "rbac": rbac, + "form": "", + "created_date": "", + "updated_date": "", + } + + mocker = Mocker() + mocker.mock_functions() + + # TODO (rudyardrichter, 2018-11-03): consolidate things needing to do this patch + indexd_patcher = patch( + "fence.blueprints.data.indexd.IndexedFile.index_document", record + ) + mocker.add_mock(indexd_patcher) + + output = { + "mocker": mocker, + # only gs or s3 for location, ignore specifiers after the _ + "indexed_file_location": request.param.split("_")[0], + } + + return output + + return do_patch + + @pytest.fixture(scope="function") def unauthorized_indexd_client(app, request): mocker = Mocker() diff --git a/tests/data/test_data.py b/tests/data/test_data.py index ee25f8f17..02dc0772f 100644 --- a/tests/data/test_data.py +++ b/tests/data/test_data.py @@ -33,7 +33,6 @@ def test_indexd_download_file( Test ``GET /data/download/1``. """ indexed_file_location = indexd_client["indexed_file_location"] - path = "/data/download/1" query_string = {"protocol": indexed_file_location} headers = { @@ -551,3 +550,52 @@ def json(self): response = client.post("/data/upload", headers=headers, data=data) data_requests.post.assert_not_called() assert response.status_code == 403, response + + +@pytest.mark.parametrize( + "indexd_client_with_arborist", ["gs", "s3", "gs_acl", "s3_acl", "s3_external"], indirect=True +) +def test_rbac( + app, + client, + mock_arborist_requests, + indexd_client_with_arborist, + user_client, + rsa_private_key, + kid, + google_proxy_group, + primary_google_service_account, + cloud_manager, + google_signed_url, +): + mock_arborist_requests({ + "arborist/auth/request": { + "POST": ('{"auth": "true"}', 200), + }, + }) + indexd_client = indexd_client_with_arborist("test_rbac") + indexed_file_location = indexd_client["indexed_file_location"] + path = "/data/download/1" + query_string = {"protocol": indexed_file_location} + headers = { + "Authorization": "Bearer " + + jwt.encode( + utils.authorized_download_context_claims( + user_client.username, user_client.user_id + ), + key=rsa_private_key, + headers={"kid": kid}, + algorithm="RS256", + ) + } + response = client.get(path, headers=headers, query_string=query_string) + assert response.status_code == 200 + assert "url" in response.json.keys() + + mock_arborist_requests({ + "arborist/auth/request": { + "POST": ('{"auth": "false"}', 403), + }, + }) + response = client.get(path, headers=headers, query_string=query_string) + assert response.status_code == 403 From b26eb627f1b1a5a64a536bff547e0bc41ca1d7ea Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 12 Apr 2019 14:16:59 -0500 Subject: [PATCH 12/30] feat(download-auth): correct methods --- fence/blueprints/data/indexd.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/fence/blueprints/data/indexd.py b/fence/blueprints/data/indexd.py index dfb744540..eb1df959f 100644 --- a/fence/blueprints/data/indexd.py +++ b/fence/blueprints/data/indexd.py @@ -311,9 +311,14 @@ def check_authorization(self, action): return self.index_document.get("uploader") == username try: + action_to_method = { + "upload": "write-storage", + "download": "read-storage", + } + method = action_to_method[action] # action should be upload or download # return bool for authorization - return self.check_rbac(action) + return self.check_rbac(method) except ValueError: # this is ok; we'll default to ACL field (previous behavior) # may want to deprecate in future From 3c5158c46d24e87ce3709000fc753dded0500e90 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 12 Apr 2019 14:27:43 -0500 Subject: [PATCH 13/30] feat(download-auth): run formatter --- fence/__init__.py | 4 +++- fence/blueprints/data/indexd.py | 14 +++----------- fence/rbac/client.py | 4 +--- tests/conftest.py | 2 -- tests/data/test_data.py | 20 +++++++++----------- tests/login/test_fence_login.py | 9 ++------- 6 files changed, 18 insertions(+), 35 deletions(-) diff --git a/fence/__init__.py b/fence/__init__.py index 2eeaa04ea..4eb2bffbf 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 diff --git a/fence/blueprints/data/indexd.py b/fence/blueprints/data/indexd.py index eb1df959f..91b04a029 100644 --- a/fence/blueprints/data/indexd.py +++ b/fence/blueprints/data/indexd.py @@ -276,15 +276,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) @@ -311,10 +306,7 @@ def check_authorization(self, action): return self.index_document.get("uploader") == username try: - action_to_method = { - "upload": "write-storage", - "download": "read-storage", - } + action_to_method = {"upload": "write-storage", "download": "read-storage"} method = action_to_method[action] # action should be upload or download # return bool for authorization diff --git a/fence/rbac/client.py b/fence/rbac/client.py index 970269f87..2351d41c3 100644 --- a/fence/rbac/client.py +++ b/fence/rbac/client.py @@ -386,9 +386,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)) diff --git a/tests/conftest.py b/tests/conftest.py index e9184db40..c97585999 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -204,7 +204,6 @@ def do_patch(urls_to_responses=None): urls_to_responses = defaults def make_mock_response(method): - def response(url, *args, **kwargs): mocked_response = MagicMock(requests.Response) if url not in urls_to_responses: @@ -498,7 +497,6 @@ def indexd_client(app, request): return output - @pytest.fixture(scope="function") def indexd_client_with_arborist(app, request): record = {} diff --git a/tests/data/test_data.py b/tests/data/test_data.py index 02dc0772f..24125e80b 100644 --- a/tests/data/test_data.py +++ b/tests/data/test_data.py @@ -553,7 +553,9 @@ def json(self): @pytest.mark.parametrize( - "indexd_client_with_arborist", ["gs", "s3", "gs_acl", "s3_acl", "s3_external"], indirect=True + "indexd_client_with_arborist", + ["gs", "s3", "gs_acl", "s3_acl", "s3_external"], + indirect=True, ) def test_rbac( app, @@ -568,11 +570,9 @@ def test_rbac( cloud_manager, google_signed_url, ): - mock_arborist_requests({ - "arborist/auth/request": { - "POST": ('{"auth": "true"}', 200), - }, - }) + mock_arborist_requests( + {"arborist/auth/request": {"POST": ('{"auth": "true"}', 200)}} + ) indexd_client = indexd_client_with_arborist("test_rbac") indexed_file_location = indexd_client["indexed_file_location"] path = "/data/download/1" @@ -592,10 +592,8 @@ def test_rbac( assert response.status_code == 200 assert "url" in response.json.keys() - mock_arborist_requests({ - "arborist/auth/request": { - "POST": ('{"auth": "false"}', 403), - }, - }) + mock_arborist_requests( + {"arborist/auth/request": {"POST": ('{"auth": "false"}', 403)}} + ) response = client.get(path, headers=headers, query_string=query_string) assert response.status_code == 403 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"]) From fe48f1caa1d90b44376d1afb406dee90ced709c8 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Tue, 9 Apr 2019 15:39:30 -0500 Subject: [PATCH 14/30] feat(arborist-sync): update sync for new RBAC --- fence/rbac/client.py | 49 ++++++++++++++++++++++++++++++++-------- fence/sync/sync_users.py | 14 +++++------- 2 files changed, 46 insertions(+), 17 deletions(-) diff --git a/fence/rbac/client.py b/fence/rbac/client.py index 970269f87..5f535b882 100644 --- a/fence/rbac/client.py +++ b/fence/rbac/client.py @@ -5,6 +5,7 @@ from functools import wraps import json +import urllib import backoff from cdislogging import get_logger @@ -232,16 +233,24 @@ def create_resource(self, parent_path, resource_json, overwrite=False): path = self._resource_url + parent_path response = requests.post(path, json=resource_json) if response.status_code == 409: - if overwrite: - resource_path = path + resource_json["name"] - return self.update_resource(resource_path, resource_json) - else: + if not overwrite: return None + # overwrite existing resource + resource_path = parent_path + resource_json["name"] + self.logger.info("trying to overwrite resource {}".format(resource_path)) + self.delete_resource(resource_path) + self.create_resource(parent_path, resource_json, overwrite=False) + return data = _request_get_json(response) - if "error" in data: - msg = data["error"].get("message", str(data["error"])) + if isinstance(data, dict) and "error" in data: + msg = data["error"] + if isinstance(data["error"], dict): + 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(path, msg) + "could not create resource `{}` in arborist: {}".format( + resource, msg + ) ) raise ArboristError(data["error"]) self.logger.info("created resource {}".format(resource_json["name"])) @@ -305,6 +314,7 @@ def create_role(self, role_json): """ response = requests.post(self._role_url, json=role_json) if response.status_code == 409: + # already exists; this is ok return None data = _request_get_json(response) if "error" in data: @@ -347,8 +357,9 @@ def create_policy(self, policy_json, skip_if_exists=True): response = requests.post(self._policy_url, json=policy_json) data = _request_get_json(response) if response.status_code == 409: + # already exists; this is ok return None - if "error" in data: + if isinstance(data, dict) and "error" in data: self.logger.error( "could not create policy `{}` in arborist: {}".format( policy_json["id"], data["error"] @@ -358,6 +369,24 @@ def create_policy(self, policy_json, skip_if_exists=True): self.logger.info("created policy {}".format(policy_json["id"])) return data + @_arborist_retry() + def create_user(self, user_info): + """ + Args: + user_info (dict): + user information that goes in the request to arborist; see arborist docs + for required field names. notably it's `name` not `username` + """ + if "name" not in user_info: + raise ValueError("create_user requires username `name` in user info") + response = requests.post(self._user_url, json=user_info) + data = _request_get_json(response) + if response.status_code == 409: + # already exists + return + elif response.status_code != 204: + msg = data.get("error", "unhelpful response from arborist") + @_arborist_retry() def grant_user_policy(self, username, policy_id): """ @@ -369,6 +398,8 @@ def grant_user_policy(self, username, policy_id): data = _request_get_json(response) if response.status_code != 204: msg = data.get("error", "unhelpful response from arborist") + if isinstance("error", dict): + msg = data["error"].get("message", msg) self.logger.error( "could not grant policy `{}` to user `{}`: {}".format( policy_id, username, msg @@ -380,7 +411,7 @@ def grant_user_policy(self, username, policy_id): @_arborist_retry() def revoke_all_policies_for_user(self, username): - url = self._url_user + "/{}/policy".format(username) + url = self._user_url + "/{}/policy".format(urllib.quote(username)) response = requests.delete(url) data = _request_get_json(response) if response.status_code != 204: diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index 9c911e086..341ec851d 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -252,7 +252,8 @@ def __init__( self.arborist_client = None if arborist: self.arborist_client = ArboristClient( - arborist_base_url=arborist, logger=self.logger + arborist_base_url=arborist, + logger=get_logger("user_syncer.arborist_client"), ) if storage_credentials: @@ -766,6 +767,8 @@ def _upsert_userinfo(self, sess, user_info): u = User(username=username) sess.add(u) + self.arborist_client.create_user({"name": username}) + u.email = user_info[username].get("email", "") u.display_name = user_info[username].get("display_name", "") u.phone_number = user_info[username].get("phone_number", "") @@ -970,7 +973,7 @@ def _sync(self, sess): self.logger.info("No users for syncing") if user_yaml.rbac: - self.logger.info("Synchronizing arborist") + self.logger.info("Synchronizing arborist...") success = self._update_arborist(sess, user_yaml) if success: self.logger.info("Finished synchronizing arborist") @@ -1011,7 +1014,7 @@ def _update_arborist(self, session, user_yaml): return False if not self.arborist_client.healthy(): # TODO (rudyardrichter, 2019-01-07): add backoff/retry here - self.logger.error("arborist service is unavailable; skipping arborist sync") + self.logger.error("arborist service is unavailable; skipping main arborist sync") return False # Set up the resource tree in arborist @@ -1090,10 +1093,5 @@ def _update_arborist(self, session, user_yaml): created_policies.add(policy_id) self.arborist_client.grant_user_policy(user.username, policy_id) - self.logger.info( - "granted policy `{}` to user `{}`".format( - policy_id, user.username - ) - ) return True From 5c431a03f01cec5a8f7ae5a9e0218487a0e2d23d Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Thu, 25 Apr 2019 11:59:47 -0500 Subject: [PATCH 15/30] feat(arborist-sync): CRFs, add group create --- fence/rbac/client.py | 24 ++++++++++++++++++++++++ fence/sync/sync_users.py | 22 ++++++++++++++++++++-- 2 files changed, 44 insertions(+), 2 deletions(-) diff --git a/fence/rbac/client.py b/fence/rbac/client.py index 5f535b882..d07c76da9 100644 --- a/fence/rbac/client.py +++ b/fence/rbac/client.py @@ -74,6 +74,7 @@ def __init__(self, logger=None, arborist_base_url="http://arborist-service/"): self._resource_url = self._base_url + "/resource" self._role_url = self._base_url + "/role/" self._user_url = self._base_url + "/user" + self._group_url = self._base_url + "/group" def healthy(self): """ @@ -424,3 +425,26 @@ def revoke_all_policies_for_user(self, username): return None self.logger.info("revoked all policies from user `{}`".format(username)) return data + + @_arborist_retry() + def create_group(self, name, description="", users=None, policies=None): + users = users or [] + policies = policies or [] + 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_codee != 201: + msg = data.get("error", "unhelpful response from arborist") + self.logger.error("could not create group {}: {}".format(name, msg)) + return None + self.logger.info("created new group `{}`".format(name)) + if users: + self.logger.info("group {} contains users: {}".format(name, list(users))) + self.logger.info("group {} has policies: {}".format(name, list(policies))) + return data diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index 341ec851d..09dfe7564 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -984,9 +984,8 @@ def _sync(self, sess): self.logger.info("No resources specified; skipping arborist sync") def _reset_user_access(self, session): - all_users = session.query(User).all() - if self.arborist_client: + all_users = session.query(User).all() usernames = {user.username for user in all_users} for username in usernames: self.arborist_client.revoke_all_policies_for_user(username) @@ -1094,4 +1093,23 @@ def _update_arborist(self, session, user_yaml): self.arborist_client.grant_user_policy(user.username, policy_id) + groups = user_yaml.rbac.get("groups", []) + for group in groups: + missing = {"name", "users", "policies"}.difference(set(group.keys())) + if missing: + name = group.get("name", "{MISSING NAME}") + self.logger.error( + "group {} missing required field(s): {}".format(name, list(missing)) + ) + continue + try: + response = self.arborist_client.create_group( + group["name"], + description=group.get("description", ""), + users=group["users"], + policies=group["policies"], + ) + except ArboristError as e: + self.logger.info("couldn't create group: {}".format(str(e))) + return True From 2830ca073208c7a94e2d6074a10236516e324364 Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Thu, 25 Apr 2019 14:42:38 -0500 Subject: [PATCH 16/30] Update client.py --- fence/rbac/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fence/rbac/client.py b/fence/rbac/client.py index d07c76da9..f0f6a4531 100644 --- a/fence/rbac/client.py +++ b/fence/rbac/client.py @@ -439,7 +439,7 @@ 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_codee != 201: + if response.status_code != 201: msg = data.get("error", "unhelpful response from arborist") self.logger.error("could not create group {}: {}".format(name, msg)) return None From 8e2ea497a815c5f369cda5413592fcacc3f175a1 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Thu, 25 Apr 2019 16:59:02 -0500 Subject: [PATCH 17/30] feat(arborist-sync): add arborist client checks --- fence/sync/sync_users.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index 09dfe7564..1cb95e275 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -767,7 +767,8 @@ def _upsert_userinfo(self, sess, user_info): u = User(username=username) sess.add(u) - self.arborist_client.create_user({"name": username}) + if self.arborist_client: + self.arborist_client.create_user({"name": username}) u.email = user_info[username].get("email", "") u.display_name = user_info[username].get("display_name", "") @@ -973,6 +974,11 @@ def _sync(self, sess): self.logger.info("No users for syncing") if user_yaml.rbac: + if not self.arborist_client: + raise EnvironmentError( + "yaml file contains rbac section but sync is not configured with" + " arborist client" + ) self.logger.info("Synchronizing arborist...") success = self._update_arborist(sess, user_yaml) if success: From 129934b322fbe6b513e1d20286f1fe04eaa34857 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 26 Apr 2019 10:54:52 -0500 Subject: [PATCH 18/30] feat(download-auth): add new userdatamodel pin --- requirements.txt | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/requirements.txt b/requirements.txt index 0c06b4e8a..26f9331f1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -29,8 +29,7 @@ SQLAlchemy==0.9.9 temps==0.3.0 Werkzeug==0.12.2 pyyaml -#userdatamodel==1.2.0 --e git+https://git@github.com/uc-cdis/userdatamodel.git@fix/remove-policy#egg=userdatamodel +userdatamodel==1.4.1 -e git+https://github.com/uc-cdis/flask-postgres-session.git@68bf5a9723a351729855c429eca8a0f4bbb830c7#egg=flask_postgres_session-0.1.3 -e git+https://git@github.com/uc-cdis/cdiserrors.git@0.1.0#egg=cdiserrors -e git+https://git@github.com/uc-cdis/cdislogging.git@master#egg=cdislogging From 6dc8851b95cff3f47e802f992b2b480861377cbb Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 26 Apr 2019 12:38:40 -0500 Subject: [PATCH 19/30] feat(download-auth): add migration --- fence/models.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/fence/models.py b/fence/models.py index 74304d846..01d1a4b44 100644 --- a/fence/models.py +++ b/fence/models.py @@ -17,6 +17,7 @@ from sqlalchemy import ( Integer, BigInteger, + DateTime, String, Column, Boolean, @@ -26,6 +27,7 @@ ) from sqlalchemy.dialects.postgresql import ARRAY from sqlalchemy.orm import relationship, backref +from sqlalchemy.sql import func from sqlalchemy import func from sqlalchemy.schema import ForeignKey from fence.jwt.token import CLIENT_ALLOWED_SCOPES @@ -563,6 +565,15 @@ def migrate(driver): _remove_policy(driver, md) + add_column_if_not_exist( + table_name=User.__tablename__, + column=Column( + "_last_auth", DateTime(timezone=False), server_default=func.now() + ), + driver=driver, + metadata=md, + ) + def add_foreign_key_column_if_not_exist( table_name, From 8223657290bf0a280c4357c0e28d1f56b88d52ae Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 26 Apr 2019 15:00:43 -0500 Subject: [PATCH 20/30] feat(download-auth): fix param in arborist request --- fence/__init__.py | 2 +- fence/blueprints/data/indexd.py | 4 ++-- fence/rbac/client.py | 7 ++++--- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/fence/__init__.py b/fence/__init__.py index 4eb2bffbf..57673c766 100644 --- a/fence/__init__.py +++ b/fence/__init__.py @@ -17,7 +17,7 @@ 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, + MicrosoftOauth2Client as MicrosoftClient ) from fence.resources.openid.orcid_oauth2 import OrcidOauth2Client as ORCIDClient from fence.resources.storage import StorageManager diff --git a/fence/blueprints/data/indexd.py b/fence/blueprints/data/indexd.py index 91b04a029..83d21247e 100644 --- a/fence/blueprints/data/indexd.py +++ b/fence/blueprints/data/indexd.py @@ -273,10 +273,10 @@ def set_acls(self): raise Unauthorized("This file is not accessible") def check_rbac(self, action): - if "rbac" not in self.index_document: + if not self.index_document.get("rbac"): raise ValueError("index record missing `rbac`") request = { - "user": {"jwt": get_jwt()}, + "user": {"token": get_jwt()}, "request": { "resource": self.index_document["rbac"], "action": {"service": "fence", "method": action}, diff --git a/fence/rbac/client.py b/fence/rbac/client.py index 2351d41c3..37708a34e 100644 --- a/fence/rbac/client.py +++ b/fence/rbac/client.py @@ -251,9 +251,10 @@ def create_resource(self, parent_path, resource_json, overwrite=False): def update_resource(self, path, resource_json): response = _request_get_json(requests.put(path, json=resource_json)) if "error" in response: - msg = response["error"].get("message", str(response["error"])) self.logger.error( - "could not update resource `{}` in arborist: {}".format(path, msg) + "could not update resource `{}` in arborist: {}".format( + path, response["error"] + ) ) raise ArboristError(response["error"]) self.logger.info("updated resource {}".format(resource_json["name"])) @@ -380,7 +381,7 @@ def grant_user_policy(self, username, policy_id): @_arborist_retry() def revoke_all_policies_for_user(self, username): - url = self._url_user + "/{}/policy".format(username) + url = self._user_url + "/{}/policy".format(username) response = requests.delete(url) data = _request_get_json(response) if response.status_code != 204: From b9ec59634bf55c48ed1f329e5d42100399965ddc Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Mon, 6 May 2019 10:55:45 -0500 Subject: [PATCH 21/30] feat(arborist-sync): fix user handling in sync --- fence/rbac/client.py | 28 +++++++++++++++++++++++++++- fence/sync/sync_users.py | 17 ++++++----------- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/fence/rbac/client.py b/fence/rbac/client.py index f0f6a4531..473bc6c2f 100644 --- a/fence/rbac/client.py +++ b/fence/rbac/client.py @@ -358,7 +358,12 @@ def create_policy(self, policy_json, skip_if_exists=True): response = requests.post(self._policy_url, json=policy_json) data = _request_get_json(response) if response.status_code == 409: - # already exists; this is ok + # already exists; this is ok, but leave warning + self.logger.warn( + "could not create policy `{}` in arborist, got 409: {}".format( + policy_json["id"], data["error"] + ) + ) return None if isinstance(data, dict) and "error" in data: self.logger.error( @@ -448,3 +453,24 @@ def create_group(self, name, description="", users=None, policies=None): self.logger.info("group {} contains users: {}".format(name, list(users))) self.logger.info("group {} has policies: {}".format(name, list(policies))) return data + + def create_user_if_not_exist(self, username): + user_json = {"name": username} + response = requests.post(self._user_url, json=user_json) + data = _request_get_json(response) + if response.status_code == 409: + self.logger.warn( + "could not create user `{}` in arborist, got 409: {}".format( + username, data["error"] + ) + ) + return None + if "error" in data: + self.logger.error( + "could not create user `{}` in arborist: {}".format( + username, data["error"] + ) + ) + raise ArboristError(data["error"]) + self.logger.info("created user {}".format(username)) + return data diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index 1cb95e275..3f624d734 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -962,8 +962,6 @@ def _sync(self, sess): self.sync_two_phsids_dict(user_yaml.projects, user_projects) self.sync_two_user_info_dict(user_yaml.user_info, user_info) - self._reset_user_access(sess) - if user_projects: self.logger.info("Sync to db and storage backend") self.sync_to_db_and_storage_backend( @@ -989,15 +987,6 @@ def _sync(self, sess): else: self.logger.info("No resources specified; skipping arborist sync") - def _reset_user_access(self, session): - if self.arborist_client: - all_users = session.query(User).all() - usernames = {user.username for user in all_users} - for username in usernames: - self.arborist_client.revoke_all_policies_for_user(username) - - # TODO (rudyardrichter 2018-09-10): revoke admin access etc - def _update_arborist(self, session, user_yaml): """ Create roles and resources in arborist from the information in @@ -1058,6 +1047,12 @@ def _update_arborist(self, session, user_yaml): self.logger.info("processing user `{}`".format(username)) user = query_for_user(session=session, username=username) + self.logger.info("making sure user exists: `{}`".format(username)) + self.arborist_client.create_user_if_not_exist(username) + + self.logger.info("revoking all policies for user `{}`".format(username)) + self.arborist_client.revoke_all_policies_for_user(username) + for path, permissions in user_resources.iteritems(): for permission in permissions: # "permission" in the dbgap sense, not the arborist sense From f6dcf97069230e55ead8d7d4fc6129f968d2fd26 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Mon, 6 May 2019 10:55:45 -0500 Subject: [PATCH 22/30] feat(arborist-sync): fix user handling in sync --- fence/rbac/client.py | 24 +++++++++++++++++++++++- fence/sync/sync_users.py | 17 ++++++----------- 2 files changed, 29 insertions(+), 12 deletions(-) diff --git a/fence/rbac/client.py b/fence/rbac/client.py index f0f6a4531..b484773e0 100644 --- a/fence/rbac/client.py +++ b/fence/rbac/client.py @@ -358,7 +358,12 @@ def create_policy(self, policy_json, skip_if_exists=True): response = requests.post(self._policy_url, json=policy_json) data = _request_get_json(response) if response.status_code == 409: - # already exists; this is ok + # already exists; this is ok, but leave warning + self.logger.warn( + "could not create policy `{}` in arborist, got 409: {}".format( + policy_json["id"], data["error"] + ) + ) return None if isinstance(data, dict) and "error" in data: self.logger.error( @@ -448,3 +453,20 @@ def create_group(self, name, description="", users=None, policies=None): self.logger.info("group {} contains users: {}".format(name, list(users))) self.logger.info("group {} has policies: {}".format(name, list(policies))) return data + + @_arborist_retry() + def create_user_if_not_exist(self, username): + user_json = {"name": username} + response = requests.post(self._user_url, json=user_json) + data = _request_get_json(response) + if response.status_code == 409: + return None + if "error" in data: + self.logger.error( + "could not create user `{}` in arborist: {}".format( + username, data["error"] + ) + ) + raise ArboristError(data["error"]) + self.logger.info("created user {}".format(username)) + return data diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index 1cb95e275..3f624d734 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -962,8 +962,6 @@ def _sync(self, sess): self.sync_two_phsids_dict(user_yaml.projects, user_projects) self.sync_two_user_info_dict(user_yaml.user_info, user_info) - self._reset_user_access(sess) - if user_projects: self.logger.info("Sync to db and storage backend") self.sync_to_db_and_storage_backend( @@ -989,15 +987,6 @@ def _sync(self, sess): else: self.logger.info("No resources specified; skipping arborist sync") - def _reset_user_access(self, session): - if self.arborist_client: - all_users = session.query(User).all() - usernames = {user.username for user in all_users} - for username in usernames: - self.arborist_client.revoke_all_policies_for_user(username) - - # TODO (rudyardrichter 2018-09-10): revoke admin access etc - def _update_arborist(self, session, user_yaml): """ Create roles and resources in arborist from the information in @@ -1058,6 +1047,12 @@ def _update_arborist(self, session, user_yaml): self.logger.info("processing user `{}`".format(username)) user = query_for_user(session=session, username=username) + self.logger.info("making sure user exists: `{}`".format(username)) + self.arborist_client.create_user_if_not_exist(username) + + self.logger.info("revoking all policies for user `{}`".format(username)) + self.arborist_client.revoke_all_policies_for_user(username) + for path, permissions in user_resources.iteritems(): for permission in permissions: # "permission" in the dbgap sense, not the arborist sense From ecf00750eb9321f3a6c6ba4af12a700afeb48fc4 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Fri, 26 Apr 2019 12:38:40 -0500 Subject: [PATCH 23/30] feat(download-auth): add migration --- fence/models.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/fence/models.py b/fence/models.py index f44534b9a..2b8d92998 100644 --- a/fence/models.py +++ b/fence/models.py @@ -17,6 +17,7 @@ from sqlalchemy import ( Integer, BigInteger, + DateTime, String, Column, Boolean, @@ -26,6 +27,7 @@ ) from sqlalchemy.dialects.postgresql import ARRAY from sqlalchemy.orm import relationship, backref +from sqlalchemy.sql import func from sqlalchemy import func from sqlalchemy.schema import ForeignKey from userdatamodel import Base @@ -684,6 +686,15 @@ def migrate(driver): _remove_policy(driver, md) + add_column_if_not_exist( + table_name=User.__tablename__, + column=Column( + "_last_auth", DateTime(timezone=False), server_default=func.now() + ), + driver=driver, + metadata=md, + ) + def add_foreign_key_column_if_not_exist( table_name, From d294f60e6e25dce511835174a01e4e88cca72c0f Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Mon, 6 May 2019 15:50:49 -0500 Subject: [PATCH 24/30] fix(imports): add missing import for syncing --- fence/sync/sync_users.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index d3cc96e3c..60c91d0a9 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -27,7 +27,7 @@ User, query_for_user, ) -from fence.rbac.client import ArboristError +from fence.rbac.client import ArboristClient, ArboristError from fence.resources.storage import StorageManager @@ -1011,7 +1011,9 @@ def _update_arborist(self, session, user_yaml): return False if not self.arborist_client.healthy(): # TODO (rudyardrichter, 2019-01-07): add backoff/retry here - self.logger.error("arborist service is unavailable; skipping main arborist sync") + self.logger.error( + "arborist service is unavailable; skipping main arborist sync" + ) return False # Set up the resource tree in arborist From c7244dea820d3d615ea29e542f15c587fd94fa1a Mon Sep 17 00:00:00 2001 From: Alexander VanTol Date: Mon, 6 May 2019 16:16:03 -0500 Subject: [PATCH 25/30] fix(fence-create): correctly pass in ArboristClient --- bin/fence-create | 13 ++++++++----- fence/sync/sync_users.py | 8 +------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/bin/fence-create b/bin/fence-create index a0719a901..101abbc6a 100755 --- a/bin/fence-create +++ b/bin/fence-create @@ -5,6 +5,8 @@ import os import sys import logging +from cdislogging import get_logger + from fence.jwt import keys from fence.config import config from fence.scripting.fence_create import ( @@ -95,9 +97,7 @@ def parse_arguments(): default=False, ) client_create.add_argument( - "--policies", - help="which RBAC policies are granted to this client", - nargs="*", + "--policies", help="which RBAC policies are granted to this client", nargs="*" ) client_modify = subparsers.add_parser("client-modify") @@ -123,7 +123,7 @@ def parse_arguments(): client_modify.add_argument( "--policies", help="which RBAC policies are granted to this client; if given, " - "previous policies will be revoked", + "previous policies will be revoked", nargs="*", ) @@ -354,7 +354,10 @@ def main(): ) arborist = None if args.arborist: - arborist = ArboristClient(arborist_base_url=args.arborist) + arborist = ArboristClient( + arborist_base_url=args.arborist, + logger=get_logger("user_syncer.arborist_client"), + ) if args.action == "create": yaml_input = args.__dict__["yaml-input"] diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index 60c91d0a9..723656a5c 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -251,13 +251,7 @@ def __init__( self.logger = get_logger( "user_syncer", log_level="debug" if config["DEBUG"] == True else "info" ) - - self.arborist_client = None - if arborist: - self.arborist_client = ArboristClient( - arborist_base_url=arborist, - logger=get_logger("user_syncer.arborist_client"), - ) + self.arborist_client = arborist if storage_credentials: self.storage_manager = StorageManager( From 59396f05262f1b02a7ab9403be5cec58d4339207 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Mon, 6 May 2019 16:28:14 -0500 Subject: [PATCH 26/30] feat(arborist-sync): overwrite policy if necessary --- fence/rbac/client.py | 26 +++++++++----------------- fence/sync/sync_users.py | 3 ++- 2 files changed, 11 insertions(+), 18 deletions(-) diff --git a/fence/rbac/client.py b/fence/rbac/client.py index 5a300a894..ba1ea4832 100644 --- a/fence/rbac/client.py +++ b/fence/rbac/client.py @@ -11,7 +11,6 @@ from cdislogging import get_logger import requests -from fence.config import config from fence.errors import Forbidden from fence.rbac.errors import ArboristError, ArboristUnhealthyError @@ -67,9 +66,7 @@ class ArboristClient(object): """ def __init__(self, logger=None, arborist_base_url="http://arborist-service/"): - self.logger = logger or get_logger( - "ArboristClient", log_level="debug" if config["DEBUG"] == True else "info" - ) + self.logger = logger or get_logger("ArboristClient") self._base_url = arborist_base_url.strip("/") self._auth_url = self._base_url + "/auth/" self._health_url = self._base_url + "/health" @@ -358,15 +355,16 @@ def delete_policy(self, path): return _request_get_json(requests.delete(self._policy_url + path)) @_arborist_retry() - def create_policy(self, policy_json, skip_if_exists=True): - response = requests.post(self._policy_url, json=policy_json) + def create_policy(self, policy_json, overwrite=False): + if overwrite: + response = requests.put(self._policy_url, json=policy_json) + else: + response = requests.post(self._policy_url, json=policy_json) data = _request_get_json(response) if response.status_code == 409: # already exists; this is ok, but leave warning self.logger.warn( - "could not create policy `{}` in arborist, got 409: {}".format( - policy_json["id"], data["error"] - ) + "policy `{}` already exists in arborist".format(policy_json["id"]) ) return None if isinstance(data, dict) and "error" in data: @@ -457,18 +455,13 @@ def create_group(self, name, description="", users=None, policies=None): self.logger.info("group {} contains users: {}".format(name, list(users))) self.logger.info("group {} has policies: {}".format(name, list(policies))) return data - + @_arborist_retry() def create_user_if_not_exist(self, username): user_json = {"name": username} response = requests.post(self._user_url, json=user_json) data = _request_get_json(response) if response.status_code == 409: - self.logger.warn( - "could not create user `{}` in arborist, got 409: {}".format( - username, data["error"] - ) - ) return None if "error" in data: self.logger.error( @@ -479,7 +472,7 @@ def create_user_if_not_exist(self, username): raise ArboristError(data["error"]) self.logger.info("created user {}".format(username)) return data - + @_arborist_retry() def create_client(self, client_id, policies): response = requests.post( @@ -550,4 +543,3 @@ def delete_client(self, client_id): response = requests.delete("/".join((self._client_url, client_id))) self.logger.info("deleted client {}".format(client_id)) return response.status_code == 204 - diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index 723656a5c..dc5bc4327 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -1083,7 +1083,8 @@ def _update_arborist(self, session, user_yaml): "description": "policy created by fence sync", "role_ids": [permission], "resource_paths": [path], - } + }, + overwrite=True, ) except ArboristError as e: self.logger.info( From cce4ea95f93e2aca86d9bb239c91a3ee5be669d6 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Mon, 6 May 2019 16:42:32 -0500 Subject: [PATCH 27/30] feat(arborist-sync): clean up logging --- fence/rbac/client.py | 1 + fence/sync/sync_users.py | 3 --- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/fence/rbac/client.py b/fence/rbac/client.py index ba1ea4832..63ec4ca9b 100644 --- a/fence/rbac/client.py +++ b/fence/rbac/client.py @@ -458,6 +458,7 @@ def create_group(self, name, description="", users=None, policies=None): @_arborist_retry() def create_user_if_not_exist(self, username): + self.logger.info("making sure user exists: `{}`".format(username)) user_json = {"name": username} response = requests.post(self._user_url, json=user_json) data = _request_get_json(response) diff --git a/fence/sync/sync_users.py b/fence/sync/sync_users.py index dc5bc4327..7eb198157 100644 --- a/fence/sync/sync_users.py +++ b/fence/sync/sync_users.py @@ -1046,10 +1046,7 @@ def _update_arborist(self, session, user_yaml): self.logger.info("processing user `{}`".format(username)) user = query_for_user(session=session, username=username) - self.logger.info("making sure user exists: `{}`".format(username)) self.arborist_client.create_user_if_not_exist(username) - - self.logger.info("revoking all policies for user `{}`".format(username)) self.arborist_client.revoke_all_policies_for_user(username) for path, permissions in user_resources.iteritems(): From 135c17ca65bfe935bb3d039fdc7a16fabdb5bf34 Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Tue, 7 May 2019 09:47:17 -0500 Subject: [PATCH 28/30] feat(download-auth): tmp userdatamodel pin --- requirements.txt | 1 - 1 file changed, 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index fdc8c794a..506103b0a 100644 --- a/requirements.txt +++ b/requirements.txt @@ -32,7 +32,6 @@ temps==0.3.0 Werkzeug==0.12.2 pyyaml==5.1 retry==0.9.2 -userdatamodel==1.4.1 -e git+https://github.com/uc-cdis/flask-postgres-session.git@68bf5a9723a351729855c429eca8a0f4bbb830c7#egg=flask_postgres_session-0.1.3 -e git+https://git@github.com/uc-cdis/cdiserrors.git@0.1.0#egg=cdiserrors -e git+https://github.com/uc-cdis/storage-client.git@0.2.2#egg=storageclient-0.2.2 From fafe1e201335c774cfc0591a4848beba9de4893d Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Tue, 7 May 2019 11:49:45 -0500 Subject: [PATCH 29/30] 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 3ecb1eec38b646dac09c6af8c0dde47b1921004e Mon Sep 17 00:00:00 2001 From: Rudyard Richter Date: Tue, 7 May 2019 13:28:29 -0500 Subject: [PATCH 30/30] 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