Skip to content

Commit

Permalink
Merge pull request #644 from asfadmin/rew/pr-3244-s3-credential-fix
Browse files Browse the repository at this point in the history
PR-3244 S3 credential fix
  • Loading branch information
reweeden authored Oct 24, 2022
2 parents 81b5753 + becb60d commit f326721
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 3 deletions.
18 changes: 17 additions & 1 deletion lambda/app.py
Original file line number Diff line number Diff line change
Expand Up @@ -976,6 +976,19 @@ def s3credentials():
policy = b_map.to_iam_policy(groups)
log.debug("policy: %s", policy)

if policy is None:
template_vars = {
"contentstring": "You do not have permission to access any data.",
"title": "Could not access data",
"requestid": get_request_id()
}
return make_html_response(
template_vars,
authorizer.get_success_response_headers(),
403,
"error.html"
)

app_name = app.current_request.headers.get("app-name", "")
role_session_name = get_role_session_name(user_profile.user_id, app_name)

Expand Down Expand Up @@ -1011,7 +1024,10 @@ def get_s3_credentials(user_id: str, role_session_name: str, policy: dict):
RoleSessionName=role_session_name,
ExternalId=user_id,
DurationSeconds=3600,
Policy=json.dumps(policy)
# NOTE: Policy max size is 2048 characters which is quite limiting.
# TODO(reweeden): We'll need to figure out how to accomodate large
# bucket maps that will push us over this limit.
Policy=json.dumps(policy, separators=(",", ":"))
)
return response["Credentials"]

Expand Down
2 changes: 1 addition & 1 deletion requirements/requirements.in
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ cachetools
cfnresponse
chalice
flatdict
git+https://github.com/asfadmin/rain-api-core.git@6acd2cb943cb552c525bc5320297f62b812a33ba
git+https://github.com/asfadmin/rain-api-core.git@e1638eec08353651ed42105840ba6a80a7141ac2
netaddr
2 changes: 1 addition & 1 deletion requirements/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ pyyaml==6.0
# via
# chalice
# rain-api-core
rain-api-core @ git+https://github.com/asfadmin/rain-api-core.git@6acd2cb943cb552c525bc5320297f62b812a33ba
rain-api-core @ git+https://github.com/asfadmin/rain-api-core.git@e1638eec08353651ed42105840ba6a80a7141ac2
# via -r requirements/requirements.in
readchar==4.0.3
# via inquirer
Expand Down
28 changes: 28 additions & 0 deletions tests/test_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -1441,6 +1441,34 @@ def test_s3credentials_unauthenticated(
assert response.status_code == 301


@mock.patch(f"{MODULE}.get_yaml_file", autospec=True)
@mock.patch(f"{MODULE}.JwtManager.get_profile_from_headers", autospec=True)
@mock.patch(f"{MODULE}.b_map", None)
def test_s3credentials_no_permissions(
mock_get_profile,
mock_get_yaml_file,
mock_get_urs_creds,
mock_make_html_response,
user_profile,
client
):
mock_get_yaml_file.return_value = {}
mock_get_profile.return_value = user_profile

client.http.get("/s3credentials")

mock_make_html_response.assert_called_once_with(
{
"contentstring": "You do not have permission to access any data.",
"title": "Could not access data",
"requestid": app.app.lambda_context.aws_request_id
},
{},
403,
"error.html"
)


@mock.patch(f"{MODULE}.boto3")
def test_get_s3_credentials(mock_boto3, monkeypatch):
monkeypatch.setenv("EGRESS_APP_DOWNLOAD_ROLE_INREGION_ARN", "aws:role:arn")
Expand Down

0 comments on commit f326721

Please sign in to comment.