Allow admin API keys for a 'parent' group to affect child groups. (#7715

)

Groups are considered related if they have the same SAML Metadata URL.

Only keys for groups marked as 'parent' are allowed access to related
groups.

<!-- Optional: Provide additional context (beyond the PR title). -->

<!-- Optional: link a GitHub issue.
Example: "Fixes #123" will auto-close #123 when the PR is merged. -->

**Related issues**: N/A

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
 	`)

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 {

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))
+}

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

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 {

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(),