Skip to content

Commit

Permalink
feat: introduce gitlab provider (#128)
Browse files Browse the repository at this point in the history
* feat: introduce gitlab provider

* chore: update go mod

* refactor: refactor pagination fetcher

* feat: handle exclusive role assignment provider by revoking current active grant to the same resource

* lint: fix lint errors

* fix: fix fetchResources append

* test: add provider test

* chore: bump go-gitlab library version

* fix: fetch resources using keyset pagination

* chore: log duration in fetch resources job

* fix(gitlab): check membership before revocation

* fix(gitlab): fetch resoruces nested under groups

* chore: enhance fetch resource error logging
  • Loading branch information
rahmatrhd authored Mar 4, 2024
1 parent d2cdd5a commit 6ea2731
Show file tree
Hide file tree
Showing 19 changed files with 1,985 additions and 51 deletions.
44 changes: 44 additions & 0 deletions core/appeal/mocks/providerService.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

20 changes: 15 additions & 5 deletions core/appeal/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const (
AuditKeyDeleteApprover = "appeal.deleteApprover"

RevokeReasonForExtension = "Automatically revoked for grant extension"
RevokeReasonForOverride = "Automatically revoked for grant override"
)

var TimeNow = time.Now
Expand Down Expand Up @@ -70,6 +71,7 @@ type providerService interface {
RevokeAccess(context.Context, domain.Grant) error
ValidateAppeal(context.Context, *domain.Appeal, *domain.Provider, *domain.Policy) error
GetPermissions(context.Context, *domain.ProviderConfig, string, string) ([]interface{}, error)
IsExclusiveRoleAssignment(context.Context, string, string) bool
}

//go:generate mockery --name=resourceService --exported --with-expecter
Expand Down Expand Up @@ -284,7 +286,7 @@ func (s *Service) Create(ctx context.Context, appeals []*domain.Appeal, opts ...
newGrant.Resource = appeal.Resource
appeal.Grant = newGrant
if revokedGrant != nil {
if _, err := s.grantService.Revoke(ctx, revokedGrant.ID, domain.SystemActorName, RevokeReasonForExtension,
if _, err := s.grantService.Revoke(ctx, revokedGrant.ID, domain.SystemActorName, revokedGrant.RevokeReason,
grant.SkipNotifications(),
grant.SkipRevokeAccessInProvider(),
); err != nil {
Expand Down Expand Up @@ -511,7 +513,7 @@ func (s *Service) UpdateApproval(ctx context.Context, approvalAction domain.Appr
newGrant.Resource = appeal.Resource
appeal.Grant = newGrant
if revokedGrant != nil {
if _, err := s.grantService.Revoke(ctx, revokedGrant.ID, domain.SystemActorName, RevokeReasonForExtension,
if _, err := s.grantService.Revoke(ctx, revokedGrant.ID, domain.SystemActorName, revokedGrant.RevokeReason,
grant.SkipNotifications(),
grant.SkipRevokeAccessInProvider(),
); err != nil {
Expand Down Expand Up @@ -1186,20 +1188,28 @@ func (s *Service) getPermissions(ctx context.Context, pc *domain.ProviderConfig,
return strPermissions, nil
}

// TODO(feature): add relation between new and revoked grant for traceability
func (s *Service) prepareGrant(ctx context.Context, appeal *domain.Appeal) (newGrant *domain.Grant, deactivatedGrant *domain.Grant, err error) {
activeGrants, err := s.grantService.List(ctx, domain.ListGrantsFilter{
filter := domain.ListGrantsFilter{
AccountIDs: []string{appeal.AccountID},
ResourceIDs: []string{appeal.ResourceID},
Statuses: []string{string(domain.GrantStatusActive)},
Permissions: appeal.Permissions,
})
}
revocationReason := RevokeReasonForExtension
if s.providerService.IsExclusiveRoleAssignment(ctx, appeal.Resource.ProviderType, appeal.Resource.Type) {
filter.Permissions = nil
revocationReason = RevokeReasonForOverride
}

activeGrants, err := s.grantService.List(ctx, filter)
if err != nil {
return nil, nil, fmt.Errorf("unable to retrieve existing active grants: %w", err)
}

if len(activeGrants) > 0 {
deactivatedGrant = &activeGrants[0]
if err := deactivatedGrant.Revoke(domain.SystemActorName, "Extended to a new grant"); err != nil {
if err := deactivatedGrant.Revoke(domain.SystemActorName, revocationReason); err != nil {
return nil, nil, fmt.Errorf("revoking previous grant: %w", err)
}
}
Expand Down
21 changes: 21 additions & 0 deletions core/appeal/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1082,6 +1082,9 @@ func (s *ServiceTestSuite) TestCreate() {
Return(nil).Once()
s.mockNotifier.On("Notify", mock.MatchedBy(func(ctx context.Context) bool { return true }), mock.Anything).Return(nil).Once()
s.mockAuditLogger.On("Log", mock.Anything, appeal.AuditKeyBulkInsert, mock.Anything).Return(nil).Once()
s.mockProviderService.EXPECT().
IsExclusiveRoleAssignment(mock.Anything, mock.Anything, mock.Anything).
Return(false).Once()
s.mockGrantService.On("List", mock.Anything, mock.Anything).Return([]domain.Grant{}, nil).Once()
s.mockGrantService.On("Prepare", mock.Anything, mock.Anything).Return(&domain.Grant{}, nil).Once()
s.mockPolicyService.On("GetOne", mock.Anything, mock.Anything, mock.Anything).Return(overriddingPolicy, nil).Once()
Expand Down Expand Up @@ -1315,6 +1318,9 @@ func (s *ServiceTestSuite) TestCreateAppeal__WithExistingAppealAndWithAutoApprov
}).
Return(expectedExistingGrants, nil).Once()
// duplicate call with slight change in filters but the code needs it in order to work. appeal create code needs to be refactored.
s.mockProviderService.EXPECT().
IsExclusiveRoleAssignment(mock.Anything, mock.Anything, mock.Anything).
Return(false).Once()
s.mockGrantService.EXPECT().
List(mock.MatchedBy(func(ctx context.Context) bool { return true }), domain.ListGrantsFilter{
Statuses: []string{string(domain.GrantStatusActive)},
Expand All @@ -1330,6 +1336,9 @@ func (s *ServiceTestSuite) TestCreateAppeal__WithExistingAppealAndWithAutoApprov
s.mockIAMManager.On("GetClient", mock.Anything).Return(s.mockIAMClient, nil)
s.mockIAMClient.On("GetUser", accountID).Return(expectedCreatorUser, nil)

s.mockProviderService.EXPECT().
IsExclusiveRoleAssignment(mock.Anything, mock.Anything, mock.Anything).
Return(false).Once()
s.mockGrantService.EXPECT().
List(mock.MatchedBy(func(ctx context.Context) bool { return true }), domain.ListGrantsFilter{
AccountIDs: []string{accountID},
Expand Down Expand Up @@ -1507,6 +1516,9 @@ func (s *ServiceTestSuite) TestCreateAppeal__WithAdditionalAppeals() {
s.mockResourceService.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx"), expectedResourceFilters).Return([]*domain.Resource{resources[1]}, nil).Once()
s.mockProviderService.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx")).Return(providers, nil).Once()
s.mockPolicyService.EXPECT().Find(mock.AnythingOfType("*context.cancelCtx")).Return(policies, nil).Once()
s.mockProviderService.EXPECT().
IsExclusiveRoleAssignment(mock.Anything, mock.Anything, mock.Anything).
Return(false).Once()
s.mockGrantService.EXPECT().List(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("domain.ListGrantsFilter")).Return([]domain.Grant{}, nil).Once().Run(func(args mock.Arguments) {
filter := args.Get(1).(domain.ListGrantsFilter)
s.Equal([]string{appealsPayload[0].AccountID}, filter.AccountIDs)
Expand All @@ -1521,6 +1533,9 @@ func (s *ServiceTestSuite) TestCreateAppeal__WithAdditionalAppeals() {
s.Equal(targetResource.ID, appeal.ResourceID)
})
s.mockProviderService.EXPECT().GetPermissions(mock.AnythingOfType("*context.cancelCtx"), providers[0].Config, resourceType, targetRole).Return([]interface{}{"test-permission-1"}, nil).Once()
s.mockProviderService.EXPECT().
IsExclusiveRoleAssignment(mock.Anything, mock.Anything, mock.Anything).
Return(false).Once()
s.mockGrantService.EXPECT().List(mock.AnythingOfType("*context.cancelCtx"), mock.AnythingOfType("domain.ListGrantsFilter")).Return([]domain.Grant{}, nil).Once().Run(func(args mock.Arguments) {
filter := args.Get(1).(domain.ListGrantsFilter)
s.Equal([]string{appealsPayload[0].AccountID}, filter.AccountIDs)
Expand Down Expand Up @@ -1903,6 +1918,9 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
Return(appealDetails, nil).Once()

s.mockPolicyService.EXPECT().GetOne(mock.Anything, mock.Anything, mock.Anything).Return(&domain.Policy{}, nil).Once()
s.mockProviderService.EXPECT().
IsExclusiveRoleAssignment(mock.Anything, mock.Anything, mock.Anything).
Return(false).Once()
s.mockGrantService.EXPECT().
List(mock.Anything, mock.Anything).Return(existingGrants, nil).Once()
expectedNewGrant := &domain.Grant{
Expand Down Expand Up @@ -2299,6 +2317,9 @@ func (s *ServiceTestSuite) TestUpdateApproval() {
Return(tc.expectedAppealDetails, nil).Once()

if tc.expectedApprovalAction.Action == "approve" {
s.mockProviderService.EXPECT().
IsExclusiveRoleAssignment(mock.Anything, mock.Anything, mock.Anything).
Return(false).Once()
s.mockGrantService.EXPECT().
List(mock.Anything, domain.ListGrantsFilter{
AccountIDs: []string{tc.expectedAppealDetails.AccountID},
Expand Down
78 changes: 78 additions & 0 deletions core/provider/mocks/assignmentTyper.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

34 changes: 28 additions & 6 deletions core/provider/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ type dormancyChecker interface {
CorrelateGrantActivities(context.Context, domain.Provider, []*domain.Grant, []*domain.Activity) error
}

//go:generate mockery --name=assignmentTyper --exported --with-expecter
type assignmentTyper interface {
IsExclusiveRoleAssignment(context.Context) bool
}

//go:generate mockery --name=resourceService --exported --with-expecter
type resourceService interface {
Find(context.Context, domain.ListResourcesFilter) ([]*domain.Resource, error)
Expand Down Expand Up @@ -253,8 +258,9 @@ func (s *Service) FetchResources(ctx context.Context) error {
return err
}

failedProviders := make([]string, 0)
failedProviders := map[string]error{}
for _, p := range providers {
startTime := time.Now()
s.logger.Info(ctx, "fetching resources", "provider_urn", p.URN)
resources, err := s.getResources(ctx, p)
if err != nil {
Expand All @@ -263,15 +269,21 @@ func (s *Service) FetchResources(ctx context.Context) error {
}
s.logger.Info(ctx, "resources added", "provider_urn", p.URN, "count", len(flattenResources(resources)))
if err := s.resourceService.BulkUpsert(ctx, resources); err != nil {
failedProviders = append(failedProviders, p.URN)
failedProviders[p.URN] = err
s.logger.Error(ctx, "failed to add resources", "provider_urn", p.URN, "error", err)
}
s.logger.Info(ctx, "fetching resources completed", "provider_urn", p.URN, "duration", time.Since(startTime))
}

if len(failedProviders) == 0 {
return nil
if len(failedProviders) > 0 {
var urns []string
for providerURN, err := range failedProviders {
s.logger.Error(ctx, "failed to add resources for provider", "provider_urn", providerURN, "error", err)
urns = append(urns, providerURN)
}
return fmt.Errorf("failed to add resources for providers: %v", urns)
}
return fmt.Errorf("failed to add resources %s - %v", "providers", failedProviders)
return nil
}

func (s *Service) GetRoles(ctx context.Context, id string, resourceType string) ([]*domain.Role, error) {
Expand Down Expand Up @@ -534,6 +546,16 @@ func (s *Service) CorrelateGrantActivities(ctx context.Context, p domain.Provide
return activityClient.CorrelateGrantActivities(ctx, p, grants, activities)
}

// IsExclusiveRoleAssignment returns true if the provider only supports exclusive role assignment
// i.e. a user can only have one role per resource
func (s *Service) IsExclusiveRoleAssignment(ctx context.Context, providerType, resourceType string) bool {
client := s.getClient(providerType)
if c, ok := client.(assignmentTyper); ok {
return c.IsExclusiveRoleAssignment(ctx)
}
return false
}

func (s *Service) getResources(ctx context.Context, p *domain.Provider) ([]*domain.Resource, error) {
c := s.getClient(p.Type)
if c == nil {
Expand Down Expand Up @@ -579,7 +601,7 @@ func (s *Service) getResources(ctx context.Context, p *domain.Provider) ([]*doma
existingProviderResources := map[string]bool{}
for _, r := range flattenedProviderResources {
for _, er := range existingGuardianResources {
if er.URN == r.URN {
if er.Type == r.Type && er.URN == r.URN {
if existingDetails := er.Details; existingDetails != nil {
if r.Details != nil {
for key, value := range existingDetails {
Expand Down
2 changes: 1 addition & 1 deletion core/provider/service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ func (s *ServiceTestSuite) TestFetchResources() {
for _, p := range providers {
s.mockProvider.On("GetResources", mockCtx, p.Config).Return([]*domain.Resource{}, nil).Once()
}
expectedError := errors.New("failed to add resources providers - [mock_provider]")
expectedError := errors.New("failed to add resources for providers: [mock_provider]")
s.mockResourceService.On("BulkUpsert", mock.Anything, mock.Anything).Return(expectedError).Once()
s.mockResourceService.On("Find", mock.Anything, mock.Anything).Return([]*domain.Resource{}, nil).Once()
actualError := s.service.FetchResources(context.Background())
Expand Down
27 changes: 8 additions & 19 deletions domain/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,16 @@ import (
)

const (
// ProviderTypeBigQuery is the type name for BigQuery provider
ProviderTypeBigQuery = "bigquery"
// ProviderTypeMetabase is the type name for Metabase provider
ProviderTypeMetabase = "metabase"
// ProviderTypeGrafana is the type name for Grafana provider
ProviderTypeGrafana = "grafana"
// ProviderTypeTableau is the type name for Tableau provider
ProviderTypeTableau = "tableau"
// ProviderTypeGCloudIAM is the type name for Google Cloud IAM provider
ProviderTypeBigQuery = "bigquery"
ProviderTypeMetabase = "metabase"
ProviderTypeGrafana = "grafana"
ProviderTypeTableau = "tableau"
ProviderTypeGCloudIAM = "gcloud_iam"
// ProviderTypeNoOp is the type name for No-Op provider
ProviderTypeNoOp = "noop"
// ProviderTypeGCS is the type name for Google Cloud Storage provider
ProviderTypeGCS = "gcs"
// ProviderTypePolicyTag is the type name for Dataplex
ProviderTypeNoOp = "noop"
ProviderTypeGCS = "gcs"
ProviderTypePolicyTag = "dataplex"
// ProviderTypeShield is the type name for Shield auth layer provider
ProviderTypeShield = "shield"
ProviderTypeShield = "shield"
ProviderTypeGitlab = "gitlab"
)

// Role is the configuration to define a role and mapping the permissions in the provider
Expand Down Expand Up @@ -64,8 +56,6 @@ type AppealConfig struct {
AllowPermanentAccess bool `json:"allow_permanent_access" yaml:"allow_permanent_access"`
AllowActiveAccessExtensionIn string `json:"allow_active_access_extension_in" yaml:"allow_active_access_extension_in" validate:"required"`
}

// ProviderConfig is the configuration for a data provider
type ProviderConfig struct {
Type string `json:"type" yaml:"type" validate:"required,oneof=google_bigquery metabase grafana tableau gcloud_iam noop gcs"`
URN string `json:"urn" yaml:"urn" validate:"required"`
Expand Down Expand Up @@ -101,7 +91,6 @@ func (pc ProviderConfig) GetFilterForResourceType(resourceType string) string {
return ""
}

// Provider domain structure
type Provider struct {
ID string `json:"id" yaml:"id"`
Type string `json:"type" yaml:"type"`
Expand Down
Loading

0 comments on commit 6ea2731

Please sign in to comment.