-
Notifications
You must be signed in to change notification settings - Fork 4k
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
fix(iam): can't use OrganizationPrincipal
for assuming Role
#5746
Conversation
`Principal: "*"` supposedly works to allow any Principal to assume a Role (restricted by `Conditions`, of course), but doesn't work in practice. The IAM API rejects it as a MalformedPolicyDocument. In order to not generate a large diff on existing policies, disable simplification of `Principal: { AWS: * }` to `Principal: *` only for AssumeRole policy documents. Fixes #5732.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
public toStatementJson(): any { | ||
const stat = super.toStatementJson(); | ||
|
||
if (stat.Principal && stat.Principal === '*') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this be simplified to just stat.Principal === '*'
? If stat.Principal
is undefined
, that will return false
, like it does now.
Thank you very much for your work guys. |
@@ -439,3 +439,18 @@ function validateMaxSessionDuration(duration?: number) { | |||
throw new Error(`maxSessionDuration is set to ${duration}, but must be >= 3600sec (1hr) and <= 43200sec (12hrs)`); | |||
} | |||
} | |||
|
|||
/** | |||
* A PolicyStatement that doesn't normalize its Principal field. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds to me like it DOES normalize the principal field (it normalizes "*"
to {AWS:"*"}
, no?).
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request is now being automatically merged. |
Thank you for contributing! Your pull request is now being automatically merged. |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Principal: "*"
supposedly works to allow any Principal to assumea Role (restricted by
Conditions
, of course), but doesn't work inpractice. The IAM API rejects it as a MalformedPolicyDocument.
In order to not generate a large diff on existing policies, disable
simplification of
Principal: { AWS: * }
toPrincipal: *
onlyfor AssumeRole policy documents.
Fixes #5732.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
Commit Message
fix(iam): can't use
OrganizationPrincipal
for assuming RolePrincipal: "*"
supposedly works to allow any Principal to assumea Role (restricted by
Conditions
, of course), but doesn't work inpractice. The IAM API rejects it as a MalformedPolicyDocument.
In order to not generate a large diff on existing policies, disable
simplification of
Principal: { AWS: * }
toPrincipal: *
onlyfor AssumeRole policy documents.
Fixes #5732.