From 7dace6d3c2e42103976c6c196a3281a1a3eb9e24 Mon Sep 17 00:00:00 2001 From: Trent Clarke Date: Fri, 29 Nov 2024 11:17:12 +1100 Subject: [PATCH 1/6] Adds IC Account Assignments to preset `access` role Adds the global-allow wildcards to the preset access role, allowing the `access` role to participate in Resource Access Requests involving Identty Center resources. --- api/types/role.go | 13 ++++++ lib/services/presets.go | 23 ++++++++++ lib/services/presets_test.go | 88 ++++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+) diff --git a/api/types/role.go b/api/types/role.go index 98263f1973d4f..7d71a16e9bc0d 100644 --- a/api/types/role.go +++ b/api/types/role.go @@ -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. @@ -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{ diff --git a/lib/services/presets.go b/lib/services/presets.go index 887545d164cf6..6209ccc6ec955 100644 --- a/lib/services/presets.go +++ b/lib/services/presets.go @@ -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, + }, + }, + } + } + + return nil +} + // 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) { @@ -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") } diff --git a/lib/services/presets_test.go b/lib/services/presets_test.go index af4dbf3b06791..2f7dd8bcf0b15 100644 --- a/lib/services/presets_test.go +++ b/lib/services/presets_test.go @@ -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{ From af4448a67e5514d7747b3583037cda3a9545f22c Mon Sep 17 00:00:00 2001 From: Trent Clarke Date: Fri, 29 Nov 2024 11:17:12 +1100 Subject: [PATCH 2/6] Adds IC Account Assignments to preset `access` role Adds the global-allow wildcards to the preset access role, allowing the `access` role to participate in Resource Access Requests involving Identty Center resources. --- api/types/role.go | 13 ++++++ lib/services/presets.go | 23 ++++++++++ lib/services/presets_test.go | 88 ++++++++++++++++++++++++++++++++++++ 3 files changed, 124 insertions(+) diff --git a/api/types/role.go b/api/types/role.go index b9f52c1bd9ad5..c24b72b84ac8b 100644 --- a/api/types/role.go +++ b/api/types/role.go @@ -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. @@ -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{ diff --git a/lib/services/presets.go b/lib/services/presets.go index 887545d164cf6..6209ccc6ec955 100644 --- a/lib/services/presets.go +++ b/lib/services/presets.go @@ -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, + }, + }, + } + } + + return nil +} + // 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) { @@ -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") } diff --git a/lib/services/presets_test.go b/lib/services/presets_test.go index af4dbf3b06791..2f7dd8bcf0b15 100644 --- a/lib/services/presets_test.go +++ b/lib/services/presets_test.go @@ -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{ From 0eb1c5a7e2e6f26c10458864014ddc1c852a5ce5 Mon Sep 17 00:00:00 2001 From: Trent Clarke Date: Fri, 13 Dec 2024 14:36:25 +1100 Subject: [PATCH 3/6] Add default roles if not present --- constants.go | 5 + lib/auth/init.go | 1 + lib/auth/init_test.go | 1 + lib/services/presets.go | 105 ++++++++++++++++++--- lib/services/presets_test.go | 177 ++++++++++++++++------------------- 5 files changed, 179 insertions(+), 110 deletions(-) diff --git a/constants.go b/constants.go index c6dcd8963fc1d..1c7ecf3226a96 100644 --- a/constants.go +++ b/constants.go @@ -704,6 +704,11 @@ const ( // access to Okta resources. This will be used by the Okta requester role to // search for Okta resources. SystemOktaAccessRoleName = "okta-access" + + // SystemIdentityCenterAccessRoleName specifies the name of a system role + // that grants a user access to AWS Identity Center resources via + // Access Requests. + SystemIdentityCenterAccessRoleName = "aws-ic-access" ) var PresetRoles = []string{PresetEditorRoleName, PresetAccessRoleName, PresetAuditorRoleName} diff --git a/lib/auth/init.go b/lib/auth/init.go index 9d86d21c1106f..215dd4aaae512 100644 --- a/lib/auth/init.go +++ b/lib/auth/init.go @@ -1037,6 +1037,7 @@ func GetPresetRoles() []types.Role { services.NewSystemOktaAccessRole(), services.NewSystemOktaRequesterRole(), services.NewPresetTerraformProviderRole(), + services.NewSystemIdentityCenterAccessRole(), } // Certain `New$FooRole()` functions will return a nil role if the diff --git a/lib/auth/init_test.go b/lib/auth/init_test.go index 59fab636b43a0..fd5ce9925c1d4 100644 --- a/lib/auth/init_test.go +++ b/lib/auth/init_test.go @@ -1115,6 +1115,7 @@ func TestPresets(t *testing.T) { enterpriseSystemRoleNames := []string{ teleport.SystemAutomaticAccessApprovalRoleName, teleport.SystemOktaAccessRoleName, + teleport.SystemIdentityCenterAccessRoleName, } enterpriseUsers := []types.User{ diff --git a/lib/services/presets.go b/lib/services/presets.go index 6209ccc6ec955..f9b996a13ee9b 100644 --- a/lib/services/presets.go +++ b/lib/services/presets.go @@ -28,8 +28,10 @@ import ( "github.com/gravitational/teleport/api/constants" apidefaults "github.com/gravitational/teleport/api/defaults" "github.com/gravitational/teleport/api/types" + "github.com/gravitational/teleport/api/types/common" apiutils "github.com/gravitational/teleport/api/utils" "github.com/gravitational/teleport/lib/modules" + "github.com/gravitational/teleport/lib/utils" ) // NewSystemAutomaticAccessApproverRole creates a new Role that is allowed to @@ -577,6 +579,32 @@ func NewSystemOktaRequesterRole() types.Role { return role } +// NewSystemIdentityCenterAccessRole creates a role that allows access to AWS +// IdentityCenter resources via Access Requests +func NewSystemIdentityCenterAccessRole() types.Role { + if modules.GetModules().BuildType() != modules.BuildEnterprise { + return nil + } + return &types.RoleV6{ + Kind: types.KindRole, + Version: types.V7, + Metadata: types.Metadata{ + Name: teleport.SystemIdentityCenterAccessRoleName, + Namespace: apidefaults.Namespace, + Description: "Access AWS IdentityCenter resources", + Labels: map[string]string{ + types.TeleportInternalResourceType: types.SystemResource, + types.OriginLabel: common.OriginAWSIdentityCenter, + }, + }, + Spec: types.RoleSpecV6{ + Allow: types.RoleConditions{ + AccountAssignments: defaultAllowAccountAssignments(true)[teleport.SystemIdentityCenterAccessRoleName], + }, + }, + } +} + // NewPresetTerraformProviderRole returns a new pre-defined role for the Teleport Terraform provider. // This role can edit any Terraform-supported resource. func NewPresetTerraformProviderRole() types.Role { @@ -716,6 +744,7 @@ func defaultAllowAccessRequestConditions(enterprise bool) map[string]*types.Acce SearchAsRoles: []string{ teleport.PresetAccessRoleName, teleport.PresetGroupAccessRoleName, + teleport.SystemIdentityCenterAccessRoleName, }, }, teleport.SystemOktaRequesterRoleName: { @@ -739,6 +768,7 @@ func defaultAllowAccessReviewConditions(enterprise bool) map[string]*types.Acces PreviewAsRoles: []string{ teleport.PresetAccessRoleName, teleport.PresetGroupAccessRoleName, + teleport.SystemIdentityCenterAccessRoleName, }, Roles: []string{ teleport.PresetAccessRoleName, @@ -754,7 +784,7 @@ func defaultAllowAccessReviewConditions(enterprise bool) map[string]*types.Acces func defaultAllowAccountAssignments(enterprise bool) map[string][]types.IdentityCenterAccountAssignment { if enterprise { return map[string][]types.IdentityCenterAccountAssignment{ - teleport.PresetAccessRoleName: { + teleport.SystemIdentityCenterAccessRoleName: { { Account: types.Wildcard, PermissionSet: types.Wildcard, @@ -763,7 +793,7 @@ func defaultAllowAccountAssignments(enterprise bool) map[string][]types.Identity } } - return nil + return map[string][]types.IdentityCenterAccountAssignment{} } // AddRoleDefaults adds default role attributes to a preset role. @@ -867,20 +897,12 @@ func AddRoleDefaults(role types.Role) (types.Role, error) { } } - if role.GetAccessRequestConditions(types.Allow).IsEmpty() { - arc := defaultAllowAccessRequestConditions(enterprise)[role.GetName()] - if arc != nil { - role.SetAccessRequestConditions(types.Allow, *arc) - changed = true - } + if roleUpdated := applyAccessRequestConditionDefaults(role, enterprise); roleUpdated { + changed = true } - if role.GetAccessReviewConditions(types.Allow).IsEmpty() { - arc := defaultAllowAccessReviewConditions(enterprise)[role.GetName()] - if arc != nil { - role.SetAccessReviewConditions(types.Allow, *arc) - changed = true - } + if roleUpdated := applyAccessReviewConditionDefaults(role, enterprise); roleUpdated { + changed = true } if len(role.GetIdentityCenterAccountAssignments(types.Allow)) == 0 { @@ -898,6 +920,61 @@ func AddRoleDefaults(role types.Role) (types.Role, error) { return role, nil } +func mergeStrings(dst, src []string, dirty *bool) []string { + items := utils.NewSet[string](dst...) + items.Add(src...) + if len(items) == len(dst) { + return dst + } + dst = items.Elements() + slices.Sort(dst) + (*dirty) = true + return dst +} + +func applyAccessRequestConditionDefaults(role types.Role, enterprise bool) bool { + defaults := defaultAllowAccessRequestConditions(enterprise)[role.GetName()] + if defaults == nil { + return false + } + + target := role.GetAccessRequestConditions(types.Allow) + changed := false + if target.IsEmpty() { + target = *defaults + changed = true + } else { + target.SearchAsRoles = mergeStrings(target.SearchAsRoles, defaults.SearchAsRoles, &changed) + } + + if changed { + role.SetAccessRequestConditions(types.Allow, target) + } + + return changed +} + +func applyAccessReviewConditionDefaults(role types.Role, enterprise bool) bool { + defaults := defaultAllowAccessReviewConditions(enterprise)[role.GetName()] + if defaults == nil { + return false + } + + target := role.GetAccessReviewConditions(types.Allow) + changed := false + if target.IsEmpty() { + target = *defaults + changed = true + } else { + target.PreviewAsRoles = mergeStrings(target.PreviewAsRoles, defaults.PreviewAsRoles, &changed) + } + + if changed { + role.SetAccessReviewConditions(types.Allow, target) + } + return changed +} + func labelMatchersUnset(role types.Role, kind string) (bool, error) { for _, cond := range []types.RoleConditionType{types.Allow, types.Deny} { labelMatchers, err := role.GetLabelMatchers(cond, kind) diff --git a/lib/services/presets_test.go b/lib/services/presets_test.go index 2f7dd8bcf0b15..3b25c1c5f5103 100644 --- a/lib/services/presets_test.go +++ b/lib/services/presets_test.go @@ -204,94 +204,6 @@ 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{ @@ -428,6 +340,25 @@ func TestAddRoleDefaults(t *testing.T) { { name: "reviewer (enterprise, existing review requests)", role: &types.RoleV6{ + Metadata: types.Metadata{ + Name: teleport.PresetReviewerRoleName, + Labels: map[string]string{ + types.TeleportInternalResourceType: types.PresetResource, + }, + }, + Spec: types.RoleSpecV6{ + Allow: types.RoleConditions{ + ReviewRequests: &types.AccessReviewConditions{ + Roles: []string{"some-role"}, + PreviewAsRoles: []string{"preview-role"}, + }, + }, + }, + }, + enterprise: true, + expectedErr: require.NoError, + reviewNotEmpty: true, + expected: &types.RoleV6{ Metadata: types.Metadata{ Name: teleport.PresetReviewerRoleName, Labels: map[string]string{ @@ -438,12 +369,16 @@ func TestAddRoleDefaults(t *testing.T) { Allow: types.RoleConditions{ ReviewRequests: &types.AccessReviewConditions{ Roles: []string{"some-role"}, + PreviewAsRoles: []string{ + teleport.PresetAccessRoleName, + teleport.SystemIdentityCenterAccessRoleName, + teleport.PresetGroupAccessRoleName, + "preview-role", + }, }, }, }, }, - enterprise: true, - expectedErr: noChange, }, { name: "requester (not enterprise)", @@ -499,6 +434,25 @@ func TestAddRoleDefaults(t *testing.T) { { name: "requester (enterprise, existing requests)", role: &types.RoleV6{ + Metadata: types.Metadata{ + Name: teleport.PresetRequesterRoleName, + Labels: map[string]string{ + types.TeleportInternalResourceType: types.PresetResource, + }, + }, + Spec: types.RoleSpecV6{ + Allow: types.RoleConditions{ + Request: &types.AccessRequestConditions{ + Roles: []string{"some-role"}, + SearchAsRoles: []string{"search-as-role"}, + }, + }, + }, + }, + enterprise: true, + expectedErr: require.NoError, + accessRequestsNotEmpty: true, + expected: &types.RoleV6{ Metadata: types.Metadata{ Name: teleport.PresetRequesterRoleName, Labels: map[string]string{ @@ -509,12 +463,16 @@ func TestAddRoleDefaults(t *testing.T) { Allow: types.RoleConditions{ Request: &types.AccessRequestConditions{ Roles: []string{"some-role"}, + SearchAsRoles: []string{ + teleport.PresetAccessRoleName, + teleport.SystemIdentityCenterAccessRoleName, + teleport.PresetGroupAccessRoleName, + "search-as-role", + }, }, }, }, }, - enterprise: true, - expectedErr: noChange, }, { name: "okta resources (not enterprise)", @@ -643,8 +601,28 @@ func TestAddRoleDefaults(t *testing.T) { }, }, }, - enterprise: true, - expectedErr: noChange, + enterprise: true, + expectedErr: require.NoError, + accessRequestsNotEmpty: true, + expected: &types.RoleV6{ + Metadata: types.Metadata{ + Name: teleport.SystemOktaRequesterRoleName, + Labels: map[string]string{ + types.TeleportInternalResourceType: types.SystemResource, + types.OriginLabel: types.OriginOkta, + }, + }, + Spec: types.RoleSpecV6{ + Allow: types.RoleConditions{ + Request: &types.AccessRequestConditions{ + Roles: []string{"some-role"}, + SearchAsRoles: []string{ + teleport.SystemOktaAccessRoleName, + }, + }, + }, + }, + }, }, } @@ -662,8 +640,15 @@ func TestAddRoleDefaults(t *testing.T) { require.Empty(t, cmp.Diff(role, test.expected)) if test.expected != nil { - require.Equal(t, test.reviewNotEmpty, !role.GetAccessReviewConditions(types.Allow).IsEmpty()) - require.Equal(t, test.accessRequestsNotEmpty, !role.GetAccessRequestConditions(types.Allow).IsEmpty()) + require.Equal(t, test.reviewNotEmpty, + !role.GetAccessReviewConditions(types.Allow).IsEmpty(), + "Expected populated Access Review Conditions (%t)", + test.reviewNotEmpty) + + require.Equal(t, test.accessRequestsNotEmpty, + !role.GetAccessRequestConditions(types.Allow).IsEmpty(), + "Expected populated Access Request Conditions (%t)", + test.accessRequestsNotEmpty) } }) } From 43470589988e184daeb7c617af92e92cb13383cb Mon Sep 17 00:00:00 2001 From: Trent Clarke Date: Fri, 13 Dec 2024 14:44:57 +1100 Subject: [PATCH 4/6] Add reviewer role --- lib/services/presets.go | 3 +++ lib/services/presets_test.go | 7 ++++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/lib/services/presets.go b/lib/services/presets.go index 1019dbd943274..f2557d1ea01e0 100644 --- a/lib/services/presets.go +++ b/lib/services/presets.go @@ -775,6 +775,7 @@ func defaultAllowAccessReviewConditions(enterprise bool) map[string]*types.Acces Roles: []string{ teleport.PresetAccessRoleName, teleport.PresetGroupAccessRoleName, + teleport.SystemIdentityCenterAccessRoleName, }, }, } @@ -946,6 +947,7 @@ func applyAccessRequestConditionDefaults(role types.Role, enterprise bool) bool target = *defaults changed = true } else { + target.Roles = mergeStrings(target.Roles, defaults.Roles, &changed) target.SearchAsRoles = mergeStrings(target.SearchAsRoles, defaults.SearchAsRoles, &changed) } @@ -968,6 +970,7 @@ func applyAccessReviewConditionDefaults(role types.Role, enterprise bool) bool { target = *defaults changed = true } else { + target.Roles = mergeStrings(target.Roles, defaults.Roles, &changed) target.PreviewAsRoles = mergeStrings(target.PreviewAsRoles, defaults.PreviewAsRoles, &changed) } diff --git a/lib/services/presets_test.go b/lib/services/presets_test.go index 3b25c1c5f5103..3e4f12c5d4084 100644 --- a/lib/services/presets_test.go +++ b/lib/services/presets_test.go @@ -368,7 +368,12 @@ func TestAddRoleDefaults(t *testing.T) { Spec: types.RoleSpecV6{ Allow: types.RoleConditions{ ReviewRequests: &types.AccessReviewConditions{ - Roles: []string{"some-role"}, + Roles: []string{ + teleport.PresetAccessRoleName, + teleport.SystemIdentityCenterAccessRoleName, + teleport.PresetGroupAccessRoleName, + "some-role", + }, PreviewAsRoles: []string{ teleport.PresetAccessRoleName, teleport.SystemIdentityCenterAccessRoleName, From c6b0a40ce6c6281db2ea51cabc2c9699a915bd66 Mon Sep 17 00:00:00 2001 From: Trent Clarke Date: Sat, 14 Dec 2024 00:33:52 +1100 Subject: [PATCH 5/6] Update lib/services/presets.go Co-authored-by: Roman Tkachenko --- lib/services/presets.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/services/presets.go b/lib/services/presets.go index f2557d1ea01e0..8744e89fd2b60 100644 --- a/lib/services/presets.go +++ b/lib/services/presets.go @@ -593,7 +593,7 @@ func NewSystemIdentityCenterAccessRole() types.Role { Metadata: types.Metadata{ Name: teleport.SystemIdentityCenterAccessRoleName, Namespace: apidefaults.Namespace, - Description: "Access AWS IdentityCenter resources", + Description: "Access AWS IAM Identity Center resources", Labels: map[string]string{ types.TeleportInternalResourceType: types.SystemResource, types.OriginLabel: common.OriginAWSIdentityCenter, From f99a58d35a1edfe00dc358b73fb8a21458f77c63 Mon Sep 17 00:00:00 2001 From: Trent Clarke Date: Sat, 14 Dec 2024 00:54:31 +1100 Subject: [PATCH 6/6] apply feedback --- lib/services/presets.go | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/lib/services/presets.go b/lib/services/presets.go index 8744e89fd2b60..ec3f8ad529c9d 100644 --- a/lib/services/presets.go +++ b/lib/services/presets.go @@ -923,16 +923,15 @@ func AddRoleDefaults(role types.Role) (types.Role, error) { return role, nil } -func mergeStrings(dst, src []string, dirty *bool) []string { +func mergeStrings(dst, src []string) (merged []string, changed bool) { items := utils.NewSet[string](dst...) items.Add(src...) if len(items) == len(dst) { - return dst + return dst, false } dst = items.Elements() slices.Sort(dst) - (*dirty) = true - return dst + return dst, true } func applyAccessRequestConditionDefaults(role types.Role, enterprise bool) bool { @@ -947,8 +946,13 @@ func applyAccessRequestConditionDefaults(role types.Role, enterprise bool) bool target = *defaults changed = true } else { - target.Roles = mergeStrings(target.Roles, defaults.Roles, &changed) - target.SearchAsRoles = mergeStrings(target.SearchAsRoles, defaults.SearchAsRoles, &changed) + var rolesUpdated bool + + target.Roles, rolesUpdated = mergeStrings(target.Roles, defaults.Roles) + changed = changed || rolesUpdated + + target.SearchAsRoles, rolesUpdated = mergeStrings(target.SearchAsRoles, defaults.SearchAsRoles) + changed = changed || rolesUpdated } if changed { @@ -970,8 +974,13 @@ func applyAccessReviewConditionDefaults(role types.Role, enterprise bool) bool { target = *defaults changed = true } else { - target.Roles = mergeStrings(target.Roles, defaults.Roles, &changed) - target.PreviewAsRoles = mergeStrings(target.PreviewAsRoles, defaults.PreviewAsRoles, &changed) + var rolesUpdated bool + + target.Roles, rolesUpdated = mergeStrings(target.Roles, defaults.Roles) + changed = changed || rolesUpdated + + target.PreviewAsRoles, rolesUpdated = mergeStrings(target.PreviewAsRoles, defaults.PreviewAsRoles) + changed = changed || rolesUpdated } if changed {