diff --git a/requirements/requirements.in b/requirements/requirements.in index 0da9b612..8e65cbbf 100644 --- a/requirements/requirements.in +++ b/requirements/requirements.in @@ -2,5 +2,5 @@ cachetools cfnresponse chalice -git+https://github.com/asfadmin/rain-api-core.git@f5186c00c8e9d576f710eac62e6ca1e51516d6d7 +git+https://github.com/asfadmin/rain-api-core.git@1be67560f7c41b50afbd2ca20473ffbdc7efae68 netaddr diff --git a/requirements/requirements.txt b/requirements/requirements.txt index ab8a9f7f..1b8a84e9 100644 --- a/requirements/requirements.txt +++ b/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@1be67560f7c41b50afbd2ca20473ffbdc7efae68 # via -r requirements/requirements.in readchar==4.2.1 # via inquirer diff --git a/tests/__init__.py b/tests/__init__.py index e69de29b..33a0b6cf 100644 --- a/tests/__init__.py +++ b/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" diff --git a/tests/test_app.py b/tests/test_app.py index f58fd561..bd882355 100644 --- a/tests/test_app.py +++ b/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 @@ -108,7 +109,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 @@ -188,7 +189,11 @@ def test_request_authorizer_bearer_header_eula_error( 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() @@ -210,11 +215,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() @@ -287,7 +297,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() @@ -335,8 +345,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 @@ -355,7 +366,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() @@ -371,7 +382,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() @@ -381,7 +393,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() diff --git a/thin_egress_app/app.py b/thin_egress_app/app.py index 640ddc69..c5686f84 100644 --- a/thin_egress_app/app.py +++ b/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, @@ -164,11 +163,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 @@ -225,16 +219,23 @@ def _get_profile_and_response_from_bearer(self, token): response = None 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'{e.payload["resolution_url"]} and try again.' + f'Could not fetch data because "{e.msg["error_description"]}". Please accept EULA here: ' + f'{e.msg["resolution_url"]} and try again.' ), "requestid": get_request_id(), } @@ -243,11 +244,8 @@ def _get_profile_and_response_from_bearer(self, token): else: response = Response(body=e.payload, status_code=403, headers={}) return None, response - - 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) + except EdlException: + user_profile = None if user_profile is None: response = do_auth_and_return(app.current_request.context) @@ -318,76 +316,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") - 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))) + client = EdlClient() 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()