diff --git a/api/v1alpha1/azurevalidator_types.go b/api/v1alpha1/azurevalidator_types.go index 22ead9c1..7a36516b 100644 --- a/api/v1alpha1/azurevalidator_types.go +++ b/api/v1alpha1/azurevalidator_types.go @@ -17,44 +17,40 @@ limitations under the License. package v1alpha1 import ( + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // AzureValidatorSpec defines the desired state of AzureValidator type AzureValidatorSpec struct { - // Rules for validating that the correct role assignments have been created in Azure RBAC to - // provide needed permissions. - // +kubebuilder:validation:MaxItems=5 - // +kubebuilder:validation:XValidation:message="RBACRules must have unique names",rule="self.all(e, size(self.filter(x, x.name == e.name)) == 1)" - RBACRules []RBACRule `json:"rbacRules,omitempty" yaml:"rbacRules,omitempty"` // Rules for validating that images exist in an Azure Compute Gallery published as a community // gallery. + // +kubebuilder:validation:MaxItems=5 + // +kubebuilder:validation:XValidation:message="CommunityGalleryImageRules must have unique names",rule="self.all(e, size(self.filter(x, x.name == e.name)) == 1)" CommunityGalleryImageRules []CommunityGalleryImageRule `json:"communityGalleryImageRules,omitempty" yaml:"communityGalleryImageRules,omitempty"` - Auth AzureAuth `json:"auth" yaml:"auth"` + // RBACRoleRules validate that a security principal has permissions at a specified scope via + // role assignments and role definitions. + // +kubebuilder:validation:MaxItems=5 + // +kubebuilder:validation:XValidation:message="RBACRoleRules must have unique names",rule="self.all(e, size(self.filter(x, x.name == e.name)) == 1)" + RBACRoleRules []RBACRoleRule `json:"rbacRoleRules,omitempty" yaml:"rbacRoleRules,omitempty"` + Auth AzureAuth `json:"auth" yaml:"auth"` } // ResultCount returns the number of validation results expected for an AzureValidatorSpec. func (s AzureValidatorSpec) ResultCount() int { - return len(s.RBACRules) + len(s.CommunityGalleryImageRules) + return len(s.CommunityGalleryImageRules) + len(s.RBACRoleRules) } -// RBACRule verifies that a security principal has permissions via role assignments and that no deny -// assignments deny the permissions. -type RBACRule struct { - // Unique identifier for the rule in the validator. Used to ensure conditions do not overwrite - // each other. - Name string `json:"name" yaml:"name"` - // The permissions that the principal must have. If the principal has permissions less than - // this, validation will fail. If the principal has permissions equal to or more than this - // (e.g., inherited permissions from higher level scope, more roles than needed) validation - // will pass. - //+kubebuilder:validation:MinItems=1 - //+kubebuilder:validation:MaxItems=20 - //+kubebuilder:validation:XValidation:message="Each permission set must have Actions, DataActions, or both defined",rule="self.all(item, size(item.actions) > 0 || size(item.dataActions) > 0)" - Permissions []PermissionSet `json:"permissionSets" yaml:"permissionSets"` - // The principal being validated. This can be any type of principal - Device, ForeignGroup, - // Group, ServicePrincipal, or User. - PrincipalID string `json:"principalId" yaml:"principalId"` +// AzureAuth defines authentication configuration for an AzureValidator. +type AzureAuth struct { + // If true, the AzureValidator will use the Azure SDK's default credential chain to authenticate. + // Set to true if using WorkloadIdentityCredentials. + Implicit bool `json:"implicit" yaml:"implicit"` + // Name of a Secret in the same namespace as the AzureValidator that contains Azure credentials. + // The secret data's keys and values are expected to align with valid Azure environment variable credentials, + // per the options defined in https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity#readme-environment-variables. + SecretName string `json:"secretName,omitempty" yaml:"secretName,omitempty"` } // CommunityGalleryImageRule verifies that one or more images in a community gallery exist and are @@ -77,49 +73,81 @@ type CommunityGalleryImageRule struct { // CommunityGallery is a community gallery in a particular location. type CommunityGallery struct { // Location is the location of the community gallery (e.g. "westus"). - // +kubebuilder:validation:MaxLength=50 Location string `json:"location" yaml:"location"` // Name is the name of the community gallery. - // +kubebuilder:validation:MaxLength=200 Name string `json:"name" yaml:"name"` } -// AzureAuth defines authentication configuration for an AzureValidator. -type AzureAuth struct { - // If true, the AzureValidator will use the Azure SDK's default credential chain to authenticate. - // Set to true if using WorkloadIdentityCredentials. - Implicit bool `json:"implicit" yaml:"implicit"` - // Name of a Secret in the same namespace as the AzureValidator that contains Azure credentials. - // The secret data's keys and values are expected to align with valid Azure environment variable credentials, - // per the options defined in https://pkg.go.dev/github.com/Azure/azure-sdk-for-go/sdk/azidentity#readme-environment-variables. - SecretName string `json:"secretName,omitempty" yaml:"secretName,omitempty"` +// RBACRoleRule verifies that a role definition with a role type, role name, and set of permissions +// exists, and that it is assigned at a scope to a security principal. +type RBACRoleRule struct { + // Name is a unique identifier for the rule in the validator. Used to ensure conditions do not + // overwrite each other. + // +kubebuilder:validation:MaxLength=200 + Name string `json:"name" yaml:"name"` + // PrincipalID is the security principal being validated. This can be any type of principal - + // Device, ForeignGroup, Group, ServicePrincipal, or User. + PrincipalID string `json:"principalId" yaml:"principalId"` + // RoleAssignments are combinations of scope and role data. + // +kubebuilder:validation:MinItems=1 + RoleAssignments []RoleAssignment `json:"roleAssignments" yaml:"roleAssignments"` } -// ActionStr is a type used for Action strings and DataAction strings. Alias exists to enable -// kubebuilder max string length validation for arrays of these. -// +kubebuilder:validation:MaxLength=200 -type ActionStr string - -// PermissionSet is part of an RBAC rule and verifies that a security principal has the specified -// permissions (via role assignments) at the specified scope. Scope can be either subscription, -// resource group, or resource. -type PermissionSet struct { - // Actions is a list of actions that the role must be able to perform. Must not contain any - // wildcards. If not specified, the role is assumed to already be able to perform all required - // actions. - //+kubebuilder:validation:MaxItems=1000 - //+kubebuilder:validation:XValidation:message="Actions cannot have wildcards.",rule="self.all(item, !item.contains('*'))" - Actions []ActionStr `json:"actions,omitempty" yaml:"actions,omitempty"` - // DataActions is a list of data actions that the role must be able to perform. Must not - // contain any wildcards. If not provided, the role is assumed to already be able to perform - // all required data actions. - //+kubebuilder:validation:MaxItems=1000 - //+kubebuilder:validation:XValidation:message="DataActions cannot have wildcards.",rule="self.all(item, !item.contains('*'))" - DataActions []ActionStr `json:"dataActions,omitempty" yaml:"dataActions,omitempty"` - // Scope is the minimum scope of the role. Role assignments found at higher level scopes will - // satisfy this. For example, a role assignment found with subscription scope will satisfy a - // permission set where the role scope specified is a resource group within that subscription. +// RoleAssignment is a combination of scope and role data. +type RoleAssignment struct { + // Scope is the exact scope the role is assigned to the security principal at. Scope string `json:"scope" yaml:"scope"` + // Role is the role data. + Role Role `json:"role" yaml:"role"` +} + +// Role is role data in a role assignment. Is it a subset of a role definition. +type Role struct { + // Name is the role name property of the role definition. + Name string `json:"name" yaml:"name"` + // Type is the role type property of the role definition. Must be "BuiltInRole" or "Custom". + // Required to disambiguate built in roles and custom roles with the same name. + // +kubebuilder:validation:Enum=BuiltInRole;CustomRole + Type string `json:"type" yaml:"type"` + // Permission is the permissions data of the role definition. + Permission Permission `json:"permissions" yaml:"permissions"` +} + +// Permission is the permission data in a role definition. +type Permission struct { + // Actions is the "actions" of the role definition. + Actions []string `json:"actions,omitempty" yaml:"actions,omitempty"` + // DataActions is the "dataActions" of the role definition. + DataActions []string `json:"dataActions,omitempty" yaml:"dataActions,omitempty"` + // NotActions is the "notActions" of the role definition. + NotActions []string `json:"notActions,omitempty" yaml:"notActions,omitempty"` + // NotDataActions is the "notDataActions" of the role definition. + NotDataActions []string `json:"notDataActions,omitempty" yaml:"notDataActions,omitempty"` +} + +// Equal compares a Permission (from the spec) to an armauthorization.Permission (from the Azure API +// response). +func (p Permission) Equal(other armauthorization.Permission) bool { + compareSlices := func(a []string, b []*string) bool { + if len(a) != len(b) { + return false + } + for i, val := range a { + if b[i] == nil { + return false + } + bVal := *b[i] + if val != bVal { + return false + } + } + return true + } + + return compareSlices(p.Actions, other.Actions) && + compareSlices(p.DataActions, other.DataActions) && + compareSlices(p.NotActions, other.NotActions) && + compareSlices(p.NotDataActions, other.NotDataActions) } // AzureValidatorStatus defines the observed state of AzureValidator diff --git a/api/v1alpha1/azurevalidator_types_test.go b/api/v1alpha1/azurevalidator_types_test.go new file mode 100644 index 00000000..35baf427 --- /dev/null +++ b/api/v1alpha1/azurevalidator_types_test.go @@ -0,0 +1,140 @@ +/* +Copyright 2024. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package v1alpha1 + +import ( + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" + "k8s.io/utils/ptr" +) + +func TestPermission_Equal(t *testing.T) { + type fields struct { + Actions []string + DataActions []string + NotActions []string + NotDataActions []string + } + type args struct { + other armauthorization.Permission + } + tests := []struct { + name string + fields fields + args args + want bool + }{ + { + name: "Returns true when both empty.", + fields: fields{}, + args: args{ + other: armauthorization.Permission{}, + }, + want: true, + }, + { + name: "Returns true when equal.", + fields: fields{ + Actions: []string{"a", "b"}, + DataActions: []string{"c", "d"}, + NotActions: []string{"d", "e"}, + NotDataActions: []string{"f", "g"}, + }, + args: args{ + other: armauthorization.Permission{ + Actions: []*string{ptr.To("a"), ptr.To("b")}, + DataActions: []*string{ptr.To("c"), ptr.To("d")}, + NotActions: []*string{ptr.To("d"), ptr.To("e")}, + NotDataActions: []*string{ptr.To("f"), ptr.To("g")}, + }, + }, + want: true, + }, + { + name: "Returns true when equal (some slices omitted).", + fields: fields{ + Actions: []string{"a", "b"}, + }, + args: args{ + other: armauthorization.Permission{ + Actions: []*string{ptr.To("a"), ptr.To("b")}, + }, + }, + want: true, + }, + { + name: "Returns false when inequal (1).", + fields: fields{ + Actions: []string{"a", "b"}, + }, + args: args{ + other: armauthorization.Permission{ + Actions: []*string{ptr.To("c"), ptr.To("d")}, + }, + }, + want: false, + }, + { + name: "Returns false when inequal (2).", + fields: fields{ + Actions: []string{"a", "b"}, + }, + args: args{ + other: armauthorization.Permission{ + Actions: []*string{ptr.To("c")}, + }, + }, + want: false, + }, + { + name: "Returns false when inequal (3).", + fields: fields{ + Actions: []string{"a", "b"}, + }, + args: args{ + other: armauthorization.Permission{}, + }, + want: false, + }, + { + name: "Returns false when inequal (4).", + fields: fields{ + Actions: []string{"a", "b"}, + }, + args: args{ + other: armauthorization.Permission{ + Actions: []*string{nil, nil}, + }, + }, + want: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := Permission{ + Actions: tt.fields.Actions, + DataActions: tt.fields.DataActions, + NotActions: tt.fields.NotActions, + NotDataActions: tt.fields.NotDataActions, + } + if got := p.Equal(tt.args.other); got != tt.want { + t.Errorf("Permission.Equal() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 3711140d..09c5a44b 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -101,16 +101,16 @@ func (in *AzureValidatorList) DeepCopyObject() runtime.Object { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *AzureValidatorSpec) DeepCopyInto(out *AzureValidatorSpec) { *out = *in - if in.RBACRules != nil { - in, out := &in.RBACRules, &out.RBACRules - *out = make([]RBACRule, len(*in)) + if in.CommunityGalleryImageRules != nil { + in, out := &in.CommunityGalleryImageRules, &out.CommunityGalleryImageRules + *out = make([]CommunityGalleryImageRule, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } } - if in.CommunityGalleryImageRules != nil { - in, out := &in.CommunityGalleryImageRules, &out.CommunityGalleryImageRules - *out = make([]CommunityGalleryImageRule, len(*in)) + if in.RBACRoleRules != nil { + in, out := &in.RBACRoleRules, &out.RBACRoleRules + *out = make([]RBACRoleRule, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } @@ -180,48 +180,90 @@ func (in *CommunityGalleryImageRule) DeepCopy() *CommunityGalleryImageRule { } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *PermissionSet) DeepCopyInto(out *PermissionSet) { +func (in *Permission) DeepCopyInto(out *Permission) { *out = *in if in.Actions != nil { in, out := &in.Actions, &out.Actions - *out = make([]ActionStr, len(*in)) + *out = make([]string, len(*in)) copy(*out, *in) } if in.DataActions != nil { in, out := &in.DataActions, &out.DataActions - *out = make([]ActionStr, len(*in)) + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.NotActions != nil { + in, out := &in.NotActions, &out.NotActions + *out = make([]string, len(*in)) + copy(*out, *in) + } + if in.NotDataActions != nil { + in, out := &in.NotDataActions, &out.NotDataActions + *out = make([]string, len(*in)) copy(*out, *in) } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PermissionSet. -func (in *PermissionSet) DeepCopy() *PermissionSet { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Permission. +func (in *Permission) DeepCopy() *Permission { if in == nil { return nil } - out := new(PermissionSet) + out := new(Permission) in.DeepCopyInto(out) return out } // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *RBACRule) DeepCopyInto(out *RBACRule) { +func (in *RBACRoleRule) DeepCopyInto(out *RBACRoleRule) { *out = *in - if in.Permissions != nil { - in, out := &in.Permissions, &out.Permissions - *out = make([]PermissionSet, len(*in)) + if in.RoleAssignments != nil { + in, out := &in.RoleAssignments, &out.RoleAssignments + *out = make([]RoleAssignment, len(*in)) for i := range *in { (*in)[i].DeepCopyInto(&(*out)[i]) } } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RBACRule. -func (in *RBACRule) DeepCopy() *RBACRule { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RBACRoleRule. +func (in *RBACRoleRule) DeepCopy() *RBACRoleRule { + if in == nil { + return nil + } + out := new(RBACRoleRule) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Role) DeepCopyInto(out *Role) { + *out = *in + in.Permission.DeepCopyInto(&out.Permission) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Role. +func (in *Role) DeepCopy() *Role { + if in == nil { + return nil + } + out := new(Role) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RoleAssignment) DeepCopyInto(out *RoleAssignment) { + *out = *in + in.Role.DeepCopyInto(&out.Role) +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RoleAssignment. +func (in *RoleAssignment) DeepCopy() *RoleAssignment { if in == nil { return nil } - out := new(RBACRule) + out := new(RoleAssignment) in.DeepCopyInto(out) return out } diff --git a/build b/build index 683974c2..e14cf18d 160000 --- a/build +++ b/build @@ -1 +1 @@ -Subproject commit 683974c2e0e6c2bd18cd6867d4033f2e2cd99c42 +Subproject commit e14cf18d15269794a35476045774bda7a85b8d34 diff --git a/chart/validator-plugin-azure/crds/validation.spectrocloud.labs_azurevalidators.yaml b/chart/validator-plugin-azure/crds/validation.spectrocloud.labs_azurevalidators.yaml index dc334b00..d0579712 100644 --- a/chart/validator-plugin-azure/crds/validation.spectrocloud.labs_azurevalidators.yaml +++ b/chart/validator-plugin-azure/crds/validation.spectrocloud.labs_azurevalidators.yaml @@ -72,11 +72,9 @@ spec: location: description: Location is the location of the community gallery (e.g. "westus"). - maxLength: 50 type: string name: description: Name is the name of the community gallery. - maxLength: 200 type: string required: - location @@ -104,96 +102,106 @@ spec: - name - subscriptionID type: object + maxItems: 5 type: array - rbacRules: + x-kubernetes-validations: + - message: CommunityGalleryImageRules must have unique names + rule: self.all(e, size(self.filter(x, x.name == e.name)) == 1) + rbacRoleRules: description: |- - Rules for validating that the correct role assignments have been created in Azure RBAC to - provide needed permissions. + RBACRoleRules validate that a security principal has permissions at a specified scope via + role assignments and role definitions. items: description: |- - RBACRule verifies that a security principal has permissions via role assignments and that no deny - assignments deny the permissions. + RBACRoleRule verifies that a role definition with a role type, role name, and set of permissions + exists, and that it is assigned at a scope to a security principal. properties: name: description: |- - Unique identifier for the rule in the validator. Used to ensure conditions do not overwrite - each other. + Name is a unique identifier for the rule in the validator. Used to ensure conditions do not + overwrite each other. + maxLength: 200 type: string - permissionSets: + principalId: description: |- - The permissions that the principal must have. If the principal has permissions less than - this, validation will fail. If the principal has permissions equal to or more than this - (e.g., inherited permissions from higher level scope, more roles than needed) validation - will pass. + PrincipalID is the security principal being validated. This can be any type of principal - + Device, ForeignGroup, Group, ServicePrincipal, or User. + type: string + roleAssignments: + description: RoleAssignments are combinations of scope and role + data. items: - description: |- - PermissionSet is part of an RBAC rule and verifies that a security principal has the specified - permissions (via role assignments) at the specified scope. Scope can be either subscription, - resource group, or resource. + description: RoleAssignment is a combination of scope and + role data. properties: - actions: - description: |- - Actions is a list of actions that the role must be able to perform. Must not contain any - wildcards. If not specified, the role is assumed to already be able to perform all required - actions. - items: - description: |- - ActionStr is a type used for Action strings and DataAction strings. Alias exists to enable - kubebuilder max string length validation for arrays of these. - maxLength: 200 - type: string - maxItems: 1000 - type: array - x-kubernetes-validations: - - message: Actions cannot have wildcards. - rule: self.all(item, !item.contains('*')) - dataActions: - description: |- - DataActions is a list of data actions that the role must be able to perform. Must not - contain any wildcards. If not provided, the role is assumed to already be able to perform - all required data actions. - items: - description: |- - ActionStr is a type used for Action strings and DataAction strings. Alias exists to enable - kubebuilder max string length validation for arrays of these. - maxLength: 200 - type: string - maxItems: 1000 - type: array - x-kubernetes-validations: - - message: DataActions cannot have wildcards. - rule: self.all(item, !item.contains('*')) + role: + description: Role is the role data. + properties: + name: + description: Name is the role name property of the + role definition. + type: string + permissions: + description: Permission is the permissions data of + the role definition. + properties: + actions: + description: Actions is the "actions" of the role + definition. + items: + type: string + type: array + dataActions: + description: DataActions is the "dataActions" + of the role definition. + items: + type: string + type: array + notActions: + description: NotActions is the "notActions" of + the role definition. + items: + type: string + type: array + notDataActions: + description: NotDataActions is the "notDataActions" + of the role definition. + items: + type: string + type: array + type: object + type: + description: |- + Type is the role type property of the role definition. Must be "BuiltInRole" or "Custom". + Required to disambiguate built in roles and custom roles with the same name. + enum: + - BuiltInRole + - CustomRole + type: string + required: + - name + - permissions + - type + type: object scope: - description: |- - Scope is the minimum scope of the role. Role assignments found at higher level scopes will - satisfy this. For example, a role assignment found with subscription scope will satisfy a - permission set where the role scope specified is a resource group within that subscription. + description: Scope is the exact scope the role is assigned + to the security principal at. type: string required: + - role - scope type: object - maxItems: 20 minItems: 1 type: array - x-kubernetes-validations: - - message: Each permission set must have Actions, DataActions, - or both defined - rule: self.all(item, size(item.actions) > 0 || size(item.dataActions) - > 0) - principalId: - description: |- - The principal being validated. This can be any type of principal - Device, ForeignGroup, - Group, ServicePrincipal, or User. - type: string required: - name - - permissionSets - principalId + - roleAssignments type: object maxItems: 5 type: array x-kubernetes-validations: - - message: RBACRules must have unique names + - message: RBACRoleRules must have unique names rule: self.all(e, size(self.filter(x, x.name == e.name)) == 1) required: - auth diff --git a/config/crd/bases/validation.spectrocloud.labs_azurevalidators.yaml b/config/crd/bases/validation.spectrocloud.labs_azurevalidators.yaml index dc334b00..d0579712 100644 --- a/config/crd/bases/validation.spectrocloud.labs_azurevalidators.yaml +++ b/config/crd/bases/validation.spectrocloud.labs_azurevalidators.yaml @@ -72,11 +72,9 @@ spec: location: description: Location is the location of the community gallery (e.g. "westus"). - maxLength: 50 type: string name: description: Name is the name of the community gallery. - maxLength: 200 type: string required: - location @@ -104,96 +102,106 @@ spec: - name - subscriptionID type: object + maxItems: 5 type: array - rbacRules: + x-kubernetes-validations: + - message: CommunityGalleryImageRules must have unique names + rule: self.all(e, size(self.filter(x, x.name == e.name)) == 1) + rbacRoleRules: description: |- - Rules for validating that the correct role assignments have been created in Azure RBAC to - provide needed permissions. + RBACRoleRules validate that a security principal has permissions at a specified scope via + role assignments and role definitions. items: description: |- - RBACRule verifies that a security principal has permissions via role assignments and that no deny - assignments deny the permissions. + RBACRoleRule verifies that a role definition with a role type, role name, and set of permissions + exists, and that it is assigned at a scope to a security principal. properties: name: description: |- - Unique identifier for the rule in the validator. Used to ensure conditions do not overwrite - each other. + Name is a unique identifier for the rule in the validator. Used to ensure conditions do not + overwrite each other. + maxLength: 200 type: string - permissionSets: + principalId: description: |- - The permissions that the principal must have. If the principal has permissions less than - this, validation will fail. If the principal has permissions equal to or more than this - (e.g., inherited permissions from higher level scope, more roles than needed) validation - will pass. + PrincipalID is the security principal being validated. This can be any type of principal - + Device, ForeignGroup, Group, ServicePrincipal, or User. + type: string + roleAssignments: + description: RoleAssignments are combinations of scope and role + data. items: - description: |- - PermissionSet is part of an RBAC rule and verifies that a security principal has the specified - permissions (via role assignments) at the specified scope. Scope can be either subscription, - resource group, or resource. + description: RoleAssignment is a combination of scope and + role data. properties: - actions: - description: |- - Actions is a list of actions that the role must be able to perform. Must not contain any - wildcards. If not specified, the role is assumed to already be able to perform all required - actions. - items: - description: |- - ActionStr is a type used for Action strings and DataAction strings. Alias exists to enable - kubebuilder max string length validation for arrays of these. - maxLength: 200 - type: string - maxItems: 1000 - type: array - x-kubernetes-validations: - - message: Actions cannot have wildcards. - rule: self.all(item, !item.contains('*')) - dataActions: - description: |- - DataActions is a list of data actions that the role must be able to perform. Must not - contain any wildcards. If not provided, the role is assumed to already be able to perform - all required data actions. - items: - description: |- - ActionStr is a type used for Action strings and DataAction strings. Alias exists to enable - kubebuilder max string length validation for arrays of these. - maxLength: 200 - type: string - maxItems: 1000 - type: array - x-kubernetes-validations: - - message: DataActions cannot have wildcards. - rule: self.all(item, !item.contains('*')) + role: + description: Role is the role data. + properties: + name: + description: Name is the role name property of the + role definition. + type: string + permissions: + description: Permission is the permissions data of + the role definition. + properties: + actions: + description: Actions is the "actions" of the role + definition. + items: + type: string + type: array + dataActions: + description: DataActions is the "dataActions" + of the role definition. + items: + type: string + type: array + notActions: + description: NotActions is the "notActions" of + the role definition. + items: + type: string + type: array + notDataActions: + description: NotDataActions is the "notDataActions" + of the role definition. + items: + type: string + type: array + type: object + type: + description: |- + Type is the role type property of the role definition. Must be "BuiltInRole" or "Custom". + Required to disambiguate built in roles and custom roles with the same name. + enum: + - BuiltInRole + - CustomRole + type: string + required: + - name + - permissions + - type + type: object scope: - description: |- - Scope is the minimum scope of the role. Role assignments found at higher level scopes will - satisfy this. For example, a role assignment found with subscription scope will satisfy a - permission set where the role scope specified is a resource group within that subscription. + description: Scope is the exact scope the role is assigned + to the security principal at. type: string required: + - role - scope type: object - maxItems: 20 minItems: 1 type: array - x-kubernetes-validations: - - message: Each permission set must have Actions, DataActions, - or both defined - rule: self.all(item, size(item.actions) > 0 || size(item.dataActions) - > 0) - principalId: - description: |- - The principal being validated. This can be any type of principal - Device, ForeignGroup, - Group, ServicePrincipal, or User. - type: string required: - name - - permissionSets - principalId + - roleAssignments type: object maxItems: 5 type: array x-kubernetes-validations: - - message: RBACRules must have unique names + - message: RBACRoleRules must have unique names rule: self.all(e, size(self.filter(x, x.name == e.name)) == 1) required: - auth diff --git a/config/samples/azurevalidator-communitygalleryimages-one-image.yaml b/config/samples/azurevalidator-communitygalleryimages-one-image.yaml index 0768ca2b..188c7922 100644 --- a/config/samples/azurevalidator-communitygalleryimages-one-image.yaml +++ b/config/samples/azurevalidator-communitygalleryimages-one-image.yaml @@ -6,7 +6,6 @@ spec: auth: implicit: false secretName: azure-creds - rbacRules: [] communityGalleryImageRules: - name: rule-1 gallery: diff --git a/config/samples/azurevalidator-rbac-invalid1.yaml b/config/samples/azurevalidator-rbac-invalid1.yaml deleted file mode 100644 index 9cc8e7b0..00000000 --- a/config/samples/azurevalidator-rbac-invalid1.yaml +++ /dev/null @@ -1,16 +0,0 @@ -apiVersion: validation.spectrocloud.labs/v1alpha1 -kind: AzureValidator -metadata: - name: azurevalidator-rbac-invalid1 -spec: - auth: - implicit: false - secretName: azure-creds - rbacRules: - - name: rule-1 - principalId: "a83574a7-53ef-4b37-b85e-99f956f0985a" - permissionSets: - # Permission set is invalid because neither actions or dataActions are specified. - # CR will be created but will result in failed validation results that report this problem at - # runtime. User would then fix and re-apply CR. - - scope: "/subscriptions/9b16dd0b-1bea-4c9a-a291-65e6f44c4745" diff --git a/config/samples/azurevalidator-rbac-invalid2.yaml b/config/samples/azurevalidator-rbac-invalid2.yaml deleted file mode 100644 index 209f8c82..00000000 --- a/config/samples/azurevalidator-rbac-invalid2.yaml +++ /dev/null @@ -1,19 +0,0 @@ -apiVersion: validation.spectrocloud.labs/v1alpha1 -kind: AzureValidator -metadata: - name: azurevalidator-rbac-invalid2 -spec: - auth: - implicit: false - secretName: azure-creds - rbacRules: - - name: rule-1 - principalId: "a83574a7-53ef-4b37-b85e-99f956f0985a" - permissionSets: - # Permission set is invalid because there are no control Actions and no DataActions to be - # validated. - # CR will be created but will result in failed validation results that report this problem at - # runtime. User would then fix and re-apply CR. - - scope: "/subscriptions/9b16dd0b-1bea-4c9a-a291-65e6f44c4745" - actions: [] - dataActions: [] diff --git a/config/samples/azurevalidator-rbac-invalid3.yaml b/config/samples/azurevalidator-rbac-invalid3.yaml deleted file mode 100644 index 2361b312..00000000 --- a/config/samples/azurevalidator-rbac-invalid3.yaml +++ /dev/null @@ -1,22 +0,0 @@ -apiVersion: validation.spectrocloud.labs/v1alpha1 -kind: AzureValidator -metadata: - name: azurevalidator-rbac-invalid3 -spec: - auth: - implicit: false - secretName: azure-creds - # rbacRules is invalid because rule names are not unique. - rbacRules: - - name: rule-1 - principalId: "a83574a7-53ef-4b37-b85e-99f956f0985a" - permissionSets: - - scope: "/subscriptions/9b16dd0b-1bea-4c9a-a291-65e6f44c4745" - actions: - - "Microsoft.Compute/virtualMachines/read" - - name: rule-1 - principalId: "a83574a7-53ef-4b37-b85e-99f956f0985b" - permissionSets: - - scope: "/subscriptions/9b16dd0b-1bea-4c9a-a291-65e6f44c4746" - actions: - - "Microsoft.Compute/virtualMachines/write" diff --git a/config/samples/azurevalidator-rbac-one-permission-set-all-actions-permitted-by-one-role.yaml b/config/samples/azurevalidator-rbac-one-permission-set-all-actions-permitted-by-one-role.yaml deleted file mode 100644 index 212ddece..00000000 --- a/config/samples/azurevalidator-rbac-one-permission-set-all-actions-permitted-by-one-role.yaml +++ /dev/null @@ -1,17 +0,0 @@ -apiVersion: validation.spectrocloud.labs/v1alpha1 -kind: AzureValidator -metadata: - name: azurevalidator-rbac-one-permission-set-all-actions-permitted-by-one-role -spec: - auth: - implicit: false - secretName: azure-creds - rbacRules: - - name: rule-1 - principalId: "a83574a7-53ef-4b37-b85e-99f956f0985a" - permissionSets: - - scope: "/subscriptions/9b16dd0b-1bea-4c9a-a291-65e6f44c4745" - # Actions provided by built-in role Contributor (b24988ac-6180-42a0-ab88-20f7382dd24c) - actions: - - "Microsoft.Compute/virtualMachines/capture/action" # provided via role's wildcard "*" - - "Microsoft.Compute/virtualMachines/write" # provided via role's wildcard "*" diff --git a/config/samples/azurevalidator-rbac-one-permission-set-some-actions-permitted-by-one-role.yaml b/config/samples/azurevalidator-rbac-one-permission-set-some-actions-permitted-by-one-role.yaml deleted file mode 100644 index cd768400..00000000 --- a/config/samples/azurevalidator-rbac-one-permission-set-some-actions-permitted-by-one-role.yaml +++ /dev/null @@ -1,18 +0,0 @@ -apiVersion: validation.spectrocloud.labs/v1alpha1 -kind: AzureValidator -metadata: - name: azurevalidator-rbac-one-permission-set-some-actions-permitted-by-one-role -spec: - auth: - implicit: false - secretName: azure-creds - rbacRules: - # Rule is expected to result in Failed validation - - name: rule-1 - principalId: "9debc6b2-56be-4f03-bf0b-4762397fc327" - permissionSets: - - scope: "/subscriptions/9b16dd0b-1bea-4c9a-a291-65e6f44c4745" - # Some actions provided by custom role TestCustomRole (d2dd3116-04f5-4c40-944a-c28eeeed6a2e) - actions: - - "Microsoft.Compute/virtualMachines/read" # role has this action - - "Microsoft.Compute/virtualMachines/write" # but not this one diff --git a/config/samples/azurevalidator-rbacrole-one-role-assignment.yaml b/config/samples/azurevalidator-rbacrole-one-role-assignment.yaml new file mode 100644 index 00000000..3159f2b9 --- /dev/null +++ b/config/samples/azurevalidator-rbacrole-one-role-assignment.yaml @@ -0,0 +1,22 @@ +apiVersion: validation.spectrocloud.labs/v1alpha1 +kind: AzureValidator +metadata: + name: azurevalidator-rbacrole-one-role-assignment +spec: + auth: + implicit: false + secretName: azure-creds + rbacRoleRules: + - name: rule-1 + principalId: 1765687d-2f89-46d4-8d42-ad39b160d31c + roleAssignments: + - scope: /subscriptions/9b16dd0b-1bea-4c9a-a291-65e6f44c4745 + role: + name: MyCustomRole + type: CustomRole + permission: + actions: + - Microsoft.Compute/virtualMachines/write + dataActions: [] + notActions: [] + notDataActions: [] diff --git a/go.mod b/go.mod index c500acf1..c273dc30 100644 --- a/go.mod +++ b/go.mod @@ -11,10 +11,10 @@ require ( github.com/onsi/ginkgo/v2 v2.19.1 github.com/onsi/gomega v1.34.1 github.com/validator-labs/validator v0.0.51 - golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 k8s.io/api v0.30.3 k8s.io/apimachinery v0.30.3 k8s.io/client-go v0.30.3 + k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 sigs.k8s.io/cluster-api v1.7.4 sigs.k8s.io/controller-runtime v0.18.4 ) @@ -62,6 +62,7 @@ require ( go.uber.org/multierr v1.11.0 // indirect go.uber.org/zap v1.27.0 // indirect golang.org/x/crypto v0.25.0 // indirect + golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect golang.org/x/net v0.27.0 // indirect golang.org/x/oauth2 v0.21.0 // indirect golang.org/x/sys v0.22.0 // indirect @@ -77,7 +78,6 @@ require ( k8s.io/apiextensions-apiserver v0.30.1 // indirect k8s.io/klog/v2 v2.130.1 // indirect k8s.io/kube-openapi v0.0.0-20240228011516-70dd3763d340 // indirect - k8s.io/utils v0.0.0-20240502163921-fe8a2dddb1d0 // indirect sigs.k8s.io/json v0.0.0-20221116044647-bc3834ca7abd // indirect sigs.k8s.io/structured-merge-diff/v4 v4.4.1 // indirect sigs.k8s.io/yaml v1.4.0 // indirect diff --git a/internal/constants/constants.go b/internal/constants/constants.go index e515497a..c12f5471 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -5,8 +5,8 @@ const ( // PluginCode is the code for the plugin. PluginCode string = "Azure" - // ValidationTypeRBAC is the validation type for RBAC rules. - ValidationTypeRBAC string = "azure-rbac" + // ValidationTypeRBACRole is the validation type for RBAC role rules. + ValidationTypeRBACRole string = "azure-rbac-role" // ValidationTypeCommunityGalleryImages is the validation type for community gallery image rules. ValidationTypeCommunityGalleryImages string = "azure-community-gallery-image" diff --git a/internal/controller/azurevalidator_controller.go b/internal/controller/azurevalidator_controller.go index 9cca3916..1d216a21 100644 --- a/internal/controller/azurevalidator_controller.go +++ b/internal/controller/azurevalidator_controller.go @@ -125,21 +125,10 @@ func (r *AzureValidatorReconciler) Reconcile(ctx context.Context, req ctrl.Reque defer cancel() } - daClient := azure_utils.NewDenyAssignmentsClient(azureCtx, azureAPI.DenyAssignmentsClient) raClient := azure_utils.NewRoleAssignmentsClient(azureCtx, azureAPI.RoleAssignmentsClient) rdClient := azure_utils.NewRoleDefinitionsClient(azureCtx, azureAPI.RoleDefinitionsClient) cgiClient := azure_utils.NewCommunityGalleryImagesClient(azureCtx, azureAPI.CommunityGalleryImagesClientProducer) - // RBAC rules - rbacSvc := validators.NewRBACRuleService(daClient, raClient, rdClient) - for _, rule := range validator.Spec.RBACRules { - vrr, err := rbacSvc.ReconcileRBACRule(rule) - if err != nil { - l.Error(err, "failed to reconcile RBAC rule") - } - resp.AddResult(vrr, err) - } - // Community gallery image rules cgiSvc := validators.NewCommunityGalleryImageRuleService(cgiClient, r.Log) for _, rule := range validator.Spec.CommunityGalleryImageRules { @@ -149,6 +138,16 @@ func (r *AzureValidatorReconciler) Reconcile(ctx context.Context, req ctrl.Reque } resp.AddResult(vrr, err) } + + // RBAC role rules + roleSvc := validators.NewRBACRoleRuleService(raClient, rdClient) + for _, rule := range validator.Spec.RBACRoleRules { + vrr, err := roleSvc.ReconcileRBACRoleRule(rule) + if err != nil { + l.Error(err, "failed to reconcile RBAC role rule") + } + resp.AddResult(vrr, err) + } } // Patch the ValidationResult with the latest ValidationRuleResults diff --git a/internal/controller/azurevalidator_controller_test.go b/internal/controller/azurevalidator_controller_test.go index 73239a6e..9bc4869d 100644 --- a/internal/controller/azurevalidator_controller_test.go +++ b/internal/controller/azurevalidator_controller_test.go @@ -25,8 +25,8 @@ var _ = Describe("AzureValidator controller", Ordered, func() { } }) - It("Should not create a ValidationResult when neither Actions nor DataActions are defined in any permission set", func() { - By("Attempting to create a new AzureValidator with one invalid permission set") + It("Should not create a ValidationResult when an invalid RBACRoleRule is applied (missing principal ID)", func() { + By("Attempting to create a new AzureValidator") ctx := context.Background() @@ -36,33 +36,29 @@ var _ = Describe("AzureValidator controller", Ordered, func() { Namespace: validatorNamespace, }, Spec: v1alpha1.AzureValidatorSpec{ - RBACRules: []v1alpha1.RBACRule{ + RBACRoleRules: []v1alpha1.RBACRoleRule{ { - Permissions: []v1alpha1.PermissionSet{ + Name: "rule-1", + RoleAssignments: []v1alpha1.RoleAssignment{ { - Scope: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/Example-Storage-rg", - Actions: []v1alpha1.ActionStr{"action_1"}, + Scope: "test-scope", + Role: v1alpha1.Role{ + Name: "test-role-name", + Type: "test-role-type", + }, }, }, - PrincipalID: "p_id", - }, - { - Permissions: []v1alpha1.PermissionSet{ - { - Scope: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/Example-Storage-rg2", - }, - }, - PrincipalID: "p_id", }, }, }, } - Expect(k8sClient.Create(ctx, val)).Should(MatchError(ContainSubstring("Each permission set must have Actions, DataActions, or both defined"))) + Expect(k8sClient.Create(ctx, val)).Should(MatchError(ContainSubstring( + "spec.rbacRoleRules[0].principalId: Required value"))) }) - It("Should not create a ValidationResult when any Action has a wildcard", func() { - By("Attempting to create a new AzureValidator with one invalid permission set") + It("Should not create a ValidationResult when an invalid RBACRoleRule is applied (missing role assignments)", func() { + By("Attempting to create a new AzureValidator") ctx := context.Background() @@ -72,24 +68,20 @@ var _ = Describe("AzureValidator controller", Ordered, func() { Namespace: validatorNamespace, }, Spec: v1alpha1.AzureValidatorSpec{ - RBACRules: []v1alpha1.RBACRule{ + RBACRoleRules: []v1alpha1.RBACRoleRule{ { - Permissions: []v1alpha1.PermissionSet{ - { - Scope: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/Example-Storage-rg", - Actions: []v1alpha1.ActionStr{"things/*"}, - }, - }, - PrincipalID: "p_id", + Name: "rule-1", + PrincipalID: "test-principal-id", }, }, }, } - Expect(k8sClient.Create(ctx, val)).Should(MatchError(ContainSubstring("Actions cannot have wildcards"))) + Expect(k8sClient.Create(ctx, val)).Should(MatchError(ContainSubstring( + "spec.rbacRoleRules[0].roleAssignments: Required value"))) }) - It("Should not create a ValidationResult when any DataAction has a wildcard", func() { + It("Should not create a ValidationResult when an invalid RBACRoleRule is applied (empty role assignments)", func() { By("Attempting to create a new AzureValidator with one invalid permission set") ctx := context.Background() @@ -100,21 +92,18 @@ var _ = Describe("AzureValidator controller", Ordered, func() { Namespace: validatorNamespace, }, Spec: v1alpha1.AzureValidatorSpec{ - RBACRules: []v1alpha1.RBACRule{ + RBACRoleRules: []v1alpha1.RBACRoleRule{ { - Permissions: []v1alpha1.PermissionSet{ - { - Scope: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/Example-Storage-rg", - DataActions: []v1alpha1.ActionStr{"things/*"}, - }, - }, - PrincipalID: "p_id", + Name: "rule-1", + PrincipalID: "test-principal-id", + RoleAssignments: []v1alpha1.RoleAssignment{}, }, }, }, } - Expect(k8sClient.Create(ctx, val)).Should(MatchError(ContainSubstring("DataActions cannot have wildcards"))) + Expect(k8sClient.Create(ctx, val)).Should(MatchError(ContainSubstring( + "spec.rbacRoleRules[0].roleAssignments: Invalid value: 0: spec.rbacRoleRules[0].roleAssignments in body should have at least 1 items"))) }) It("Should create a ValidationResult and update its Status with a failed condition", func() { @@ -144,15 +133,19 @@ var _ = Describe("AzureValidator controller", Ordered, func() { Implicit: false, SecretName: "azure-creds", }, - RBACRules: []v1alpha1.RBACRule{ + RBACRoleRules: []v1alpha1.RBACRoleRule{ { - Permissions: []v1alpha1.PermissionSet{ + Name: "rule-1", + PrincipalID: "test-principal-id", + RoleAssignments: []v1alpha1.RoleAssignment{ { - Scope: "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/Example-Storage-rg", - Actions: []v1alpha1.ActionStr{"action_1"}, + Scope: "test-scope", + Role: v1alpha1.Role{ + Name: "test-role-name", + Type: "test-role-type", + }, }, }, - PrincipalID: "p_id", }, }, }, diff --git a/internal/utils/azure/azure.go b/internal/utils/azure/azure.go index da1c0026..38c83b1b 100644 --- a/internal/utils/azure/azure.go +++ b/internal/utils/azure/azure.go @@ -6,8 +6,8 @@ import ( "context" "fmt" "net/http" + "net/url" "os" - "strings" "time" armpolicy "github.com/Azure/azure-sdk-for-go/sdk/azcore/arm/policy" @@ -207,14 +207,6 @@ func (c *RoleDefinitionsClient) GetByID(roleID string) (*armauthorization.RoleDe return &roleDefinitionResp.RoleDefinition, nil } -// RoleNameFromRoleDefinitionID extracts the name of a role (aka the non-fully-qualified ID of the -// role) from an Azure role definition ID (aka the fully-qualified ID of the role definition). -func RoleNameFromRoleDefinitionID(roleDefinitionID string) string { - split := strings.Split(roleDefinitionID, "/") - roleName := split[len(split)-1] - return roleName -} - // CommunityGalleryImagesClient is a facade over the Azure community gallery images client. // Exists to make our code easier to test (it handles paging). type CommunityGalleryImagesClient struct { @@ -263,3 +255,11 @@ func (c *CommunityGalleryImagesClient) GetImagesForGallery(location, name, subsc return images, fmt.Errorf("context cancelled") } } + +// RoleAssignmentRequestFilter returns a filter string for Azure role assignments API calls. +func RoleAssignmentRequestFilter(principalID string) string { + // Azure's Go SDK for their API has a bug where it doesn't escape the filter string for the role + // assignments call we do here, so we manually escape it ourselves. + // https://github.com/Azure/azure-sdk-for-go/issues/20847 + return url.QueryEscape(fmt.Sprintf("principalId eq '%s'", principalID)) +} diff --git a/internal/utils/azure/azure_test.go b/internal/utils/azure/azure_test.go deleted file mode 100644 index 466c1999..00000000 --- a/internal/utils/azure/azure_test.go +++ /dev/null @@ -1,31 +0,0 @@ -package azure - -import ( - "testing" -) - -func Test_RoleNameFromRoleDefinitionID(t *testing.T) { - type args struct { - roleDefinitionID string - } - tests := []struct { - name string - args args - want string - }{ - { - name: "Extracts a role ID successfully from a role definition ID.", - args: args{ - roleDefinitionID: "/subscriptions/45e41ba5-078e-4f83-893c-f7fd7f5aed19/providers/Microsoft.Authorization/roleDefinitions/a8fd7d79-1dee-4829-8ef8-8b7b97711fe9", - }, - want: "a8fd7d79-1dee-4829-8ef8-8b7b97711fe9", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := RoleNameFromRoleDefinitionID(tt.args.roleDefinitionID); got != tt.want { - t.Errorf("RoleNameFromRoleDefinitionID() = %v, want %v", got, tt.want) - } - }) - } -} diff --git a/internal/validators/rbac.go b/internal/validators/rbac.go deleted file mode 100644 index 73b5dec6..00000000 --- a/internal/validators/rbac.go +++ /dev/null @@ -1,154 +0,0 @@ -package validators - -import ( - "fmt" - "net/url" - - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" - corev1 "k8s.io/api/core/v1" - - "github.com/validator-labs/validator-plugin-azure/api/v1alpha1" - "github.com/validator-labs/validator-plugin-azure/internal/constants" - azerr "github.com/validator-labs/validator-plugin-azure/internal/utils/azureerrors" - vapi "github.com/validator-labs/validator/api/v1alpha1" - vapiconstants "github.com/validator-labs/validator/pkg/constants" - vapitypes "github.com/validator-labs/validator/pkg/types" - "github.com/validator-labs/validator/pkg/util" -) - -// denyAssignmentAPI contains methods that allow getting all deny assignments for a scope and -// optional filter. -type denyAssignmentAPI interface { - GetDenyAssignmentsForScope(scope string, filter *string) ([]*armauthorization.DenyAssignment, error) -} - -// roleAssignmentAPI contains methods that allow getting all role assignments for a scope and -// optional filter. -type roleAssignmentAPI interface { - GetRoleAssignmentsForScope(scope string, filter *string) ([]*armauthorization.RoleAssignment, error) -} - -// roleDefinitionAPI contains methods that allow getting all the information we need for an existing -// role definition. -type roleDefinitionAPI interface { - GetByID(roleID string) (*armauthorization.RoleDefinition, error) -} - -// RBACRuleService reconciles RBAC rules. -type RBACRuleService struct { - daAPI denyAssignmentAPI - raAPI roleAssignmentAPI - rdAPI roleDefinitionAPI -} - -// NewRBACRuleService creates a new RBACRuleService. Requires Azure client facades that support -// getting deny assignments, role assignments, and role definitions. -func NewRBACRuleService(daAPI denyAssignmentAPI, raAPI roleAssignmentAPI, rdAPI roleDefinitionAPI) *RBACRuleService { - return &RBACRuleService{ - daAPI: daAPI, - raAPI: raAPI, - rdAPI: rdAPI, - } -} - -// ReconcileRBACRule reconciles an RBAC rule. -func (s *RBACRuleService) ReconcileRBACRule(rule v1alpha1.RBACRule) (*vapitypes.ValidationRuleResult, error) { - - // Build the default ValidationResult for this role assignment rule. - state := vapi.ValidationSucceeded - latestCondition := vapi.DefaultValidationCondition() - latestCondition.Failures = []string{} - latestCondition.Message = "Principal has all required permissions." - latestCondition.ValidationRule = fmt.Sprintf("%s-%s", vapiconstants.ValidationRulePrefix, rule.Name) - latestCondition.ValidationType = constants.ValidationTypeRBAC - validationResult := &vapitypes.ValidationRuleResult{Condition: &latestCondition, State: &state} - - for _, set := range rule.Permissions { - if err := s.processPermissionSet(set, rule.PrincipalID, &latestCondition.Failures); err != nil { - // Code this is returning to will take care of changing the validation result to a - // failed validation, using the error returned. - return validationResult, err - } - } - - if len(latestCondition.Failures) > 0 { - state = vapi.ValidationFailed - latestCondition.Message = "Principal lacks required permissions. See failures for details." - latestCondition.Status = corev1.ConditionFalse - } - - return validationResult, nil -} - -// processPermissionSet processes a permission set from the rule. -func (s *RBACRuleService) processPermissionSet(set v1alpha1.PermissionSet, principalID string, failures *[]string) error { - - // Get all deny assignments and role assignments for specified scope and principal. - // Note that in this filter, Azure checks "principalId" to make sure it's a UUID, so we don't - // need to escape the principal ID user input from the spec. - daFilter := util.Ptr(fmt.Sprintf("principalId eq '%s'", principalID)) - denyAssignments, err := s.daAPI.GetDenyAssignmentsForScope(set.Scope, daFilter) - if err != nil { - return fmt.Errorf("failed to get deny assignments: %w", azerr.AsAugmented(err)) - } - // Note that Azure's Go SDK for their API has a bug where it doesn't escape the filter string - // for the role assignments call we do here, so we manually escape it ourselves. - // https://github.com/Azure/azure-sdk-for-go/issues/20847 - raFilter := util.Ptr(url.QueryEscape(fmt.Sprintf("principalId eq '%s'", principalID))) - roleAssignments, err := s.raAPI.GetRoleAssignmentsForScope(set.Scope, raFilter) - if err != nil { - return fmt.Errorf("failed to get role assignments: %w", azerr.AsAugmented(err)) - } - - // For each role assignment found, get its role definition, because that's what we actually need - // to do validation. We need to know which Actions and DataActions the role permits. - roleDefinitions := []*armauthorization.RoleDefinition{} - for _, ra := range roleAssignments { - if ra.Properties == nil { - return fmt.Errorf("role assignment properties nil") - } - if ra.Properties.RoleDefinitionID == nil { - return fmt.Errorf("role assignment properties role definition ID nil") - } - rdID := *ra.Properties.RoleDefinitionID - // Note that, in Azure, in the role assignments API, the value is called "role definition - // ID", but in the role definitions API, it is called "role ID". - roleDefinition, err := s.rdAPI.GetByID(rdID) - if err != nil { - return fmt.Errorf("failed to get role definition using role definition ID of role assignment: %w", azerr.AsAugmented(err)) - } - roleDefinitions = append(roleDefinitions, roleDefinition) - } - - // Convert from ActionStr to string. - setActions := []string{} - for _, a := range set.Actions { - setActions = append(setActions, string(a)) - } - setDataActions := []string{} - for _, da := range set.DataActions { - setDataActions = append(setDataActions, string(da)) - } - - // Get the results and append failure messages if needed. - result, err := processAllCandidateActions(setActions, setDataActions, denyAssignments, roleDefinitions) - if err != nil { - return fmt.Errorf("failed to determine which candidate Actions and DataActions were denied and/or unpermitted: %w", err) - } - for denied, by := range result.actions.denied { - *failures = append(*failures, fmt.Sprintf("Action %s denied by deny assignment %s.", denied, by)) - } - for _, unpermitted := range result.actions.unpermitted { - *failures = append(*failures, fmt.Sprintf("Action %s unpermitted because no role assignment permits it.", unpermitted)) - } - for denied, by := range result.dataActions.denied { - *failures = append(*failures, fmt.Sprintf("DataAction %s denied by deny assignment %s.", denied, by)) - } - for _, unpermitted := range result.dataActions.unpermitted { - *failures = append(*failures, fmt.Sprintf("DataAction %s unpermitted because no role assignment permits it.", unpermitted)) - } - - // The `failures` slice will have been changed appropriately by here. Calling code will handle - // this appropriately. - return nil -} diff --git a/internal/validators/rbac_permissions.go b/internal/validators/rbac_permissions.go deleted file mode 100644 index b86cc258..00000000 --- a/internal/validators/rbac_permissions.go +++ /dev/null @@ -1,398 +0,0 @@ -package validators - -import ( - "fmt" - "strings" - - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" - "golang.org/x/exp/maps" - - map_utils "github.com/validator-labs/validator-plugin-azure/internal/utils/maps" -) - -const ( - wildcard = "*" -) - -// deniedAndUnpermitted is data about which candidate actions were denied because a deny assignment -// denied them and which were unpermitted because no role assignment permitted them. -type deniedAndUnpermitted struct { - // denied are the candidate Actions that were denied by a deny assignment and the name of the - // deny assignment that denied them. - denied map[string]string - // unpermitted are the candidate Actions that weren't permitted because no role assignment - // permitted them. - unpermitted []string -} - -// result is the data about which Actions and DataActions were denied and unpermitted. -type result struct { - actions deniedAndUnpermitted - dataActions deniedAndUnpermitted -} - -// denyAssignmentInfo is a container for permission data from the Azure API response that we've -// validated as non-nil. Used for both control Actions and DataActions. Important to use instead of -// slices of strings because it lets us tie NotActions to the Actions they subtract from on a per -// role basis. Unlike roleInfo below, this lets us retain deny assignment ID (the fully-qualified ID -// where it's a path with the scope on the left and the name of the deny assignment on the right) -// too so that we can report the names of deny assignments that deny candidate Actions or -// DataActions to the user. -type denyAssignmentInfo struct { - actions []string - notActions []string - id string -} - -// roleInfo is a container for permission data from the Azure API response that we've validated as -// non-nil. Used for both control Actions and DataActions. Important to use instead of slices of -// strings because it lets us tie NotActions to the Actions they subtract from on a per role basis. -type roleInfo struct { - actions []string - notActions []string -} - -// processAllCandidateActions determines, based on a set of deny assignments and roles (associated -// with role assignments), which required actions and data actions are denied by presence of deny -// assignment and/or unpermitted by lack of role assignment. -// -// It is assumed that all required actions and data actions have no wildcards because of CRD -// validation. -// nolint:gocyclo -func processAllCandidateActions(candidateActions, candidateDataActions []string, denyAssignments []*armauthorization.DenyAssignment, roles []*armauthorization.RoleDefinition) (result, error) { - errNil := func(subject string) error { - return fmt.Errorf("%s nil", subject) - } - appendStr := func(vals *[]string, val string) { - *vals = append(*vals, val) - } - appendDenyInfo := func(vals *[]denyAssignmentInfo, val denyAssignmentInfo) { - *vals = append(*vals, val) - } - appendRoleInfo := func(vals *[]roleInfo, val roleInfo) { - *vals = append(*vals, val) - } - - // Dereference all the data from Azure, while validating it for our algorithm's constraints. - // Deny assignments and role assignments that exist in the user's Azure account must have at - // most one wildcard each. - denyAssignmentInfoControl := []denyAssignmentInfo{} - denyAssignmentInfoData := []denyAssignmentInfo{} - roleInfoControl := []roleInfo{} - roleInfoData := []roleInfo{} - for _, denyAssignment := range denyAssignments { - if denyAssignment == nil { - return result{}, errNil("deny assignment") - } - if denyAssignment.ID == nil { - return result{}, errNil("deny assignment ID") - } - denyAssignmentID := *denyAssignment.ID - if denyAssignment.Properties == nil { - return result{}, errNil("deny assignment properties") - } - if denyAssignment.Properties.Permissions == nil { - return result{}, errNil("deny assignment properties permissions") - } - permissions := denyAssignment.Properties.Permissions - // We can expect there to only be one permissions item in the list of permissions. That's - // just how Azure works. - if len(permissions) != 1 { - return result{}, errNil("deny assignment permissions length not equal to 1") - } - permission := permissions[0] - if permission.Actions == nil { - return result{}, errNil("deny assignment Actions") - } - actions := []string{} - for _, ptr := range permission.Actions { - if ptr == nil { - return result{}, errNil("deny assignment Action") - } - action := *ptr - if numWildcards(action) > 1 { - return result{}, fmt.Errorf("deny assignment Action %s has multiple wildcards", action) - } - appendStr(&actions, action) - } - if permission.NotActions == nil { - return result{}, errNil("deny assignment NotActions") - } - notActions := []string{} - for _, ptr := range permission.NotActions { - if ptr == nil { - return result{}, errNil("deny assignment NotAction") - } - notAction := *ptr - if numWildcards(notAction) > 1 { - return result{}, fmt.Errorf("deny assignment NotAction %s has multiple wildcards", notAction) - } - appendStr(¬Actions, notAction) - } - appendDenyInfo(&denyAssignmentInfoControl, denyAssignmentInfo{ - actions: actions, - notActions: notActions, - id: denyAssignmentID, - }) - if permission.DataActions == nil { - return result{}, errNil("deny assignment DataActions") - } - dataActions := []string{} - for _, ptr := range permission.DataActions { - if ptr == nil { - return result{}, errNil("deny assignment DataAction") - } - dataAction := *ptr - if numWildcards(dataAction) > 1 { - return result{}, fmt.Errorf("deny assignment DataAction %s has multiple wildcards", dataAction) - } - appendStr(&dataActions, dataAction) - } - if permission.NotDataActions == nil { - return result{}, errNil("deny assignment NotDataActions") - } - notDataActions := []string{} - for _, ptr := range permission.NotDataActions { - if ptr == nil { - return result{}, errNil("deny assignment NotDataAction") - } - notDataAction := *ptr - if numWildcards(notDataAction) > 1 { - return result{}, fmt.Errorf("deny assignment NotDataAction %s has multiple wildcards", notDataAction) - } - appendStr(¬DataActions, notDataAction) - } - appendDenyInfo(&denyAssignmentInfoData, denyAssignmentInfo{ - actions: dataActions, - notActions: notDataActions, - id: denyAssignmentID, - }) - } - for _, role := range roles { - if role == nil { - return result{}, errNil("role") - } - if role.Properties == nil { - return result{}, errNil("role properties") - } - if role.Properties.Permissions == nil { - return result{}, errNil("role properties permissions") - } - permissions := role.Properties.Permissions - // We can expect there to only be one permissions item in the list of permissions. That's - // just how Azure works. - if len(permissions) != 1 { - return result{}, errNil("role permissions length not equal to 1") - } - permission := permissions[0] - if permission.Actions == nil { - return result{}, errNil("role Actions") - } - actions := []string{} - for _, ptr := range permission.Actions { - if ptr == nil { - return result{}, errNil("role Action") - } - action := *ptr - if numWildcards(action) > 1 { - return result{}, fmt.Errorf("role Action %s has multiple wildcards", action) - } - appendStr(&actions, action) - } - if permission.NotActions == nil { - return result{}, errNil("role NotActions") - } - notActions := []string{} - for _, ptr := range permission.NotActions { - if ptr == nil { - return result{}, errNil("role NotAction") - } - notAction := *ptr - if numWildcards(notAction) > 1 { - return result{}, fmt.Errorf("role NotAction %s has multiple wildcards", notAction) - } - appendStr(¬Actions, notAction) - } - appendRoleInfo(&roleInfoControl, roleInfo{ - actions: actions, - notActions: notActions, - }) - if permission.DataActions == nil { - return result{}, errNil("role DataActions") - } - dataActions := []string{} - for _, ptr := range permission.DataActions { - if ptr == nil { - return result{}, errNil("role DataAction") - } - dataAction := *ptr - if numWildcards(dataAction) > 1 { - return result{}, fmt.Errorf("role DataAction %s has multiple wildcards", dataAction) - } - appendStr(&dataActions, dataAction) - } - if permission.NotDataActions == nil { - return result{}, errNil("role NotDataActions") - } - notDataActions := []string{} - for _, ptr := range permission.NotDataActions { - if ptr == nil { - return result{}, errNil("role NotDataAction") - } - notDataAction := *ptr - if numWildcards(notDataAction) > 1 { - return result{}, fmt.Errorf("role NotDataAction %s has multiple wildcards", notDataAction) - } - appendStr(¬DataActions, notDataAction) - } - appendRoleInfo(&roleInfoData, roleInfo{ - actions: dataActions, - notActions: notDataActions, - }) - } - - // Use dereferenced data to find denied and unpermitted for control Actions, and then for - // DataActions. - return result{ - actions: findDeniedAndUnpermitted(candidateActions, denyAssignmentInfoControl, roleInfoControl), - dataActions: findDeniedAndUnpermitted(candidateDataActions, denyAssignmentInfoData, roleInfoData), - }, nil -} - -// findDeniedAndUnpermitted determines, based on a set of NotActions and Actions, from deny -// assignments and from role assignments, which candidate actions should be denied due to the deny -// assignments and which should be unpermitted due to the role assignments. -// -// This logic can be used for both control Actions and DataActions. They just need to come from the -// right source (deny assignment vs. role). -func findDeniedAndUnpermitted(candidateActions []string, denyAssignments []denyAssignmentInfo, roles []roleInfo) deniedAndUnpermitted { - // Begin with all candidate Actions marked as "unpermitted". It's better to start with all - // candidates considered unpermitted and marking them as permitted later instead of starting - // with an empty list of unpermitted actions and adding candidates to it later because of how - // the algorithm below works. - unpermitted := map_utils.FromKeys(candidateActions, true) - // Begin with no candidate Actions marked as denied. - // keys = candidate Actions - // values = names of denying deny assignments - denied := make(map[string]string, 0) - -candidateActions: - for _, candidateAction := range candidateActions { - for _, denyAssignment := range denyAssignments { - // Does any NotAction in the deny assignment match the candidate Action? - if matches, _ := candidateActionMatches(candidateAction, denyAssignment.notActions); matches { - // Move on to next deny assignment because this NotAction matching means the deny - // assignment does not deny the candidate Action. - continue - } - // Does any Action in the deny assignment match the candidate Action? - if matches, _ := candidateActionMatches(candidateAction, denyAssignment.actions); matches { - // Mark candidate action as "denied by deny assignment {denyAssignmentId}". - denied[candidateAction] = denyAssignment.id - } - } - for _, role := range roles { - // Does any NotAction in the role match the candidate Action? - if matches, _ := candidateActionMatches(candidateAction, role.notActions); matches { - // Move on to next role because this NotAction matching means the role does not - // permit the candidate Action. - continue - } - // Does any Action in the role match the candidate Action? - if matches, _ := candidateActionMatches(candidateAction, role.actions); matches { - // Mark candidate action as permitted. - delete(unpermitted, candidateAction) - // Move on to next candidate Action because this Action matching means the role - // permits the candidate Action. - continue candidateActions - } - } - } - - // We've now considered all deny assignments and roles for all candidate actions. We've - // determined which candidate Actions were denied and which were simply not permitted in the - // first place. It's possible for a candidate Action to be both denied and not permitted. We - // report two failures for such candidate Actions so that the user gets as much info as possible - // each time they observe the validation result. - return deniedAndUnpermitted{ - denied: denied, - unpermitted: maps.Keys(unpermitted), - } -} - -// candidateActionMatches determines whether a candidate Action matches any compared Actions, where -// the compared Actions are Actions or NotActions, from roles or deny assignments. Returns the -// matching compared Action when a match is found. -// -// The candidate Action must have no wildcards. The compared Actions must have no more than one -// wildcard each. -func candidateActionMatches(candidateAction string, comparedActions []string) (bool, string) { - for _, comparedAction := range comparedActions { - if !hasWildcard(comparedAction) { - // If allowed action has no wildcard, candidate action must be equal to it exactly in - // order for the candidate action to be permitted. - if candidateAction == comparedAction { - return true, comparedAction - } - // Whether the action permitted the candidate action because it was equal to it or it - // didn't, we can move on to the next action, because if it has no wildcard, it is - // impossible for it to permit the candidate action via wildcard. - continue - } - - // Special case for when string is just a single char - the wildcard. - if comparedAction == wildcard { - return true, comparedAction - } - - // If allowed action string has a wildcard, candidate action must match when we take the - // wildcard into account. - - if comparedAction[0:1] == wildcard { - // Wildcard is at beginning of allowed action. No prefix in action string to take into - // account. - // Find the suffix of the action string. If that suffix is also a suffix of the - // candidate action, permit the candidate action. - actionSuffix := strings.TrimPrefix(comparedAction, wildcard) - if strings.HasSuffix(candidateAction, actionSuffix) { - return true, comparedAction - } - } - - if comparedAction[len(comparedAction)-1:] == wildcard { - // Wildcard is at end of allowed action. No suffix in action string to take into - // account. - // Find the prefix of the action string. If that prefix is also a prefix of the - // candidate action, permit the candidate action. - actionPrefix := strings.TrimSuffix(comparedAction, wildcard) - if strings.HasPrefix(candidateAction, actionPrefix) { - return true, comparedAction - } - } - - // Wildcard is somewhere in the middle. Must take into account prefix and suffix. - // Split the action string by the wildcard. The first segment is a prefix. The second is a - // suffix. If the candidate action has this prefix as a prefix and has this suffix as a - // suffix, permit the candidate action. - splitAction := strings.Split(comparedAction, wildcard) - actionPrefix := splitAction[0] - actionSuffix := splitAction[1] - if strings.HasPrefix(candidateAction, actionPrefix) && strings.HasSuffix(candidateAction, actionSuffix) { - return true, comparedAction - } - } - - // Compared action not relevant when there was no match. - return false, "" -} - -// numWildcards returns how many wildcards an action string has. -func numWildcards(action string) int { - return strings.Count(action, wildcard) -} - -// hasWildcard returns true IFF an action has one wildcard. Helper is useful for clarity in -// algorithm. -func hasWildcard(action string) bool { - return numWildcards(action) == 1 -} diff --git a/internal/validators/rbac_permissions_test.go b/internal/validators/rbac_permissions_test.go deleted file mode 100644 index 12e6160c..00000000 --- a/internal/validators/rbac_permissions_test.go +++ /dev/null @@ -1,2045 +0,0 @@ -package validators - -import ( - "reflect" - "testing" - - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" - - "github.com/validator-labs/validator/pkg/util" -) - -func Test_processAllCandidateActions(t *testing.T) { - type args struct { - candidateActions []string - candidateDataActions []string - denyAssignments []*armauthorization.DenyAssignment - roles []*armauthorization.RoleDefinition - } - tests := []struct { - name string - args args - want result - wantErr bool - }{ - // Note that these tests test code that calls code that is already covered by tests below. - // Therefore, we don't need to test some functionality (see below). Here, we test how input - // checked for invalid values (nils and wildcard uses we're not allowing), whether control - // Actions are handled separately from DataActions, for both deny assignments and role - // assignments, and whether parsing the raw Azure data (with pointers) into the data for the - // helper functions works correctly. - - // Cases that test invalid input. - { - name: "nil deny assignment", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{nil}, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil deny assignment ID", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - ID: nil, - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil deny assignment properties", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: nil, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil deny assignment properties permissions", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: nil, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "deny assignment properties permissions length 0", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{}, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "deny assignment properties permissions length greater than 1", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - {}, - {}, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil deny assignment Actions", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: nil, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil deny assignment Action", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{nil}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "deny assignment Action with multiple wildcards", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*/*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil deny assignment NotActions", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: nil, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil deny assignment NotAction", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{nil}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "deny assignment NotAction with multiple wildcards", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*/*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil deny assignment DataActions", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: nil, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil deny assignment DataAction", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{nil}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "deny assignment DataAction with multiple wildcards", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*/*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil deny assignment NotDataActions", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: nil, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil deny assignment NotDataAction", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{nil}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "deny assignment NotDataAction with multiple wildcards", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*/*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil role", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{nil}, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil role properties", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: nil, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil role properties permissions", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: nil, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "role properties permissions length 0", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{}, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "role properties permissions length greater than 1", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - {}, - {}, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil role Actions", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: nil, - }, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil role Action", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{nil}, - }, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "role NotAction with multiple wildcards", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("*/*")}, - }, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil role NotActions", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: nil, - }, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil role NotAction", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{nil}, - }, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "role NotAction with multiple wildcards", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*/*")}, - }, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil role DataActions", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: nil, - }, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil role DataAction", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{nil}, - }, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "role DataAction with multiple wildcards", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*/*")}, - }, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil role NotDataActions", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: nil, - }, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "nil role NotDataAction", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{nil}, - }, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - { - name: "role NotDataAction with multiple wildcards", - args: args{ - candidateActions: []string{}, - candidateDataActions: []string{}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*")}, - }, - }, - }, - ID: util.Ptr("da"), - }, - }, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("*")}, - NotActions: []*string{util.Ptr("*")}, - DataActions: []*string{util.Ptr("*")}, - NotDataActions: []*string{util.Ptr("*/*")}, - }, - }, - }, - }, - }, - }, - want: result{}, - wantErr: true, - }, - - // Cases that test handling Actions and DataActions separately with them fed into the - // algorithm helpers correctly. - { - name: "1 candidate Action that should be permitted, 1 candidate DataAction that should be permitted", - args: args{ - candidateActions: []string{"a"}, - candidateDataActions: []string{"b"}, - denyAssignments: []*armauthorization.DenyAssignment{}, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("a")}, - NotActions: []*string{}, - DataActions: []*string{util.Ptr("b")}, - NotDataActions: []*string{}, - }, - }, - }, - }, - }, - }, - want: result{ - actions: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{}, - }, - dataActions: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{}, - }, - }, - wantErr: false, - }, - { - name: "1 candidate Action that should not be permitted, 1 candidate DataAction that should not be permitted", - args: args{ - candidateActions: []string{"a"}, - candidateDataActions: []string{"b"}, - denyAssignments: []*armauthorization.DenyAssignment{}, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{}, - NotActions: []*string{}, - DataActions: []*string{}, - NotDataActions: []*string{}, - }, - }, - }, - }, - }, - }, - want: result{ - actions: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - dataActions: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"b"}, - }, - }, - wantErr: false, - }, - { - name: "1 candidate Action that should not be permitted, 1 candidate DataAction that should be permitted", - args: args{ - candidateActions: []string{"a"}, - candidateDataActions: []string{"b"}, - denyAssignments: []*armauthorization.DenyAssignment{}, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("a")}, - NotActions: []*string{util.Ptr("a")}, - DataActions: []*string{util.Ptr("b")}, - NotDataActions: []*string{}, - }, - }, - }, - }, - }, - }, - want: result{ - actions: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - dataActions: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{}, - }, - }, - wantErr: false, - }, - { - name: "1 candidate Action that should be permitted, 1 candidate DataAction that should not permitted", - args: args{ - candidateActions: []string{"a"}, - candidateDataActions: []string{"b"}, - denyAssignments: []*armauthorization.DenyAssignment{}, - roles: []*armauthorization.RoleDefinition{ - { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("a")}, - NotActions: []*string{}, - DataActions: []*string{util.Ptr("b")}, - NotDataActions: []*string{util.Ptr("b")}, - }, - }, - }, - }, - }, - }, - want: result{ - actions: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{}, - }, - dataActions: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"b"}, - }, - }, - wantErr: false, - }, - - // Cases that test parsing into deny assignments (because cases above happen to already - // test parsing into roles) with them fed into the algorithm helpers correctly. - { - name: "Parsing a deny assignment.", - args: args{ - candidateActions: []string{"a"}, - candidateDataActions: []string{"b"}, - denyAssignments: []*armauthorization.DenyAssignment{ - { - ID: util.Ptr("da1"), - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("a")}, - NotActions: []*string{util.Ptr("a")}, - DataActions: []*string{util.Ptr("b")}, - NotDataActions: []*string{util.Ptr("b")}, - }, - }, - }, - }, - }, - roles: []*armauthorization.RoleDefinition{}, - }, - want: result{ - actions: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - dataActions: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"b"}, - }, - }, - wantErr: false, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, err := processAllCandidateActions(tt.args.candidateActions, tt.args.candidateDataActions, tt.args.denyAssignments, tt.args.roles) - if (err != nil) != tt.wantErr { - t.Errorf("processAllCandidateActions() error = %v, wantErr %v", err, tt.wantErr) - return - } - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("processAllCandidateActions() = %v, want %v", got, tt.want) - } - }) - } -} - -func Test_findDeniedAndUnpermitted(t *testing.T) { - type args struct { - candidateActions []string - denyAssignments []denyAssignmentInfo - roles []roleInfo - } - tests := []struct { - name string - args args - want deniedAndUnpermitted - }{ - // Note that in these tests, "candidate action" means either a candidate Action or a candidate DataAction. This - // is the part of the algorithm generic with respect to type of Action. - - // Also note that these tests test code that calls code that is already covered by tests below. Therefore, we - // don't need to test some functionality (see below). Here, we test how different combinations of candidate - // Actions, deny assignments, and roles result in denied and/or unpermitted candidate Actions. - - // Cases that test, for 1 candidate Action, every combination of deny assignment and role, where there is 0 or 1 - // matching Actions or NotActions. - { - name: "0 deny assign, 0 role", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{}, - roles: []roleInfo{}, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (0 matching A, 0 matching NA), 0 role", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{}, - notActions: []string{}, - id: "da1", - }, - }, - roles: []roleInfo{}, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (1 matching A, 0 matching NA), 0 role", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{"a"}, - notActions: []string{}, - id: "da1", - }, - }, - roles: []roleInfo{}, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{ - "a": "da1", - }, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (0 matching A, 1 matching NA), 0 role", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{}, - notActions: []string{"a"}, - id: "da1", - }, - }, - roles: []roleInfo{}, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (1 matching A, 1 matching NA), 0 role", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{"a"}, - notActions: []string{"a"}, - id: "da1", - }, - }, - roles: []roleInfo{}, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "0 deny assign, 1 role (0 matching A, 0 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{}, - roles: []roleInfo{ - { - actions: []string{}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "0 deny assign, 1 role (1 matching A, 0 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{}, - roles: []roleInfo{ - { - actions: []string{"a"}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{}, - }, - }, - { - name: "0 deny assign, 1 role (0 matching A, 1 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{}, - roles: []roleInfo{ - { - actions: []string{}, - notActions: []string{"a"}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "0 deny assign, 1 role (1 matching A, 1 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{}, - roles: []roleInfo{ - { - actions: []string{"a"}, - notActions: []string{"a"}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (0 matching A, 0 matching NA), 1 role (0 matching A, 0 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{}, - notActions: []string{}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (0 matching A, 0 matching NA), 1 role (0 matching A, 1 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{}, - notActions: []string{}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{}, - notActions: []string{"a"}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (0 matching A, 0 matching NA), 1 role (1 matching A, 0 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{}, - notActions: []string{}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{"a"}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{}, - }, - }, - { - name: "1 deny assign (0 matching A, 0 matching NA), 1 role (1 matching A, 1 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{}, - notActions: []string{}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{"a"}, - notActions: []string{"a"}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (1 matching A, 0 matching NA), 1 role (0 matching A, 0 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{"a"}, - notActions: []string{}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{ - "a": "da1", - }, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (1 matching A, 0 matching NA), 1 role (0 matching A, 1 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{"a"}, - notActions: []string{}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{}, - notActions: []string{"a"}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{ - "a": "da1", - }, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (1 matching A, 0 matching NA), 1 role (1 matching A, 0 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{"a"}, - notActions: []string{}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{"a"}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{ - "a": "da1", - }, - unpermitted: []string{}, - }, - }, - { - name: "1 deny assign (1 matching A, 0 matching NA), 1 role (1 matching A, 1 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{"a"}, - notActions: []string{}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{"a"}, - notActions: []string{"a"}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{ - "a": "da1", - }, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (0 matching A, 1 matching NA), 1 role (0 matching A, 0 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{}, - notActions: []string{"a"}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (0 matching A, 1 matching NA), 1 role (0 matching A, 1 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{}, - notActions: []string{"a"}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{}, - notActions: []string{"a"}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (0 matching A, 1 matching NA), 1 role (1 matching A, 0 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{}, - notActions: []string{"a"}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{"a"}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{}, - }, - }, - { - name: "1 deny assign (0 matching A, 1 matching NA), 1 role (1 matching A, 1 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{}, - notActions: []string{"a"}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{"a"}, - notActions: []string{"a"}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (1 matching A, 1 matching NA), 1 role (0 matching A, 0 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{"a"}, - notActions: []string{"a"}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (1 matching A, 1 matching NA), 1 role (0 matching A, 1 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{"a"}, - notActions: []string{"a"}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{}, - notActions: []string{"a"}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 deny assign (1 matching A, 1 matching NA), 1 role (1 matching A, 0 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{"a"}, - notActions: []string{"a"}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{"a"}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{}, - }, - }, - { - name: "1 deny assign (1 matching A, 1 matching NA), 1 role (1 matching A, 1 matching NA)", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{"a"}, - notActions: []string{"a"}, - id: "da1", - }, - }, - roles: []roleInfo{ - { - actions: []string{"a"}, - notActions: []string{"a"}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"a"}, - }, - }, - - // Cases for other situations. Not exhaustive like above. Best effort. - { - name: "1 deny assignment that denies but is not the first deny assignment", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{ - { - actions: []string{"b"}, - notActions: []string{}, - id: "da1", - }, - { - actions: []string{"a"}, - notActions: []string{}, - id: "da2", - }, - }, - roles: []roleInfo{}, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{ - "a": "da2", - }, - unpermitted: []string{"a"}, - }, - }, - { - name: "1 role that permits but is not the first role", - args: args{ - candidateActions: []string{"a"}, - denyAssignments: []denyAssignmentInfo{}, - roles: []roleInfo{ - { - actions: []string{"b"}, - notActions: []string{}, - }, - { - actions: []string{"a"}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{}, - }, - }, - { - name: "2 candidate actions, both permitted", - args: args{ - candidateActions: []string{"a", "b"}, - denyAssignments: []denyAssignmentInfo{}, - roles: []roleInfo{ - { - actions: []string{"a"}, - notActions: []string{}, - }, - { - actions: []string{"b"}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{}, - }, - }, - { - name: "2 candidate actions, 1 unpermitted and 1 permitted", - args: args{ - candidateActions: []string{"a", "b"}, - denyAssignments: []denyAssignmentInfo{}, - roles: []roleInfo{ - { - actions: []string{"a"}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{"b"}, - }, - }, - { - name: "2 candidate actions, both permitted by the same action (wildcard only)", - args: args{ - candidateActions: []string{"a", "b"}, - denyAssignments: []denyAssignmentInfo{}, - roles: []roleInfo{ - { - actions: []string{"*"}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{}, - }, - }, - { - name: "2 candidate actions, both permitted by the same action (not wildcard only)", - args: args{ - candidateActions: []string{"a/b", "a/c"}, - denyAssignments: []denyAssignmentInfo{}, - roles: []roleInfo{ - { - actions: []string{"a/*"}, - notActions: []string{}, - }, - }, - }, - want: deniedAndUnpermitted{ - denied: map[string]string{}, - unpermitted: []string{}, - }, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - if got := findDeniedAndUnpermitted(tt.args.candidateActions, tt.args.denyAssignments, tt.args.roles); !reflect.DeepEqual(got, tt.want) { - t.Errorf("findDeniedAndUnpermitted() = %v, want %v", got, tt.want) - } - }) - } -} - -func Test_candidateActionMatches(t *testing.T) { - type args struct { - candidateAction string - comparedActions []string - } - tests := []struct { - name string - args args - want bool - want1 string - }{ - // Note that in these tests, "candidate Action" means either a candidate Action or a candidate DataAction. This - // part of the algorithm is generic with respect to type of Action. - - // Cases that test when a candidate Action matches because the compared Action is the wildcard only or the - // candidate Action exactly, regardless of compared Action order. - { - name: "No compared Actions", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{}, - }, - want: false, - want1: "", - }, - { - name: "1 compared Action, which is candidate Action exactly", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"a/b/c/d"}, - }, - want: true, - want1: "a/b/c/d", - }, - { - name: "1 compared Action, which is wildcard only", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"*"}, - }, - want: true, - want1: "*", - }, - { - name: "2 compared Actions, first is candidate Action exactly, second is some other Action", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"a/b/c/d", "e/f/g/h"}, - }, - want: true, - want1: "a/b/c/d", - }, - { - name: "2 compared Actions, first is some other candidate Action, second is candidate Action exactly", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"e/f/g/h", "a/b/c/d"}, - }, - want: true, - want1: "a/b/c/d", - }, - { - name: "2 compared Actions, first is wildcard only, second is candidate Action exactly", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"*", "a/b/c/d"}, - }, - want: true, - want1: "*", - }, - { - name: "2 compared Actions, first is candidate Action exactly, second is wildcard only", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"a/b/c/d", "*"}, - }, - want: true, - want1: "a/b/c/d", - }, - - // Cases that test, when a compared action isn't the wildcard only, how a wildcard in a compared Action is able - // to result in a match. We must confirm that wildcards are able to to used in Actions with up to 4 components, - // in any place among the components, and taking the place of up to 3 components. - { - name: "Wildcard is like */b/c/d", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"*/b/c/d"}, - }, - want: true, - want1: "*/b/c/d", - }, - { - name: "Wildcard is like a/*/c/d", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"a/*/c/d"}, - }, - want: true, - want1: "a/*/c/d", - }, - { - name: "Wildcard is like a/b/*/d", - args: args{ - candidateAction: "a/b/*/d", - comparedActions: []string{"a/b/*/d"}, - }, - want: true, - want1: "a/b/*/d", - }, - { - name: "Wildcard is like a/b/c/*", - args: args{ - candidateAction: "a/b/c/*", - comparedActions: []string{"a/b/c/*"}, - }, - want: true, - want1: "a/b/c/*", - }, - { - name: "Wildcard is like */c/d", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"*/c/d"}, - }, - want: true, - want1: "*/c/d", - }, - { - name: "Wildcard is like a/*/d", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"a/*/d"}, - }, - want: true, - want1: "a/*/d", - }, - { - name: "Wildcard is like a/b/*", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"a/b/*"}, - }, - want: true, - want1: "a/b/*", - }, - { - name: "Wildcard is like */d", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"*/d"}, - }, - want: true, - want1: "*/d", - }, - { - name: "Wildcard is like a/*", - args: args{ - candidateAction: "a/b/c/d", - comparedActions: []string{"a/*"}, - }, - want: true, - want1: "a/*", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - got, got1 := candidateActionMatches(tt.args.candidateAction, tt.args.comparedActions) - if got != tt.want { - t.Errorf("candidateActionMatches() got = %v, want %v", got, tt.want) - } - if got1 != tt.want1 { - t.Errorf("candidateActionMatches() got1 = %v, want %v", got1, tt.want1) - } - }) - } -} diff --git a/internal/validators/rbac_role_rule.go b/internal/validators/rbac_role_rule.go new file mode 100644 index 00000000..d2d18a34 --- /dev/null +++ b/internal/validators/rbac_role_rule.go @@ -0,0 +1,147 @@ +package validators + +import ( + "fmt" + + corev1 "k8s.io/api/core/v1" + + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" + "github.com/validator-labs/validator-plugin-azure/api/v1alpha1" + "github.com/validator-labs/validator-plugin-azure/internal/constants" + "github.com/validator-labs/validator-plugin-azure/internal/utils/azure" + vapi "github.com/validator-labs/validator/api/v1alpha1" + vapiconstants "github.com/validator-labs/validator/pkg/constants" + vapitypes "github.com/validator-labs/validator/pkg/types" +) + +// roleAssignmentAPI contains methods that allow getting all role assignments for a scope and +// optional filter. +type roleAssignmentAPI interface { + GetRoleAssignmentsForScope(scope string, filter *string) ([]*armauthorization.RoleAssignment, error) +} + +// roleDefinitionAPI contains methods that allow getting all the information we need for an existing +// role definition. +type roleDefinitionAPI interface { + GetByID(roleID string) (*armauthorization.RoleDefinition, error) +} + +// RBACRoleRuleService reconciles RBAC role rules. +type RBACRoleRuleService struct { + raAPI roleAssignmentAPI + rdAPI roleDefinitionAPI +} + +// NewRBACRoleRuleService creates a new RBACRoleRuleService. Requires Azure client facades that +// support, role assignments, and role definitions. +func NewRBACRoleRuleService(raAPI roleAssignmentAPI, rdAPI roleDefinitionAPI) *RBACRoleRuleService { + return &RBACRoleRuleService{ + raAPI: raAPI, + rdAPI: rdAPI, + } +} + +// ReconcileRBACRoleRule reconciles an RBAC role rule. +func (s *RBACRoleRuleService) ReconcileRBACRoleRule(rule v1alpha1.RBACRoleRule) (*vapitypes.ValidationRuleResult, error) { + + // Build the default ValidationResult for this role assignment rule. + state := vapi.ValidationSucceeded + latestCondition := vapi.DefaultValidationCondition() + latestCondition.Failures = []string{} + latestCondition.Message = "All role assignments correct. Roles contain permissions and assigned to principal at scopes." + latestCondition.ValidationRule = fmt.Sprintf("%s-%s", vapiconstants.ValidationRulePrefix, rule.Name) + latestCondition.ValidationType = constants.ValidationTypeRBACRole + validationResult := &vapitypes.ValidationRuleResult{Condition: &latestCondition, State: &state} + + for _, raSpec := range rule.RoleAssignments { + newFailure, err := s.processRoleAssignment(raSpec, rule.PrincipalID) + if err != nil { + // Code this is returning to will take care of changing the validation result to a + // failed validation, using the error returned. + return validationResult, err + } + if newFailure != "" { + latestCondition.Failures = append(latestCondition.Failures, newFailure) + } + } + + if len(latestCondition.Failures) > 0 { + state = vapi.ValidationFailed + latestCondition.Message = "Principal lacks required permissions. See failures for details." + latestCondition.Status = corev1.ConditionFalse + } + + return validationResult, nil +} + +// processRoleAssignment processes one role assignment in the rule. Returns a string for a new +// failure to append to the rule's failures or an error is something stopped it from validating the +// role assignment. +func (s *RBACRoleRuleService) processRoleAssignment(raSpec v1alpha1.RoleAssignment, principalID string) (string, error) { + // Get all role assignments for principal at scope. + raFilter := azure.RoleAssignmentRequestFilter(principalID) + roleAssignments, err := s.raAPI.GetRoleAssignmentsForScope(raSpec.Scope, &raFilter) + if err != nil { + return "", fmt.Errorf("failed to get role assignments for principal '%s' at scope '%s': %w", principalID, principalID, err) + } + + // Optimization for better failure message. If there are no role assignments, the principal + // definitely isn't assigned an appropriate role. + if len(roleAssignments) == 0 { + return fmt.Sprintf("No role assignments found for principal '%s' at scope '%s'.", principalID, raSpec.Scope), nil + } + + // For each role assignment, use its role definition ID to fetch the corresponding role + // definition. If the role definition's role type, role name, and permissions match, and the + // role assignment's scope matches the scope defined in the rule (meaning the role isn't + // assigned at a scope greater than required), validation passes. + for _, ra := range roleAssignments { + // Check role assignment API response for nils. Not expected, but theoretically possible. + if ra == nil || ra.ID == nil || ra.Properties == nil || ra.Properties.RoleDefinitionID == nil || ra.Properties.Scope == nil { + return "", fmt.Errorf("role assignment from API response missing expected properties") + } + + // Get role definition for role assignment. + // + // This happens within the loop, so it results in sequential API calls (until a suitable + // role is found), and it would be nice to be able to get all role definitions we need at + // once so that it finishes making API calls sooner. But, that won't work for our use case. + // To list role definitions, we need to know their scope (which means whether they're at the + // subscription or tenant level, and which subscriptions or tenants those are). It's + // impossible to know all of the roles that could be assigned to the principal, but it's + // possible to start from the role assignments and use the role definition IDs in them to + // see which roles are assigned. So, we do it that way instead. In the real world, Azure + // users typically assign fewer, larger roles instead of many, smaller roles, so the issue + // of time to complete sequential API calls should be negligible. + roleDef, err := s.rdAPI.GetByID(*ra.Properties.RoleDefinitionID) + if err != nil { + return "", fmt.Errorf("failed to get role definition '%s' (for role assignment '%s'): %w", + *ra.Properties.RoleDefinitionID, *ra.ID, err) + } + + // Check role def API response for nils or permissions with lenth not equal to 1. Not + // expected, but theoretically possible. + if roleDef.Properties == nil || roleDef.Properties.Permissions == nil || roleDef.Properties.RoleType == nil || roleDef.Properties.RoleName == nil { + return "", fmt.Errorf("role definition '%s' from API response missing expected properties", *ra.Properties.RoleDefinitionID) + } + if len(roleDef.Properties.Permissions) != 1 { + return "", fmt.Errorf("role definition '%s' from API response has unexpected number of permissions", *ra.Properties.RoleDefinitionID) + } + + if *roleDef.Properties.RoleType == raSpec.Role.Type && + *roleDef.Properties.RoleName == raSpec.Role.Name && + *ra.Properties.Scope == raSpec.Scope && + raSpec.Role.Permission.Equal(*roleDef.Properties.Permissions[0]) { + // Validation passes, no need to continue checking other role assignments. + return "", nil + } + } + + // If no role assignments that match were found, validation fails. + // + // We cannot return unique failure messages for each issue that could occur because we need to + // check all role assignments. The failure means there is no suitable role assignment, not that + // a particular role assignment is wrong. + return fmt.Sprintf("Principal '%s' does not have role with type '%s' and role name '%s' assigned at scope '%s' with required permissions.", + principalID, raSpec.Role.Type, raSpec.Role.Name, raSpec.Scope), nil +} diff --git a/internal/validators/rbac_role_rule_test.go b/internal/validators/rbac_role_rule_test.go new file mode 100644 index 00000000..073bbf62 --- /dev/null +++ b/internal/validators/rbac_role_rule_test.go @@ -0,0 +1,681 @@ +package validators + +import ( + "errors" + "fmt" + "testing" + + "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" + + corev1 "k8s.io/api/core/v1" + "k8s.io/utils/ptr" + + "github.com/validator-labs/validator-plugin-azure/api/v1alpha1" + vapi "github.com/validator-labs/validator/api/v1alpha1" + vapitypes "github.com/validator-labs/validator/pkg/types" + "github.com/validator-labs/validator/pkg/util" +) + +const ( + testScope = "/subscriptions/00000000-0000-0000-0000-000000000000" + testPrincipalID = "00000000-0000-0000-0000-000000000000" + testRoleAssignmentID = "test-role-assignment-id" + testRoleDefinitionID = "test-role-definition-id" + testRoleType = "test-role-type" + testRoleName = "test-role-name" + testAction = "test-action" +) + +// fakeRAAPI is a raAPI implementation for testing. +type fakeRAAPI struct { + d1 []*armauthorization.RoleAssignment + d2 error +} + +func (api fakeRAAPI) GetRoleAssignmentsForScope(_ string, _ *string) ([]*armauthorization.RoleAssignment, error) { + return api.d1, api.d2 +} + +// fakeRDAPI is a rdAPI implementation for testing. +type fakeRDAPI struct { + // Map to store role definitions by key + roleDefinitions map[string]*armauthorization.RoleDefinition + // Default error to return if key not found + defaultError error + // Error to force, regardless of key + forcedError error +} + +func (api fakeRDAPI) GetByID(key string) (*armauthorization.RoleDefinition, error) { + if api.forcedError != nil { + return nil, api.forcedError + } + + if roleDef, ok := api.roleDefinitions[key]; ok { + return roleDef, nil + } + + return nil, api.defaultError +} + +func TestRBACRoleRuleService_ReconcileRBACRoleRule(t *testing.T) { + + type testCase struct { + name string + rule v1alpha1.RBACRoleRule + raAPIMock fakeRAAPI + rdAPIMock fakeRDAPI + expectedError error + expectedResult vapitypes.ValidationRuleResult + } + + testCases := []testCase{ + { + name: "Pass (a role assignment provides the required permissions)", + rule: v1alpha1.RBACRoleRule{ + Name: "rule-1", + PrincipalID: testPrincipalID, + RoleAssignments: []v1alpha1.RoleAssignment{ + { + Scope: testScope, + Role: v1alpha1.Role{ + Type: testRoleType, + Name: testRoleName, + Permission: v1alpha1.Permission{ + Actions: []string{testAction}, + }, + }, + }, + }, + }, + raAPIMock: fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{ + { + Properties: &armauthorization.RoleAssignmentProperties{ + RoleDefinitionID: ptr.To(testRoleDefinitionID), + Scope: ptr.To(testScope), + }, + ID: ptr.To(testRoleAssignmentID), + }, + }, + }, + rdAPIMock: fakeRDAPI{ + roleDefinitions: map[string]*armauthorization.RoleDefinition{ + testRoleDefinitionID: { + Properties: &armauthorization.RoleDefinitionProperties{ + RoleType: ptr.To(testRoleType), + RoleName: ptr.To(testRoleName), + Permissions: []*armauthorization.Permission{ + { + Actions: []*string{ptr.To(testAction)}, + DataActions: []*string{}, + NotActions: []*string{}, + NotDataActions: []*string{}, + }, + }, + }, + }, + }, + }, + expectedError: nil, + expectedResult: vapitypes.ValidationRuleResult{ + Condition: &vapi.ValidationCondition{ + ValidationType: "azure-rbac-role", + ValidationRule: "validation-rule-1", + Message: "All role assignments correct. Roles contain permissions and assigned to principal at scopes.", + Details: []string{}, + Failures: []string{}, + Status: corev1.ConditionTrue, + }, + State: util.Ptr(vapi.ValidationSucceeded), + }, + }, + { + name: "Fail (at least one failure occurs)", + rule: v1alpha1.RBACRoleRule{ + Name: "rule-1", + PrincipalID: testPrincipalID, + RoleAssignments: []v1alpha1.RoleAssignment{ + { + Scope: testScope, + Role: v1alpha1.Role{ + Type: testRoleType, + Name: testRoleName, + Permission: v1alpha1.Permission{ + Actions: []string{testAction}, + }, + }, + }, + }, + }, + raAPIMock: fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{}, + }, + expectedError: nil, + expectedResult: vapitypes.ValidationRuleResult{ + Condition: &vapi.ValidationCondition{ + ValidationType: "azure-rbac-role", + ValidationRule: "validation-rule-1", + Message: "Principal lacks required permissions. See failures for details.", + Details: []string{}, + Failures: []string{fmt.Sprintf("No role assignments found for principal '%s' at scope '%s'.", + testPrincipalID, testScope)}, + Status: corev1.ConditionFalse, + }, + State: util.Ptr(vapi.ValidationFailed), + }, + }, + } + for _, tc := range testCases { + svc := NewRBACRoleRuleService(tc.raAPIMock, tc.rdAPIMock) + result, err := svc.ReconcileRBACRoleRule(tc.rule) + util.CheckTestCase(t, result, tc.expectedResult, err, tc.expectedError) + } +} + +func TestRBACRoleRuleService_processRoleAssignment(t *testing.T) { + testErrMsg := "test error" + + type fields struct { + raAPI roleAssignmentAPI + rdAPI roleDefinitionAPI + } + type args struct { + raSpec v1alpha1.RoleAssignment + principalID string + } + tests := []struct { + name string + fields fields + args args + want string + wantErr bool + wantErrMsg string + }{ + { + name: "Returns an error when the role assignments API returns an error.", + fields: fields{ + raAPI: &fakeRAAPI{ + d2: errors.New(testErrMsg), + }, + }, + args: args{ + raSpec: v1alpha1.RoleAssignment{ + Scope: testScope, + }, + principalID: testPrincipalID, + }, + want: "", + wantErr: true, + wantErrMsg: fmt.Sprintf("failed to get role assignments for principal '%s' at scope '%s': %s", + testPrincipalID, testPrincipalID, testErrMsg), + }, + { + name: "Returns no error and a failure when role assignments API returns no role assignments.", + fields: fields{ + raAPI: &fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{}, + }, + }, + args: args{ + raSpec: v1alpha1.RoleAssignment{ + Scope: testScope, + }, + principalID: testPrincipalID, + }, + want: fmt.Sprintf("No role assignments found for principal '%s' at scope '%s'.", testPrincipalID, testScope), + wantErr: false, + }, + { + name: "Returns an error when the role assignments API returns data that is missing expected properties.", + fields: fields{ + raAPI: &fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{{}}, + }, + }, + args: args{ + raSpec: v1alpha1.RoleAssignment{ + Scope: testScope, + }, + principalID: testPrincipalID, + }, + want: "", + wantErr: true, + wantErrMsg: "role assignment from API response missing expected properties", + }, + { + name: "Returns an error when the role definitions API returns an error.", + fields: fields{ + raAPI: &fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{ + { + Properties: &armauthorization.RoleAssignmentProperties{ + RoleDefinitionID: ptr.To(testRoleDefinitionID), + Scope: ptr.To(testScope), + }, + ID: ptr.To(testRoleAssignmentID), + }, + }, + }, + rdAPI: &fakeRDAPI{ + forcedError: errors.New(testErrMsg), + }, + }, + args: args{ + raSpec: v1alpha1.RoleAssignment{ + Scope: testScope, + }, + principalID: testPrincipalID, + }, + want: "", + wantErr: true, + wantErrMsg: fmt.Sprintf("failed to get role definition '%s' (for role assignment '%s'): %s", + testRoleDefinitionID, testRoleAssignmentID, testErrMsg), + }, + { + name: "Returns an error when the role definitions API returns data that is missing expected properties.", + fields: fields{ + raAPI: &fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{ + { + Properties: &armauthorization.RoleAssignmentProperties{ + RoleDefinitionID: ptr.To(testRoleDefinitionID), + Scope: ptr.To(testScope), + }, + ID: ptr.To(testRoleAssignmentID), + }, + }, + }, + rdAPI: &fakeRDAPI{ + roleDefinitions: map[string]*armauthorization.RoleDefinition{ + testRoleDefinitionID: {}, + }, + }, + }, + args: args{ + raSpec: v1alpha1.RoleAssignment{ + Scope: testScope, + }, + principalID: testPrincipalID, + }, + want: "", + wantErr: true, + wantErrMsg: fmt.Sprintf("role definition '%s' from API response missing expected properties", testRoleDefinitionID), + }, + { + name: "Returns an error when the role definitions API returns data that doesn't have exactly one permission in its properties.", + fields: fields{ + raAPI: &fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{ + { + Properties: &armauthorization.RoleAssignmentProperties{ + RoleDefinitionID: ptr.To(testRoleDefinitionID), + Scope: ptr.To(testScope), + }, + ID: ptr.To(testRoleAssignmentID), + }, + }, + }, + rdAPI: &fakeRDAPI{ + roleDefinitions: map[string]*armauthorization.RoleDefinition{ + testRoleDefinitionID: { + Properties: &armauthorization.RoleDefinitionProperties{ + RoleType: ptr.To(testRoleType), + RoleName: ptr.To(testRoleName), + Permissions: []*armauthorization.Permission{}, + }, + }, + }, + }, + }, + args: args{ + raSpec: v1alpha1.RoleAssignment{ + Scope: testScope, + }, + principalID: testPrincipalID, + }, + want: "", + wantErr: true, + wantErrMsg: fmt.Sprintf("role definition '%s' from API response has unexpected number of permissions", testRoleDefinitionID), + }, + { + name: "Returns no error and no failure when there is a role assignment for a role with the specified permissions.", + fields: fields{ + raAPI: &fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{ + { + Properties: &armauthorization.RoleAssignmentProperties{ + RoleDefinitionID: ptr.To(testRoleDefinitionID), + Scope: ptr.To(testScope), + }, + ID: ptr.To(testRoleAssignmentID), + }, + }, + }, + rdAPI: &fakeRDAPI{ + roleDefinitions: map[string]*armauthorization.RoleDefinition{ + testRoleDefinitionID: { + Properties: &armauthorization.RoleDefinitionProperties{ + RoleType: ptr.To(testRoleType), + RoleName: ptr.To(testRoleName), + Permissions: []*armauthorization.Permission{ + { + Actions: []*string{ptr.To(testAction)}, + DataActions: []*string{}, + NotActions: []*string{}, + NotDataActions: []*string{}, + }, + }, + }, + }, + }, + }, + }, + args: args{ + raSpec: v1alpha1.RoleAssignment{ + Scope: testScope, + Role: v1alpha1.Role{ + Type: testRoleType, + Name: testRoleName, + Permission: v1alpha1.Permission{ + Actions: []string{}, + }, + }, + }, + principalID: testPrincipalID, + }, + want: "", + wantErr: false, + }, + { + name: "Returns no error and no failure when the specified permissions are provided by a role assignment other than the first one encountered.", + fields: fields{ + raAPI: &fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{ + { + Properties: &armauthorization.RoleAssignmentProperties{ + RoleDefinitionID: ptr.To(testRoleDefinitionID), + Scope: ptr.To(testScope), + }, + ID: ptr.To(testRoleAssignmentID), + }, + { + Properties: &armauthorization.RoleAssignmentProperties{ + RoleDefinitionID: ptr.To(testRoleDefinitionID + "2"), + Scope: ptr.To(testScope), + }, + ID: ptr.To(testRoleAssignmentID + "2"), + }, + }, + }, + rdAPI: &fakeRDAPI{ + roleDefinitions: map[string]*armauthorization.RoleDefinition{ + testRoleDefinitionID: { + Properties: &armauthorization.RoleDefinitionProperties{ + RoleType: ptr.To(testRoleType), + RoleName: ptr.To(testRoleName), + Permissions: []*armauthorization.Permission{ + { + Actions: []*string{ptr.To("some-other-action")}, + DataActions: []*string{}, + NotActions: []*string{}, + NotDataActions: []*string{}, + }, + }, + }, + }, + testRoleDefinitionID + "2": { + Properties: &armauthorization.RoleDefinitionProperties{ + RoleType: ptr.To(testRoleType), + RoleName: ptr.To(testRoleName), + Permissions: []*armauthorization.Permission{ + { + Actions: []*string{ptr.To(testAction)}, + DataActions: []*string{}, + NotActions: []*string{}, + NotDataActions: []*string{}, + }, + }, + }, + }, + }, + }, + }, + args: args{ + raSpec: v1alpha1.RoleAssignment{ + Scope: testScope, + Role: v1alpha1.Role{ + Type: testRoleType, + Name: testRoleName, + Permission: v1alpha1.Permission{ + Actions: []string{}, + }, + }, + }, + principalID: testPrincipalID, + }, + want: "", + wantErr: false, + }, + { + name: "Returns no error and a failure when a role assignment and role definition are found with everything matching except role type.", + fields: fields{ + raAPI: &fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{ + { + Properties: &armauthorization.RoleAssignmentProperties{ + RoleDefinitionID: ptr.To(testRoleDefinitionID), + Scope: ptr.To(testScope), + }, + ID: ptr.To(testRoleAssignmentID), + }, + }, + }, + rdAPI: &fakeRDAPI{ + roleDefinitions: map[string]*armauthorization.RoleDefinition{ + testRoleDefinitionID: { + Properties: &armauthorization.RoleDefinitionProperties{ + RoleType: ptr.To("some-other-role-type"), + RoleName: ptr.To(testRoleName), + Permissions: []*armauthorization.Permission{ + { + Actions: []*string{ptr.To(testAction)}, + DataActions: []*string{}, + NotActions: []*string{}, + NotDataActions: []*string{}, + }, + }, + }, + }, + }, + }, + }, + args: args{ + raSpec: v1alpha1.RoleAssignment{ + Scope: testScope, + Role: v1alpha1.Role{ + Type: testRoleType, + Name: testRoleName, + Permission: v1alpha1.Permission{ + Actions: []string{}, + }, + }, + }, + principalID: testPrincipalID, + }, + want: fmt.Sprintf("Principal '%s' does not have role with type '%s' and role name '%s' assigned at scope '%s' with required permissions.", + testPrincipalID, testRoleType, testRoleName, testScope), + wantErr: false, + }, + { + name: "Returns no error and a failure when a role assignment and role definition are found with everything matching except role name.", + fields: fields{ + raAPI: &fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{ + { + Properties: &armauthorization.RoleAssignmentProperties{ + RoleDefinitionID: ptr.To(testRoleDefinitionID), + Scope: ptr.To(testScope), + }, + ID: ptr.To(testRoleAssignmentID), + }, + }, + }, + rdAPI: &fakeRDAPI{ + roleDefinitions: map[string]*armauthorization.RoleDefinition{ + testRoleDefinitionID: { + Properties: &armauthorization.RoleDefinitionProperties{ + RoleType: ptr.To(testRoleType), + RoleName: ptr.To("some-other-role-name"), + Permissions: []*armauthorization.Permission{ + { + Actions: []*string{ptr.To(testAction)}, + DataActions: []*string{}, + NotActions: []*string{}, + NotDataActions: []*string{}, + }, + }, + }, + }, + }, + }, + }, + args: args{ + raSpec: v1alpha1.RoleAssignment{ + Scope: testScope, + Role: v1alpha1.Role{ + Type: testRoleType, + Name: testRoleName, + Permission: v1alpha1.Permission{ + Actions: []string{}, + }, + }, + }, + principalID: testPrincipalID, + }, + want: fmt.Sprintf("Principal '%s' does not have role with type '%s' and role name '%s' assigned at scope '%s' with required permissions.", + testPrincipalID, testRoleType, testRoleName, testScope), + wantErr: false, + }, + { + name: "Returns no error and a failure when a role assignment and role definition are found with everything matching except role assignment scope.", + fields: fields{ + raAPI: &fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{ + { + Properties: &armauthorization.RoleAssignmentProperties{ + RoleDefinitionID: ptr.To(testRoleDefinitionID), + Scope: ptr.To("some-other-scope"), + }, + ID: ptr.To(testRoleAssignmentID), + }, + }, + }, + rdAPI: &fakeRDAPI{ + roleDefinitions: map[string]*armauthorization.RoleDefinition{ + testRoleDefinitionID: { + Properties: &armauthorization.RoleDefinitionProperties{ + RoleType: ptr.To(testRoleType), + RoleName: ptr.To(testRoleName), + Permissions: []*armauthorization.Permission{ + { + Actions: []*string{ptr.To(testAction)}, + DataActions: []*string{}, + NotActions: []*string{}, + NotDataActions: []*string{}, + }, + }, + }, + }, + }, + }, + }, + args: args{ + raSpec: v1alpha1.RoleAssignment{ + Scope: testScope, + Role: v1alpha1.Role{ + Type: testRoleType, + Name: testRoleName, + Permission: v1alpha1.Permission{ + Actions: []string{}, + }, + }, + }, + principalID: testPrincipalID, + }, + want: fmt.Sprintf("Principal '%s' does not have role with type '%s' and role name '%s' assigned at scope '%s' with required permissions.", + testPrincipalID, testRoleType, testRoleName, testScope), + wantErr: false, + }, + { + name: "Returns no error and a failure when a role assignment and role definition are found with everything matching except permissions.", + fields: fields{ + raAPI: &fakeRAAPI{ + d1: []*armauthorization.RoleAssignment{ + { + Properties: &armauthorization.RoleAssignmentProperties{ + RoleDefinitionID: ptr.To(testRoleDefinitionID), + Scope: ptr.To(testScope), + }, + ID: ptr.To(testRoleAssignmentID), + }, + }, + }, + rdAPI: &fakeRDAPI{ + roleDefinitions: map[string]*armauthorization.RoleDefinition{ + testRoleDefinitionID: { + Properties: &armauthorization.RoleDefinitionProperties{ + RoleType: ptr.To(testRoleType), + RoleName: ptr.To(testRoleName), + Permissions: []*armauthorization.Permission{ + { + Actions: []*string{ptr.To("some-other-action")}, + DataActions: []*string{}, + NotActions: []*string{}, + NotDataActions: []*string{}, + }, + }, + }, + }, + }, + }, + }, + args: args{ + raSpec: v1alpha1.RoleAssignment{ + Scope: testScope, + Role: v1alpha1.Role{ + Type: testRoleType, + Name: testRoleName, + Permission: v1alpha1.Permission{ + Actions: []string{}, + }, + }, + }, + principalID: testPrincipalID, + }, + want: fmt.Sprintf("Principal '%s' does not have role with type '%s' and role name '%s' assigned at scope '%s' with required permissions.", + testPrincipalID, testRoleType, testRoleName, testScope), + wantErr: false, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &RBACRoleRuleService{ + raAPI: tt.fields.raAPI, + rdAPI: tt.fields.rdAPI, + } + got, err := s.processRoleAssignment(tt.args.raSpec, tt.args.principalID) + if (err != nil) != tt.wantErr { + t.Errorf("RBACRoleRuleService.processRoleAssignment() error = %v, wantErr %v", err, tt.wantErr) + return + } + if err != nil && err.Error() != tt.wantErrMsg { + t.Errorf("RBACRoleRuleService.processRoleAssignment() errorMsg = '%s', wantErrMsg '%s'", err.Error(), tt.wantErrMsg) + return + } + if got != tt.want { + t.Errorf("RBACRoleRuleService.processRoleAssignment() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/validators/rbac_test.go b/internal/validators/rbac_test.go deleted file mode 100644 index cf207d77..00000000 --- a/internal/validators/rbac_test.go +++ /dev/null @@ -1,420 +0,0 @@ -package validators - -import ( - "errors" - "testing" - - "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/authorization/armauthorization/v2" - corev1 "k8s.io/api/core/v1" - - "github.com/validator-labs/validator-plugin-azure/api/v1alpha1" - vapi "github.com/validator-labs/validator/api/v1alpha1" - vapitypes "github.com/validator-labs/validator/pkg/types" - "github.com/validator-labs/validator/pkg/util" -) - -type denyAssignmentAPIMock struct { - data []*armauthorization.DenyAssignment - err error -} - -func (m denyAssignmentAPIMock) GetDenyAssignmentsForScope(_ string, _ *string) ([]*armauthorization.DenyAssignment, error) { - return m.data, nil -} - -type roleAssignmentAPIMock struct { - data []*armauthorization.RoleAssignment - err error -} - -func (m roleAssignmentAPIMock) GetRoleAssignmentsForScope(_ string, _ *string) ([]*armauthorization.RoleAssignment, error) { - return m.data, m.err -} - -type roleDefinitionAPIMock struct { - // key = roleID - data map[string]*armauthorization.RoleDefinition - err error -} - -func (m roleDefinitionAPIMock) GetByID(roleID string) (*armauthorization.RoleDefinition, error) { - return m.data[roleID], nil -} - -func TestRBACRuleService_ReconcileRBACRule(t *testing.T) { - - // Example scopes taken from: - // https://learn.microsoft.com/en-us/azure/role-based-access-control/scope-overview - subscriptionScope := "/subscriptions/00000000-0000-0000-0000-000000000000" - - type testCase struct { - name string - rule v1alpha1.RBACRule - daAPIMock denyAssignmentAPIMock - raAPIMock roleAssignmentAPIMock - rdAPIMock roleDefinitionAPIMock - expectedError error - expectedResult vapitypes.ValidationRuleResult - } - - // Note that these test cases test code that calls code in rbac_permissions.go, which is already - // covered by tests. Therefore, we don't need to test some functionality (see - // rbac_permissions_test.go). Here, we test that the expected failure messages are included in - // the validation result for conditions that should cause failures. Note that the input is based - // on how Azure responds to our API requests for a given principal and scope. Therefore, we are - // *not* testing whether Azure is using the correct logic to determine which deny assignments - // and role assignments match the queries. We're trusting that it does this correctly, including - // when scope should inherit or not inherit an assignment because of the subscription->resource - // group etc hierarchy. - - testCases := []testCase{ - { - name: "Pass (required actions and data actions provided by role assignments and not denied by deny assignments)", - rule: v1alpha1.RBACRule{ - Name: "rule-1", - Permissions: []v1alpha1.PermissionSet{ - { - Actions: []v1alpha1.ActionStr{"a"}, - DataActions: []v1alpha1.ActionStr{"b"}, - Scope: subscriptionScope, - }, - }, - PrincipalID: "p_id", - }, - daAPIMock: denyAssignmentAPIMock{ - data: []*armauthorization.DenyAssignment{}, - err: nil, - }, - raAPIMock: roleAssignmentAPIMock{ - data: []*armauthorization.RoleAssignment{ - { - Properties: &armauthorization.RoleAssignmentProperties{ - RoleDefinitionID: util.Ptr("role_id"), - }, - }, - }, - err: nil, - }, - rdAPIMock: roleDefinitionAPIMock{ - data: map[string]*armauthorization.RoleDefinition{ - "role_id": { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("a")}, - DataActions: []*string{util.Ptr("b")}, - NotActions: []*string{}, - NotDataActions: []*string{}, - }, - }, - }, - }, - }, - err: nil, - }, - expectedError: nil, - expectedResult: vapitypes.ValidationRuleResult{ - Condition: &vapi.ValidationCondition{ - ValidationType: "azure-rbac", - ValidationRule: "validation-rule-1", - Message: "Principal has all required permissions.", - Details: []string{}, - Failures: []string{}, - Status: corev1.ConditionTrue, - }, - State: util.Ptr(vapi.ValidationSucceeded), - }, - }, - { - name: "Fail (required actions and data actions provided by role assignments but denied by deny assignments)", - rule: v1alpha1.RBACRule{ - Name: "rule-1", - Permissions: []v1alpha1.PermissionSet{ - { - Actions: []v1alpha1.ActionStr{"a"}, - DataActions: []v1alpha1.ActionStr{"b"}, - Scope: subscriptionScope, - }, - }, - PrincipalID: "p_id", - }, - daAPIMock: denyAssignmentAPIMock{ - data: []*armauthorization.DenyAssignment{ - { - Properties: &armauthorization.DenyAssignmentProperties{ - Permissions: []*armauthorization.DenyAssignmentPermission{ - { - Actions: []*string{util.Ptr("a")}, - DataActions: []*string{util.Ptr("b")}, - NotActions: []*string{}, - NotDataActions: []*string{}, - }, - }, - }, - ID: util.Ptr("d"), - }, - }, - err: nil, - }, - raAPIMock: roleAssignmentAPIMock{ - data: []*armauthorization.RoleAssignment{ - { - Properties: &armauthorization.RoleAssignmentProperties{ - RoleDefinitionID: util.Ptr("role_id"), - }, - }, - }, - err: nil, - }, - rdAPIMock: roleDefinitionAPIMock{ - data: map[string]*armauthorization.RoleDefinition{ - "role_id": { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{util.Ptr("a")}, - DataActions: []*string{util.Ptr("b")}, - NotActions: []*string{}, - NotDataActions: []*string{}, - }, - }, - }, - }, - }, - err: nil, - }, - expectedError: nil, - expectedResult: vapitypes.ValidationRuleResult{ - Condition: &vapi.ValidationCondition{ - ValidationType: "azure-rbac", - ValidationRule: "validation-rule-1", - Message: "Principal lacks required permissions. See failures for details.", - Details: []string{}, - Failures: []string{ - "Action a denied by deny assignment d.", - "DataAction b denied by deny assignment d.", - }, - Status: corev1.ConditionFalse, - }, - State: util.Ptr(vapi.ValidationFailed), - }, - }, - { - name: "Fail (required actions and data actions not provided by role assignments)", - rule: v1alpha1.RBACRule{ - Name: "rule-1", - Permissions: []v1alpha1.PermissionSet{ - { - Actions: []v1alpha1.ActionStr{"a"}, - DataActions: []v1alpha1.ActionStr{"b"}, - Scope: subscriptionScope, - }, - }, - PrincipalID: "p_id", - }, - daAPIMock: denyAssignmentAPIMock{ - data: []*armauthorization.DenyAssignment{}, - err: nil, - }, - raAPIMock: roleAssignmentAPIMock{ - data: []*armauthorization.RoleAssignment{ - { - Properties: &armauthorization.RoleAssignmentProperties{ - RoleDefinitionID: util.Ptr("role_id"), - }, - }, - }, - err: nil, - }, - rdAPIMock: roleDefinitionAPIMock{ - data: map[string]*armauthorization.RoleDefinition{ - "role_id": { - Properties: &armauthorization.RoleDefinitionProperties{ - Permissions: []*armauthorization.Permission{ - { - Actions: []*string{}, - DataActions: []*string{}, - NotActions: []*string{}, - NotDataActions: []*string{}, - }, - }, - }, - }, - }, - err: nil, - }, - expectedError: nil, - expectedResult: vapitypes.ValidationRuleResult{ - Condition: &vapi.ValidationCondition{ - ValidationType: "azure-rbac", - ValidationRule: "validation-rule-1", - Message: "Principal lacks required permissions. See failures for details.", - Details: []string{}, - Failures: []string{ - "Action a unpermitted because no role assignment permits it.", - "DataAction b unpermitted because no role assignment permits it.", - }, - Status: corev1.ConditionFalse, - }, - State: util.Ptr(vapi.ValidationFailed), - }, - }, - } - for _, tc := range testCases { - svc := NewRBACRuleService(tc.daAPIMock, tc.raAPIMock, tc.rdAPIMock) - result, err := svc.ReconcileRBACRule(tc.rule) - util.CheckTestCase(t, result, tc.expectedResult, err, tc.expectedError) - } -} - -// fakeDAAPI is a daAPI implementation for testing. -type fakeDAAPI struct { - d1 []*armauthorization.DenyAssignment - d2 error -} - -func (api *fakeDAAPI) GetDenyAssignmentsForScope(_ string, _ *string) ([]*armauthorization.DenyAssignment, error) { - return api.d1, api.d2 -} - -// fakeRAAPI is a raAPI implementation for testing. -type fakeRAAPI struct { - d1 []*armauthorization.RoleAssignment - d2 error -} - -func (api *fakeRAAPI) GetRoleAssignmentsForScope(_ string, _ *string) ([]*armauthorization.RoleAssignment, error) { - return api.d1, api.d2 -} - -// fakeRDAPI is a rdAPI implementation for testing. -type fakeRDAPI struct { - d1 *armauthorization.RoleDefinition - d2 error -} - -func (api *fakeRDAPI) GetByID(_ string) (*armauthorization.RoleDefinition, error) { - return api.d1, api.d2 -} - -func TestRBACRuleService_processPermissionSet(t *testing.T) { - - type fields struct { - daAPI denyAssignmentAPI - raAPI roleAssignmentAPI - rdAPI roleDefinitionAPI - } - type args struct { - set v1alpha1.PermissionSet - principalID string - failures *[]string - } - tests := []struct { - name string - fields fields - args args - wantErr bool - }{ - { - name: "Returns an error when the deny assignments API returns an error.", - fields: fields{ - daAPI: &fakeDAAPI{ - d1: []*armauthorization.DenyAssignment{}, - d2: errors.New("fail"), - }, - }, - args: args{}, - wantErr: true, - }, - { - name: "Returns an error when the deny assignments API returns data but the role assignments API returns an error.", - fields: fields{ - daAPI: &fakeDAAPI{ - d1: []*armauthorization.DenyAssignment{}, - d2: nil, - }, - raAPI: &fakeRAAPI{ - d1: []*armauthorization.RoleAssignment{}, - d2: errors.New("fail"), - }, - }, - args: args{}, - wantErr: true, - }, - { - name: "Returns an error when the deny assignments API and role assignments API return data but the role assignment has no properties.", - fields: fields{ - daAPI: &fakeDAAPI{ - d1: []*armauthorization.DenyAssignment{}, - d2: nil, - }, - raAPI: &fakeRAAPI{ - d1: []*armauthorization.RoleAssignment{{}}, - d2: nil, - }, - rdAPI: &fakeRDAPI{ - d1: &armauthorization.RoleDefinition{}, - d2: nil, - }, - }, - args: args{}, - wantErr: true, - }, - { - name: "Returns an error when the deny assignments API, role assignments API, and role definitions API return data but the role assignment has no role definition ID property.", - fields: fields{ - daAPI: &fakeDAAPI{ - d1: []*armauthorization.DenyAssignment{}, - d2: nil, - }, - raAPI: &fakeRAAPI{ - d1: []*armauthorization.RoleAssignment{{ - Properties: &armauthorization.RoleAssignmentProperties{}, - }}, - d2: nil, - }, - rdAPI: &fakeRDAPI{ - d1: &armauthorization.RoleDefinition{}, - d2: nil, - }, - }, - args: args{}, - wantErr: true, - }, - { - name: "Returns an error when the deny assignments API and role assignments API return data but the role definitions API returns an error.", - fields: fields{ - daAPI: &fakeDAAPI{ - d1: []*armauthorization.DenyAssignment{}, - d2: nil, - }, - raAPI: &fakeRAAPI{ - d1: []*armauthorization.RoleAssignment{{ - Properties: &armauthorization.RoleAssignmentProperties{ - RoleDefinitionID: util.Ptr("abc123"), - }, - }}, - d2: nil, - }, - rdAPI: &fakeRDAPI{ - d1: &armauthorization.RoleDefinition{}, - d2: errors.New("fail"), - }, - }, - args: args{}, - wantErr: true, - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - s := &RBACRuleService{ - daAPI: tt.fields.daAPI, - raAPI: tt.fields.raAPI, - rdAPI: tt.fields.rdAPI, - } - if err := s.processPermissionSet(tt.args.set, tt.args.principalID, tt.args.failures); (err != nil) != tt.wantErr { - t.Errorf("RBACRuleService.processPermissionSet() error = %v, wantErr %v", err, tt.wantErr) - } - }) - } -}