Merge pull request #673 from asfadmin/devel

Update master branch

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

@@ -32,7 +32,8 @@ on:
 jobs:
   # Deploy to the test environment and run end to end tests
   test-end-to-end:
-    runs-on: ubuntu-latest
+    # Version of gdal installed depends on ubuntu version
+    runs-on: ubuntu-20.04
     environment: ${{ inputs.environment }}
     env:
       AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}

.github/workflows/release.yml

@@ -6,7 +6,7 @@ name: Release
 on:
   release:
     types:
-      - released
+      - published
 
 jobs:
   build:

cloudformation/thin-egress-app.yaml.j2

@@ -620,6 +620,7 @@ Resources:
           HTML_TEMPLATE_DIR: !Ref HtmlTemplateDir
           JWT_KEY_SECRET_NAME: !Ref JwtKeySecretName
           JWT_ALGO: !Ref JwtAlgo
+          ENABLE_S3_CREDENTIALS_ENDPOINT: !Ref EnableS3CredentialsEndpoint
           AWS_LAMBDA_EXEC_WRAPPER: !Ref OtLambdaExecWrapper
           OPENTELEMETRY_COLLECTOR_CONFIG_FILE: !Ref OtCollectorConfigFile
           OTEL_LOG_LEVEL: !Ref OtLogLevel

docs/configuration.md

@@ -101,23 +101,96 @@ object prefixing to control access:
 
 ```yaml
 MAP:
-   data-path: data-bucket
+    data-path: data-bucket
 
 PUBLIC_BUCKETS:
-   data-bucket/browse/: "Browse image"
+    data-bucket/browse/: "Browse image"
 
 PRIVATE_BUCKETS:
-   data-bucket/pre-commission-data/:
-     - internal_users
-     - external_team
+    data-bucket/:
+      - internal_users
+      - external_team
 ```
 
-In the above example, while access to data in `data-bucket` requires simple auth,
-accessing an object with the prefix `browse/` will require NO auth, and
-`pre-commission-data/` will require EDL App group access as specified.
+In the above example, while access to data in `data-bucket` requires EDL App
+group access to one of the specified groups, accessing an object with the
+prefix `browse/` will not require any auth.
+
+TEA will always check the rules from most deeply nested to most shallow. So
+for example, in this bucket map:
+
+```yaml
+MAP:
+    data-path: data-bucket
+
+PUBLIC_BUCKETS:
+    data-bucket/external/public/: "Browse image"
+
+PRIVATE_BUCKETS:
+    data-bucket/:
+      - internal_users
+    data-bucket/external/:
+      - internal_users
+      - external_team
+```
+
+Any data in the prefix `external/public/` will be public, data in the prefix
+`external/` but not in the prefix `external/public/` will be available to users
+in either of the defined EDL App groups, and everything else in the bucket will
+be available to users in the `internal_users` group only.
+
+##### S3 Direct Access Compatibility
+
+Some access configurations supported by the standard HTTP methods are not allowed when S3 direct access is enabled. Of note:
+
+1. The first prefix in the bucket map will need to be set to the most restrictive access level and subsequent prefixes must have access levels that become successively more open. This is due to a limitation with how IAM policies work (For more information, see [S3 direct access](#s3-direct-access)).
+2. Public buckets will require EDL authentication for S3 direct access. e.g. "Browse image"
+
+All of the bucket maps shown above are compatible with S3 direct access; however, long time users of
+TEA might recognize the following configuration example from previous versions which will be rejected when S3 direct access is enabled.
+
+Bad Example:
+```yaml
+MAP:
+    data-path: data-bucket
+
+PUBLIC_BUCKETS:
+    data-bucket/browse/: "Browse image"
+
+PRIVATE_BUCKETS:
+    # Since no permission is specified for `data-bucket/`, the default is used
+    # which allows access to any authenticated user. Therefore, adding a
+    # prefix `pre-commission-data/` that is more restrictive will break IAM
+    # compatibility.
+    data-bucket/pre-commission-data/:
+      - internal_users
+      - external_team
+```
+
+To fix this, the bucket map could be modified as follows:
+
+Good Example:
+```yaml
+MAP:
+    data-path: data-bucket
+
+PUBLIC_BUCKETS:
+    data-bucket/browse/: "Browse image"
+
+PRIVATE_BUCKETS:
+    data-bucket/:
+      - internal_users
+      - external_team
+    # The following rule is redundant, but might not be in the presence of
+    # other rules.
+    data-bucket/pre-commission-data/:
+      - internal_users
+      - external_team
+```
 
 #### S3 Direct Access
-*NOTE: Support for S3 direct access is currently experimental*
+
+*NOTE: S3 direct access is currently experimental*
 
 TEA can be deployed with an `/s3credentials` endpoint (See
 [Enabling S3 direct access](deploying.md#enabling-s3-direct-access)) for

docs/s3access.md

@@ -48,10 +48,10 @@ The response is your temporary credentials as returned by Amazon STS. See the
 **Example:**
 ```json
 {
-    "AccessKeyId": "AKIAIOSFODNN7EXAMPLE",
-    "SecretAccessKey": "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
-    "SessionToken": "LONGSTRINGOFCHARACTERS.../HJLgV91QJFCMlmY8slIEOjrOChLQYmzAqrb5U1ekoQAK6f86HKJFTT2dONzPgmJN9ZvW5DBwt6XUxC9HAQ0LDPEYEwbjGVKkzSNQh/",
-    "Expiration": "2021-01-27 00:50:09+00:00"
+    "accessKeyId": "AKIAIOSFODNN7EXAMPLE",
+    "secretAccessKey": "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY",
+    "sessionToken": "LONGSTRINGOFCHARACTERS.../HJLgV91QJFCMlmY8slIEOjrOChLQYmzAqrb5U1ekoQAK6f86HKJFTT2dONzPgmJN9ZvW5DBwt6XUxC9HAQ0LDPEYEwbjGVKkzSNQh/",
+    "expiration": "2021-01-27 00:50:09+00:00"
 }
 ```
 
@@ -94,9 +94,9 @@ def lambda_handler(event, context):
     # Set up separate boto3 clients for download and upload
     download_client = boto3.client(
         "s3",
-        aws_access_key_id=creds["AccessKeyId"],
-        aws_secret_access_key=creds["SecretAccessKey"],
-        aws_session_token=creds["SessionToken"]
+        aws_access_key_id=creds["accessKeyId"],
+        aws_secret_access_key=creds["secretAccessKey"],
+        aws_session_token=creds["sessionToken"]
     )
     # Lambda needs to have permission to upload to destination bucket
     upload_client = boto3.client("s3")

lambda/app.py

@@ -381,13 +381,25 @@ def restore_bucket_vars():
 
         b_map_dict = get_yaml_file(conf_bucket, bucket_map_file)
         reverse = os.getenv('USE_REVERSE_BUCKET_MAP', 'FALSE').lower() == 'true'
+        iam_compatible = os.getenv('ENABLE_S3_CREDENTIALS_ENDPOINT', 'FALSE').lower() == 'true'
 
         log.debug('bucket map: %s', b_map_dict)
-        b_map = BucketMap(
-            b_map_dict,
-            bucket_name_prefix=get_bucket_name_prefix(),
-            reverse=reverse
-        )
+        try:
+            b_map = BucketMap(
+                b_map_dict,
+                bucket_name_prefix=get_bucket_name_prefix(),
+                reverse=reverse,
+                iam_compatible=iam_compatible
+            )
+        except ValueError:
+            log.error('Invalid bucket map, please consult the TEA documentation')
+            if iam_compatible:
+                log.info(
+                    'Your bucket map permissions are configured in such a way '
+                    'that they cannot be converted to an IAM policy. Either '
+                    'fix your bucket map, or disable S3 credentials.'
+                )
+            raise
     else:
         log.info('reusing old bucket configs')
 
@@ -996,6 +1008,13 @@ def s3credentials():
     creds = get_s3_credentials(user_profile.user_id, role_session_name, policy)
     timer.mark()
 
+    creds = {
+        "accessKeyId": creds["AccessKeyId"],
+        "secretAccessKey": creds["SecretAccessKey"],
+        "sessionToken": creds["SessionToken"],
+        "expiration": creds["Expiration"]
+    }
+
     log.debug("timing for s3credentials()")
     timer.log_all(log)
 

requirements/requirements-dev.txt

@@ -8,14 +8,14 @@ attrs==21.4.0
     # via
     #   -c requirements/requirements.txt
     #   pytest
-boto3==1.24.94
+boto3==1.25.3
     # via -r requirements/requirements-dev.in
-botocore==1.27.94
+botocore==1.28.3
     # via
     #   -c requirements/requirements.txt
     #   boto3
     #   s3transfer
-build==0.8.0
+build==0.9.0
     # via pip-tools
 click==8.1.3
     # via
@@ -25,6 +25,8 @@ coverage[toml]==6.5.0
     # via pytest-cov
 deprecated==1.2.13
     # via opentelemetry-api
+exceptiongroup==1.0.0
+    # via pytest
 iniconfig==1.1.1
     # via pytest
 jmespath==1.0.1
@@ -46,11 +48,9 @@ pip-tools==6.9.0
     # via -r requirements/requirements-dev.in
 pluggy==1.0.0
     # via pytest
-py==1.11.0
-    # via pytest
 pyparsing==3.0.9
     # via packaging
-pytest==7.1.3
+pytest==7.2.0
     # via
     #   -r requirements/requirements-dev.in
     #   pytest-cov
@@ -80,7 +80,7 @@ urllib3==1.26.12
     # via
     #   -c requirements/requirements.txt
     #   botocore
-wheel==0.37.1
+wheel==0.38.4
     # via
     #   -c requirements/requirements.txt
     #   pip-tools

requirements/requirements.in

@@ -2,5 +2,5 @@ cachetools
 cfnresponse
 chalice
 flatdict
-git+https://github.com/asfadmin/rain-api-core.git@e1638eec08353651ed42105840ba6a80a7141ac2
+git+https://github.com/asfadmin/rain-api-core.git@16329f3fa7326edd668e26c2d817cf430268faad
 netaddr

requirements/requirements.txt

@@ -8,7 +8,7 @@ attrs==21.4.0
     # via chalice
 blessed==1.19.1
     # via inquirer
-botocore==1.27.94
+botocore==1.28.3
     # via chalice
 cachetools==5.0.0
     # via
@@ -42,7 +42,7 @@ netaddr==0.8.0
     #   rain-api-core
 pycparser==2.21
     # via cffi
-pyjwt[crypto]==2.5.0
+pyjwt[crypto]==2.6.0
     # via rain-api-core
 python-dateutil==2.8.2
     # via botocore
@@ -52,7 +52,7 @@ pyyaml==6.0
     # via
     #   chalice
     #   rain-api-core
-rain-api-core @ git+https://github.com/asfadmin/rain-api-core.git@e1638eec08353651ed42105840ba6a80a7141ac2
+rain-api-core @ git+https://github.com/asfadmin/rain-api-core.git@16329f3fa7326edd668e26c2d817cf430268faad
     # via -r requirements/requirements.in
 readchar==4.0.3
     # via inquirer
@@ -61,8 +61,6 @@ six==1.16.0
     #   blessed
     #   chalice
     #   python-dateutil
-types-cryptography==3.3.23.1
-    # via pyjwt
 typing-extensions==4.4.0
     # via chalice
 urllib3==1.26.12
@@ -71,7 +69,7 @@ urllib3==1.26.12
     #   cfnresponse
 wcwidth==0.2.5
     # via blessed
-wheel==0.37.1
+wheel==0.38.4
     # via chalice
 
 # The following packages are considered to be unsafe in a requirements file:

setup_jwt_cookie.sh

@@ -25,7 +25,7 @@ function GENERATE_TEA_CREDS {
 }
 
 GENERATE_TEA_CREDS
-aws secretsmanager create-secret --name jwt_secret_for_tea --profile ${profile_name:-default} --region ${aws_region:-us-east-1} \
+aws secretsmanager create-secret --name jwt_secret_for_tea --profile ${profile_name:-default} --region ${aws_region:-us-west-2} \
     --description "RS256 keys for TEA app JWT cookies" \
     --secret-string file:///tmp/jwtkeys.json
 

tests/test_app.py

@@ -384,6 +384,28 @@ def test_restore_bucket_vars(mock_get_yaml_file, resources):
     assert app.b_map.bucket_map == buckets
 
 
+@mock.patch(f"{MODULE}.get_yaml_file", autospec=True)
+def test_restore_bucket_vars_iam_compatibility_error(
+    mock_get_yaml_file,
+    monkeypatch
+):
+    mock_get_yaml_file.return_value = {
+        "PATH": "bucket",
+        "PRIVATE_BUCKETS": {
+            "bucket/prefix/": ["group"]
+        }
+    }
+
+    app.b_map = None
+    monkeypatch.setenv("ENABLE_S3_CREDENTIALS_ENDPOINT", "False")
+    app.restore_bucket_vars()
+
+    app.b_map = None
+    monkeypatch.setenv("ENABLE_S3_CREDENTIALS_ENDPOINT", "True")
+    with pytest.raises(ValueError, match="Invalid prefix permissions"):
+        app.restore_bucket_vars()
+
+
 @mock.patch(f"{MODULE}.get_urs_url", autospec=True)
 def test_do_auth_and_return(mock_get_urs_url, monkeypatch):
     mock_get_urs_url.side_effect = lambda _ctx, redirect: redirect
@@ -556,6 +578,7 @@ def test_try_download_from_bucket(
     client = mock_get_role_session().client()
     client.get_bucket_location.return_value = {"LocationConstraint": "us-east-1"}
     client.head_object.return_value = {"ContentLength": 2048}
+    app.b_map = None
 
     response = app.try_download_from_bucket("somebucket", "somefile", user_profile, {})
     client.head_object.assert_called_once()
@@ -1407,10 +1430,10 @@ def test_s3credentials(
     response = client.http.get("/s3credentials")
 
     assert response.json_body == {
-        "AccessKeyId": "access_key",
-        "SecretAccessKey": "secret_access_key",
-        "SessionToken": "session_token",
-        "Expiration": "expiration"
+        "accessKeyId": "access_key",
+        "secretAccessKey": "secret_access_key",
+        "sessionToken": "session_token",
+        "expiration": "expiration"
     }
     assert response.status_code == 200
 

tests_e2e/test_s3credentials.py

@@ -17,8 +17,8 @@ def test_authenticated_user_can_get_creds(urls, auth_cookies):
 
     assert r.status_code == 200
     assert list(data.keys()) == [
-        "AccessKeyId",
-        "SecretAccessKey",
-        "SessionToken",
-        "Expiration"
+        "accessKeyId",
+        "secretAccessKey",
+        "sessionToken",
+        "expiration"
     ]