Skip to content
New issue

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

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

Already on GitHub? # to your account

[Feature request] The ability to reuse groups in eligibility policies. #39

Open
tawoyinfa opened this issue Jul 12, 2023 · 20 comments
Open
Labels
enhancement New feature or request

Comments

@tawoyinfa
Copy link
Contributor

At present I believe you can only use the group once. I have two use case - I have a support team i'm happy to give unapproved access to x roles, but for administrative roles, I want an approval. I believe under the current model i can't reference the group twice. The second use case, I have a permissionset with an customer managed policy available to only the Audit account, should be be available to group x. At the same time group x should also be able to assume other roles in the org.

@tawoyinfa tawoyinfa added the enhancement New feature or request label Jul 12, 2023
@tawoyinfa tawoyinfa moved this to Backlog in TEAM Roadmap Jul 12, 2023
@tawoyinfa tawoyinfa changed the title [Feature request] The ability to reuse groups in elibility policies. [Feature request] The ability to reuse groups in eligibility policies. Jul 12, 2023
@rapides
Copy link

rapides commented Jul 13, 2023

@tawoyinfa

I think that there is another use case. A scenario when one group has an eligibility policy where there is Permission Set A and B, and AWS Account A and B, and I don't want to allow requesting temporary access using Permission Set B with AWS Account A. But only Permission Set A with AWS account A, and Permission Set B with AWS account B.

Currently, while being an admin I need to select all possible permission sets and AWS accounts, and requesters can select and mix them in any way which is not safe (approvers can make a mistake during the verification). The only solution is to have more and more user groups but this is not easy when you are using 3rd party provider like Azure AD to manage them, so you cannot simply add new groups in AWS IAM IC.

Let me know if that makes sense.

@andyt10
Copy link

andyt10 commented Aug 14, 2023

I think this is a very necessary change to the current model for eligibility.

Both the use case of having some accounts with unapproved temporary access, as well as mapping specific permissions sets to specific accounts (and not others) as @rapides mentions.

Ideally this would be for both group and individual user eligibility policies but right now I'd settle for this just working for groups.

I'd happily look at adding this myself if it's the direction the owners want to take the solution. :)

@tawoyinfa
Copy link
Contributor Author

@andyt10 This is definitely the direction we are looking at going forward. Would be great if you can take a look at supporting this.

@andyt10
Copy link

andyt10 commented Aug 14, 2023

This is honestly a great offering, and I am so happy someone from AWS has finally looked to fill this void.
I'd be very keen to put effort into maintaining this solution, a reason to finally place with GraphQL as well... :)

I think there needs to be some consideration around evaluation logic. The more I think about this, the more important it actually seems to make sure it's 'right' whilst this Solution is in its infancy.

From examining this Solution, and reading the points above from yourself and rapides it seems like the following is the desired behaviour

  • We need to be able to reference the same Group/User & Account/OU tuple in different policies.
  • A uniqueness constraint needs to exist for a tuple of Group/User & Account/OU & Permission Set to prevent ambiguity. (i.e. creating one policy that requires approval and one that does not)
  • There needs to be a known policy evaluation logic where Account level policies overlap with OU level ones. Right now it seems to always opt for requiring approval..even where you might specify an Account in that OU is not requiring approval. It feels like a 'most specific policy' option is what is actually desired. (Happy to discuss this part, or treat it as a separate change...not sure how it relates to [Feature Request] Inherited Eligibility & Approver Policies #41)

I'm currently evaluating this Solution for use within our Organisation. So far it seems like a great offering, and would potentially replace something we have in-house for most use cases. If this gets the buy-in, I would expect some time directed to making improvements as we need them too.

The eligibility evaluation logic and data model seems to be the biggest place for improvement so far. (Slack is desired as well, but I see another PR already for that).

@tawoyinfa

This comment was marked as outdated.

@andyt10
Copy link

andyt10 commented Sep 1, 2023

@tawoyinfa sorry mate, I didn't see your reply or get to that discord link before it expired.
discord user is vkandy :)

@matthewcbaker
Copy link

I am evaluating TEAM right now, and it looks fantastic so far, but I came looking to see if someone else had noted this issue as I can also see it being a problem.

It's essentially exactly the scenario that @rapides describes above that will cause the problem, but to provide a named example:

  • Developers can get ReadOnly access to Production account.
  • Developers can get Admin access to Test account, but shouldn't be able to get Admin access to Production account (either accidentally or deliberately).

In a similar way, adding to the above scenario:

  • Developers can get ReadOnly access to the Test account without requiring approval.

As noted, it's possible to solve this by adding the Developers to multiple Roles. Whilst possible, when you expand it out to all relevant scenarios, maintaining this would be incredibly difficult.

(Unfortunately the approval process cannot be solved this way, even if it's very hard to maintain, and I have added a comment to #81 to describe it.)

I am glad that @tawoyinfa has already seen this as the future direction. Thank you.

Copy link

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 7 days it will automatically be closed.

@emanuelhjertzen
Copy link

We have deployed this solution (v1.10) and worked with it for a few months now. This feature request is definitely our most wanted!

@fatbasstard
Copy link
Contributor

Hi @tawoyinfa

Can this issue be reopened? I'm really looking forward to this feature. I need/want to setup different permissions. e.g. Assume Role A without approval, but Role B with approval.

This is now not possible

@matthewcbaker
Copy link

I still see this as a worthwhile enhancement

@kstromeiraos
Copy link

Same here, this would definitely be very useful for us!

@matthewcbaker
Copy link

Still useful, and would save repetition within the authentication system.

@danluera
Copy link
Contributor

My knowledge of JS is pretty non-existent. But, it seems to me that the logic of greying out an entity that's already been assigned is controlled in the UI and not by the underlying DynamoDB entry. I've not dug far enough into the rest of the code to see what the impact of removing the filter from the UI would do to downstream logic. But... in 1.1.2's Eligible.js file the users are filtered on line 736 and the groups are filtered on line 765.

``disabled: allItems.map(({ id }) => id).includes(user.UserId),

``disabled: allItems.map(({ id }) => id).includes(group.GroupId),

Copy link

github-actions bot commented Sep 8, 2024

Marking this issue as stale due to inactivity. This helps our maintainers find and focus on the active issues. If this issue receives no comments in the next 7 days it will automatically be closed.

@fatbasstard
Copy link
Contributor

Still useful

@mbaker-itrs
Copy link

Still useful

Current solution is having "Group A" manually/regularly/automatically copied into additional "Group A Pre-Approved", "Group A Prod" (or whatever is relevant for RBAC) within the Identity Provider so that by the time it reaches TEAM it /looks/ like several groups of people.

As noted above, this is a messy and hard to maintain solution to a problem that only exists within TEAM.

@jvanenckevort
Copy link

This causes a lot of Entra ID overhead, creating multiple groups with essentially the same people in it.

@matthewcbaker
Copy link

Still an issue

@CloudNinjaDev
Copy link

Still needed this feature

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests