Skip to content

Commit b3cac85

Browse files
anjaliagg9791anjaliagg9791
committed
add test cases
1 parent cf8c009 commit b3cac85

File tree

8 files changed

+963
-64
lines changed

8 files changed

+963
-64
lines changed

‎core/appeal/service.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -894,11 +894,10 @@ func (s *Service) UpdateApproval(ctx context.Context, approvalAction domain.Appr
894894
}
895895
}
896896

897-
isStepValid := true
898897
isSelfApprovalNotAllowed := false
899898
policyStep := appeal.Policy.GetStepByName(currentApproval.Name)
900899
if policyStep == nil {
901-
isStepValid = false
900+
isStepValid := false
902901
if appeal.Policy.HasCustomSteps() {
903902
for _, ap := range appeal.Approvals {
904903
if ap.Name == currentApproval.Name {

‎core/appeal/service_test.go‎

Lines changed: 208 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -5311,11 +5311,13 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
53115311
name: "approve",
53125312
expectedApprovalAction: validApprovalActionParam,
53135313
expectedAppealDetails: &domain.Appeal{
5314-
ID: validApprovalActionParam.AppealID,
5315-
AccountID: "[email protected]",
5316-
CreatedBy: creator,
5317-
ResourceID: "1",
5318-
Role: "test-role",
5314+
ID: validApprovalActionParam.AppealID,
5315+
AccountID: "[email protected]",
5316+
CreatedBy: creator,
5317+
ResourceID: "1",
5318+
Role: "test-role",
5319+
PolicyID: "test-policy-id",
5320+
PolicyVersion: 1,
53195321
Resource: &domain.Resource{
53205322
ID: "1",
53215323
URN: "urn",
@@ -5351,13 +5353,15 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
53515353
},
53525354
},
53535355
expectedResult: &domain.Appeal{
5354-
ID: validApprovalActionParam.AppealID,
5355-
AccountID: "[email protected]",
5356-
CreatedBy: creator,
5357-
ResourceID: "1",
5358-
Role: "test-role",
5359-
Resource: dummyResource,
5360-
Status: domain.AppealStatusApproved,
5356+
ID: validApprovalActionParam.AppealID,
5357+
AccountID: "[email protected]",
5358+
CreatedBy: creator,
5359+
ResourceID: "1",
5360+
Role: "test-role",
5361+
PolicyID: "test-policy-id",
5362+
PolicyVersion: 1,
5363+
Resource: dummyResource,
5364+
Status: domain.AppealStatusApproved,
53615365
Approvals: []*domain.Approval{
53625366
{
53635367
Name: "approval_0",
@@ -5428,11 +5432,13 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
54285432
Reason: "test-reason",
54295433
},
54305434
expectedAppealDetails: &domain.Appeal{
5431-
ID: validApprovalActionParam.AppealID,
5432-
AccountID: "[email protected]",
5433-
CreatedBy: creator,
5434-
ResourceID: "1",
5435-
Role: "test-role",
5435+
ID: validApprovalActionParam.AppealID,
5436+
AccountID: "[email protected]",
5437+
CreatedBy: creator,
5438+
ResourceID: "1",
5439+
Role: "test-role",
5440+
PolicyID: "test-policy-id",
5441+
PolicyVersion: 1,
54365442
Resource: &domain.Resource{
54375443
ID: "1",
54385444
URN: "urn",
@@ -5455,11 +5461,13 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
54555461
},
54565462
},
54575463
expectedResult: &domain.Appeal{
5458-
ID: validApprovalActionParam.AppealID,
5459-
AccountID: "[email protected]",
5460-
CreatedBy: creator,
5461-
ResourceID: "1",
5462-
Role: "test-role",
5464+
ID: validApprovalActionParam.AppealID,
5465+
AccountID: "[email protected]",
5466+
CreatedBy: creator,
5467+
ResourceID: "1",
5468+
Role: "test-role",
5469+
PolicyID: "test-policy-id",
5470+
PolicyVersion: 1,
54635471
Resource: &domain.Resource{
54645472
ID: "1",
54655473
URN: "urn",
@@ -5506,11 +5514,13 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
55065514
Action: domain.AppealActionNameReject,
55075515
},
55085516
expectedAppealDetails: &domain.Appeal{
5509-
ID: validApprovalActionParam.AppealID,
5510-
AccountID: "[email protected]",
5511-
CreatedBy: creator,
5512-
ResourceID: "1",
5513-
Role: "test-role",
5517+
ID: validApprovalActionParam.AppealID,
5518+
AccountID: "[email protected]",
5519+
CreatedBy: creator,
5520+
ResourceID: "1",
5521+
Role: "test-role",
5522+
PolicyID: "test-policy-id",
5523+
PolicyVersion: 1,
55145524
Resource: &domain.Resource{
55155525
ID: "1",
55165526
URN: "urn",
@@ -5538,11 +5548,13 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
55385548
},
55395549
},
55405550
expectedResult: &domain.Appeal{
5541-
ID: validApprovalActionParam.AppealID,
5542-
AccountID: "[email protected]",
5543-
CreatedBy: creator,
5544-
ResourceID: "1",
5545-
Role: "test-role",
5551+
ID: validApprovalActionParam.AppealID,
5552+
AccountID: "[email protected]",
5553+
CreatedBy: creator,
5554+
ResourceID: "1",
5555+
Role: "test-role",
5556+
PolicyID: "test-policy-id",
5557+
PolicyVersion: 1,
55465558
Resource: &domain.Resource{
55475559
ID: "1",
55485560
URN: "urn",
@@ -5693,9 +5705,10 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
56935705
GetByID(h.ctxMatcher, validApprovalActionParam.AppealID).
56945706
Return(tc.expectedAppealDetails, nil).Once()
56955707

5696-
if tc.expectedApprovalAction.Action == domain.AppealActionNameApprove &&
5697-
tc.expectedAppealDetails.Policy == nil {
5708+
if tc.expectedAppealDetails.Policy == nil && tc.expectedApprovalAction.Action == domain.AppealActionNameApprove {
56985709
mockPolicy := &domain.Policy{
5710+
ID: tc.expectedAppealDetails.PolicyID,
5711+
Version: tc.expectedAppealDetails.PolicyVersion,
56995712
Steps: []*domain.Step{
57005713
{Name: "approval_0"},
57015714
{Name: "approval_1"},
@@ -5725,7 +5738,17 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
57255738
h.mockProviderService.EXPECT().GrantAccess(mock.Anything, grantArgMatcher(*tc.expectedGrant)).Return(nil).Once()
57265739
}
57275740

5728-
h.mockRepository.EXPECT().Update(h.ctxMatcher, tc.expectedResult).Return(nil).Once()
5741+
h.mockRepository.EXPECT().Update(h.ctxMatcher, mock.MatchedBy(func(appeal *domain.Appeal) bool {
5742+
// Compare without Policy field as it may not be set during Update
5743+
return appeal.ID == tc.expectedResult.ID &&
5744+
appeal.ResourceID == tc.expectedResult.ResourceID &&
5745+
appeal.PolicyID == tc.expectedResult.PolicyID &&
5746+
appeal.PolicyVersion == tc.expectedResult.PolicyVersion &&
5747+
appeal.Status == tc.expectedResult.Status &&
5748+
appeal.AccountID == tc.expectedResult.AccountID &&
5749+
appeal.Role == tc.expectedResult.Role &&
5750+
appeal.CreatedBy == tc.expectedResult.CreatedBy
5751+
})).Return(nil).Once()
57295752
h.mockNotifier.EXPECT().Notify(h.ctxMatcher, mock.Anything).Return(nil).Once()
57305753
h.mockAuditLogger.EXPECT().Log(h.ctxMatcher, mock.Anything, mock.Anything).
57315754
Return(nil).Once()
@@ -5851,9 +5874,10 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
58515874
GetByID(h.ctxMatcher, validApprovalActionParam.AppealID).
58525875
Return(tc.expectedAppealDetails, nil).Once()
58535876

5854-
if tc.expectedApprovalAction.Action == domain.AppealActionNameApprove &&
5855-
tc.expectedAppealDetails.Policy == nil {
5877+
if tc.expectedAppealDetails.Policy == nil && tc.expectedApprovalAction.Action == domain.AppealActionNameApprove {
58565878
mockPolicy := &domain.Policy{
5879+
ID: tc.expectedAppealDetails.PolicyID,
5880+
Version: tc.expectedAppealDetails.PolicyVersion,
58575881
Steps: []*domain.Step{
58585882
{Name: "approval_0"},
58595883
{Name: "approval_1"},
@@ -5883,7 +5907,17 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
58835907
h.mockProviderService.EXPECT().GrantAccess(mock.Anything, grantArgMatcher(*tc.expectedGrant)).Return(nil).Once()
58845908
}
58855909

5886-
h.mockRepository.EXPECT().Update(h.ctxMatcher, tc.expectedResult).Return(nil).Once()
5910+
h.mockRepository.EXPECT().Update(h.ctxMatcher, mock.MatchedBy(func(appeal *domain.Appeal) bool {
5911+
// Compare without Policy field as it may not be set during Update
5912+
return appeal.ID == tc.expectedResult.ID &&
5913+
appeal.ResourceID == tc.expectedResult.ResourceID &&
5914+
appeal.PolicyID == tc.expectedResult.PolicyID &&
5915+
appeal.PolicyVersion == tc.expectedResult.PolicyVersion &&
5916+
appeal.Status == tc.expectedResult.Status &&
5917+
appeal.AccountID == tc.expectedResult.AccountID &&
5918+
appeal.Role == tc.expectedResult.Role &&
5919+
appeal.CreatedBy == tc.expectedResult.CreatedBy
5920+
})).Return(nil).Once()
58875921
h.mockNotifier.EXPECT().Notify(h.ctxMatcher, mock.Anything).Return(nil).Once()
58885922
h.mockAuditLogger.EXPECT().Log(h.ctxMatcher, mock.Anything, mock.Anything).
58895923
Return(nil).Once()
@@ -5955,6 +5989,8 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
59555989
if tc.expectedApprovalAction.Action == domain.AppealActionNameApprove &&
59565990
tc.expectedAppealDetails.Policy == nil {
59575991
mockPolicy := &domain.Policy{
5992+
ID: tc.expectedAppealDetails.PolicyID,
5993+
Version: tc.expectedAppealDetails.PolicyVersion,
59585994
Steps: []*domain.Step{
59595995
{Name: "approval_0"},
59605996
{Name: "approval_1", DontAllowSelfApproval: true},
@@ -5963,6 +5999,7 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
59635999
h.mockPolicyService.EXPECT().
59646000
GetOne(mock.Anything, tc.expectedAppealDetails.PolicyID, tc.expectedAppealDetails.PolicyVersion).
59656001
Return(mockPolicy, nil).Once()
6002+
tc.expectedAppealDetails.Policy = mockPolicy
59666003
}
59676004

59686005
h.mockProviderService.EXPECT().
@@ -5993,6 +6030,139 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
59936030
}
59946031
})
59956032

6033+
s.Run("should handle custom steps with self-approval check", func() {
6034+
h := newServiceTestHelper()
6035+
defer h.assertExpectations(s.T())
6036+
6037+
appealID := uuid.New().String()
6038+
userActor := "[email protected]"
6039+
6040+
testAppeal := &domain.Appeal{
6041+
ID: appealID,
6042+
CreatedBy: userActor,
6043+
Status: domain.AppealStatusPending,
6044+
PolicyID: "policy-1",
6045+
PolicyVersion: 1,
6046+
ResourceID: "resource-1",
6047+
Resource: &domain.Resource{
6048+
ID: "resource-1",
6049+
URN: "test:resource:1",
6050+
Name: "Test Resource",
6051+
ProviderType: "test-provider",
6052+
},
6053+
Policy: &domain.Policy{
6054+
ID: "policy-1",
6055+
Version: 1,
6056+
CustomSteps: &domain.CustomSteps{
6057+
Type: "custom",
6058+
},
6059+
},
6060+
Approvals: []*domain.Approval{
6061+
{
6062+
ID: "approval-1",
6063+
Name: "custom_approval",
6064+
Index: 0,
6065+
Status: domain.ApprovalStatusPending,
6066+
DontAllowSelfApproval: true,
6067+
Approvers: []string{userActor},
6068+
},
6069+
},
6070+
}
6071+
6072+
action := domain.ApprovalAction{
6073+
AppealID: appealID,
6074+
ApprovalName: "custom_approval",
6075+
Actor: userActor,
6076+
Action: "approve",
6077+
}
6078+
6079+
h.mockRepository.EXPECT().GetByID(h.ctxMatcher, appealID).Return(testAppeal, nil).Once()
6080+
6081+
actualResult, actualError := h.service.UpdateApproval(context.Background(), action)
6082+
6083+
s.Nil(actualResult)
6084+
s.ErrorIs(actualError, appeal.ErrSelfApprovalNotAllowed)
6085+
})
6086+
6087+
s.Run("should allow custom step approval when self-approval is allowed", func() {
6088+
h := newServiceTestHelper()
6089+
defer h.assertExpectations(s.T())
6090+
6091+
appealID := uuid.New().String()
6092+
userActor := "[email protected]"
6093+
6094+
testAppeal := &domain.Appeal{
6095+
ID: appealID,
6096+
CreatedBy: userActor,
6097+
Status: domain.AppealStatusPending,
6098+
PolicyID: "policy-1",
6099+
PolicyVersion: 1,
6100+
ResourceID: "resource-1",
6101+
Resource: &domain.Resource{
6102+
ID: "resource-1",
6103+
URN: "test:resource:1",
6104+
Name: "Test Resource",
6105+
ProviderType: "test-provider",
6106+
},
6107+
Policy: &domain.Policy{
6108+
ID: "policy-1",
6109+
Version: 1,
6110+
CustomSteps: &domain.CustomSteps{
6111+
Type: "custom",
6112+
},
6113+
},
6114+
Approvals: []*domain.Approval{
6115+
{
6116+
ID: "approval-1",
6117+
Name: "custom_approval",
6118+
Index: 0,
6119+
Status: domain.ApprovalStatusPending,
6120+
DontAllowSelfApproval: false,
6121+
Approvers: []string{userActor},
6122+
},
6123+
},
6124+
}
6125+
6126+
action := domain.ApprovalAction{
6127+
AppealID: appealID,
6128+
ApprovalName: "custom_approval",
6129+
Actor: userActor,
6130+
Action: "approve",
6131+
Reason: "approved",
6132+
}
6133+
6134+
h.mockRepository.EXPECT().GetByID(h.ctxMatcher, appealID).Return(testAppeal, nil).Once()
6135+
h.mockProviderService.EXPECT().
6136+
IsExclusiveRoleAssignment(mock.Anything, mock.Anything, mock.Anything).
6137+
Return(false).Once()
6138+
h.mockGrantService.EXPECT().
6139+
List(h.ctxMatcher, domain.ListGrantsFilter{
6140+
AccountIDs: []string{testAppeal.AccountID},
6141+
ResourceIDs: []string{testAppeal.ResourceID},
6142+
Statuses: []string{string(domain.GrantStatusActive)},
6143+
Permissions: testAppeal.Permissions,
6144+
}).Return([]domain.Grant{}, nil).Once()
6145+
h.mockGrantService.EXPECT().
6146+
Prepare(mock.Anything, mock.Anything).Return(&domain.Grant{
6147+
Status: domain.GrantStatusActive,
6148+
AccountID: userActor,
6149+
AccountType: domain.DefaultAppealAccountType,
6150+
ResourceID: "resource-1",
6151+
Role: "test-role",
6152+
IsPermanent: true,
6153+
}, nil).Once()
6154+
h.mockProviderService.EXPECT().GetDependencyGrants(mock.Anything, mock.AnythingOfType("domain.Grant")).Return(nil, nil).Once()
6155+
h.mockProviderService.EXPECT().GrantAccess(mock.Anything, mock.Anything).Return(nil).Once()
6156+
h.mockRepository.EXPECT().Update(h.ctxMatcher, mock.Anything).Return(nil).Once()
6157+
h.mockNotifier.EXPECT().Notify(h.ctxMatcher, mock.Anything).Return(nil).Once()
6158+
h.mockAuditLogger.EXPECT().Log(h.ctxMatcher, mock.Anything, mock.Anything).Return(nil).Once()
6159+
6160+
actualResult, actualError := h.service.UpdateApproval(context.Background(), action)
6161+
6162+
s.NoError(actualError)
6163+
s.NotNil(actualResult)
6164+
})
6165+
59966166
}
59976167

59986168
func (s *ServiceTestSuite) TestGrantAccessToProvider() {

‎core/policy/service.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -146,7 +146,7 @@ func (s *Service) Create(ctx context.Context, p *domain.Policy) error {
146146
}
147147

148148
if p.HasCustomSteps() {
149-
if err := s.decryptAppealMetadata(p); err != nil {
149+
if err := s.decryptCustomSteps(p); err != nil {
150150
return err
151151
}
152152
}

0 commit comments

Comments
 (0)