-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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): User.fromUserName not implementing IUSER functions #10527
Conversation
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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.
I don't think any of these changes will actually do anything, so the only thing that happens is that those changes are now silently ignored.
Also, if you want to carry this PR forward, you will need to add tests.
public addToGroup(_group: IGroup): void { | ||
throw new Error('Cannot add imported User to Group'); | ||
public addToGroup(group: IGroup): void { | ||
this.groups.push(group.groupName); |
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.
I don't think this is sufficient. If the User
is being created you can add it to a group in this way, but fairly sure that's not how you do it if the User already exists.
} | ||
|
||
public attachInlinePolicy(_policy: Policy): void { | ||
throw new Error('Cannot add inline policy to imported User'); | ||
public attachInlinePolicy(policy: Policy): void { |
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.
Will this actually add an inline policy to an already-existing User
object? I don't think so.
public addManagedPolicy(_policy: IManagedPolicy): void { | ||
throw new Error('Cannot add managed policy to imported User'); | ||
public addManagedPolicy(policy: IManagedPolicy): void { | ||
if (this.managedPolicies.find(mp => mp === policy)) { return; } |
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.
Same comment here.
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*
User.fromUserName not implementing IUSER functions
closes #5797
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license