fix(iam): Error for addToPolicy and addToPrincipalPolicy in User.fromUserName corrected... #11046
Conversation
|
|
|
Title does not follow the guidelines of Conventional Commits. Please adjust title before merge. |
ariain
changed the title
ariain
changed the title
ariain
changed the title
|
@wtho can you help me with these errors?? |
|
Just go through the list of checks at the end of this PR
|
|
@wtho who will review and merge these changes now?? |
|
Haha, just wait till someone from aws assignes a reviewer, it usually takes 2-4 days. |
|
Okay, thanks for guiding me @wtho ... |
|
@rix0rrr please review these changes... |
| this.defaultPolicy.addStatements(statement); | ||
| return { statementAdded: true, policyDependable: this.defaultPolicy }; | ||
| public addToPrincipalPolicy(_statement: PolicyStatement): AddToPrincipalPolicyResult { | ||
| throw new Error('Cannot add imported User to principal policy'); |
There was a problem hiding this comment.
The error message is the wrong way around.
Cannot add policy to imported User principal makes more sense to me.
There was a problem hiding this comment.
I'm sort of fine having this throw an error, but as discussed in #10913 this could just be made to work.
If you add an AWS::IAM::Policy and you give it the right UserName, it becomes possible to add an identity policy to an existing identity, as far as I can tell.
Why don't we just do that?
Maybe the correct fix is to figure out where the exception that was mentioned in #10913 is thrown, and make sure that error isn't thrown anymore. Because the content of addToPrincipalPolicy mostly seems correct, except it probably throws when attachToUser is called... (?)
There was a problem hiding this comment.
The error message will be common with addToPolicy function so maybe we can throw this message Cannot add policy to imported User.
If we don't want to throw an error then there are several problems here if we try to correct the functionality because its not like other functions are working. The attachToUser will call to attachInlinePolicy which will throw an error, I can try to fix that function...
There was a problem hiding this comment.
So, should I check for the correct functionality or throw an error instead??
mergify
bot
dismissed
rix0rrr’s stale review
Pull request has been modified.
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
It was not possible to attach policies to imported `User` objects. This can be made to work though, as the underlying CloudFormation resource allows doing so. Make this work in the class library. Fixes #10913, closes #11046, closes #10527. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
iam.User.fromUserNameaddToPolicyandaddToPrincipalPolicyfixedError is corrected to
Cannot add policy to imported Userfor both functions...fixes #10913
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license