Refactor EDL call to use EdlClient from rain-api-core

requirements/requirements.txt

@@ -48,7 +48,7 @@ pyyaml==6.0.2
     # via
     #   chalice
     #   rain-api-core
-rain-api-core @ git+https://github.com/asfadmin/rain-api-core.git@f5186c00c8e9d576f710eac62e6ca1e51516d6d7
+rain-api-core @ git+https://github.com/asfadmin/rain-api-core.git@rew/pr-3242-refactor-edl-client
     # via -r requirements/requirements.in
 readchar==4.2.1
     # via inquirer

tests/__init__.py

@@ -0,0 +1,7 @@
+import os
+
+# Override environment variables that we don't want to be accidentally pulling
+# defaults from in tests. These need to be set before app.py is imported by
+# pytest in order to ensure that initialization code that runs at import time
+# will get the fake values.
+os.environ["AUTH_BASE_URL"] = "http://testing-auth-base-url"

tests/test_app.py

@@ -10,6 +10,7 @@
 from botocore.exceptions import ClientError
 from chalice.test import Client
 from rain_api_core.auth import UserProfile
+from rain_api_core.edl import EdlException, EulaException
 
 from thin_egress_app import app
 
@@ -107,7 +108,7 @@ def mock_make_html_response():
 
 @pytest.fixture
 def mock_request():
-    with mock.patch(f"{MODULE}.request", autospec=True) as m:
+    with mock.patch(f"{MODULE}.urllib.request", autospec=True) as m:
         yield m
 
 
@@ -174,7 +175,11 @@ def test_request_authorizer_basic_header(mock_do_auth_and_return, current_reques
 @mock.patch(f"{MODULE}.get_user_from_token", autospec=True)
 def test_request_authorizer_bearer_header_eula_error(mock_get_user_from_token, current_request):
     current_request.headers = {"Authorization": "Bearer token"}
-    mock_get_user_from_token.side_effect = app.EulaException({})
+    mock_get_user_from_token.side_effect = EulaException(
+        HTTPError("", 403, "Forbidden", {}, io.StringIO()),
+        {},
+        "",
+    )
 
     authorizer = app.RequestAuthorizer()
 
@@ -195,11 +200,16 @@ def test_request_authorizer_bearer_header_eula_error_browser(
         "Authorization": "Bearer token",
         "user-agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64)"
     }
-    mock_get_user_from_token.side_effect = app.EulaException({
+    msg = {
         "status_code": 403,
         "error_description": "EULA Acceptance Failure",
-        "resolution_url": "http://resolution_url"
-    })
+        "resolution_url": "http://resolution_url",
+    }
+    mock_get_user_from_token.side_effect = EulaException(
+        HTTPError("", 403, "Forbidden", {}, None),
+        msg,
+        None,
+    )
 
     authorizer = app.RequestAuthorizer()
 
@@ -239,7 +249,7 @@ def test_request_authorizer_bearer_header_no_profile(
     }
     mock_response = mock.Mock()
     mock_do_auth_and_return.return_value = mock_response
-    mock_get_new_token_and_profile.return_value = False
+    mock_get_new_token_and_profile.return_value = None
     mock_get_user_from_token.return_value = "user_name"
 
     authorizer = app.RequestAuthorizer()
@@ -270,7 +280,7 @@ def test_request_authorizer_bearer_header_no_user_id(
     }
     mock_response = mock.Mock()
     mock_do_auth_and_return.return_value = mock_response
-    mock_get_user_from_token.return_value = None
+    mock_get_user_from_token.side_effect = EdlException(KeyError("uid"), {}, None)
 
     authorizer = app.RequestAuthorizer()
 
@@ -318,8 +328,9 @@ def test_get_user_from_token(mock_request, mock_get_urs_creds, current_request):
     del current_request
 
     payload = '{"uid": "user_name"}'
-    mock_response = mock.Mock()
-    mock_response.read.return_value = payload
+    mock_response = mock.MagicMock()
+    with mock_response as mock_f:
+        mock_f.read.return_value = payload
     mock_response.code = 200
     mock_request.urlopen.return_value = mock_response
 
@@ -338,7 +349,7 @@ def test_get_user_from_token_eula_error(mock_request, mock_get_urs_creds, curren
     """
     mock_request.urlopen.side_effect = HTTPError("", 403, "Forbidden", {}, io.StringIO(payload))
 
-    with pytest.raises(app.EulaException):
+    with pytest.raises(EulaException):
         app.get_user_from_token("token")
     mock_get_urs_creds.assert_called_once()
 
@@ -354,7 +365,8 @@ def test_get_user_from_token_other_error(mock_request, mock_get_urs_creds, curre
     """
     mock_request.urlopen.side_effect = HTTPError("", 401, "Bad Request", {}, io.StringIO(payload))
 
-    assert app.get_user_from_token("token") is None
+    with pytest.raises(EdlException):
+        assert app.get_user_from_token("token")
     mock_get_urs_creds.assert_called_once()
 
 
@@ -364,7 +376,8 @@ def test_get_user_from_token_json_error(mock_request, mock_get_urs_creds, curren
 
     mock_request.urlopen.side_effect = HTTPError("", code, "Message", {}, io.StringIO("not valid json"))
 
-    assert app.get_user_from_token("token") is None
+    with pytest.raises(EdlException):
+        assert app.get_user_from_token("token")
     mock_get_urs_creds.assert_called_once()
 
 

thin_egress_app/app.py

@@ -6,9 +6,7 @@
 import urllib.request
 from functools import wraps
 from typing import Optional
-from urllib import request
-from urllib.error import HTTPError
-from urllib.parse import quote_plus, urlencode, urlparse
+from urllib.parse import quote_plus, urlparse
 
 import boto3
 import cachetools
@@ -37,6 +35,7 @@ def inject(obj):
     retrieve_secret,
 )
 from rain_api_core.bucket_map import BucketMap
+from rain_api_core.edl import EdlClient, EdlException, EulaException
 from rain_api_core.egress_util import get_bucket_name_prefix, get_presigned_url
 from rain_api_core.general_util import (
     duration,
@@ -167,11 +166,6 @@ class TeaException(Exception):
     """ base exception for TEA """
 
 
-class EulaException(TeaException):
-    def __init__(self, payload: dict):
-        self.payload = payload
-
-
 class RequestAuthorizer:
     def __init__(self):
         self._response = None
@@ -222,16 +216,23 @@ def _handle_auth_bearer_header(self, token) -> Optional[UserProfile]:
         """
         try:
             user_id = get_user_from_token(token)
+            log_context(user_id=user_id)
+
+            user_profile = get_new_token_and_profile(
+                user_id,
+                True,
+                aux_headers=get_aux_request_headers(),
+            )
         except EulaException as e:
             log.warning("user has not accepted EULA")
             # TODO(reweeden): changing the response based on user agent looks like a really bad idea...
             if check_for_browser(app.current_request.headers):
                 template_vars = {
-                    "title": e.payload["error_description"],
+                    "title": e.msg["error_description"],
                     "status_code": 403,
                     "contentstring": (
-                        f'Could not fetch data because "{e.payload["error_description"]}". Please accept EULA here: '
-                        f'<a href="{e.payload["resolution_url"]}">{e.payload["resolution_url"]}</a> and try again.'
+                        f'Could not fetch data because "{e.msg["error_description"]}". Please accept EULA here: '
+                        f'<a href="{e.msg["resolution_url"]}">{e.msg["resolution_url"]}</a> and try again.'
                     ),
                     "requestid": get_request_id(),
                 }
@@ -240,16 +241,13 @@ def _handle_auth_bearer_header(self, token) -> Optional[UserProfile]:
             else:
                 self._response = Response(body=e.payload, status_code=403, headers={})
             return None
+        except EdlException:
+            user_profile = None
 
-        if user_id:
-            log_context(user_id=user_id)
-            aux_headers = get_aux_request_headers()
-            user_profile = get_new_token_and_profile(user_id, True, aux_headers=aux_headers)
-            if user_profile:
-                return user_profile
+        if user_profile is None:
+            self._response = do_auth_and_return(app.current_request.context)
 
-        self._response = do_auth_and_return(app.current_request.context)
-        return None
+        return user_profile
 
     def get_error_response(self) -> Optional[Response]:
         """Get the response to return if the user was not authenticated. This
@@ -315,76 +313,35 @@ def get_user_from_token(token):
         "token": token
     }
 
-    url = "{}/oauth/tokens/user?{}".format(
-        os.getenv("AUTH_BASE_URL", "https://urs.earthdata.nasa.gov"),
-        urlencode(params)
-    )
-
     authval = f"Basic {urs_creds['UrsAuth']}"
     headers = {"Authorization": authval}
 
     # Tack on auxillary headers
     headers.update(get_aux_request_headers())
-    log.debug("headers: %s, params: %s", headers, params)
 
-    _time = time.time()
-
-    req = request.Request(url, headers=headers, method="POST")
+    client = EdlClient()
     try:
-        response = request.urlopen(req)
-    except HTTPError as e:
-        response = e
-        log.debug("%s", e)
-
-    payload = response.read()
-    log.info(return_timing_object(service="EDL", endpoint=url, method="POST", duration=duration(_time)))
-
-    try:
-        msg = json.loads(payload)
-    except json.JSONDecodeError:
-        log.error("could not get json message from payload: %s", payload)
-        msg = {}
-
-    log.debug("raw payload: %s", payload)
-    log.debug("json loads: %s", msg)
-    log.debug("code: %s", response.code)
-
-    if response.code == 200:
-        try:
-            return msg["uid"]
-        except KeyError as e:
-            log.error(
-                "Problem with return from URS: e: %s, url: %s, params: %s, response payload: %s",
-                e,
-                url,
-                params,
-                payload,
-            )
-            return None
-    elif response.code == 403:
-        if "error_description" in msg and "eula" in msg["error_description"].lower():
-            # sample json in this case:
-            # `{"status_code": 403, "error_description": "EULA Acceptance Failure",
-            #   "resolution_url": "http://uat.urs.earthdata.nasa.gov/approve_app?client_id=LqWhtVpLmwaD4VqHeoN7ww"}`
-            log.warning("user needs to sign the EULA")
-            raise EulaException(msg)
-        # Probably an expired token if here
-        log.warning("403 error from URS: %s", msg)
-    else:
-        if "error" in msg:
-            errtxt = msg["error"]
-        else:
-            errtxt = ""
-        if "error_description" in msg:
-            errtxt = errtxt + " " + msg["error_description"]
+        msg = client.request(
+            "POST",
+            "/oauth/tokens/user",
+            params=params,
+            headers=headers,
+        )
 
+        user_id = msg.get("uid")
+        if user_id is None:
+            log.error("Problem with return from URS: msg: %s", msg)
+            raise EdlException(KeyError("uid"), msg, None)
+        return user_id
+    except EulaException:
+        raise
+    except EdlException as e:
         log.error(
-            "Error getting URS userid from token: %s with code %s",
-            errtxt,
-            response.code,
+            "Error getting URS userid from token: %s, response: %s",
+            e.inner,
+            e.payload,
         )
-        log.debug("url: %s, params: %s", url, params)
-    return None
+        raise
 
 
 @with_trace()