fix: bug in appeal creation (#390)

- bug: If a final approval step of the policy gets skipped or allow_failed is true, then the appeal gets auto_approved, but the grant will not get created and no access is provided.
- fix bug by adding a check for last step
- add test-short cmd to makefile to be able to run quick service layer tests by skipping docker led store test cases.

* chore: add vendor dir to .gitignore

---------

Co-authored-by: Ravi Suhag <suhag.ravi@gmail.com>

.gitignore

@@ -13,7 +13,7 @@
 *.out
 
 # # Dependency directories (remove the comment below to include it)
-# vendor/
+vendor/
 
 # Config
 /config.yaml

Makefile

@@ -34,7 +34,11 @@ clean: tidy ## Clean the build artifacts
 	@rm -rf $coverage.out ${BUILD_DIR}
 
 test:  ## Run the tests
-	go test ./... -race -coverprofile=coverage.out
+	go test ./ ... -race -coverprofile=coverage.out
+
+test-short:
+	@echo "Running short tests by disabling store tests..."
+	go test ./... -race -short -coverprofile=coverage.out
 
 coverage: test ## Print the code coverage
 	@echo "Generating coverage report..."

core/appeal/service.go

@@ -276,7 +276,7 @@ func (s *Service) Create(ctx context.Context, appeals []*domain.Appeal, opts ...
 		appeal.Policy = nil
 
 		for _, approval := range appeal.Approvals {
-			if approval.Index == len(appeal.Approvals)-1 && approval.Status == domain.ApprovalStatusApproved {
+			if approval.Index == len(appeal.Approvals)-1 && (approval.Status == domain.ApprovalStatusApproved || appeal.Status == domain.AppealStatusApproved) {
 				newGrant, revokedGrant, err := s.prepareGrant(ctx, appeal)
 				if err != nil {
 					return fmt.Errorf("preparing grant: %w", err)

core/appeal/service_test.go

@@ -1153,6 +1153,13 @@ func (s *ServiceTestSuite) TestCreateAppeal__WithExistingAppealAndWithAutoApprov
 					AllowFailed: false,
 					ApproveIf:   "1==1",
 				},
+				{
+					Name:        "step_2",
+					Strategy:    "manual",
+					When:        "1==0",
+					AllowFailed: false,
+					Approvers:   []string{"test-approver@email.com"},
+				},
 			},
 			IAM: &domain.IAMConfig{
 				Provider: "http",
@@ -1188,6 +1195,13 @@ func (s *ServiceTestSuite) TestCreateAppeal__WithExistingAppealAndWithAutoApprov
 					Status:        domain.ApprovalStatusApproved,
 					PolicyID:      "policy_1",
 					PolicyVersion: 1,
+				}, {
+					Name:          "step_2",
+					Index:         1,
+					Status:        domain.ApprovalStatusSkipped,
+					PolicyID:      "policy_1",
+					PolicyVersion: 1,
+					Approvers:     []string{"test-approver@email.com"},
 				},
 			},
 			Grant: &domain.Grant{
@@ -1224,6 +1238,14 @@ func (s *ServiceTestSuite) TestCreateAppeal__WithExistingAppealAndWithAutoApprov
 					Status:        domain.ApprovalStatusApproved,
 					PolicyID:      "policy_1",
 					PolicyVersion: 1,
+				}, {
+					ID:            "2",
+					Name:          "step_2",
+					Index:         1,
+					Status:        domain.ApprovalStatusSkipped,
+					PolicyID:      "policy_1",
+					PolicyVersion: 1,
+					Approvers:     []string{"test-approver@email.com"},
 				},
 			},
 			Grant: &domain.Grant{
@@ -1271,6 +1293,14 @@ func (s *ServiceTestSuite) TestCreateAppeal__WithExistingAppealAndWithAutoApprov
 			OrderBy:     []string{"updated_at:desc"},
 		}).
 		Return(expectedExistingGrants, nil).Once()
+	s.mockGrantService.EXPECT().
+		List(mock.AnythingOfType("*context.emptyCtx"), domain.ListGrantsFilter{
+			Statuses:    []string{string(domain.GrantStatusActive)},
+			AccountIDs:  []string{appeals[0].AccountID},
+			ResourceIDs: []string{appeals[0].ResourceID},
+			Permissions: []string{"test-permission"},
+		}).
+		Return(expectedExistingGrants, nil).Once()
 	s.mockProviderService.On("ValidateAppeal", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil)
 	s.mockProviderService.On("GetPermissions", mock.Anything, mock.Anything, mock.Anything, mock.Anything).
 		Return([]interface{}{"test-permission"}, nil)