Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Roles for our pathogen-repo-build GitHub Actions workflow, take 2 #7

Closed
wants to merge 1 commit into from

Conversation

tsibley
Copy link
Member

@tsibley tsibley commented May 17, 2024

GitHub OIDC → Cognito Identity → AssumeRoleWithWebSession

Let's us map GitHub token claims to principal tags, which is great. No need to customize the "sub" claim any more, and granular matching.

But ultimately not workable because GitHub OIDC doesn't include the repository name sans owner in its token claims (e.g. "zika" for "nextstrain/zika"), so we don't have any policy variable to interpolate into policy conditions.

I thought maybe we could (ab)use role session names for this and pass the unqualified repo name via it ourselves, but that hit a wall because while sts:RoleSessionName is available in the AssumeRoleWithWebIdentity request, it's not available on subsequent requests which we need to be able to use in policy conditions.

Then I thought maybe we could use sts:SourceIdentity instead in the role assumption request which is available as aws:SourceIdentity in subsequent requests, except that GitHub's token would have to include an "https://aws.amazonaws.com/source_identity" claim, and it ofc does not provide any way to do that. If it did, it would likely also let us provide a custom "https://aws.amazonaws.com/tags" claim at which point all of this nonsense would be moot anyway.

Finally, I thought maybe we could use a custom audience ("aud" claim) which GitHub does let you specify. This is available in AWS requests as the "token.actions.githubusercontent.com:aud" key and we can use it as a policy variable. \o/ But to use it we'd need to cut out Cognito Identity (e.g. go back to where we started without it) and lose the token claims → principal tags mapping. So we'd need to customize the GitHub token's "sub" claim and match against it. We'd also need to explicitly list out all possible valid "aud" on the AWS OIDC IdP configuration (as "client_id_list"), and that would be frustrating.

This all feels more complex than it's worth at this point.

Resolves: #4
Related-to: nextstrain/.github#81
Related-to: #6

Let's us map GitHub token claims to principal tags, which is great.
No need to customize the "sub" claim any more, and granular matching.

But ultimately not workable because GitHub OIDC doesn't include the
repository name _sans_ owner in its token claims (e.g. "zika" for
"nextstrain/zika"), so we don't have any policy variable to interpolate
into policy conditions.

I thought maybe we could (ab)use role session names for this and pass
the unqualified repo name via it ourselves, but that hit a wall because
while sts:RoleSessionName is available in the AssumeRoleWithWebIdentity
request, it's not available on subsequent requests which we need to be
able to use in policy conditions.

Then I thought maybe we could use sts:SourceIdentity instead in the role
assumption request which is available as aws:SourceIdentity in
subsequent requests, except that GitHub's token would have to include an
"https://aws.amazonaws.com/source_identity" claim, and it ofc does not
provide any way to do that.  If it did, it would likely also let us
provide a custom "https://aws.amazonaws.com/tags" claim at which point
all of this nonsense would be moot anyway.

Finally, I thought maybe we could use a custom audience ("aud" claim)
which GitHub _does_ let you specify.  This is available in AWS requests
as the "token.actions.githubusercontent.com:aud" key and we can use it
as a policy variable.  \o/  But to use it we'd need to cut out Cognito
Identity (e.g. go back to where we started without it) and lose the
token claims → principal tags mapping.  So we'd need to customize the
GitHub token's "sub" claim and match against it.  We'd also need to
explicitly list out all possible valid "aud" on the AWS OIDC IdP
configuration (as "client_id_list"), and that would be frustrating.

This all feels more complex than it's worth at this point.

Resolves: <#4>
Related-to: <nextstrain/.github#81>
@tsibley tsibley closed this May 17, 2024
tsibley added a commit that referenced this pull request May 20, 2024
tsibley added a commit that referenced this pull request May 20, 2024
tsibley added a commit that referenced this pull request May 20, 2024
tsibley added a commit that referenced this pull request May 20, 2024
tsibley added a commit that referenced this pull request May 21, 2024
A collection of templated repo-specific roles for inside a Nextstrain
runtime and one role for the GitHub Actions job itself, i.e. outside the
runtime.

The inside-the-runtime roles are given pathogen-specific permissions
necessary for the ingest and phylogenetic workflows of a pathogen repo.

The outside-the-runtime role is only necessary/used at the moment for
access to AWS Batch.

The roles can only be assumed by specific repos when performed by our
centralized pathogen-repo-build workflow.  While this doesn't completely
prevent off-label use by other GitHub Actions workflows launched from a
pathogen repo, it does make it more involved to do so, hopefully to the
point of discouragement.  The associated GitHub repository configuration
is managed by Terraform now as well since the customization of the "sub"
claim in GitHub Action's OIDC token is tightly coupled to our AWS role
trust policies.

Resolves: <#4>
Related-to: <nextstrain/private#96>,
            <nextstrain/.github#81>
Supersedes: <#6>,
            <#7>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create new role for GH Action access to S3
1 participant