diff --git a/enterprise/server/backends/authdb/authdb.go b/enterprise/server/backends/authdb/authdb.go index da5b500a41f..b6a6022cac1 100644 --- a/enterprise/server/backends/authdb/authdb.go +++ b/enterprise/server/backends/authdb/authdb.go @@ -199,9 +199,17 @@ func (d *AuthDB) backfillUnencryptedKeys() error { } type apiKeyGroup struct { - APIKeyID string - UserID string - GroupID string + APIKeyID string + UserID string + GroupID string + // The ChildGroupIDs below is only populated if a group is marked as a + // 'parent' group. + IsParent bool + // If an API key from a parent group is allowed to manage child groups, + // those child groups will appear in this list. This is currently only used + // for external user management via SCIM/API where an admin API key from the + // parent group is used to manage all the related groups and their users. + ChildGroupIDs []string `gorm:"-"` Capabilities int32 UseGroupOwnedExecutors bool CacheEncryptionEnabled bool @@ -220,10 +228,18 @@ func (g *apiKeyGroup) GetGroupID() string { return g.GroupID } +func (g *apiKeyGroup) GetChildGroupIDs() []string { + return g.ChildGroupIDs +} + func (g *apiKeyGroup) GetCapabilities() int32 { return g.Capabilities } +func (g *apiKeyGroup) HasCapability(cap akpb.ApiKey_Capability) bool { + return g.Capabilities&int32(cap) != 0 +} + func (g *apiKeyGroup) GetUseGroupOwnedExecutors() bool { return g.UseGroupOwnedExecutors } @@ -339,6 +355,32 @@ func (d *AuthDB) fillDecryptedAPIKey(ak *tables.APIKey) error { return nil } +func (d *AuthDB) fillChildGroupIDs(ctx context.Context, akg *apiKeyGroup) error { + // If the group is not designated as a parent then don't bother doing a + // query. + if !akg.IsParent { + return nil + } + if !akg.HasCapability(akpb.ApiKey_ORG_ADMIN_CAPABILITY) { + return nil + } + rq := d.h.NewQuery(ctx, "authdb_get_child_group_ids").Raw(` + SELECT child.group_id + FROM "Groups" parent + JOIN "Groups" child on parent.saml_idp_metadata_url = child.saml_idp_metadata_url + WHERE parent.group_id = ? + AND child.group_id != ? + `, akg.GetGroupID(), akg.GetGroupID()) + groupIDs, err := db.ScanAll(rq, &struct{ GroupID string }{}) + if err != nil { + return err + } + for _, groupID := range groupIDs { + akg.ChildGroupIDs = append(akg.ChildGroupIDs, groupID.GroupID) + } + return nil +} + func (d *AuthDB) GetAPIKeyGroupFromAPIKey(ctx context.Context, apiKey string) (interfaces.APIKeyGroup, error) { apiKey = strings.TrimSpace(apiKey) if strings.Contains(apiKey, " ") || len(apiKey) != apiKeyLength { @@ -394,6 +436,9 @@ func (d *AuthDB) GetAPIKeyGroupFromAPIKey(ctx context.Context, apiKey string) (i } return nil, err } + if err := d.fillChildGroupIDs(ctx, akg); err != nil { + return nil, err + } if d.apiKeyGroupCache != nil { metrics.APIKeyLookupCount.With(prometheus.Labels{metrics.APIKeyLookupStatus: "cache_miss"}).Inc() d.apiKeyGroupCache.Add(cacheKey, akg) @@ -435,6 +480,9 @@ func (d *AuthDB) GetAPIKeyGroupFromAPIKeyID(ctx context.Context, apiKeyID string } return nil, err } + if err := d.fillChildGroupIDs(ctx, akg); err != nil { + return nil, err + } if d.apiKeyGroupCache != nil { d.apiKeyGroupCache.Add(cacheKey, akg) } @@ -494,7 +542,8 @@ func (d *AuthDB) newAPIKeyGroupQuery(subDomain string, allowUserOwnedKeys bool) g.group_id, g.use_group_owned_executors, g.cache_encryption_enabled, - g.enforce_ip_rules + g.enforce_ip_rules, + g.is_parent FROM "Groups" AS g, "APIKeys" AS ak `) diff --git a/enterprise/server/backends/userdb/userdb.go b/enterprise/server/backends/userdb/userdb.go index 89baf58f953..8a0a90391a2 100644 --- a/enterprise/server/backends/userdb/userdb.go +++ b/enterprise/server/backends/userdb/userdb.go @@ -363,7 +363,8 @@ func (d *UserDB) UpdateGroup(ctx context.Context, g *tables.Group) (string, erro cache_encryption_enabled = ?, suggestion_preference = ?, restrict_clean_workflow_runs_to_admins = ?, - enforce_ip_rules = ? + enforce_ip_rules = ?, + is_parent = ? WHERE group_id = ?`, g.Name, g.URLIdentifier, @@ -378,6 +379,7 @@ func (d *UserDB) UpdateGroup(ctx context.Context, g *tables.Group) (string, erro g.SuggestionPreference, g.RestrictCleanWorkflowRunsToAdmins, g.EnforceIPRules, + g.IsParent, g.GroupID, ).Exec().Error if err != nil { diff --git a/enterprise/server/backends/userdb/userdb_test.go b/enterprise/server/backends/userdb/userdb_test.go index a9e166d54bd..5d8588d0b3d 100644 --- a/enterprise/server/backends/userdb/userdb_test.go +++ b/enterprise/server/backends/userdb/userdb_test.go @@ -1828,3 +1828,94 @@ func TestCapabilitiesForUserRole(t *testing.T) { }) } } + +func TestChildGroupAuth(t *testing.T) { + flags.Set(t, "auth.api_key_group_cache_ttl", 0) + env := newTestEnv(t) + flags.Set(t, "app.create_group_per_user", true) + flags.Set(t, "app.no_default_user_group", true) + udb := env.GetUserDB() + ctx := context.Background() + + auth := env.GetAuthenticator().(*testauth.TestAuthenticator) + auth.APIKeyProvider = func(apiKey string) interfaces.UserInfo { + ui, err := env.GetAuthDB().GetAPIKeyGroupFromAPIKey(context.Background(), apiKey) + require.NoError(t, err) + return claims.APIKeyGroupClaims(ui) + } + + // Start with two independent groups. + createUser(t, ctx, env, "US1", "org1.io") + ctx1 := authUserCtx(ctx, env, t, "US1") + us1Group := getGroup(t, ctx1, env).Group + key1, err := env.GetAuthDB().CreateAPIKey( + ctx1, us1Group.GroupID, "admin", + []akpb.ApiKey_Capability{akpb.ApiKey_ORG_ADMIN_CAPABILITY}, + false /*=visibleToDevelopers*/) + require.NoError(t, err) + adminCtx1 := env.GetAuthenticator().AuthContextFromAPIKey(ctx, key1.Value) + + createUser(t, ctx, env, "US2", "org2.io") + ctx2 := authUserCtx(ctx, env, t, "US2") + us2Group := getGroup(t, ctx2, env).Group + key2, err := env.GetAuthDB().CreateAPIKey( + ctx2, us2Group.GroupID, "admin", + []akpb.ApiKey_Capability{akpb.ApiKey_ORG_ADMIN_CAPABILITY}, + false /*=visibleToDevelopers*/) + require.NoError(t, err) + //adminCtx2 := env.GetAuthenticator().AuthContextFromAPIKey(ctx, key2.Value) + + // Admin key for group1 shouldn't be able to affect anything in group2. + err = udb.UpdateGroupUsers(adminCtx1, us2Group.GroupID, []*grpb.UpdateGroupUsersRequest_Update{{ + UserId: &uidpb.UserId{Id: "US2"}, + Role: grpb.Group_DEVELOPER_ROLE, + }}) + require.Error(t, err, "should not be able to update role of US2") + require.True(t, status.IsPermissionDeniedError(err)) + err = udb.UpdateGroupUsers(adminCtx1, us1Group.GroupID, []*grpb.UpdateGroupUsersRequest_Update{{ + UserId: &uidpb.UserId{Id: "US1"}, + Role: grpb.Group_DEVELOPER_ROLE, + }}) + require.NoError(t, err, "should be able to update role of US2") + + us1Group.SamlIdpMetadataUrl = "https://some/saml/url" + us1Group.URLIdentifier = "org1" + _, err = udb.UpdateGroup(ctx1, &us1Group) + require.NoError(t, err) + us2Group.SamlIdpMetadataUrl = us1Group.SamlIdpMetadataUrl + us2Group.URLIdentifier = "org2" + _, err = udb.UpdateGroup(ctx2, &us2Group) + require.NoError(t, err) + + // Re-auth and try again. US1 should still not be able to affect the second + // group. + adminCtx1 = env.GetAuthenticator().AuthContextFromAPIKey(ctx, key1.Value) + err = udb.UpdateGroupUsers(adminCtx1, us2Group.GroupID, []*grpb.UpdateGroupUsersRequest_Update{{ + UserId: &uidpb.UserId{Id: "US2"}, + Role: grpb.Group_DEVELOPER_ROLE, + }}) + require.Error(t, err, "should not be able to update role of US2") + require.True(t, status.IsPermissionDeniedError(err)) + + // Now mark the first group as a "parent" organization. + // Since they share the same SAML IDP Metadata URL, a group administrator + // from the first group should be able to affect the child group, but + // not vice versa. + us1Group.IsParent = true + _, err = udb.UpdateGroup(adminCtx1, &us1Group) + require.NoError(t, err) + adminCtx1 = env.GetAuthenticator().AuthContextFromAPIKey(ctx, key1.Value) + err = udb.UpdateGroupUsers(adminCtx1, us2Group.GroupID, []*grpb.UpdateGroupUsersRequest_Update{{ + UserId: &uidpb.UserId{Id: "US2"}, + Role: grpb.Group_DEVELOPER_ROLE, + }}) + require.NoError(t, err, "should be able to update role of US2") + + adminCtx2 := env.GetAuthenticator().AuthContextFromAPIKey(ctx, key2.Value) + err = udb.UpdateGroupUsers(adminCtx2, us1Group.GroupID, []*grpb.UpdateGroupUsersRequest_Update{{ + UserId: &uidpb.UserId{Id: "US1"}, + Role: grpb.Group_DEVELOPER_ROLE, + }}) + require.Error(t, err, "should not be able to update role of US1") + require.True(t, status.IsPermissionDeniedError(err)) +} diff --git a/server/interfaces/interfaces.go b/server/interfaces/interfaces.go index 754f33b416f..9134e18f5dd 100644 --- a/server/interfaces/interfaces.go +++ b/server/interfaces/interfaces.go @@ -426,6 +426,7 @@ type APIKeyGroup interface { GetAPIKeyID() string GetUserID() string GetGroupID() string + GetChildGroupIDs() []string GetUseGroupOwnedExecutors() bool GetCacheEncryptionEnabled() bool GetEnforceIPRules() bool diff --git a/server/tables/tables.go b/server/tables/tables.go index 24226eec768..502276b20d1 100644 --- a/server/tables/tables.go +++ b/server/tables/tables.go @@ -239,7 +239,7 @@ type Group struct { EnforceIPRules bool `gorm:"not null;default:0"` // The SAML IDP Metadata URL for this group. - SamlIdpMetadataUrl string + SamlIdpMetadataUrl string `gorm:"index:group_saml_idp_metadata_url_idx"` InvocationWebhookURL string `gorm:"not null;default:''"` @@ -248,6 +248,10 @@ type Group struct { // The public key and encrypted private key. Used to upload secrets. PublicKey string EncryptedPrivateKey string + + // When a Group is designated as a "parent" then any Admin keys from that + // org also work for managing groups with the same SAML IDP Metadata URL. + IsParent bool `gorm:"not null;default:0"` } func (g *Group) TableName() string { diff --git a/server/util/claims/claims.go b/server/util/claims/claims.go index 8d20534520a..3f7f7120994 100644 --- a/server/util/claims/claims.go +++ b/server/util/claims/claims.go @@ -141,18 +141,26 @@ func APIKeyGroupClaims(akg interfaces.APIKeyGroup) *Claims { if akg.GetCapabilities()&int32(akpb.ApiKey_ORG_ADMIN_CAPABILITY) > 0 { keyRole = role.Admin } + allowedGroups := []string{akg.GetGroupID()} + groupMemberships := []*interfaces.GroupMembership{{ + GroupID: akg.GetGroupID(), + Capabilities: capabilities.FromInt(akg.GetCapabilities()), + Role: keyRole, + }} + for _, cg := range akg.GetChildGroupIDs() { + allowedGroups = append(allowedGroups, cg) + groupMemberships = append(groupMemberships, &interfaces.GroupMembership{ + GroupID: cg, + Capabilities: capabilities.FromInt(akg.GetCapabilities()), + Role: keyRole, + }) + } return &Claims{ - APIKeyID: akg.GetAPIKeyID(), - UserID: akg.GetUserID(), - GroupID: akg.GetGroupID(), - AllowedGroups: []string{akg.GetGroupID()}, - GroupMemberships: []*interfaces.GroupMembership{ - { - GroupID: akg.GetGroupID(), - Capabilities: capabilities.FromInt(akg.GetCapabilities()), - Role: keyRole, - }, - }, + APIKeyID: akg.GetAPIKeyID(), + UserID: akg.GetUserID(), + GroupID: akg.GetGroupID(), + AllowedGroups: allowedGroups, + GroupMemberships: groupMemberships, Capabilities: capabilities.FromInt(akg.GetCapabilities()), UseGroupOwnedExecutors: akg.GetUseGroupOwnedExecutors(), CacheEncryptionEnabled: akg.GetCacheEncryptionEnabled(),