Skip to content

Commit

Permalink
[DDO-3839] add Broad Institute group syncing (#632)
Browse files Browse the repository at this point in the history
  • Loading branch information
jack-r-warren authored Aug 13, 2024
1 parent 3390df7 commit b08627d
Show file tree
Hide file tree
Showing 17 changed files with 565 additions and 44 deletions.
1 change: 1 addition & 0 deletions sherlock/.mockery.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,4 @@ packages:
github.com/broadinstitute/sherlock/sherlock/internal/role_propagation/propagation_engines:
interfaces:
PropagationEngine:
ToleratedUserCalculator:
3 changes: 3 additions & 0 deletions sherlock/config/default_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,9 @@ rolePropagation:
userEmailSuffixesToReplace:
- "@broadinstitute.org"

broadInstituteGroup:
enable: false

suitabilitySynchronization:
enable: true
behaviors:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
drop index if exists roles_grants_broad_institute_group_unique;

alter table roles
drop column if exists grants_broad_institute_group;

alter table role_operations
drop column if exists from_grants_broad_institute_group;

alter table role_operations
drop column if exists to_grants_broad_institute_group;
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
alter table roles
add column if not exists grants_broad_institute_group text;

create unique index if not exists roles_grants_broad_institute_group_unique
on roles (grants_broad_institute_group)
where deleted_at is null and grants_broad_institute_group is not null and grants_broad_institute_group != '';

alter table role_operations
add column if not exists from_grants_broad_institute_group text;

alter table role_operations
add column if not exists to_grants_broad_institute_group text;
35 changes: 19 additions & 16 deletions sherlock/internal/api/sherlock/roles_v3.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,22 +25,24 @@ type RoleV3Edit struct {
GrantsProdFirecloudGroup *string `json:"grantsProdFirecloudGroup,omitempty" form:"grantsProdFirecloudGroup"`
GrantsDevAzureGroup *string `json:"grantsDevAzureGroup,omitempty" form:"grantsDevAzureGroup"`
GrantsProdAzureGroup *string `json:"grantsProdAzureGroup,omitempty" form:"grantsProdAzureGroup"`
GrantsBroadInstituteGroup *string `json:"grantsBroadInstituteGroup,omitempty" form:"grantsBroadInstituteGroup"`
}

func (r RoleV3) toModel() models.Role {
ret := models.Role{
Model: r.toGormModel(),
RoleFields: models.RoleFields{
Name: r.Name,
SuspendNonSuitableUsers: r.SuspendNonSuitableUsers,
AutoAssignAllUsers: r.AutoAssignAllUsers,
CanBeGlassBrokenByRoleID: r.CanBeGlassBrokenByRole,
GrantsSherlockSuperAdmin: r.GrantsSherlockSuperAdmin,
GrantsDevFirecloudGroup: r.GrantsDevFirecloudGroup,
GrantsQaFirecloudGroup: r.GrantsQaFirecloudGroup,
GrantsProdFirecloudGroup: r.GrantsProdFirecloudGroup,
GrantsDevAzureGroup: r.GrantsDevAzureGroup,
GrantsProdAzureGroup: r.GrantsProdAzureGroup,
Name: r.Name,
SuspendNonSuitableUsers: r.SuspendNonSuitableUsers,
AutoAssignAllUsers: r.AutoAssignAllUsers,
CanBeGlassBrokenByRoleID: r.CanBeGlassBrokenByRole,
GrantsSherlockSuperAdmin: r.GrantsSherlockSuperAdmin,
GrantsDevFirecloudGroup: r.GrantsDevFirecloudGroup,
GrantsQaFirecloudGroup: r.GrantsQaFirecloudGroup,
GrantsProdFirecloudGroup: r.GrantsProdFirecloudGroup,
GrantsDevAzureGroup: r.GrantsDevAzureGroup,
GrantsProdAzureGroup: r.GrantsProdAzureGroup,
GrantsBroadInstituteGroup: r.GrantsBroadInstituteGroup,
},
}
if r.DefaultGlassBreakDuration != nil {
Expand All @@ -65,12 +67,13 @@ func roleFromModel(model models.Role) RoleV3 {
DefaultGlassBreakDuration: utils.NilOrCall(func(nanoseconds int64) Duration {
return Duration{time.Duration(nanoseconds)}
}, model.DefaultGlassBreakDuration),
GrantsSherlockSuperAdmin: model.GrantsSherlockSuperAdmin,
GrantsDevFirecloudGroup: model.GrantsDevFirecloudGroup,
GrantsQaFirecloudGroup: model.GrantsQaFirecloudGroup,
GrantsProdFirecloudGroup: model.GrantsProdFirecloudGroup,
GrantsDevAzureGroup: model.GrantsDevAzureGroup,
GrantsProdAzureGroup: model.GrantsProdAzureGroup,
GrantsSherlockSuperAdmin: model.GrantsSherlockSuperAdmin,
GrantsDevFirecloudGroup: model.GrantsDevFirecloudGroup,
GrantsQaFirecloudGroup: model.GrantsQaFirecloudGroup,
GrantsProdFirecloudGroup: model.GrantsProdFirecloudGroup,
GrantsDevAzureGroup: model.GrantsDevAzureGroup,
GrantsProdAzureGroup: model.GrantsProdAzureGroup,
GrantsBroadInstituteGroup: model.GrantsBroadInstituteGroup,
},
}
if len(model.Assignments) > 0 {
Expand Down
1 change: 1 addition & 0 deletions sherlock/internal/api/sherlock/roles_v3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ func (s *handlerSuite) Test_roleFromModel() {
GrantsProdFirecloudGroup: model.GrantsProdFirecloudGroup,
GrantsDevAzureGroup: model.GrantsDevAzureGroup,
GrantsProdAzureGroup: model.GrantsProdAzureGroup,
GrantsBroadInstituteGroup: model.GrantsBroadInstituteGroup,
},
}
role := roleFromModel(model)
Expand Down
4 changes: 4 additions & 0 deletions sherlock/internal/models/role.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ type RoleFields struct {
// GrantsProdAzureGroup, when not null, indicates that a User with an unsuspended RoleAssignment to this Role
// should have their Azure account (if they have one) added to this group.
GrantsProdAzureGroup *string

// GrantsBroadInstituteGroup, when not null, indicates that a User with an unsuspended RoleAssignment to this
// Role should have their Broad Institute account (assuming it is active) added to this group.
GrantsBroadInstituteGroup *string
}

type Role struct {
Expand Down
2 changes: 1 addition & 1 deletion sherlock/internal/models/role_assignment.go
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ func (ra *RoleAssignment) AfterUpdate(tx *gorm.DB) error {
slack.SendPermissionChangeNotification(tx.Statement.Context, user.SlackReference(true), slack.PermissionChangeNotificationInputs{
Summary: fmt.Sprintf("edited RoleAssignment for %s", ra.Description(tx)),
Results: []string{
"Old fields: " + slack.EscapeText(litter.Sdump(ra.RoleAssignmentFields)),
"Old fields: " + slack.EscapeText(litter.Sdump(ra.previousFields)),
"New fields: " + slack.EscapeText(litter.Sdump(ra.RoleAssignmentFields)),
},
})
Expand Down
17 changes: 9 additions & 8 deletions sherlock/internal/models/test_data.go
Original file line number Diff line number Diff line change
Expand Up @@ -357,14 +357,15 @@ func (td *testDataImpl) Role_TerraSuitableEngineer() Role {
if td.role_terraSuitableEngineer.ID == 0 {
td.role_terraSuitableEngineer = Role{
RoleFields: RoleFields{
Name: utils.PointerTo("terra-suitable-engineer"),
SuspendNonSuitableUsers: utils.PointerTo(true),
AutoAssignAllUsers: utils.PointerTo(false),
GrantsDevFirecloudGroup: utils.PointerTo("terra-suitable-engineer-dev"),
GrantsQaFirecloudGroup: utils.PointerTo("terra-suitable-engineer-qa"),
GrantsProdFirecloudGroup: utils.PointerTo("terra-suitable-engineer-prod"),
GrantsDevAzureGroup: utils.PointerTo("00000000-0000-0000-0000-000000000001"),
GrantsProdAzureGroup: utils.PointerTo("00000000-0000-0000-0000-000000000002"),
Name: utils.PointerTo("terra-suitable-engineer"),
SuspendNonSuitableUsers: utils.PointerTo(true),
AutoAssignAllUsers: utils.PointerTo(false),
GrantsDevFirecloudGroup: utils.PointerTo("terra-suitable-engineer-dev"),
GrantsQaFirecloudGroup: utils.PointerTo("terra-suitable-engineer-qa"),
GrantsProdFirecloudGroup: utils.PointerTo("terra-suitable-engineer-prod"),
GrantsDevAzureGroup: utils.PointerTo("00000000-0000-0000-0000-000000000001"),
GrantsProdAzureGroup: utils.PointerTo("00000000-0000-0000-0000-000000000002"),
GrantsBroadInstituteGroup: utils.PointerTo("terra-suitable-engineer-broadinstitute"),
},
}
td.h.SetSelfSuperAdminForDB()
Expand Down
6 changes: 6 additions & 0 deletions sherlock/internal/role_propagation/boot.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,12 @@ func Init(ctx context.Context) error {
getGrant: func(role models.Role) *string { return role.GrantsProdAzureGroup },
engine: &propagation_engines.AzureGroupEngine{},
},

&propagatorImpl[string, propagation_engines.NonAdminGoogleGroupIdentifier, propagation_engines.NonAdminGoogleGroupFields]{
configKey: "broadInstituteGroup",
getGrant: func(role models.Role) *string { return role.GrantsBroadInstituteGroup },
engine: &propagation_engines.NonAdminGoogleGroupEngine{},
},
}
for _, p := range propagators {
if err := p.Init(ctx); err != nil {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
package propagation_engines

import (
"context"
"fmt"
"github.com/broadinstitute/sherlock/sherlock/internal/clients/bits_data_warehouse"
"github.com/broadinstitute/sherlock/sherlock/internal/models"
"github.com/broadinstitute/sherlock/sherlock/internal/models/self"
"github.com/broadinstitute/sherlock/sherlock/internal/role_propagation/intermediary_user"
"github.com/knadh/koanf"
cloudidentity "google.golang.org/api/cloudidentity/v1beta1"
"google.golang.org/api/option"
)

type NonAdminGoogleGroupIdentifier struct {
Email string `koanf:"email"`

// resourceName is like "groups/<ID>/memberships/<ID>". It's output-only from the Cloud Identity API.
// We need it to delete or update membership, but it obviously can't exist before creation. This
// field here is basically a note-taking field. In NonAdminGoogleGroupEngine.LoadCurrentState we
// learn of the value and we write it down. In NonAdminGoogleGroupEngine.Remove we check to see
// if we already have a value. If not, we can make an extra round trip to look it up.
//
// This isn't an exported field because it's an unpredictable value we're comfortable encapsulating
// within this engine. It makes sense to try to carry it between LoadCurrentState and Remove, but
// it isn't something relevant for determining if someone should be in a group or not (so we don't
// want it to be checked in EqualTo, for example).
resourceName *string
}

func (n NonAdminGoogleGroupIdentifier) EqualTo(other intermediary_user.Identifier) bool {
switch other := other.(type) {
case NonAdminGoogleGroupIdentifier:
return n.Email == other.Email
default:
return false
}
}

type NonAdminGoogleGroupFields struct{}

func (n NonAdminGoogleGroupFields) EqualTo(other intermediary_user.Fields) bool {
switch other.(type) {
case NonAdminGoogleGroupFields:
return true
default:
return false
}
}

// NonAdminGoogleGroupEngine is like GoogleWorkspaceGroupEngine but it's designed to operate without workspace-level
// admin privileges.
type NonAdminGoogleGroupEngine struct {
service *cloudidentity.Service
}

func (n *NonAdminGoogleGroupEngine) Init(ctx context.Context, k *koanf.Koanf) error {
var err error
n.service, err = cloudidentity.NewService(ctx, option.WithScopes(cloudidentity.CloudIdentityGroupsScope))
return err
}

func (n *NonAdminGoogleGroupEngine) CalculateToleratedUsers(_ context.Context) ([]NonAdminGoogleGroupIdentifier, error) {
return []NonAdminGoogleGroupIdentifier{
{
Email: self.Email,
},
}, nil
}

func (n *NonAdminGoogleGroupEngine) LoadCurrentState(ctx context.Context, grant string) ([]intermediary_user.IntermediaryUser[NonAdminGoogleGroupIdentifier, NonAdminGoogleGroupFields], error) {
lookupResponse, err := n.service.Groups.Lookup().GroupKeyId(grant).Context(ctx).Do()
if err != nil {
return nil, fmt.Errorf("failed to lookup cloudidentity group ID for %s: %w", grant, err)
}

currentState := make([]intermediary_user.IntermediaryUser[NonAdminGoogleGroupIdentifier, NonAdminGoogleGroupFields], 0)
err = n.service.Groups.Memberships.List(lookupResponse.Name).Pages(ctx, func(memberships *cloudidentity.ListMembershipsResponse) error {
for _, membership := range memberships.Memberships {
resourceName := membership.Name
currentState = append(currentState, intermediary_user.IntermediaryUser[NonAdminGoogleGroupIdentifier, NonAdminGoogleGroupFields]{
Identifier: NonAdminGoogleGroupIdentifier{
Email: membership.PreferredMemberKey.Id,
resourceName: &resourceName,
},
Fields: NonAdminGoogleGroupFields{},
})
}
return nil
})
return currentState, err
}

func (n *NonAdminGoogleGroupEngine) GenerateDesiredState(_ context.Context, roleAssignments map[uint]models.RoleAssignment) (map[uint]intermediary_user.IntermediaryUser[NonAdminGoogleGroupIdentifier, NonAdminGoogleGroupFields], error) {
desiredState := make(map[uint]intermediary_user.IntermediaryUser[NonAdminGoogleGroupIdentifier, NonAdminGoogleGroupFields])
for id, roleAssignment := range roleAssignments {
if !roleAssignment.IsActive() {
// There's no concept of a suspended group member, so we just exclude any non-active users
continue
}

if _, found, err := bits_data_warehouse.GetPerson(roleAssignment.User.Email); err != nil {
return nil, fmt.Errorf("bits_data_warehouse error: %w", err)
} else if found {
// If the user is found in bits_data_warehouse, that means their account exists enough
// that it's reasonable for us to add them to the desired state
desiredState[id] = intermediary_user.IntermediaryUser[NonAdminGoogleGroupIdentifier, NonAdminGoogleGroupFields]{
Identifier: NonAdminGoogleGroupIdentifier{Email: roleAssignment.User.Email},
Fields: NonAdminGoogleGroupFields{},
}
}
}
return desiredState, nil
}

func (n *NonAdminGoogleGroupEngine) Add(ctx context.Context, grant string, identifier NonAdminGoogleGroupIdentifier, _ NonAdminGoogleGroupFields) (string, error) {
lookupResponse, err := n.service.Groups.Lookup().GroupKeyId(grant).Context(ctx).Do()
if err != nil {
return "", fmt.Errorf("failed to add %s to %s due to cloudidentity lookup error: %w", identifier.Email, grant, err)
}

_, err = n.service.Groups.Memberships.Create(lookupResponse.Name, &cloudidentity.Membership{
PreferredMemberKey: &cloudidentity.EntityKey{
Id: identifier.Email,
},
Roles: []*cloudidentity.MembershipRole{
{
Name: "MEMBER",
},
},
}).Context(ctx).Do()
if err != nil {
return "", fmt.Errorf("failed to add %s to %s: %w", identifier.Email, grant, err)
} else {
return fmt.Sprintf("added %s to %s", identifier.Email, grant), nil
}

}

func (n *NonAdminGoogleGroupEngine) Update(_ context.Context, _ string, _ NonAdminGoogleGroupIdentifier, _ NonAdminGoogleGroupFields, _ NonAdminGoogleGroupFields) (string, error) {
return "", fmt.Errorf("%T.Update not umplemented; %T.EqualTo should always equal true", n, NonAdminGoogleGroupFields{})
}

func (n *NonAdminGoogleGroupEngine) Remove(ctx context.Context, grant string, identifier NonAdminGoogleGroupIdentifier) (string, error) {
var resourceName string

if identifier.resourceName != nil {
resourceName = *identifier.resourceName
} else {
// Not sure we can hit this case but the code is trivial and it's easier to reason about with it being independent like this
groupLookupResponse, err := n.service.Groups.Lookup().GroupKeyId(grant).Context(ctx).Do()
if err != nil {
return "", fmt.Errorf("failed to remove %s from %s due to cloudidentity lookup error: %w", identifier.Email, grant, err)
}
membershipLookupResponse, err := n.service.Groups.Memberships.Lookup(groupLookupResponse.Name).MemberKeyId(identifier.Email).Context(ctx).Do()
if err != nil {
return "", fmt.Errorf("failed to remove %s from %s due to cloudidentity lookup error: %w", identifier.Email, grant, err)
}
resourceName = membershipLookupResponse.Name
}

_, err := n.service.Groups.Memberships.Delete(resourceName).Context(ctx).Do()
if err != nil {
return "", fmt.Errorf("failed to remove %s from %s: %w", identifier.Email, grant, err)
} else {
return fmt.Sprintf("removed %s from %s", identifier.Email, grant), nil
}
}
Loading

0 comments on commit b08627d

Please sign in to comment.