Merge pull request #856 from asfadmin/rew/pr-3243-add-bearer-token-ca…

…ching

PR-3243 Add short TTL caching to bearer tokens

.github/workflows/re-test-e2e.yml

@@ -31,7 +31,7 @@ jobs:
   # Deploy to the test environment and run end to end tests
   test-end-to-end:
     # Version of gdal installed depends on ubuntu version
-    runs-on: ubuntu-20.04
+    runs-on: ubuntu-22.04
     environment: ${{ inputs.environment }}
     env:
       AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
@@ -177,6 +177,10 @@ jobs:
                   VPCSecurityGroupIDs= \
                   VPCSubnetIDs=
 
+      - uses: actions/setup-python@v5
+        with:
+          python-version: "3.10"
+
       - name: Install GDAL library
         run: |
           sudo apt-get update
@@ -187,7 +191,7 @@ jobs:
           URS_USERNAME: ${{ secrets.URS_USERNAME }}
           URS_PASSWORD: ${{ secrets.URS_PASSWORD }}
         run: |
-          pip install -r requirements/requirements-test.txt GDAL==3.0.4
+          pip install -r requirements/requirements-test.txt GDAL==3.4.1
           pytest tests_e2e \
               --stack-name=$STACK_NAME \
               --test-results=asf.public.code/thin-egress-app/testresults.json \

tests/test_app.py

@@ -42,8 +42,9 @@ def user_profile():
 
 @pytest.fixture
 def _clear_caches():
-    app.get_bc_config_client.cache_clear()
-    app.get_bucket_region_cache.clear()
+    app.get_bc_config_client.cache.clear()
+    app.get_bucket_region.cache.clear()
+    app.RequestAuthorizer._get_profile_and_response_from_bearer.__wrapped__.cache.clear()
 
 
 @pytest.fixture(scope="module")
@@ -134,7 +135,12 @@ def test_request_authorizer_no_headers(current_request, mock_get_urs_url):
 
 @mock.patch(f"{MODULE}.get_user_from_token", autospec=True)
 @mock.patch(f"{MODULE}.get_new_token_and_profile", autospec=True)
-def test_request_authorizer_bearer_header(mock_get_new_token_and_profile, mock_get_user_from_token, current_request):
+def test_request_authorizer_bearer_header(
+    mock_get_new_token_and_profile,
+    mock_get_user_from_token,
+    _clear_caches,
+    current_request,
+):
     current_request.headers = {
         "Authorization": "Bearer token",
         "x-origin-request-id": "origin_request_id"
@@ -157,7 +163,11 @@ def test_request_authorizer_bearer_header(mock_get_new_token_and_profile, mock_g
 
 
 @mock.patch(f"{MODULE}.do_auth_and_return", autospec=True)
-def test_request_authorizer_basic_header(mock_do_auth_and_return, current_request):
+def test_request_authorizer_basic_header(
+    mock_do_auth_and_return,
+    _clear_caches,
+    current_request,
+):
     current_request.headers = {
         "Authorization": "Basic token",
         "x-origin-request-id": "origin_request_id"
@@ -172,7 +182,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):
+def test_request_authorizer_bearer_header_eula_error(
+    mock_get_user_from_token,
+    _clear_caches,
+    current_request,
+):
     current_request.headers = {"Authorization": "Bearer token"}
     mock_get_user_from_token.side_effect = app.EulaException({})
 
@@ -189,7 +203,8 @@ def test_request_authorizer_bearer_header_eula_error(mock_get_user_from_token, c
 def test_request_authorizer_bearer_header_eula_error_browser(
     mock_get_user_from_token,
     mock_make_html_response,
-    current_request
+    _clear_caches,
+    current_request,
 ):
     current_request.headers = {
         "Authorization": "Bearer token",
@@ -231,15 +246,16 @@ def test_request_authorizer_bearer_header_no_profile(
     mock_do_auth_and_return,
     mock_get_new_token_and_profile,
     mock_get_user_from_token,
-    current_request
+    _clear_caches,
+    current_request,
 ):
     current_request.headers = {
         "Authorization": "Bearer token",
         "x-origin-request-id": "origin_request_id"
     }
     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()
@@ -262,7 +278,8 @@ def test_request_authorizer_bearer_header_no_profile(
 def test_request_authorizer_bearer_header_no_user_id(
     mock_do_auth_and_return,
     mock_get_user_from_token,
-    current_request
+    _clear_caches,
+    current_request,
 ):
     current_request.headers = {
         "Authorization": "Bearer token",
@@ -1530,29 +1547,32 @@ def test_dynamic_url_directory(
 @mock.patch(f"{MODULE}.get_yaml_file", autospec=True)
 @mock.patch(f"{MODULE}.get_api_request_uuid", autospec=True)
 @mock.patch(f"{MODULE}.try_download_from_bucket", autospec=True)
+@mock.patch(f"{MODULE}.get_user_from_token", autospec=True)
+@mock.patch(f"{MODULE}.get_new_token_and_profile", autospec=True)
 @mock.patch(f"{MODULE}.JwtManager.get_profile_from_headers", autospec=True)
-@mock.patch(f"{MODULE}.RequestAuthorizer._handle_auth_bearer_header", autospec=True)
 @mock.patch(f"{MODULE}.JwtManager.get_header_to_set_auth_cookie", autospec=True)
 @mock.patch(f"{MODULE}.JWT_COOKIE_NAME", "asf-cookie")
 @mock.patch(f"{MODULE}.b_map", None)
 def test_dynamic_url_bearer_auth(
     mock_get_header_to_set_auth_cookie,
-    mock_handle_auth_bearer_header,
-    mock_get_profile,
+    mock_get_profile_from_headers,
+    mock_get_new_token_and_profile,
+    mock_get_user_from_token,
     mock_try_download_from_bucket,
     mock_get_api_request_uuid,
     mock_get_yaml_file,
     data_path,
     user_profile,
-    current_request
+    current_request,
 ):
     mock_try_download_from_bucket.return_value = chalice.Response(body="Mock response", headers={}, status_code=200)
-    mock_handle_auth_bearer_header.return_value = user_profile
+    mock_get_new_token_and_profile.return_value = user_profile
+    mock_get_user_from_token.return_value = user_profile.user_id
     mock_get_header_to_set_auth_cookie.return_value = {"SET-COOKIE": "cookie"}
     with open(data_path / "bucket_map_example.yaml") as f:
         mock_get_yaml_file.return_value = yaml.full_load(f)
 
-    mock_get_profile.return_value = None
+    mock_get_profile_from_headers.return_value = None
     mock_get_api_request_uuid.return_value = None
     current_request.uri_params = {"proxy": "DATA-TYPE-1/PLATFORM-A/OBJECT_1"}
     current_request.headers = {"Authorization": "bearer b64token"}
@@ -1611,22 +1631,19 @@ def test_s3credentials(
 
 
 @mock.patch(f"{MODULE}.get_yaml_file", autospec=True)
-@mock.patch(f"{MODULE}.RequestAuthorizer._handle_auth_bearer_header", autospec=True)
 @mock.patch(f"{MODULE}.JwtManager.get_profile_from_headers", autospec=True)
 @mock.patch(f"{MODULE}.do_auth_and_return", autospec=True)
 @mock.patch(f"{MODULE}.b_map", None)
 def test_s3credentials_unauthenticated(
     mock_do_auth_and_return,
     mock_get_profile,
-    mock_handle_auth_bearer_header,
     mock_get_yaml_file,
     mock_retrieve_secret,
     data_path,
     client
 ):
     del mock_retrieve_secret
 
-    mock_handle_auth_bearer_header.return_value = None
     with open(data_path / "bucket_map_example.yaml") as f:
         mock_get_yaml_file.return_value = yaml.full_load(f)
     mock_get_profile.return_value = None

tests_e2e/conftest.py

@@ -239,6 +239,32 @@ def auth_cookies(earthdata_auth_session, url_earthdata, api_host, urs_username,
     return cookiejar
 
 
+@pytest.fixture(scope="module")
+def user_bearer_token(url_earthdata, urs_username, urs_password):
+    parse_result = urllib.parse.urlparse(url_earthdata)
+    edl_url = f"{parse_result.scheme}://{parse_result.netloc}"
+
+    # Create a new token
+    response = requests.post(
+        f"{edl_url}/api/users/token",
+        auth=requests.auth.HTTPBasicAuth(urs_username, str(urs_password)),
+    )
+    response.raise_for_status()
+
+    token = response.json()["access_token"]
+
+    yield token
+
+    # Revoke the token to clean up after ourselves. EDL only allows 2 active
+    # tokens at a time.
+    response = requests.post(
+        f"{edl_url}/api/users/revoke_token",
+        params={"token": token},
+        auth=requests.auth.HTTPBasicAuth(urs_username, str(urs_password)),
+    )
+    response.raise_for_status()
+
+
 # Functions that generate the JSON report file
 def pytest_sessionstart(session):
     session.results = {}

tests_e2e/test_protected.py

@@ -106,19 +106,33 @@ def find_bearer_token(auth_cookies):
     return None
 
 
-def validate_bearer_token_works(auth_cookies, url):
+def test_validate_app_bearer_token(urls, auth_cookies):
+    url = urls.join(urls.METADATA_FILE)
     token = find_bearer_token(auth_cookies)
     assert token is not None
 
     r = requests.get(url, headers={"Authorization": f"Bearer {token}"})
     assert r.status_code == 200
 
 
-def test_validate_bearer_token_works(urls, auth_cookies):
+def test_validate_app_bearer_token_private_file(urls, auth_cookies):
+    url = urls.join("PRIVATE", "ACCESS", "testfile")
+    token = find_bearer_token(auth_cookies)
+    assert token is not None
+
+    r = requests.get(url, headers={"Authorization": f"Bearer {token}"})
+    assert r.status_code == 200
+
+
+def test_validate_user_bearer_token(urls, user_bearer_token):
     url = urls.join(urls.METADATA_FILE)
-    validate_bearer_token_works(auth_cookies, url)
+
+    r = requests.get(url, headers={"Authorization": f"Bearer {user_bearer_token}"})
+    assert r.status_code == 200
 
 
-def test_validate_private_file_bearer_token_works(urls, auth_cookies):
+def test_validate_user_bearer_token_private_file(urls, user_bearer_token):
     url = urls.join("PRIVATE", "ACCESS", "testfile")
-    validate_bearer_token_works(auth_cookies, url)
+
+    r = requests.get(url, headers={"Authorization": f"Bearer {user_bearer_token}"})
+    assert r.status_code == 200

thin_egress_app/app.py

@@ -16,7 +16,7 @@
 from botocore.config import Config as bc_Config
 from botocore.exceptions import ClientError
 from cachetools.func import ttl_cache
-from cachetools.keys import hashkey
+from cachetools.keys import hashkey, methodkey
 from chalice import Chalice, Response
 
 try:
@@ -88,9 +88,6 @@ def wrapper(*args, **kwargs):
 # Here's a lifetime-of lambda cache of these values:
 bucket_map_file = os.getenv("BUCKET_MAP_FILE", "bucket_map.yaml")
 b_map = None
-# TODO(reweeden): Refactor when wrapped attributes are implemented
-# https://github.com/tkem/cachetools/issues/176
-get_bucket_region_cache = cachetools.LRUCache(maxsize=128)
 
 STAGE = os.getenv("STAGE_NAME", "DEV")
 
@@ -194,7 +191,7 @@ def get_profile(self) -> Optional[UserProfile]:
         if method == "bearer":
             # we will deal with "bearer" auth here. "Basic" auth will be handled by do_auth_and_return()
             log.debug("we got an Authorization header. %s", authorization)
-            user_profile = self._handle_auth_bearer_header(token)
+            user_profile, self._response = self._get_profile_and_response_from_bearer(token)
 
             if user_profile is None:
                 # Not a successful event.
@@ -212,14 +209,20 @@ def get_profile(self) -> Optional[UserProfile]:
         return None
 
     @with_trace()
-    def _handle_auth_bearer_header(self, token) -> Optional[UserProfile]:
+    @cachetools.cached(
+        cachetools.TTLCache(maxsize=32, ttl=1 * 60),
+        key=methodkey,
+    )
+    def _get_profile_and_response_from_bearer(self, token):
         """
         Will handle the output from get_user_from_token in context of a chalice function. If user_id is determined,
         returns it. If user_id is not determined returns data to be returned
 
         :param token:
         :return: action, data
         """
+        user_profile = None
+        response = None
         try:
             user_id = get_user_from_token(token)
         except EulaException as e:
@@ -236,20 +239,20 @@ def _handle_auth_bearer_header(self, token) -> Optional[UserProfile]:
                     "requestid": get_request_id(),
                 }
 
-                self._response = make_html_response(template_vars, {}, 403, "error.html")
+                response = make_html_response(template_vars, {}, 403, "error.html")
             else:
-                self._response = Response(body=e.payload, status_code=403, headers={})
-            return None
+                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)
-            if user_profile:
-                return user_profile
 
-        self._response = do_auth_and_return(app.current_request.context)
-        return None
+        if user_profile is None:
+            response = do_auth_and_return(app.current_request.context)
+
+        return user_profile, response
 
     def get_error_response(self) -> Optional[Response]:
         """Get the response to return if the user was not authenticated. This
@@ -552,7 +555,7 @@ def get_bcconfig(user_id: str) -> dict:
 
 @with_trace()
 @cachetools.cached(
-    get_bucket_region_cache,
+    cachetools.LRUCache(maxsize=128),
     # Cache by bucketname only
     key=lambda _, bucketname: hashkey(bucketname)
 )