Skip to content

Commit

Permalink
feat(appeal): introduce appeal metadata (#139)
Browse files Browse the repository at this point in the history 
* feat: support appeal metadata to story metadata in policy details

* fix: review fixes

* chore: db migration for appeal_metadata_sources column

* chore: generate proto

* chore: change AppealMetadataSource.Config type to interface{}; encrypt and decrypt at Config level

* fix: fix err == nil

* fix: squash mapstructure embedded struct

* feat: evaluate url if contains "$appeal" string

* fix: cherry pick response struct into map

* fix: resolve metadata value

* refactor: move evaluate value logic to domain

* fix: make appealMetadata concurent safe

* feat: use context in http fetch

* feat!: move metadata sources config into policy.appeal

* refactor: revert debugging leftover

* lint: fix lint errors

* feat: use reflection for evaluating metadata value

* feat: remove sensitive values before sending response

* fix: nil check for config.Auth in http pkg

* test: increase coverage

* test: add unit test for appeal metadata

* lint: fix lint errors

* test: add unit test for policy service

* test: add unit test for metadata source encrypt & decrypt

---------

Co-authored-by: Ayushi Sharma <[email protected]>
Co-authored-by: Rahmat Hidayat <[email protected]>
  • Loading branch information
@ayushi0014 @rahmatrhd
3 people authored Apr 29, 2024
1 parent 3a9dc95 commit 1998a3c
Show file tree
Hide file tree
Showing 19 changed files with 2,455 additions and 1,071 deletions.
54 changes: 50 additions & 4 deletions api/handler/v1beta1/adapter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@ func (a *adapter) FromPolicyProto(p *guardianv1beta1.Policy) *domain.Policy {
if p.GetAppeal() != nil {
var durationOptions []domain.AppealDurationOption
var questions []domain.Question
var metadataSources map[string]*domain.AppealMetadataSource
for _, d := range p.GetAppeal().GetDurationOptions() {
option := domain.AppealDurationOption{
Name: d.GetName(),
Expand All @@ -328,13 +329,27 @@ func (a *adapter) FromPolicyProto(p *guardianv1beta1.Policy) *domain.Policy {
questions = append(questions, question)
}

if mds := p.GetAppeal().GetMetadataSources(); mds != nil {
metadataSources = make(map[string]*domain.AppealMetadataSource)
for key, metadataCfg := range mds {
metadataSources[key] = &domain.AppealMetadataSource{
Name: metadataCfg.GetName(),
Description: metadataCfg.GetDescription(),
Type: metadataCfg.GetType(),
Config: metadataCfg.GetConfig().AsInterface(),
Value: metadataCfg.GetValue().AsInterface(),
}
}
}

policy.AppealConfig = &domain.PolicyAppealConfig{
DurationOptions: durationOptions,
AllowOnBehalf: p.GetAppeal().GetAllowOnBehalf(),
Questions: questions,
AllowPermanentAccess: p.GetAppeal().GetAllowPermanentAccess(),
AllowActiveAccessExtensionIn: p.GetAppeal().GetAllowActiveAccessExtensionIn(),
AllowCreatorDetailsFailure: p.GetAppeal().GetAllowCreatorDetailsFailure(),
MetadataSources: metadataSources,
}
}

Expand Down Expand Up @@ -444,7 +459,11 @@ func (a *adapter) ToPolicyProto(p *domain.Policy) (*guardianv1beta1.Policy, erro
}
}

policyProto.Appeal = a.ToPolicyAppealConfigProto(p)
appealConfig, err := a.ToPolicyAppealConfigProto(p)
if err != nil {
return nil, fmt.Errorf("failed to convert appeal config to proto: %w", err)
}
policyProto.Appeal = appealConfig

if !p.CreatedAt.IsZero() {
policyProto.CreatedAt = timestamppb.New(p.CreatedAt)
Expand All @@ -456,9 +475,9 @@ func (a *adapter) ToPolicyProto(p *domain.Policy) (*guardianv1beta1.Policy, erro
return policyProto, nil
}

func (a *adapter) ToPolicyAppealConfigProto(p *domain.Policy) *guardianv1beta1.PolicyAppealConfig {
func (a *adapter) ToPolicyAppealConfigProto(p *domain.Policy) (*guardianv1beta1.PolicyAppealConfig, error) {
if p.AppealConfig == nil {
return nil
return nil, nil
}

policyAppealConfigProto := &guardianv1beta1.PolicyAppealConfig{}
Expand All @@ -485,7 +504,34 @@ func (a *adapter) ToPolicyAppealConfigProto(p *domain.Policy) *guardianv1beta1.P
Description: q.Description,
})
}
return policyAppealConfigProto

for key, metadataSource := range p.AppealConfig.MetadataSources {
if policyAppealConfigProto.MetadataSources == nil {
policyAppealConfigProto.MetadataSources = make(map[string]*guardianv1beta1.PolicyAppealConfig_MetadataSource)
}
policyAppealConfigProto.MetadataSources[key] = &guardianv1beta1.PolicyAppealConfig_MetadataSource{
Name: metadataSource.Name,
Description: metadataSource.Description,
Type: metadataSource.Type,
}

if metadataSource.Config != nil {
cfg, err := structpb.NewValue(metadataSource.Config)
if err != nil {
return nil, err
}
policyAppealConfigProto.MetadataSources[key].Config = cfg
}
if metadataSource.Value != nil {
value, err := structpb.NewValue(metadataSource.Value)
if err != nil {
return nil, err
}
policyAppealConfigProto.MetadataSources[key].Value = value
}
}

return policyAppealConfigProto, nil
}

func (a *adapter) FromResourceProto(r *guardianv1beta1.Resource) *domain.Resource {
Expand Down
2 changes: 1 addition & 1 deletion api/handler/v1beta1/grpc.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type ProtoAdapter interface {
FromPolicyProto(*guardianv1beta1.Policy) *domain.Policy
ToPolicyProto(*domain.Policy) (*guardianv1beta1.Policy, error)

ToPolicyAppealConfigProto(policy *domain.Policy) *guardianv1beta1.PolicyAppealConfig
ToPolicyAppealConfigProto(policy *domain.Policy) (*guardianv1beta1.PolicyAppealConfig, error)

FromResourceProto(*guardianv1beta1.Resource) *domain.Resource
ToResourceProto(*domain.Resource) (*guardianv1beta1.Resource, error)
Expand Down
14 changes: 7 additions & 7 deletions api/handler/v1beta1/policy.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ func (s *GRPCServer) ListPolicies(ctx context.Context, req *guardianv1beta1.List

policyProtos := []*guardianv1beta1.Policy{}
for _, p := range policies {
if p.IAM != nil {
p.IAM.Config = nil
}
p.RemoveSensitiveValues()
policyProto, err := s.adapter.ToPolicyProto(p)
if err != nil {
return nil, s.internalError(ctx, "failed to parse policy %v: %v", p.ID, err)
Expand All @@ -44,9 +42,7 @@ func (s *GRPCServer) GetPolicy(ctx context.Context, req *guardianv1beta1.GetPoli
}
}

if p.IAM != nil {
p.IAM.Config = nil
}
p.RemoveSensitiveValues()
policyProto, err := s.adapter.ToPolicyProto(p)
if err != nil {
return nil, s.internalError(ctx, "failed to parse policy: %v", err)
Expand Down Expand Up @@ -116,7 +112,11 @@ func (s *GRPCServer) GetPolicyPreferences(ctx context.Context, req *guardianv1be
}
}

appealConfigProto := s.adapter.ToPolicyAppealConfigProto(p)
p.RemoveSensitiveValues()
appealConfigProto, err := s.adapter.ToPolicyAppealConfigProto(p)
if err != nil {
return nil, s.internalError(ctx, "failed to parse policy preferences: %v", err)
}

return &guardianv1beta1.GetPolicyPreferencesResponse{
Appeal: appealConfigProto,
Expand Down
135 changes: 135 additions & 0 deletions api/handler/v1beta1/policy_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@ func (s *GrpcHandlersSuite) TestListPolicies() {
Config: expectedIAMConfig,
Provider: "provider",
},
Appeal: &guardianv1beta1.PolicyAppealConfig{
MetadataSources: map[string]*guardianv1beta1.PolicyAppealConfig_MetadataSource{
"test-source": {
Name: "test-source",
},
},
},
},
},
}
Expand All @@ -38,6 +45,14 @@ func (s *GrpcHandlersSuite) TestListPolicies() {
Config: map[string]interface{}{"foo": "bar"},
Provider: "provider",
},
AppealConfig: &domain.PolicyAppealConfig{
MetadataSources: map[string]*domain.AppealMetadataSource{
"test-source": {
Name: "test-source",
Config: map[string]interface{}{"foo": "bar"},
},
},
},
},
}
s.policyService.EXPECT().Find(mock.MatchedBy(func(ctx context.Context) bool { return true })).Return(dummyPolicies, nil).Once()
Expand Down Expand Up @@ -403,11 +418,24 @@ func (s *GrpcHandlersSuite) TestCreatePolicy() {
Description: "Please provide the name of the team you are in",
},
},
MetadataSources: map[string]*domain.AppealMetadataSource{
"test-source": {
Name: "test-source",
Description: "test-description",
Type: "test-type",
Config: map[string]interface{}{"foo": "bar"},
Value: "test-value",
},
},
},
}
expectedVersion := uint(1)
expectedIAMConfig, err := structpb.NewValue(expectedPolicy.IAM.Config)
s.Require().NoError(err)
expectedMetadataSourceConfig, err := structpb.NewValue(expectedPolicy.AppealConfig.MetadataSources["test-source"].Config)
s.Require().NoError(err)
expectedMetadataSourceValue, err := structpb.NewValue(expectedPolicy.AppealConfig.MetadataSources["test-source"].Value)
s.Require().NoError(err)
expectedResponse := &guardianv1beta1.CreatePolicyResponse{
Policy: &guardianv1beta1.Policy{
Id: expectedPolicy.ID,
Expand Down Expand Up @@ -464,6 +492,15 @@ func (s *GrpcHandlersSuite) TestCreatePolicy() {
Description: "Please provide the name of the team you are in",
},
},
MetadataSources: map[string]*guardianv1beta1.PolicyAppealConfig_MetadataSource{
"test-source": {
Name: "test-source",
Description: "test-description",
Type: "test-type",
Config: expectedMetadataSourceConfig,
Value: expectedMetadataSourceValue,
},
},
},
CreatedAt: timestamppb.New(timeNow),
UpdatedAt: timestamppb.New(timeNow),
Expand Down Expand Up @@ -531,6 +568,15 @@ func (s *GrpcHandlersSuite) TestCreatePolicy() {
Description: "Please provide the name of the team you are in",
},
},
MetadataSources: map[string]*guardianv1beta1.PolicyAppealConfig_MetadataSource{
"test-source": {
Name: "test-source",
Description: "test-description",
Type: "test-type",
Config: expectedMetadataSourceConfig,
Value: expectedMetadataSourceValue,
},
},
},
},
}
Expand Down Expand Up @@ -798,3 +844,92 @@ func (s *GrpcHandlersSuite) TestUpdatePolicy() {
s.policyService.AssertExpectations(s.T())
})
}

func (s *GrpcHandlersSuite) TestGetPolicyPreferences() {
s.Run("should return policy preferences on success", func() {
s.setup()

dummyPolicy := &domain.Policy{
ID: "test-policy",
Version: 1,
AppealConfig: &domain.PolicyAppealConfig{
AllowActiveAccessExtensionIn: "24h",
AllowPermanentAccess: true,
MetadataSources: map[string]*domain.AppealMetadataSource{
"test-source": {
Name: "test-source",
Config: map[string]interface{}{"foo": "bar"},
},
},
},
}

s.policyService.EXPECT().
GetOne(mock.MatchedBy(func(ctx context.Context) bool { return true }), dummyPolicy.ID, dummyPolicy.Version).
Return(dummyPolicy, nil).Once()
expectedResponse := &guardianv1beta1.GetPolicyPreferencesResponse{
Appeal: &guardianv1beta1.PolicyAppealConfig{
AllowActiveAccessExtensionIn: "24h",
AllowPermanentAccess: true,
MetadataSources: map[string]*guardianv1beta1.PolicyAppealConfig_MetadataSource{
"test-source": {
Name: "test-source",
},
},
},
}

req := &guardianv1beta1.GetPolicyPreferencesRequest{
Id: dummyPolicy.ID,
Version: uint32(dummyPolicy.Version),
}
res, err := s.grpcServer.GetPolicyPreferences(context.Background(), req)

s.NoError(err)
s.Equal(expectedResponse, res)
s.Nil(res.Appeal.MetadataSources["test-source"].Config) // config that might contain sensitive value should be removed
s.policyService.AssertExpectations(s.T())
})

s.Run("should return not found error if policy not found", func() {
s.setup()

policyID := "test-policy"
policyVersion := uint(1)
expectedError := policy.ErrPolicyNotFound
s.policyService.EXPECT().
GetOne(mock.MatchedBy(func(ctx context.Context) bool { return true }), policyID, policyVersion).
Return(nil, expectedError).Once()

req := &guardianv1beta1.GetPolicyPreferencesRequest{
Id: policyID,
Version: uint32(policyVersion),
}
res, err := s.grpcServer.GetPolicyPreferences(context.Background(), req)

s.Equal(codes.NotFound, status.Code(err))
s.Nil(res)
s.policyService.AssertExpectations(s.T())
})

s.Run("should return internal error if policy service returns error", func() {
s.setup()

policyID := "test-policy"
policyVersion := uint(1)
expectedError := errors.New("random error")
s.policyService.EXPECT().
GetOne(mock.MatchedBy(func(ctx context.Context) bool { return true }), policyID, policyVersion).
Return(nil, expectedError).Once()

req := &guardianv1beta1.GetPolicyPreferencesRequest{
Id: policyID,
Version: uint32(policyVersion),
}
res, err := s.grpcServer.GetPolicyPreferences(context.Background(), req)

s.Equal(codes.Internal, status.Code(err))
s.Nil(res)
s.policyService.AssertExpectations(s.T())
})
}
Loading

0 comments on commit 1998a3c

Please sign in to comment.