Tenant filter by Privileged Sites

Author: hwadekar-nv

rest-api/api/pkg/api/handler/expectedmachine.go

@@ -60,7 +60,7 @@ func ValidateProviderOrTenantSiteAccess(ctx context.Context, logger zerolog.Logg
 		// Privileged tenants may access Sites via a Ready TenantAccount with
 		// TargetedInstanceCreation even without an explicit TenantSite row.
 		if !hasAccess {
-			enabled, err := common.EffectiveTargetedInstanceCreation(ctx, nil, dbSession, tenant, site.ID)
+			enabled, err := common.TenantHasTargetedInstanceCreation(ctx, nil, dbSession, tenant, site)
 			if err != nil {
 				logger.Error().Err(err).Msg("error resolving TargetedInstanceCreation for Tenant/Site")
 				return false, cutil.NewAPIError(http.StatusInternalServerError, "Failed to resolve Tenant capability for Site due to DB error", nil)
@@ -347,7 +347,7 @@ func (gaemh GetAllExpectedMachineHandler) Handle(c echo.Context) error {
 	}
 
 	if tenant != nil {
-		privileged, err := common.TenantHasTargetedInstanceCreation(ctx, nil, gaemh.dbSession, tenant)
+		privileged, err := common.TenantHasTargetedInstanceCreation(ctx, nil, gaemh.dbSession, tenant, nil)
 		if err != nil {
 			logger.Error().Err(err).Msg("error resolving TargetedInstanceCreation for Tenant")
 			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to resolve Tenant capability due to DB error", nil)

rest-api/api/pkg/api/handler/instance.go

@@ -892,12 +892,12 @@ func (cih CreateInstanceHandler) Handle(c echo.Context) error {
 
 		// Begin validating Machine ID
 		if apiRequest.MachineID != nil {
-			enabledForSite, derr := common.EffectiveTargetedInstanceCreation(ctx, tx, cih.dbSession, tenant, site.ID)
+			privilegedAccess, derr := common.TenantHasTargetedInstanceCreation(ctx, tx, cih.dbSession, tenant, site)
 			if derr != nil {
 				logger.Error().Err(derr).Msg("error checking effective targeted instance creation for Site")
 				return cutil.NewAPIError(http.StatusInternalServerError, "Failed to verify capability for Site", nil)
 			}
-			if !enabledForSite {
+			if !privilegedAccess {
 				logger.Warn().Msg("tenant does not have capability to create instances from specific machine")
 				return cutil.NewAPIError(http.StatusForbidden, "Tenant does not have capability to create Instances using specific Machine ID", nil)
 			}
@@ -4905,7 +4905,10 @@ func (dih DeleteInstanceHandler) Handle(c echo.Context) error {
 		// capability. By the time `ToProto` runs the request is safe
 		// to trust.
 		if apiRequest.IsRepairTenant != nil && *apiRequest.IsRepairTenant {
-			enabledForSite, derr := common.EffectiveTargetedInstanceCreation(ctx, tx, dih.dbSession, instance.Tenant, instance.SiteID)
+			enabledForSite, derr := common.TenantHasTargetedInstanceCreation(ctx, tx, dih.dbSession, instance.Tenant, &cdbm.Site{
+				ID:                       instance.SiteID,
+				InfrastructureProviderID: instance.InfrastructureProviderID,
+			})
 			if derr != nil {
 				logger.Error().Err(derr).Msg("error checking effective targeted instance creation for Instance's Site")
 				return cutil.NewAPIError(http.StatusInternalServerError, "Failed to verify capability for Instance's Site", nil)

rest-api/api/pkg/api/handler/machine.go

@@ -206,10 +206,8 @@ func (gamh GetAllMachineHandler) Handle(c echo.Context) error {
 	// request the privileged-tenant pre-gate here (requirePrivilegedTenant=false):
 	// that gate keys off a Ready TenantAccount without site context and would
 	// reject site-privileged tenants (or those whose privilege is per-site)
-	// before machine.SiteID is known. The site-scoped
-	// EffectiveTargetedInstanceCreation checks below are authoritative and
-	// filter results to the providers/sites where the capability is effective,
-	// yielding an empty list rather than a spurious 403.
+	// before siteId is known. Privileged Tenant access is resolved below via
+	// GetPrivilegedAccessSiteIDsForTenant, which honors per-site overrides.
 	infrastructureProvider, tenant, apiError := common.IsProviderOrTenant(ctx, logger, gamh.dbSession, org, dbUser, true, false)
 	if apiError != nil {
 		return cutil.NewAPIErrorResponse(c, apiError.Code, apiError.Message, apiError.Data)
@@ -238,35 +236,27 @@ func (gamh GetAllMachineHandler) Handle(c echo.Context) error {
 		return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, errMsg, nil)
 	}
 
-	filterInput := cdbm.MachineFilterInput{
-		InfrastructureProviderIDs: []uuid.UUID{},
-	}
+	filterInput := cdbm.MachineFilterInput{}
 
-	// Validate other query params
 	if infrastructureProvider != nil {
-		filterInput.InfrastructureProviderIDs = append(filterInput.InfrastructureProviderIDs, infrastructureProvider.ID)
+		filterInput.InfrastructureProviderIDs = []uuid.UUID{infrastructureProvider.ID}
 	}
 
-	var tenantAccountProviderIDs []uuid.UUID
+	var privilegedSiteIDs []uuid.UUID
 	if tenant != nil {
-		taDAO := cdbm.NewTenantAccountDAO(gamh.dbSession)
-		tas, _, serr := taDAO.GetAll(ctx, nil, cdbm.TenantAccountFilterInput{
-			TenantIDs: []uuid.UUID{tenant.ID},
-			Statuses:  []string{cdbm.TenantAccountStatusReady},
-		}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, []string{})
+		var serr error
+		privilegedSiteIDs, serr = common.GetPrivilegedAccessSiteIDsForTenant(ctx, nil, gamh.dbSession, tenant)
 		if serr != nil {
-			logger.Error().Err(serr).Msg("error retrieving Tenant Accounts for Tenant")
-			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Error retrieving Tenant Accounts for Tenant", nil)
+			logger.Error().Err(serr).Msg("error resolving privileged Site access for Tenant")
+			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to verify privileges for Tenant", nil)
 		}
-
-		for _, ta := range tas {
-			tenantAccountProviderIDs = append(tenantAccountProviderIDs, ta.InfrastructureProviderID)
+		if len(privilegedSiteIDs) == 0 {
+			return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Tenant does not have Targeted Instance Creation capability enabled for any Site", nil)
 		}
 	}
 
 	// Validate site id if provided
 	qSiteID := c.QueryParam("siteId")
-	var filterSite *cdbm.Site
 	if qSiteID != "" {
 		site, serr := common.GetSiteFromIDString(ctx, nil, qSiteID, gamh.dbSession)
 		if serr != nil {
@@ -276,71 +266,22 @@ func (gamh GetAllMachineHandler) Handle(c echo.Context) error {
 			logger.Error().Err(serr).Str("Site ID", qSiteID).Msg("error retrieving Site specified in query")
 			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Error retrieving Site specified in query", nil)
 		}
-		filterSite = site
 
-		isAssociated := false
 		if infrastructureProvider != nil {
-			// Check if Site belongs to org's Infrastructure Provider
-			if site.InfrastructureProviderID == infrastructureProvider.ID {
-				isAssociated = true
+			if site.InfrastructureProviderID != infrastructureProvider.ID {
+				logger.Error().Msg("Site is not associated with org")
+				return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Current org is not associated with the Site specified in query", nil)
 			}
-		}
-
-		if !isAssociated && tenant != nil {
-			isAssociated = slices.Contains(tenantAccountProviderIDs, site.InfrastructureProviderID)
-		}
-
-		if isAssociated {
 			filterInput.SiteIDs = []uuid.UUID{site.ID}
 		} else {
-			logger.Error().Msg("Site is not associated with org")
-			return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Current org is not associated with the Site specified in query", nil)
-		}
-	}
-
-	if tenant != nil {
-		if filterSite != nil {
-			enabledForSite, serr := common.EffectiveTargetedInstanceCreation(ctx, nil, gamh.dbSession, tenant, filterSite.ID)
-			if serr != nil {
-				logger.Error().Err(serr).Msg("error checking effective targeted instance creation for Site")
-				return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to verify privileges for Site", nil)
-			}
-			if enabledForSite {
-				filterInput.InfrastructureProviderIDs = append(filterInput.InfrastructureProviderIDs, filterSite.InfrastructureProviderID)
-			}
-		} else {
-			siteDAO := cdbm.NewSiteDAO(gamh.dbSession)
-			privilegedProviderIDs := make(map[uuid.UUID]struct{})
-			for _, providerID := range tenantAccountProviderIDs {
-				if _, listed := privilegedProviderIDs[providerID]; listed {
-					continue
-				}
-
-				sites, _, serr := siteDAO.GetAll(ctx, nil, cdbm.SiteFilterInput{
-					InfrastructureProviderIDs: []uuid.UUID{providerID},
-				}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, nil)
-				if serr != nil {
-					logger.Error().Err(serr).Msg("error retrieving Sites for privileged Tenant")
-					return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Error retrieving Sites for privileged Tenant", nil)
-				}
-
-				for _, site := range sites {
-					enabledForSite, serr := common.EffectiveTargetedInstanceCreation(ctx, nil, gamh.dbSession, tenant, site.ID)
-					if serr != nil {
-						logger.Error().Err(serr).Msg("error checking effective targeted instance creation for Site")
-						return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to verify privileges for Site", nil)
-					}
-					if enabledForSite {
-						privilegedProviderIDs[providerID] = struct{}{}
-						break
-					}
-				}
-			}
-
-			for providerID := range privilegedProviderIDs {
-				filterInput.InfrastructureProviderIDs = append(filterInput.InfrastructureProviderIDs, providerID)
+			if !slices.Contains(privilegedSiteIDs, site.ID) {
+				logger.Error().Msg("Site is not associated with org")
+				return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Current org is not associated with the Site specified in query", nil)
 			}
+			filterInput.SiteIDs = []uuid.UUID{site.ID}
 		}
+	} else if tenant != nil {
+		filterInput.SiteIDs = privilegedSiteIDs
 	}
 
 	// Validate InstanceType ID if provided
@@ -665,7 +606,10 @@ func (gmh GetMachineHandler) Handle(c echo.Context) error {
 		isAssociated = true
 		isProviderOrPrivilegedTenant = true
 	} else if tenant != nil {
-		enabledForSite, serr := common.EffectiveTargetedInstanceCreation(ctx, nil, gmh.dbSession, tenant, machine.SiteID)
+		enabledForSite, serr := common.TenantHasTargetedInstanceCreation(ctx, nil, gmh.dbSession, tenant, &cdbm.Site{
+			ID:                       machine.SiteID,
+			InfrastructureProviderID: machine.InfrastructureProviderID,
+		})
 		if serr != nil {
 			logger.Error().Err(serr).Msg("error checking effective targeted instance creation for Machine's Site")
 			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Error verifying capability for Machine's Site", nil)
@@ -766,7 +710,7 @@ func (umh UpdateMachineHandler) Handle(c echo.Context) error {
 	// pre-gate here (requirePrivilegedTenant=false): that gate keys off the
 	// legacy tenant-level flag and would reject TenantAccount-privileged
 	// tenants before machine.SiteID is known. The site-scoped
-	// EffectiveTargetedInstanceCreation check is the authoritative decision.
+	// TenantHasTargetedInstanceCreation check is the authoritative decision.
 	infrastructureProvider, tenant, apiError := common.IsProviderOrTenant(ctx, logger, umh.dbSession, org, dbUser, false, false)
 	if apiError != nil {
 		return cutil.NewAPIErrorResponse(c, apiError.Code, apiError.Message, apiError.Data)
@@ -807,7 +751,10 @@ func (umh UpdateMachineHandler) Handle(c echo.Context) error {
 	// Validate if Tenant is allowed to update Machine
 	if tenant != nil {
 		// Check if Tenant is privileged
-		enabledForSite, serr := common.EffectiveTargetedInstanceCreation(ctx, nil, umh.dbSession, tenant, machine.SiteID)
+		enabledForSite, serr := common.TenantHasTargetedInstanceCreation(ctx, nil, umh.dbSession, tenant, &cdbm.Site{
+			ID:                       machine.SiteID,
+			InfrastructureProviderID: machine.InfrastructureProviderID,
+		})
 		if serr != nil {
 			logger.Error().Err(serr).Msg("error checking effective targeted instance creation for Machine's Site")
 			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Error verifying capability for Machine's Site, DB error", nil)

rest-api/api/pkg/api/handler/machine_test.go

@@ -1057,15 +1057,12 @@ func TestMachineHandler_GetAll(t *testing.T) {
 			verifyChildSpanner: true,
 		},
 		{
-			name:               "success case when Tenant has TargetedInstanceCreation capability and filters by Site ID",
+			name:               "failure case when Tenant has no effective TargetedInstanceCreation for Site",
 			reqOrgName:         tnOrg2,
 			user:               tnu2,
 			querySiteID:        cutil.GetPtr(siteT2.ID.String()),
-			expectedErr:        false,
-			expectedStatus:     http.StatusOK,
-			expectedCnt:        0,
-			expectedTotal:      cutil.GetPtr(0),
-			expectInstance:     false,
+			expectedErr:        true,
+			expectedStatus:     http.StatusForbidden,
 			verifyChildSpanner: true,
 		},
 		{
@@ -1078,13 +1075,11 @@ func TestMachineHandler_GetAll(t *testing.T) {
 			expectedTotal:  cutil.GetPtr(totalCount / 2),
 		},
 		{
-			name:           "empty result when Tenant has TargetedInstanceCreation capability but no Tenant Account",
+			name:           "failure case when Tenant has no effective TargetedInstanceCreation on any Site",
 			reqOrgName:     tnOrg8,
 			user:           tnu8,
-			expectedErr:    false,
-			expectedStatus: http.StatusOK,
-			expectedCnt:    0,
-			expectedTotal:  cutil.GetPtr(0),
+			expectedErr:    true,
+			expectedStatus: http.StatusForbidden,
 		},
 		{
 			name:                "success case when Instance Type ID specified in query",

rest-api/api/pkg/api/handler/site.go

@@ -651,7 +651,7 @@ func (gsh GetSiteHandler) Handle(c echo.Context) error {
 		}
 
 		if !isAssociated {
-			enabled, serr := common.EffectiveTargetedInstanceCreation(ctx, nil, gsh.dbSession, tenant, stID)
+			enabled, serr := common.TenantHasTargetedInstanceCreation(ctx, nil, gsh.dbSession, tenant, st)
 			if serr != nil {
 				logger.Error().Err(serr).Msg("error resolving TargetedInstanceCreation for Tenant/Site")
 				return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to resolve Tenant capability for Site, DB error", nil)
@@ -888,7 +888,7 @@ func (gash GetAllSiteHandler) Handle(c echo.Context) error {
 
 		// If Tenant has TargetedInstanceCreation on any Ready TenantAccount,
 		// also retrieve all Sites from Providers where that capability is enabled.
-		privileged, serr := common.TenantHasTargetedInstanceCreation(ctx, nil, gash.dbSession, tenant)
+		privileged, serr := common.TenantHasTargetedInstanceCreation(ctx, nil, gash.dbSession, tenant, nil)
 		if serr != nil {
 			logger.Error().Err(serr).Msg("error resolving TargetedInstanceCreation for Tenant")
 			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to resolve Tenant capability, DB error", nil)
@@ -1226,7 +1226,7 @@ func (gssdh GetSiteStatusDetailsHandler) Handle(c echo.Context) error {
 		}
 
 		if !isAssociated {
-			enabled, serr := common.EffectiveTargetedInstanceCreation(ctx, nil, gssdh.dbSession, tenant, stID)
+			enabled, serr := common.TenantHasTargetedInstanceCreation(ctx, nil, gssdh.dbSession, tenant, st)
 			if serr != nil {
 				logger.Error().Err(serr).Msg("error resolving TargetedInstanceCreation for Tenant/Site")
 				return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to resolve Tenant capability for Site, DB error", nil)

rest-api/api/pkg/api/handler/sku.go

@@ -103,7 +103,7 @@ func (gash GetAllSkuHandler) Handle(c echo.Context) error {
 			return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Site specified in request data is does not belong to current org", nil)
 		}
 	} else if tenant != nil {
-		enabled, err := common.EffectiveTargetedInstanceCreation(ctx, nil, gash.dbSession, tenant, site.ID)
+		enabled, err := common.TenantHasTargetedInstanceCreation(ctx, nil, gash.dbSession, tenant, site)
 		if err != nil {
 			logger.Error().Err(err).Msg("error resolving TargetedInstanceCreation for Tenant/Site")
 			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to resolve Tenant capability for Site due to DB error", nil)
@@ -260,7 +260,7 @@ func (gsh GetSkuHandler) Handle(c echo.Context) error {
 			return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "SKU does not belong to a Site owned by current org", nil)
 		}
 	} else if tenant != nil {
-		enabled, err := common.EffectiveTargetedInstanceCreation(ctx, nil, gsh.dbSession, tenant, site.ID)
+		enabled, err := common.TenantHasTargetedInstanceCreation(ctx, nil, gsh.dbSession, tenant, site)
 		if err != nil {
 			logger.Error().Err(err).Msg("error resolving TargetedInstanceCreation for Tenant/Site")
 			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to resolve Tenant capability for Site due to DB error", nil)

rest-api/api/pkg/api/handler/tenant.go

@@ -207,7 +207,7 @@ func (gcth GetCurrentTenantHandler) Handle(c echo.Context) error {
 	}
 
 	// Create response
-	hasTargetedInstanceCreation, err := common.TenantHasTargetedInstanceCreation(ctx, nil, gcth.dbSession, tn)
+	hasTargetedInstanceCreation, err := common.TenantHasTargetedInstanceCreation(ctx, nil, gcth.dbSession, tn, nil)
 	if err != nil {
 		logger.Error().Err(err).Msg("error resolving TargetedInstanceCreation for Tenant")
 		return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to resolve Tenant capability due to DB error", nil)

rest-api/api/pkg/api/handler/tenantaccount.go

@@ -576,8 +576,8 @@ func (gtah GetTenantAccountHandler) Handle(c echo.Context) error {
 			TenantIDs: []uuid.UUID{*ta.TenantID},
 		}, cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, []string{"Site"})
 		if err != nil {
-			logger.Error().Err(err).Msg("error retrieving Tenant Sites for Tenant Account")
-			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant Sites for Tenant Account", nil)
+			logger.Error().Err(err).Msg("error retrieving Tenant Sites for Tenant")
+			return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Tenant Sites for Tenant", nil)
 		}
 	}