Skip to content

Commit

Permalink
fix(appeal): handle duplicate active grants (#120)
Browse files Browse the repository at this point in the history
* fix(appeal): handle duplicate active grants

* chore: refactor error handling

* chore: remove unused type

* fix(appeal): handle duplicate active grants

* chore: remove unused error

* feat(appeal): update repository test

* chore: resolve comment
  • Loading branch information
abhishekv24 authored Feb 15, 2024
1 parent 57cd794 commit 4cdfc56
Show file tree
Hide file tree
Showing 4 changed files with 63 additions and 6 deletions.
6 changes: 4 additions & 2 deletions core/appeal/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -525,8 +525,10 @@ func (s *Service) UpdateApproval(ctx context.Context, approvalAction domain.Appr
}

if err := s.Update(ctx, appeal); err != nil {
if err := s.providerService.RevokeAccess(ctx, *appeal.Grant); err != nil {
return nil, fmt.Errorf("revoking access: %w", err)
if !errors.Is(err, domain.ErrDuplicateActiveGrant) {
if err := s.providerService.RevokeAccess(ctx, *appeal.Grant); err != nil {
return nil, fmt.Errorf("revoking access: %w", err)
}
}
return nil, fmt.Errorf("updating appeal: %w", err)
}
Expand Down
2 changes: 2 additions & 0 deletions domain/grant.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ const (
GrantExpirationReasonDormant = "grant/access hasn't been used for a while"
)

var ErrDuplicateActiveGrant = errors.New("grant already exists")

type Grant struct {
ID string `json:"id" yaml:"id"`
Status GrantStatus `json:"status" yaml:"status"`
Expand Down
10 changes: 10 additions & 0 deletions internal/store/postgres/appeal_repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,16 @@ import (
"github.com/goto/guardian/domain"
"github.com/goto/guardian/internal/store/postgres/model"
"github.com/goto/guardian/utils"
"github.com/jackc/pgx/v5/pgconn"
"gorm.io/gorm"
"gorm.io/gorm/clause"
)

const (
pgUniqueViolationErrorCode = "23505"
grantUniqueConstraintName = "unique_active_grants_index"
)

var (
AppealStatusDefaultSort = []string{
domain.AppealStatusPending,
Expand Down Expand Up @@ -135,6 +141,10 @@ func (r *AppealRepository) Update(ctx context.Context, a *domain.Appeal) error {

return r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error {
if err := tx.Omit("Approvals.Approvers").Session(&gorm.Session{FullSaveAssociations: true}).Save(&m).Error; err != nil {
var pgError *pgconn.PgError
if errors.As(err, &pgError) && pgError.Code == pgUniqueViolationErrorCode && pgError.ConstraintName == grantUniqueConstraintName {
return domain.ErrDuplicateActiveGrant
}
return err
}

Expand Down
51 changes: 47 additions & 4 deletions internal/store/postgres/appeal_repository_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@ import (

type AppealRepositoryTestSuite struct {
suite.Suite
store *postgres.Store
pool *dockertest.Pool
resource *dockertest.Resource
repository *postgres.AppealRepository
store *postgres.Store
pool *dockertest.Pool
resource *dockertest.Resource
repository *postgres.AppealRepository
grantRepository *postgres.GrantRepository

dummyProvider *domain.Provider
dummyPolicy *domain.Policy
Expand All @@ -40,6 +41,8 @@ func (s *AppealRepositoryTestSuite) SetupSuite() {

s.repository = postgres.NewAppealRepository(s.store.DB())

s.grantRepository = postgres.NewGrantRepository(s.store.DB())

s.dummyPolicy = &domain.Policy{
ID: "policy_test",
Version: 1,
Expand Down Expand Up @@ -455,6 +458,46 @@ func (s *AppealRepositoryTestSuite) TestUpdate() {
s.EqualError(actualError, "json: unsupported type: chan int")
})

s.Run("should return error if grant already exists", func() {
pendingAppeal := &domain.Appeal{
ID: uuid.NewString(),
ResourceID: s.dummyResource.ID,
PolicyID: s.dummyPolicy.ID,
PolicyVersion: s.dummyPolicy.Version,
AccountID: "[email protected]",
AccountType: domain.DefaultAppealAccountType,
Role: "role_test",
Permissions: []string{"permission_test"},
CreatedBy: "[email protected]",
Status: domain.AppealStatusPending,
}
dummyAppealError := s.repository.BulkUpsert(context.Background(), []*domain.Appeal{pendingAppeal})
s.Require().NoError(dummyAppealError)

dummyGrants := &domain.Grant{
Status: domain.GrantStatusActive,
AccountID: "[email protected]",
ResourceID: s.dummyResource.ID,
Permissions: []string{"permission_test"},
}

dummyGrantError := s.grantRepository.BulkUpsert(context.Background(), []*domain.Grant{dummyGrants})
s.Require().NoError(dummyGrantError)

appealApprovalErr := pendingAppeal.Approve()
s.Require().NoError(appealApprovalErr)

pendingAppeal.Grant = &domain.Grant{ //new duplicate grant
Status: domain.GrantStatusActive,
AccountID: "[email protected]",
ResourceID: s.dummyResource.ID,
Permissions: []string{"permission_test"},
}

err := s.repository.Update(context.Background(), pendingAppeal)
s.ErrorIs(err, domain.ErrDuplicateActiveGrant)
})

s.Run("should return nil on success", func() {
dummyAppeals := []*domain.Appeal{
{
Expand Down

0 comments on commit 4cdfc56

Please sign in to comment.