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.

Sign up for GitHub

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

Adds Roles for IC resource access requests #49565

Merged
merged 9 commits into from
Dec 16, 2024
Merged

Adds Roles for IC resource access requests #49565

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7dace6d
Adds IC Account Assignments to preset `access` role
tcsc Nov 29, 2024
114f00a
Merge branch 'master' into tcsc/idc-roles
tcsc Dec 12, 2024
af4448a
Adds IC Account Assignments to preset `access` role
tcsc Nov 29, 2024
c043d75
Merge remote-tracking branch 'refs/remotes/origin/tcsc/idc-roles' int…
tcsc Dec 12, 2024
0eb1c5a
Add default roles if not present
tcsc Dec 13, 2024
16b5f33
Merge branch 'master' into tcsc/idc-roles
tcsc Dec 13, 2024
4347058
Add reviewer role
tcsc Dec 13, 2024
c6b0a40
Update lib/services/presets.go
tcsc Dec 13, 2024
f99a58d
apply feedback
tcsc Dec 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions api/types/role.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,6 +288,9 @@ type Role interface {
// GetIdentityCenterAccountAssignments fetches the allow or deny Account
// Assignments for the role
GetIdentityCenterAccountAssignments(RoleConditionType) []IdentityCenterAccountAssignment
// GetIdentityCenterAccountAssignments sets the allow or deny Account
// Assignments for the role
SetIdentityCenterAccountAssignments(RoleConditionType, []IdentityCenterAccountAssignment)
}

// NewRole constructs new standard V7 role.
Expand Down Expand Up @@ -2094,6 +2097,16 @@ func (r *RoleV6) GetIdentityCenterAccountAssignments(rct RoleConditionType) []Id
return r.Spec.Deny.AccountAssignments
}

// SetIdentityCenterAccountAssignments sets the allow or deny Identity Center
// Account Assignments for the role
func (r *RoleV6) SetIdentityCenterAccountAssignments(rct RoleConditionType, assignments []IdentityCenterAccountAssignment) {
cond := &r.Spec.Deny
if rct == Allow {
cond = &r.Spec.Allow
}
cond.AccountAssignments = assignments
}

// LabelMatcherKinds is the complete list of resource kinds that support label
// matchers.
var LabelMatcherKinds = []string{
Expand Down
23 changes: 23 additions & 0 deletions lib/services/presets.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -751,6 +751,21 @@ func defaultAllowAccessReviewConditions(enterprise bool) map[string]*types.Acces
return map[string]*types.AccessReviewConditions{}
}

func defaultAllowAccountAssignments(enterprise bool) map[string][]types.IdentityCenterAccountAssignment {
if enterprise {
return map[string][]types.IdentityCenterAccountAssignment{
teleport.PresetAccessRoleName: {
{
Account: types.Wildcard,
PermissionSet: types.Wildcard,
},
},
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, wouldn't this assign people with this role to each account and permission set pair? Don't we want to only grant access request privileges to them?

Copy link
Contributor

@smallinsky smallinsky Dec 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this will ensure that anyone with the appropriate access role is assigned to WS IC resources.

However we should consider following the existing pattern:

// defaultAllowAccessRequestConditions has the access request conditions that should be set as default when they were
// not explicitly defined.
func defaultAllowAccessRequestConditions(enterprise bool) map[string]*types.AccessRequestConditions {
	if enterprise {
		return map[string]*types.AccessRequestConditions{
			teleport.PresetRequesterRoleName: {
				SearchAsRoles: []string{
					teleport.PresetAccessRoleName,
					teleport.PresetGroupAccessRoleName,
				},
			},
			teleport.SystemOktaRequesterRoleName: {
				SearchAsRoles: []string{
					teleport.SystemOktaAccessRoleName,
				},
				MaxDuration: types.NewDuration(MaxAccessDuration),
			},
		}
	}

	return map[string]*types.AccessRequestConditions{}
}

// defaultAllowAccessReviewConditions has the access review conditions that should be set as default when they were
// not explicitly defined.
func defaultAllowAccessReviewConditions(enterprise bool) map[string]*types.AccessReviewConditions {
	if enterprise {
		return map[string]*types.AccessReviewConditions{
			teleport.PresetReviewerRoleName: {
				PreviewAsRoles: []string{
					teleport.PresetAccessRoleName,
					teleport.PresetGroupAccessRoleName,
				},
				Roles: []string{
					teleport.PresetAccessRoleName,
					teleport.PresetGroupAccessRoleName,
				},
			},
		}
	}

	return map[string]*types.AccessReviewConditions{}
}

To extend this we just need to add PresetAWSIdentityCenterAccessRoleName in both defaultAllowAccessRequestConditions and defaultAllowAccessReviewConditions. This ensures that the access role does not interfere with AWS IC resources.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I agree let's do that as it's safer.

@tcsc Sorry for going back and forth but I think it makes sense to have preset role for AWS IC resources to make sure users don't accidentally grant everyone access to everything by assigning access role.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have to admit that I was a bit concerned about that myself.

}
}

return nil
tcsc marked this conversation as resolved.
Show resolved Hide resolved
}

// AddRoleDefaults adds default role attributes to a preset role.
// Only attributes whose resources are not already defined (either allowing or denying) are added.
func AddRoleDefaults(role types.Role) (types.Role, error) {
Expand Down Expand Up @@ -868,6 +883,14 @@ func AddRoleDefaults(role types.Role) (types.Role, error) {
}
}

if len(role.GetIdentityCenterAccountAssignments(types.Allow)) == 0 {
assignments := defaultAllowAccountAssignments(enterprise)[role.GetName()]
if assignments != nil {
role.SetIdentityCenterAccountAssignments(types.Allow, assignments)
changed = true
}
}

if !changed {
return nil, trace.AlreadyExists("no change")
}
Expand Down
88 changes: 88 additions & 0 deletions lib/services/presets_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,94 @@ func TestAddRoleDefaults(t *testing.T) {
},
},
},
{
name: "access (account assignments set on enterprise)",
role: &types.RoleV6{
Metadata: types.Metadata{
Name: teleport.PresetAccessRoleName,
Labels: map[string]string{
types.TeleportInternalResourceType: types.PresetResource,
},
},
Spec: types.RoleSpecV6{
Allow: types.RoleConditions{
Rules: defaultAllowRules()[teleport.PresetAccessRoleName],
DatabaseServiceLabels: defaultAllowLabels(false)[teleport.PresetAccessRoleName].DatabaseServiceLabels,
DatabaseRoles: defaultAllowLabels(false)[teleport.PresetAccessRoleName].DatabaseRoles,
},
},
},
enterprise: true,
expectedErr: require.NoError,
expected: &types.RoleV6{
Metadata: types.Metadata{
Name: teleport.PresetAccessRoleName,
Labels: map[string]string{
types.TeleportInternalResourceType: types.PresetResource,
},
},
Spec: types.RoleSpecV6{
Allow: types.RoleConditions{
Rules: defaultAllowRules()[teleport.PresetAccessRoleName],
DatabaseServiceLabels: defaultAllowLabels(true)[teleport.PresetAccessRoleName].DatabaseServiceLabels,
DatabaseRoles: defaultAllowLabels(true)[teleport.PresetAccessRoleName].DatabaseRoles,

AccountAssignments: []types.IdentityCenterAccountAssignment{
{
Account: types.Wildcard,
PermissionSet: types.Wildcard,
},
},
},
},
},
},
{
name: "access (account assignments not set on OSS)",
role: &types.RoleV6{
Metadata: types.Metadata{
Name: teleport.PresetAccessRoleName,
Labels: map[string]string{
types.TeleportInternalResourceType: types.PresetResource,
},
},
Spec: types.RoleSpecV6{
Allow: types.RoleConditions{
Rules: defaultAllowRules()[teleport.PresetAccessRoleName],
DatabaseServiceLabels: defaultAllowLabels(false)[teleport.PresetAccessRoleName].DatabaseServiceLabels,
DatabaseRoles: defaultAllowLabels(false)[teleport.PresetAccessRoleName].DatabaseRoles,
},
},
},
enterprise: false,
expectedErr: noChange,
},
{
name: "access (account assignments unchanged if already set)",
role: &types.RoleV6{
Metadata: types.Metadata{
Name: teleport.PresetAccessRoleName,
Labels: map[string]string{
types.TeleportInternalResourceType: types.PresetResource,
},
},
Spec: types.RoleSpecV6{
Allow: types.RoleConditions{
AccountAssignments: []types.IdentityCenterAccountAssignment{
{
Account: "Some Account",
PermissionSet: "Some PermissionSet",
},
},
Rules: defaultAllowRules()[teleport.PresetAccessRoleName],
DatabaseServiceLabels: defaultAllowLabels(true)[teleport.PresetAccessRoleName].DatabaseServiceLabels,
DatabaseRoles: defaultAllowLabels(true)[teleport.PresetAccessRoleName].DatabaseRoles,
},
},
},
enterprise: true,
expectedErr: noChange,
},
{
name: "auditor (default rules match preset rules)",
role: &types.RoleV6{
Expand Down
Loading