From 06245a933db687cdbe952e50752a8a51c9d2142b Mon Sep 17 00:00:00 2001 From: Chief-Rishab Date: Sat, 13 May 2023 16:09:50 +0530 Subject: [PATCH] feat(policy): introduce expression in additional appeal requirement trigger --- api/handler/v1beta1/adapter.go | 2 + core/appeal/service_test.go | 188 ++++++++++++++++++++++++++++++++ domain/policy.go | 30 ++++- domain/policy_test.go | 194 +++++++++++++++++++++++++++++++++ 4 files changed, 408 insertions(+), 6 deletions(-) diff --git a/api/handler/v1beta1/adapter.go b/api/handler/v1beta1/adapter.go index e4b4593bb..fc4a82b95 100644 --- a/api/handler/v1beta1/adapter.go +++ b/api/handler/v1beta1/adapter.go @@ -265,6 +265,7 @@ func (a *adapter) FromPolicyProto(p *guardianv1beta1.Policy) *domain.Policy { ResourceURN: r.GetOn().GetResourceUrn(), Role: r.GetOn().GetRole(), Conditions: conditions, + Expression: r.GetOn().GetExpression(), } } @@ -394,6 +395,7 @@ func (a *adapter) ToPolicyProto(p *domain.Policy) (*guardianv1beta1.Policy, erro ResourceUrn: r.On.ResourceURN, Role: r.On.Role, Conditions: conditions, + Expression: r.On.Expression, } } diff --git a/core/appeal/service_test.go b/core/appeal/service_test.go index b1586a7fe..fc9d6cd5f 100644 --- a/core/appeal/service_test.go +++ b/core/appeal/service_test.go @@ -1240,6 +1240,194 @@ func (s *ServiceTestSuite) TestCreateAppeal__WithExistingAppealAndWithAutoApprov s.Equal(expectedResult, appeals) } +func (s *ServiceTestSuite) TestCreateAppeal__WithAdditionalAppeals() { + s.setup() + providerType := "test-provider-type" + providerURN := "test-provider-urn" + resourceType := "test-resource-type" + targetResource := &domain.ResourceIdentifier{ + ID: "test-resource-id-2", + } + targetRole := "test-role-1" + + resources := []*domain.Resource{ + { + ID: "test-resource-id-1", + URN: "test-resource-urn-1", + Type: resourceType, + ProviderType: providerType, + ProviderURN: providerURN, + }, + { + ID: "test-resource-id-2", + URN: "test-resource-urn-2", + Type: resourceType, + ProviderType: providerType, + ProviderURN: providerURN, + }, + } + policies := []*domain.Policy{ + { + ID: "test-policy-id-1", + Version: 1, + Steps: []*domain.Step{ + { + Name: "test-step-1", + Strategy: domain.ApprovalStepStrategyAuto, + ApproveIf: `true`, + }, + }, + Requirements: []*domain.Requirement{ + { + On: &domain.RequirementTrigger{ + Expression: `$appeal.resource.urn == "test-resource-urn-1"`, + }, + Appeals: []*domain.AdditionalAppeal{ + { + Resource: targetResource, + Role: targetRole, + }, + }, + }, + }, + }, + } + providers := []*domain.Provider{ + { + ID: "test-provider-id", + Type: providerType, + URN: providerURN, + Config: &domain.ProviderConfig{ + Resources: []*domain.ResourceConfig{ + { + Type: resourceType, + Policy: &domain.PolicyConfig{ + ID: policies[0].ID, + Version: int(policies[0].Version), + }, + Roles: []*domain.Role{ + { + ID: "test-role-1", + Permissions: []interface{}{"test-permission-1"}, + }, + }, + }, + }, + }, + }, + } + + appealsPayload := []*domain.Appeal{ + { + CreatedBy: "user@example.com", + AccountID: "user@example.com", + ResourceID: "test-resource-id-1", + Resource: &domain.Resource{ + ID: "test-resource-id-1", + URN: "test-resource-urn-1", + Type: resourceType, + ProviderType: providerType, + ProviderURN: providerURN, + }, + Role: "test-role-1", + }, + } + + // 1.a main appeal creation + expectedResourceFilters := domain.ListResourcesFilter{IDs: []string{appealsPayload[0].Resource.ID}} + s.mockResourceService.EXPECT().Find(mock.AnythingOfType("*context.emptyCtx"), expectedResourceFilters).Return([]*domain.Resource{resources[0]}, nil).Once() + s.mockProviderService.EXPECT().Find(mock.AnythingOfType("*context.emptyCtx")).Return(providers, nil).Once() + s.mockPolicyService.EXPECT().Find(mock.AnythingOfType("*context.emptyCtx")).Return(policies, nil).Once() + s.mockGrantService.EXPECT().List(mock.AnythingOfType("*context.emptyCtx"), mock.AnythingOfType("domain.ListGrantsFilter")).Return([]domain.Grant{}, nil).Once().Run(func(args mock.Arguments) { + filter := args.Get(1).(domain.ListGrantsFilter) + s.Equal([]string{appealsPayload[0].AccountID}, filter.AccountIDs) + s.Equal([]string{appealsPayload[0].Resource.ID}, filter.ResourceIDs) + s.Equal([]string{appealsPayload[0].Role}, filter.Roles) + }) + s.mockRepository.EXPECT().Find(mock.AnythingOfType("*context.emptyCtx"), mock.AnythingOfType("*domain.ListAppealsFilter")).Return([]*domain.Appeal{}, nil).Once() + s.mockProviderService.EXPECT().ValidateAppeal(mock.AnythingOfType("*context.emptyCtx"), appealsPayload[0], providers[0], policies[0]).Return(nil).Once() + s.mockProviderService.EXPECT().GetPermissions(mock.AnythingOfType("*context.emptyCtx"), providers[0].Config, appealsPayload[0].Resource.Type, appealsPayload[0].Role).Return([]interface{}{"test-permission-1"}, nil).Once() + s.mockGrantService.EXPECT().List(mock.AnythingOfType("*context.emptyCtx"), mock.AnythingOfType("domain.ListGrantsFilter")).Return([]domain.Grant{}, nil).Once().Run(func(args mock.Arguments) { + filter := args.Get(1).(domain.ListGrantsFilter) + s.Equal([]string{appealsPayload[0].AccountID}, filter.AccountIDs) + s.Equal([]string{appealsPayload[0].Resource.ID}, filter.ResourceIDs) + }) + expectedGrant := &domain.Grant{ID: "main-grant"} + s.mockGrantService.EXPECT().Prepare(mock.AnythingOfType("*context.emptyCtx"), mock.AnythingOfType("domain.Appeal")).Return(expectedGrant, nil).Once().Run(func(args mock.Arguments) { + appeal := args.Get(1).(domain.Appeal) + s.Equal(appealsPayload[0].AccountID, appeal.AccountID) + s.Equal(appealsPayload[0].Role, appeal.Role) + s.Equal(appealsPayload[0].ResourceID, appeal.ResourceID) + s.Equal(len(policies[0].Steps), len(appeal.Approvals)) + }) + s.mockPolicyService.EXPECT().GetOne(mock.AnythingOfType("*context.emptyCtx"), policies[0].ID, policies[0].Version).Return(policies[0], nil).Once() + + // 2.a additional appeal creation + s.mockResourceService.EXPECT().Get(mock.AnythingOfType("*context.cancelCtx"), targetResource).Return(resources[1], nil).Once() + expectedResourceFilters = domain.ListResourcesFilter{IDs: []string{resources[1].ID}} + s.mockResourceService.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx"), expectedResourceFilters).Return([]*domain.Resource{resources[1]}, nil).Once() + s.mockProviderService.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx")).Return(providers, nil).Once() + s.mockPolicyService.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx")).Return(policies, nil).Once() + s.mockGrantService.EXPECT().List(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("domain.ListGrantsFilter")).Return([]domain.Grant{}, nil).Once().Run(func(args mock.Arguments) { + filter := args.Get(1).(domain.ListGrantsFilter) + s.Equal([]string{appealsPayload[0].AccountID}, filter.AccountIDs) + s.Equal([]string{targetResource.ID}, filter.ResourceIDs) + s.Equal([]string{targetRole}, filter.Roles) + }) + s.mockRepository.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("*domain.ListAppealsFilter")).Return([]*domain.Appeal{}, nil).Once() + s.mockProviderService.EXPECT().ValidateAppeal(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("*domain.Appeal"), providers[0], policies[0]).Return(nil).Once().Run(func(args mock.Arguments) { + appeal := args.Get(1).(*domain.Appeal) + s.Equal(appealsPayload[0].AccountID, appeal.AccountID) + s.Equal(targetRole, appeal.Role) + s.Equal(targetResource.ID, appeal.ResourceID) + }) + s.mockProviderService.EXPECT().GetPermissions(mock.AnythingOfType("*context.cancelCtx"), providers[0].Config, resourceType, targetRole).Return([]interface{}{"test-permission-1"}, nil).Once() + s.mockGrantService.EXPECT().List(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("domain.ListGrantsFilter")).Return([]domain.Grant{}, nil).Once().Run(func(args mock.Arguments) { + filter := args.Get(1).(domain.ListGrantsFilter) + s.Equal([]string{appealsPayload[0].AccountID}, filter.AccountIDs) + s.Equal([]string{targetResource.ID}, filter.ResourceIDs) + }) + expectedAdditionalGrant := &domain.Grant{ID: "additional-grant"} + s.mockGrantService.EXPECT().Prepare(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("domain.Appeal")).Return(expectedAdditionalGrant, nil).Once().Run(func(args mock.Arguments) { + appeal := args.Get(1).(domain.Appeal) + s.Equal(appealsPayload[0].AccountID, appeal.AccountID) + s.Equal(targetRole, appeal.Role) + s.Equal(targetResource.ID, appeal.ResourceID) + s.Equal(len(policies[0].Steps), len(appeal.Approvals)) + }) + s.mockPolicyService.EXPECT().GetOne(mock.AnythingOfType("*context.cancelCtx"), policies[0].ID, policies[0].Version).Return(policies[0], nil).Once() + + // 2.b grant access for the additional appeal + s.mockProviderService.EXPECT().GrantAccess(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("domain.Grant")).Return(nil).Once().Run(func(args mock.Arguments) { + grant := args.Get(1).(domain.Grant) + s.Equal(expectedAdditionalGrant.ID, grant.ID) + }) + s.mockRepository.EXPECT().BulkUpsert(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("[]*domain.Appeal")).Return(nil).Once().Run(func(args mock.Arguments) { + appeals := args.Get(1).([]*domain.Appeal) + appeal := appeals[0] + s.Equal(targetResource.ID, appeal.Resource.ID) + }) + s.mockAuditLogger.EXPECT().Log(mock.AnythingOfType("*context.cancelCtx"), appeal.AuditKeyBulkInsert, mock.Anything).Return(nil).Once() + s.mockNotifier.EXPECT().Notify(mock.Anything).Return(nil).Once() + + // 1.b grant access for the main appeal + s.mockProviderService.EXPECT().GrantAccess(mock.AnythingOfType("*context.emptyCtx"), mock.AnythingOfType("domain.Grant")).Return(nil).Once().Run(func(args mock.Arguments) { + grant := args.Get(1).(domain.Grant) + s.Equal(expectedGrant.ID, grant.ID) + }) + s.mockRepository.EXPECT().BulkUpsert(mock.AnythingOfType("*context.emptyCtx"), mock.AnythingOfType("[]*domain.Appeal")).Return(nil).Once().Run(func(args mock.Arguments) { + appeals := args.Get(1).([]*domain.Appeal) + appeal := appeals[0] + s.Equal(appealsPayload[0].Resource.ID, appeal.Resource.ID) + }) + s.mockAuditLogger.EXPECT().Log(mock.AnythingOfType("*context.emptyCtx"), appeal.AuditKeyBulkInsert, mock.Anything).Return(nil).Once() + s.mockNotifier.EXPECT().Notify(mock.Anything).Return(nil).Once() + + err := s.service.Create(context.Background(), appealsPayload) + + s.NoError(err) +} + func (s *ServiceTestSuite) TestUpdateApproval() { timeNow := time.Now() appeal.TimeNow = func() time.Time { diff --git a/domain/policy.go b/domain/policy.go index 4c11293e3..30a42f915 100644 --- a/domain/policy.go +++ b/domain/policy.go @@ -181,12 +181,14 @@ func (s Step) ToApproval(a *Appeal, p *Policy, index int) (*Approval, error) { } type RequirementTrigger struct { - ProviderType string `json:"provider_type" yaml:"provider_type" validate:"required_without_all=ProviderURN ResourceType ResourceURN Role Conditions"` - ProviderURN string `json:"provider_urn" yaml:"provider_urn" validate:"required_without_all=ProviderType ResourceType ResourceURN Role Conditions"` - ResourceType string `json:"resource_type" yaml:"resource_type" validate:"required_without_all=ProviderType ProviderURN ResourceURN Role Conditions"` - ResourceURN string `json:"resource_urn" yaml:"resource_urn" validate:"required_without_all=ProviderType ProviderURN ResourceType Role Conditions"` - Role string `json:"role" yaml:"role" validate:"required_without_all=ProviderType ProviderURN ResourceType ResourceType Conditions"` - Conditions []*Condition `json:"conditions" yaml:"conditions" validate:"required_without_all=ProviderType ProviderURN ResourceType ResourceType Role"` + ProviderType string `json:"provider_type" yaml:"provider_type" validate:"required_without_all=ProviderURN ResourceType ResourceURN Role Conditions Expression"` + ProviderURN string `json:"provider_urn" yaml:"provider_urn" validate:"required_without_all=ProviderType ResourceType ResourceURN Role Conditions Expression"` + ResourceType string `json:"resource_type" yaml:"resource_type" validate:"required_without_all=ProviderType ProviderURN ResourceURN Role Conditions Expression"` + ResourceURN string `json:"resource_urn" yaml:"resource_urn" validate:"required_without_all=ProviderType ProviderURN ResourceType Role Conditions Expression"` + Role string `json:"role" yaml:"role" validate:"required_without_all=ProviderType ProviderURN ResourceType ResourceType Conditions Expression"` + // Deprecated: use Expression instead + Conditions []*Condition `json:"conditions" yaml:"conditions" validate:"required_without_all=ProviderType ProviderURN ResourceType ResourceType Role Expression"` + Expression string `json:"expression" yaml:"expression" validate:"required_without_all=ProviderType ProviderURN ResourceType ResourceType Role Conditions"` } func (r *RequirementTrigger) IsMatch(a *Appeal) (bool, error) { @@ -234,6 +236,22 @@ func (r *RequirementTrigger) IsMatch(a *Appeal) (bool, error) { } } } + if r.Expression != "" { + appealMap, err := structToMap(a) + if err != nil { + return false, fmt.Errorf("parsing appeal to map: %w", err) + } + params := map[string]interface{}{"appeal": appealMap} + v, err := evaluator.Expression(r.Expression).EvaluateWithVars(params) + if err != nil { + return false, fmt.Errorf("evaluating expression %q: %w", r.Expression, err) + } + if match, ok := v.(bool); !ok { + return false, fmt.Errorf("expression %q did not evaluate to a boolean, evaluated value: %q", r.Expression, v) + } else { + return match, nil + } + } return true, nil } diff --git a/domain/policy_test.go b/domain/policy_test.go index 046851036..e399fb6c3 100644 --- a/domain/policy_test.go +++ b/domain/policy_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/odpf/guardian/domain" + "github.com/stretchr/testify/assert" ) func TestStep_ResolveApprovers(t *testing.T) { @@ -181,6 +182,199 @@ func TestStep_ResolveApprovers(t *testing.T) { } } +func TestRequirementTrigger(t *testing.T) { + t.Run("should return error if got error when parsing appeal to map", func(t *testing.T) { + brokenAppeal := &domain.Appeal{ + Creator: map[string]interface{}{ + "foobar": &brokenType{}, + }, + } + r := domain.RequirementTrigger{ + Expression: "$appeal.creator.foobar == true", + } + match, err := r.IsMatch(brokenAppeal) + assert.ErrorContains(t, err, "parsing appeal to map:") + assert.False(t, match) + }) + + t.Run("should return error if got error when evaluating expression", func(t *testing.T) { + expr := "invalid expression" + r := domain.RequirementTrigger{ + Expression: expr, + } + match, err := r.IsMatch(&domain.Appeal{}) + assert.ErrorContains(t, err, fmt.Sprintf("evaluating expression %q:", expr)) + assert.False(t, match) + }) + + t.Run("should return error if expression result is not boolean", func(t *testing.T) { + expr := "$appeal.resource.details.foo" + appeal := &domain.Appeal{ + Resource: &domain.Resource{ + Details: map[string]interface{}{ + "foo": "bar", + }, + }, + } + r := domain.RequirementTrigger{ + Expression: expr, + } + match, err := r.IsMatch(appeal) + assert.ErrorContains(t, err, fmt.Sprintf("expression %q did not evaluate to a boolean, evaluated value: %q", expr, "bar")) + assert.False(t, match) + }) + + t.Run("test trigger matching", func(t *testing.T) { + testCases := []struct { + name string + trigger domain.RequirementTrigger + appeal *domain.Appeal + expectedMatch bool + }{ + { + name: "using provider_type and resource_type", + trigger: domain.RequirementTrigger{ + ProviderType: "my-provider", + ResourceType: "my-resource", + }, + appeal: &domain.Appeal{ + Resource: &domain.Resource{ + Type: "my-resource", + ProviderType: "my-provider", + }, + }, + expectedMatch: true, + }, + { + name: "check if resource_urn is matched expression", + trigger: domain.RequirementTrigger{ + Expression: `$appeal.resource.urn == "urn:my-resource:123"`, + }, + appeal: &domain.Appeal{ + Resource: &domain.Resource{ + URN: "urn:my-resource:123", + }, + }, + expectedMatch: true, + }, + { + name: "check if resource_urn is matched expression (false condition)", + trigger: domain.RequirementTrigger{ + Expression: `$appeal.resource.urn == "urn:my-resource:123"`, + }, + appeal: &domain.Appeal{ + Resource: &domain.Resource{ + URN: "urn:my-resource:123456", + }, + }, + expectedMatch: false, + }, + { + name: "expression returns boolean value", + trigger: domain.RequirementTrigger{ + Expression: `$appeal.resource.details.foo`, + }, + appeal: &domain.Appeal{ + Resource: &domain.Resource{ + Details: map[string]interface{}{ + "foo": true, + }, + }, + }, + expectedMatch: true, + }, + { + name: "using conditions", + trigger: domain.RequirementTrigger{ + Conditions: []*domain.Condition{ + { + Field: "$resource.type", + Match: &domain.MatchCondition{ + Eq: "test-resource-type", + }, + }, + }, + }, + appeal: &domain.Appeal{ + Resource: &domain.Resource{ + Type: "test-resource-type", + }, + }, + expectedMatch: true, + }, + { + name: "should return false on matched conditions and not matched expression", + trigger: domain.RequirementTrigger{ + Conditions: []*domain.Condition{ + { + Field: "$resource.type", + Match: &domain.MatchCondition{ + Eq: "test-resource-type-incorrect", + }, + }, + }, + Expression: `$appeal.resource.type == "test-resource-type"`, + }, + appeal: &domain.Appeal{ + Resource: &domain.Resource{ + Type: "test-resource-type", + }, + }, + expectedMatch: false, + }, + { + name: "should return false on not matched conditions and matched expression", + trigger: domain.RequirementTrigger{ + Conditions: []*domain.Condition{ + { + Field: "$resource.type", + Match: &domain.MatchCondition{ + Eq: "test-resource-type", + }, + }, + }, + Expression: `$appeal.resource.type == "test-resource-type-incorrect"`, + }, + appeal: &domain.Appeal{ + Resource: &domain.Resource{ + Type: "test-resource-type", + }, + }, + expectedMatch: false, + }, + { + name: "should return true on matched conditions and expression", + trigger: domain.RequirementTrigger{ + Conditions: []*domain.Condition{ + { + Field: "$resource.type", + Match: &domain.MatchCondition{ + Eq: "test-resource-type", + }, + }, + }, + Expression: `$appeal.resource.provider_type == "test-provider-type"`, + }, + appeal: &domain.Appeal{ + Resource: &domain.Resource{ + Type: "test-resource-type", + ProviderType: "test-provider-type", + }, + }, + expectedMatch: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + match, err := tc.trigger.IsMatch(tc.appeal) + assert.NoError(t, err) + assert.Equal(t, tc.expectedMatch, match) + }) + } + }) +} + type brokenType struct{} func (c *brokenType) MarshalJSON() ([]byte, error) {