Merge 11439fd into 8883117

.secrets.baseline

@@ -250,7 +250,7 @@
         "filename": "tests/ras/test_ras.py",
         "hashed_secret": "d9db6fe5c14dc55edd34115cdf3958845ac30882",
         "is_verified": false,
-        "line_number": 103
+        "line_number": 105
       }
     ],
     "tests/test-fence-config.yaml": [
@@ -260,7 +260,6 @@
         "hashed_secret": "afc848c316af1a89d49826c5ae9d00ed769415f3",
         "is_verified": false,
         "line_number": 31
-
       },
       {
         "type": "Secret Keyword",
@@ -271,5 +270,5 @@
       }
     ]
   },
-  "generated_at": "2021-11-15T23:28:25Z"
+  "generated_at": "2021-12-01T02:28:44Z"
 }

fence/resources/openid/ras_oauth2.py

@@ -10,6 +10,7 @@
 
 from authutils.errors import JWTError
 from authutils.token.core import get_iss, get_kid
+from gen3authz.client.arborist.errors import ArboristError
 
 
 from fence.config import config
@@ -23,6 +24,7 @@
 )
 from fence.jwt.validate import validate_jwt
 from fence.utils import DEFAULT_BACKOFF_SETTINGS
+from fence.errors import InternalError
 from .idp_oauth2 import Oauth2ClientBase
 
 
@@ -160,6 +162,8 @@ def get_user_id(self, code):
             flask.g.tokens = token
             flask.g.keys = keys
 
+        except InternalError:
+            raise
         except Exception as e:
             self.logger.exception("{}: {}".format(err_msg, e))
             return {"error": err_msg}
@@ -206,9 +210,33 @@ def map_iss_sub_pair_to_user(self, issuer, subject_id, username, email):
                         "from the DRS endpoint. Changing said user's username"
                         f' to "{username}".'
                     )
-                    # TODO also change username in Arborist
+
+                    tries = 2
+                    for i in range(tries):
+                        try:
+                            flask.current_app.arborist.update_user(
+                                iss_sub_pair_to_user.user.username,
+                                new_username=username,
+                                new_email=email,
+                            )
+                        except ArboristError as e:
+                            self.logger.warning(
+                                f"Try {i+1}: could not update user's username in Arborist: {e}"
+                            )
+                            if i == tries - 1:
+                                err_msg = f"Failed to update user's username in Arborist after {tries} tries"
+                                self.logger.exception(err_msg)
+                                raise InternalError(err_msg)
+                        else:
+                            self.logger.info(
+                                "Successfully changed Arborist user's username from "
+                                f'"{iss_sub_pair_to_user.user.username}" to "{username}"'
+                            )
+                            break
+
                     iss_sub_pair_to_user.user.username = username
-                    iss_sub_pair_to_user.user.email = email
+                    if email:
+                        iss_sub_pair_to_user.user.email = email
                     db_session.commit()
                 elif iss_sub_pair_to_user.user.username != username:
                     self.logger.warning(

fence/sync/sync_users.py

@@ -5,6 +5,7 @@
 import subprocess as sp
 import yaml
 import copy
+import datetime
 
 from contextlib import contextmanager
 from collections import defaultdict
@@ -1612,7 +1613,8 @@ def _update_authz_in_arborist(
         Args:
             user_projects (dict)
             user_yaml (UserYAML) optional, if there are policies for users in a user.yaml
-            expiration (datetime.datetime): expiration time
+            single_user_sync (bool) whether authz update is for a single user
+            expires (int) time at which authz info in Arborist should expire
 
         Return:
             bool: success
@@ -1731,9 +1733,12 @@ def _update_authz_in_arborist(
                                     "not creating policy in arborist; {}".format(str(e))
                                 )
                             self._created_policies.add(policy_id)
-                        self.arborist_client.grant_user_policy(username, policy_id)
-                        # TODO need to add expiration to this function in gen3authz
-                        # self.arborist_client.grant_user_policy(username, policy_id, expiration=expiration)
+
+                        self.arborist_client.grant_user_policy(
+                            username,
+                            policy_id,
+                            expires_at=expires,
+                        )
 
             # TODO As of 10-11-2021, there's no endpoint yet in Arborist to
             # support the creation of policies in bulk. When syncing RAS
@@ -1766,7 +1771,11 @@ def _update_authz_in_arborist(
             #
             if user_yaml:
                 for policy in user_yaml.policies.get(username, []):
-                    self.arborist_client.grant_user_policy(username, policy)
+                    self.arborist_client.grant_user_policy(
+                        username,
+                        policy,
+                        expires_at=expires,
+                    )
 
         if user_yaml:
             for client_name, client_details in user_yaml.clients.items():

pyproject.toml

@@ -27,7 +27,7 @@ flask-cors = "^3.0.3"
 flask-restful = "^0.3.6"
 flask_sqlalchemy_session = "^1.1"
 email_validator = "^1.1.1"
-gen3authz = "^1.3.0"  
+gen3authz = "^1.4.0"
 gen3cirrus = "^2.0.0"
 gen3config = "^0.1.7"
 gen3users = "^0.6.0"

tests/ras/test_ras.py

@@ -4,6 +4,7 @@
 import time
 import mock
 import jwt
+import pytest
 
 from cdislogging import get_logger
 
@@ -19,6 +20,7 @@
 )
 from fence.resources.openid.ras_oauth2 import RASOauth2Client as RASClient
 from fence.resources.ga4gh.passports import get_or_create_gen3_user_from_iss_sub
+from fence.errors import InternalError
 
 from tests.dbgap_sync.conftest import add_visa_manually
 from fence.job.visa_update_cronjob import Visa_Token_Update
@@ -650,7 +652,7 @@ def test_map_iss_sub_pair_to_user_with_no_prior_DRS_access(db_session):
     Test RASOauth2Client.map_iss_sub_pair_to_user when the username passed in
     (e.g. eRA username) does not already exist in the Fence database and that
     user's <iss, sub> combination has not already been mapped through a prior
-    DRS/data access request.
+    DRS access request.
     """
     iss = "https://domain.tld"
     sub = "123_abc"
@@ -677,15 +679,19 @@ def test_map_iss_sub_pair_to_user_with_no_prior_DRS_access(db_session):
     assert len(iss_sub_pair_to_user_records) == 1
 
 
-def test_map_iss_sub_pair_to_user_with_prior_DRS_access(db_session):
+def test_map_iss_sub_pair_to_user_with_prior_DRS_access(
+    db_session, mock_arborist_requests
+):
     """
     Test RASOauth2Client.map_iss_sub_pair_to_user when the username passed in
     (e.g. eRA username) does not already exist in the Fence database but that
     user's <iss, sub> combination has already been mapped to an existing user
-    created during a prior DRS/data access request. In this case, that
+    created during a prior DRS access request. In this case, that
     existing user's username is changed from sub+iss to the username passed
     in.
     """
+    mock_arborist_requests({"arborist/user/123_abcdomain.tld": {"PATCH": (None, 204)}})
+
     iss = "https://domain.tld"
     sub = "123_abc"
     username = "johnsmith"
@@ -713,14 +719,39 @@ def test_map_iss_sub_pair_to_user_with_prior_DRS_access(db_session):
     assert iss_sub_pair_to_user.user.email == email
 
 
+def test_map_iss_sub_pair_to_user_with_prior_DRS_access_and_arborist_error(
+    db_session, mock_arborist_requests
+):
+    """
+    Test that RASOauth2Client.map_iss_sub_pair_to_user raises an internal error
+    when Arborist fails to return a successful response.
+    """
+    mock_arborist_requests({"arborist/user/123_abcdomain.tld": {"PATCH": (None, 500)}})
+
+    iss = "https://domain.tld"
+    sub = "123_abc"
+    username = "johnsmith"
+    email = "johnsmith@domain.tld"
+    oidc = config.get("OPENID_CONNECT", {})
+    ras_client = RASClient(
+        oidc["ras"],
+        HTTP_PROXY=config.get("HTTP_PROXY"),
+        logger=logger,
+    )
+    get_or_create_gen3_user_from_iss_sub(iss, sub)
+
+    with pytest.raises(InternalError):
+        ras_client.map_iss_sub_pair_to_user(iss, sub, username, email)
+
+
 def test_map_iss_sub_pair_to_user_with_prior_login_and_prior_DRS_access(
     db_session,
 ):
     """
     Test RASOauth2Client.map_iss_sub_pair_to_user when the username passed in
     (e.g. eRA username) already exists in the Fence database and that
     user's <iss, sub> combination has already been mapped to a separate user
-    created during a prior DRS/data access request. In this case,
+    created during a prior DRS access request. In this case,
     map_iss_sub_pair_to_user returns the user created from prior DRS/data
     access, rendering the other user (e.g. the eRA one) inaccessible.
     """