Skip to content

Commit

Permalink
fix: bug fixes (#135)
Browse files Browse the repository at this point in the history 
* chore(policy): set appeal default values before validating additional appeals

* fix(appeal): transform revoke information domain-model

* fix(policy): store requirements config properly

* fix: ignore permission exists when granting, and permission not found when revoking access to bigquery and gcloud iam

* fix(cmd): read output flag from persistent flags
  • Loading branch information
@rahmatrhd
rahmatrhd authored Feb 14, 2022
1 parent 0797693 commit 08553e3
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 26 deletions.
21 changes: 15 additions & 6 deletions cmd/resource.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ func ResourceCmd(c *app.CLIConfig, adapter handlerv1beta1.ProtoAdapter) *cobra.C
}

func listResourcesCmd(c *app.CLIConfig, adapter handlerv1beta1.ProtoAdapter) *cobra.Command {
var providerType, providerURN, resourceType, resourceURN, name, format string
var providerType, providerURN, resourceType, resourceURN, name string
var isDeleted bool
var details []string

Expand Down Expand Up @@ -78,6 +78,11 @@ func listResourcesCmd(c *app.CLIConfig, adapter handlerv1beta1.ProtoAdapter) *co
return err
}

format, err := cmd.Flags().GetString("output")
if err != nil {
return err
}

if format != "" {
var resources []*domain.Resource
for _, r := range res.GetResources() {
Expand Down Expand Up @@ -121,20 +126,18 @@ func listResourcesCmd(c *app.CLIConfig, adapter handlerv1beta1.ProtoAdapter) *co
cmd.Flags().StringVarP(&name, "name", "n", "", "Filter by name")
cmd.Flags().StringArrayVarP(&details, "details", "d", nil, "Filter by details object values. Example: --details=key1.key2:value")
cmd.Flags().BoolVarP(&isDeleted, "deleted", "D", false, "Show deleted resources")
cmd.Flags().StringVarP(&format, "format", "f", "", "Format of output - json yaml prettyjson etc")

return cmd
}

func viewResourceCmd(c *app.CLIConfig, adapter handlerv1beta1.ProtoAdapter) *cobra.Command {
var format string
var metadata bool

cmd := &cobra.Command{
Use: "view",
Short: "View a resource details",
Example: heredoc.Doc(`
$ guardian resource view <resource-id> --format=json --metadata=true
$ guardian resource view <resource-id> --output=json --metadata=true
`),
Annotations: map[string]string{
"group:core": "true",
Expand All @@ -159,8 +162,14 @@ func viewResourceCmd(c *app.CLIConfig, adapter handlerv1beta1.ProtoAdapter) *cob
return err
}

format, err := cmd.Flags().GetString("output")
if err != nil {
return err
}

if format != "" {
r := adapter.FromResourceProto(res.GetResource())
spinner.Stop()
if err := printer.Text(r, format); err != nil {
return fmt.Errorf("failed to parse resources: %v", err)
}
Expand All @@ -178,6 +187,7 @@ func viewResourceCmd(c *app.CLIConfig, adapter handlerv1beta1.ProtoAdapter) *cob
r.GetName(),
})

spinner.Stop()
printer.Table(os.Stdout, report)
}

Expand All @@ -196,12 +206,12 @@ func viewResourceCmd(c *app.CLIConfig, adapter handlerv1beta1.ProtoAdapter) *cob
}
}

spinner.Stop()
return nil
},
}

cmd.Flags().BoolVarP(&metadata, "metadata", "m", false, "Set if you want to see metadata")
cmd.Flags().StringVarP(&format, "format", "f", "", "Format of output - json yaml prettyjson etc")
return cmd
}

Expand Down Expand Up @@ -249,7 +259,6 @@ func setResourceCmd(c *app.CLIConfig, adapter handlerv1beta1.ProtoAdapter) *cobr
}

spinner.Stop()

fmt.Println("Successfully updated metadata")

return nil
Expand Down
1 change: 1 addition & 0 deletions core/policy/service.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,7 @@ func (s *Service) validateRequirements(requirements []*domain.Requirement) error
Role: aa.Role,
Options: aa.Options,
}
appeal.SetDefaults()
if err := s.providerService.ValidateAppeal(appeal, provider); err != nil {
return fmt.Errorf("requirement[%v].appeals[%v]: %w", i, j, err)
}
Expand Down
1 change: 1 addition & 0 deletions core/policy/service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,7 @@ func (s *ServiceTestSuite) TestPolicyRequirements() {
Role: aa.Role,
Options: aa.Options,
}
expectedAppeal.SetDefaults()
s.mockProviderService.
On("ValidateAppeal", expectedAppeal, expectedProvider).
Return(nil).
Expand Down
13 changes: 13 additions & 0 deletions plugins/providers/bigquery/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package bigquery

import (
"context"
"errors"
"strings"

"github.com/mitchellh/mapstructure"
Expand Down Expand Up @@ -118,6 +119,9 @@ func (p *Provider) GrantAccess(pc *domain.ProviderConfig, a *domain.Appeal) erro

for _, p := range permissions {
if err := bqClient.GrantDatasetAccess(ctx, d, a.AccountID, string(p)); err != nil {
if errors.Is(err, ErrPermissionAlreadyExists) {
return nil
}
return err
}
}
Expand All @@ -131,6 +135,9 @@ func (p *Provider) GrantAccess(pc *domain.ProviderConfig, a *domain.Appeal) erro

for _, p := range permissions {
if err := bqClient.GrantTableAccess(ctx, t, a.AccountType, a.AccountID, string(p)); err != nil {
if errors.Is(err, ErrPermissionAlreadyExists) {
return nil
}
return err
}
}
Expand Down Expand Up @@ -169,6 +176,9 @@ func (p *Provider) RevokeAccess(pc *domain.ProviderConfig, a *domain.Appeal) err

for _, p := range permissions {
if err := bqClient.RevokeDatasetAccess(ctx, d, a.AccountID, string(p)); err != nil {
if errors.Is(err, ErrPermissionNotFound) {
return nil
}
return err
}
}
Expand All @@ -182,6 +192,9 @@ func (p *Provider) RevokeAccess(pc *domain.ProviderConfig, a *domain.Appeal) err

for _, p := range permissions {
if err := bqClient.RevokeTableAccess(ctx, t, a.AccountType, a.AccountID, string(p)); err != nil {
if errors.Is(err, ErrPermissionNotFound) {
return nil
}
return err
}
}
Expand Down
17 changes: 15 additions & 2 deletions plugins/providers/gcloudiam/provider.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package gcloudiam

import (
"errors"
"fmt"
"strings"

Expand Down Expand Up @@ -74,7 +75,13 @@ func (p *Provider) GrantAccess(pc *domain.ProviderConfig, a *domain.Appeal) erro
}

if a.Resource.Type == ResourceTypeProject || a.Resource.Type == ResourceTypeOrganization {
return client.GrantAccess(a.AccountType, a.AccountID, a.Role)
if err := client.GrantAccess(a.AccountType, a.AccountID, a.Role); err != nil {
if errors.Is(err, ErrPermissionAlreadyExists) {
return nil
}
return err
}
return nil
}

return ErrInvalidResourceType
Expand All @@ -92,7 +99,13 @@ func (p *Provider) RevokeAccess(pc *domain.ProviderConfig, a *domain.Appeal) err
}

if a.Resource.Type == ResourceTypeProject || a.Resource.Type == ResourceTypeOrganization {
return client.RevokeAccess(a.AccountType, a.AccountID, a.Role)
if err := client.RevokeAccess(a.AccountType, a.AccountID, a.Role); err != nil {
if errors.Is(err, ErrPermissionNotFound) {
return nil
}
return err
}
return nil
}

return ErrInvalidResourceType
Expand Down
6 changes: 6 additions & 0 deletions store/model/appeal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ func (m *Appeal) FromDomain(a *domain.Appeal) error {
m.Options = datatypes.JSON(options)
m.Labels = datatypes.JSON(labels)
m.Details = datatypes.JSON(details)
m.RevokedBy = a.RevokedBy
m.RevokedAt = a.RevokedAt
m.RevokeReason = a.RevokeReason
m.Approvals = approvals
m.CreatedAt = a.CreatedAt
m.UpdatedAt = a.UpdatedAt
Expand Down Expand Up @@ -172,6 +175,9 @@ func (m *Appeal) ToDomain() (*domain.Appeal, error) {
Role: m.Role,
Options: options,
Details: details,
RevokedBy: m.RevokedBy,
RevokedAt: m.RevokedAt,
RevokeReason: m.RevokeReason,
Labels: labels,
Approvals: approvals,
Resource: resource,
Expand Down
49 changes: 32 additions & 17 deletions store/model/policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@ import (

// Policy is the database model for policy
type Policy struct {
ID string `gorm:"primaryKey"`
Version uint `gorm:"primaryKey"`
Description string
Steps datatypes.JSON
Labels datatypes.JSON
IAM datatypes.JSON
CreatedAt time.Time `gorm:"autoCreateTime"`
UpdatedAt time.Time `gorm:"autoUpdateTime"`
DeletedAt gorm.DeletedAt `gorm:"index"`
ID string `gorm:"primaryKey"`
Version uint `gorm:"primaryKey"`
Description string
Steps datatypes.JSON
Labels datatypes.JSON
Requirements datatypes.JSON
IAM datatypes.JSON
CreatedAt time.Time `gorm:"autoCreateTime"`
UpdatedAt time.Time `gorm:"autoUpdateTime"`
DeletedAt gorm.DeletedAt `gorm:"index"`
}

// TableName overrides the table name
Expand All @@ -39,6 +40,11 @@ func (m *Policy) FromDomain(p *domain.Policy) error {
return err
}

requirements, err := json.Marshal(p.Requirements)
if err != nil {
return err
}

iam, err := json.Marshal(p.IAM)
if err != nil {
return err
Expand All @@ -49,6 +55,7 @@ func (m *Policy) FromDomain(p *domain.Policy) error {
m.Description = p.Description
m.Steps = datatypes.JSON(steps)
m.Labels = datatypes.JSON(labels)
m.Requirements = datatypes.JSON(requirements)
m.IAM = datatypes.JSON(iam)
m.CreatedAt = p.CreatedAt
m.UpdatedAt = p.UpdatedAt
Expand All @@ -68,6 +75,13 @@ func (m *Policy) ToDomain() (*domain.Policy, error) {
return nil, err
}

var requirements []*domain.Requirement
if m.Requirements != nil {
if err := json.Unmarshal(m.Requirements, &requirements); err != nil {
return nil, err
}
}

var iam *domain.IAMConfig
if m.IAM != nil {
if err := json.Unmarshal(m.IAM, &iam); err != nil {
Expand All @@ -76,13 +90,14 @@ func (m *Policy) ToDomain() (*domain.Policy, error) {
}

return &domain.Policy{
ID: m.ID,
Version: m.Version,
Description: m.Description,
Steps: steps,
Labels: labels,
IAM: iam,
CreatedAt: m.CreatedAt,
UpdatedAt: m.UpdatedAt,
ID: m.ID,
Version: m.Version,
Description: m.Description,
Steps: steps,
Labels: labels,
Requirements: requirements,
IAM: iam,
CreatedAt: m.CreatedAt,
UpdatedAt: m.UpdatedAt,
}, nil
}
7 changes: 6 additions & 1 deletion store/postgres/policy_repository_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ func (s *PolicyRepositoryTestSuite) SetupTest() {
"description",
"steps",
"labels",
"requirements",
"iam",
"created_at",
"updated_at",
Expand All @@ -50,7 +51,7 @@ func (s *PolicyRepositoryTestSuite) TearDownTest() {
}

func (s *PolicyRepositoryTestSuite) TestCreate() {
expectedQuery := regexp.QuoteMeta(`INSERT INTO "policies" ("id","version","description","steps","labels","iam","created_at","updated_at","deleted_at") VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9)`)
expectedQuery := regexp.QuoteMeta(`INSERT INTO "policies" ("id","version","description","steps","labels","requirements","iam","created_at","updated_at","deleted_at") VALUES ($1,$2,$3,$4,$5,$6,$7,$8,$9,$10)`)

s.Run("should return error if got error from db transaction", func() {
p := &domain.Policy{}
Expand All @@ -62,6 +63,7 @@ func (s *PolicyRepositoryTestSuite) TestCreate() {
"null",
"null",
"null",
"null",
utils.AnyTime{},
utils.AnyTime{},
gorm.DeletedAt{},
Expand Down Expand Up @@ -89,6 +91,7 @@ func (s *PolicyRepositoryTestSuite) TestCreate() {
"null",
"null",
"null",
"null",
utils.AnyTime{},
utils.AnyTime{},
gorm.DeletedAt{},
Expand Down Expand Up @@ -141,6 +144,7 @@ func (s *PolicyRepositoryTestSuite) TestFind() {
"null",
"null",
"null",
"null",
now,
now,
)
Expand Down Expand Up @@ -212,6 +216,7 @@ func (s *PolicyRepositoryTestSuite) TestGetOne() {
"null",
"null",
"null",
"null",
now,
now,
}
Expand Down

0 comments on commit 08553e3

Please sign in to comment.