diff --git a/rest-api/api/pkg/api/handler/operatingsystem.go b/rest-api/api/pkg/api/handler/operatingsystem.go index 75128ae184..c4accd62e0 100644 --- a/rest-api/api/pkg/api/handler/operatingsystem.go +++ b/rest-api/api/pkg/api/handler/operatingsystem.go @@ -26,7 +26,6 @@ import ( "github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/model" "github.com/NVIDIA/infra-controller/rest-api/api/pkg/api/pagination" sc "github.com/NVIDIA/infra-controller/rest-api/api/pkg/client/site" - auth "github.com/NVIDIA/infra-controller/rest-api/auth/pkg/authorization" cutil "github.com/NVIDIA/infra-controller/rest-api/common/pkg/util" cdb "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db" cdbm "github.com/NVIDIA/infra-controller/rest-api/db/pkg/db/model" @@ -236,6 +235,26 @@ func validateIpxeTemplateAvailableAtSites(ctx context.Context, dbSession *cdb.Se return nil } +// getTenantSiteIDs returns the IDs of all sites the tenant has access to, +// regardless of site status. Used to scope provider-owned Operating System +// visibility for tenant admins. +func getTenantSiteIDs(ctx context.Context, dbSession *cdb.Session, tenantID uuid.UUID) ([]uuid.UUID, error) { + tsDAO := cdbm.NewTenantSiteDAO(dbSession) + tss, _, err := tsDAO.GetAll(ctx, nil, + cdbm.TenantSiteFilterInput{TenantIDs: []uuid.UUID{tenantID}}, + cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, + nil, + ) + if err != nil { + return nil, err + } + ids := make([]uuid.UUID, len(tss)) + for i, ts := range tss { + ids[i] = ts.SiteID + } + return ids, nil +} + // ~~~~~ Create Handler ~~~~~ // // CreateOperatingSystemHandler is the API Handler for creating new OperatingSystem @@ -278,47 +297,37 @@ func (csh CreateOperatingSystemHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve current user", nil) } - // Validate org - ok, err := auth.ValidateOrgMembership(dbUser, org) - if !ok { - if err != nil { - logger.Error().Err(err).Msg("error validating org membership for User in request") - } else { - logger.Warn().Msg("could not validate org membership for user, access denied") - } - return cutil.NewAPIErrorResponse(c, http.StatusForbidden, fmt.Sprintf("Failed to validate membership for org: %s", org), nil) - } - - // Validate role, only Tenant Admins are allowed to create OperatingSystem - ok = auth.ValidateUserRoles(dbUser, org, nil, auth.TenantAdminRole) - if !ok { - logger.Warn().Msg("user does not have Tenant Admin role, access denied") - return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "User does not have Tenant Admin role with org", nil) + ip, tenant, apiError := common.IsProviderOrTenant(ctx, logger, csh.dbSession, org, dbUser, false, false) + if apiError != nil { + return cutil.NewAPIErrorResponse(c, apiError.Code, apiError.Message, apiError.Data) } // Validate request // Bind request data to API model apiRequest := model.APIOperatingSystemCreateRequest{} - err = c.Bind(&apiRequest) + err := c.Bind(&apiRequest) if err != nil { logger.Warn().Err(err).Msg("error binding request data into API model") return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Failed to parse request data, potentially invalid structure", nil) } - // Validate the tenant for which this OperatingSystem is being created - tenant, err := common.GetTenantForOrg(ctx, nil, csh.dbSession, org) - if err != nil { - if err == common.ErrOrgTenantNotFound { - logger.Warn().Err(err).Msg("Org does not have a Tenant associated") - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Org does not have a Tenant associated", nil) - } - logger.Error().Err(err).Msg("unable to retrieve tenant for org") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve tenant for org", nil) + + // Infer OS type from the provided source fields (ipxeScript -> iPXE, + // ipxeTemplateId -> Templated iPXE, otherwise Image). + osType := apiRequest.GetOperatingSystemType() + + // Provider Admin is limited to iPXE Template-based OSes. When both roles + // allow the action, Provider Admin takes priority (provider-owned OS). + allowedByProvider := ip != nil && osType == cdbm.OperatingSystemTypeTemplatedIPXE + allowedByTenant := tenant != nil + if !allowedByProvider && !allowedByTenant { + logger.Warn().Msg("caller is not permitted to create this Operating System") + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Provider Admin can only create iPXE Template-based Operating Systems", nil) } - // Default TenantID to org's Tenant when nil; validate when set - if apiRequest.TenantID == nil { - apiRequest.TenantID = cutil.GetPtr(tenant.ID.String()) - } else if *apiRequest.TenantID != tenant.ID.String() { + // If the caller provided an explicit tenantId in the body (deprecated), validate + // it matches the org's tenant. + // TODO: tenantId as parameter is deprecated and will need to be removed by 2026-10-01. + if tenant != nil && apiRequest.TenantID != nil && *apiRequest.TenantID != tenant.ID.String() { logger.Warn().Str("tenantId", *apiRequest.TenantID).Msg("TenantID in request does not match org's Tenant") return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "TenantID specified in request does not match org's Tenant", nil) } @@ -337,99 +346,120 @@ func (csh CreateOperatingSystemHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Error validating user data in Operating System creation request", verr) } - // check for name uniqueness for the tenant, ie, tenant cannot have another os with same name + // Check for name uniqueness within the owner's scope (provider or tenant). // TODO consider doing this with an advisory lock for correctness osDAO := cdbm.NewOperatingSystemDAO(csh.dbSession) - oss, tot, err := osDAO.GetAll( - ctx, - nil, - cdbm.OperatingSystemFilterInput{ - TenantIDs: []uuid.UUID{tenant.ID}, - Names: []string{apiRequest.Name}, - }, - cdbp.PageInput{}, - nil, - ) + uniquenessFilter := cdbm.OperatingSystemFilterInput{Names: []string{apiRequest.Name}} + if allowedByProvider { + uniquenessFilter.InfrastructureProviderID = &ip.ID + } else { + uniquenessFilter.TenantIDs = []uuid.UUID{tenant.ID} + } + oss, tot, err := osDAO.GetAll(ctx, nil, uniquenessFilter, cdbp.PageInput{}, nil) if err != nil { - logger.Error().Err(err).Msg("db error checking for name uniqueness of tenant os") + logger.Error().Err(err).Msg("db error checking for name uniqueness of os") return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to create OperatingSystem due to DB error", nil) } if tot > 0 { - logger.Warn().Str("tenantId", tenant.ID.String()).Str("name", apiRequest.Name).Msg("Operating System with same name already exists for tenant") - return cutil.NewAPIErrorResponse(c, http.StatusConflict, "Another Operating System with specified name already exists for Tenant", validation.Errors{ + logger.Warn().Str("name", apiRequest.Name).Msg("Operating System with same name already exists") + return cutil.NewAPIErrorResponse(c, http.StatusConflict, "Another Operating System with specified name already exists", validation.Errors{ "id": errors.New(oss[0].ID.String()), }) } - // Infer OS type from the provided source fields (ipxeScript -> iPXE, - // ipxeTemplateId -> Templated iPXE, otherwise Image). - osType := apiRequest.GetOperatingSystemType() - // Set the phoneHomeEnabled if provided in request phoneHomeEnabled := false if apiRequest.PhoneHomeEnabled != nil { phoneHomeEnabled = *apiRequest.PhoneHomeEnabled } - // Verify or validate site + // Resolve and validate target sites. Site ownership differs by caller: + // provider-owned OSes (Templated iPXE only) target the provider's own sites; + // tenant-owned OSes target sites the tenant has access to. tsDAO := cdbm.NewTenantSiteDAO(csh.dbSession) rdbst := []cdbm.Site{} sttsmap := map[uuid.UUID]*cdbm.TenantSite{} dbossd := []cdbm.StatusDetail{} - // Get all TenantSite records for the Tenant - tss, _, err := tsDAO.GetAll( - ctx, - nil, - cdbm.TenantSiteFilterInput{ - TenantIDs: []uuid.UUID{tenant.ID}, - }, - cdbp.PageInput{ - Limit: cutil.GetPtr(cdbp.TotalLimit), - }, - nil, - ) - if err != nil { - logger.Error().Err(err).Msg("db error retrieving TenantSite records for Tenant") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Site associations for Tenant, DB error", nil) - } - for _, ts := range tss { - cts := ts - sttsmap[ts.SiteID] = &cts - } - - // Validate the site for which this image based Operating System is being created - for _, stID := range apiRequest.SiteIDs { - site, serr := common.GetSiteFromIDString(ctx, nil, stID, csh.dbSession) - if serr != nil { - if serr == common.ErrInvalidID { - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Failed to create Operating System, invalid Site ID: %s", stID), nil) + if allowedByProvider { + // Provider-owned Templated iPXE: sites are the explicitly requested ones, + // validated to be Registered and to belong to the provider. + for _, stID := range apiRequest.SiteIDs { + site, serr := common.GetSiteFromIDString(ctx, nil, stID, csh.dbSession) + if serr != nil { + if serr == common.ErrInvalidID { + return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Failed to create Operating System, invalid Site ID: %s", stID), nil) + } + if serr == cdb.ErrDoesNotExist { + return cutil.NewAPIErrorResponse(c, http.StatusNotFound, fmt.Sprintf("Failed to create Operating System, could not find Site with ID: %s ", stID), nil) + } + logger.Warn().Err(serr).Str("Site ID", stID).Msg("error retrieving Site from DB by ID") + return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Failed to create Operating System, could not retrieve Site with ID: %s, DB error", stID), nil) + } + if site.Status != cdbm.SiteStatusRegistered { + return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Failed to create Operating System, Site: %s specified in request is not in Registered state", site.ID.String()), nil) } - if serr == cdb.ErrDoesNotExist { - return cutil.NewAPIErrorResponse(c, http.StatusNotFound, fmt.Sprintf("Failed to create Operating System, could not find Site with ID: %s ", stID), nil) + if site.InfrastructureProviderID != ip.ID { + return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Unable to associate Operating System with Site: %s, Site does not belong to provider", stID), nil) } - logger.Warn().Err(serr).Str("Site ID", stID).Msg("error retrieving Site from DB by ID") - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Failed to create Operating System, could not retrieve Site with ID: %s, DB error", stID), nil) + rdbst = append(rdbst, *site) } - - if site.Status != cdbm.SiteStatusRegistered { - logger.Warn().Msg(fmt.Sprintf("Unable to associate Operating System to Site: %s. Site is not in Registered state", site.ID.String())) - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Failed to create Operating System, Site: %s specified in request is not in Registered state", site.ID.String()), nil) + } else { + // Tenant-owned: validate requested sites against the tenant's site access. + // Get all TenantSite records for the Tenant + tss, _, terr := tsDAO.GetAll( + ctx, + nil, + cdbm.TenantSiteFilterInput{ + TenantIDs: []uuid.UUID{tenant.ID}, + }, + cdbp.PageInput{ + Limit: cutil.GetPtr(cdbp.TotalLimit), + }, + nil, + ) + if terr != nil { + logger.Error().Err(terr).Msg("db error retrieving TenantSite records for Tenant") + return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Site associations for Tenant, DB error", nil) } - - // Validate the TenantSite exists for current tenant and this site - _, ok := sttsmap[site.ID] - if !ok { - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Unable to associate Operating System with Site: %s, Tenant does not have access to Site", stID), nil) + for _, ts := range tss { + cts := ts + sttsmap[ts.SiteID] = &cts } - // Validate the Site has the ImageBasedOperatingSystem capability enabled for Image based Operating Systems - if osType == cdbm.OperatingSystemTypeImage && (site.Config == nil || !site.Config.ImageBasedOperatingSystem) { - logger.Warn().Str("siteId", stID).Msg("Image based Operating System is not supported for Site, ImageBasedOperatingSystem capability is not enabled") - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Creation of Image based Operating Systems is not supported. Site must have ImageBasedOperatingSystem capability enabled.", nil) - } + // Validate the site for which this image based Operating System is being created + for _, stID := range apiRequest.SiteIDs { + site, serr := common.GetSiteFromIDString(ctx, nil, stID, csh.dbSession) + if serr != nil { + if serr == common.ErrInvalidID { + return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Failed to create Operating System, invalid Site ID: %s", stID), nil) + } + if serr == cdb.ErrDoesNotExist { + return cutil.NewAPIErrorResponse(c, http.StatusNotFound, fmt.Sprintf("Failed to create Operating System, could not find Site with ID: %s ", stID), nil) + } + logger.Warn().Err(serr).Str("Site ID", stID).Msg("error retrieving Site from DB by ID") + return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Failed to create Operating System, could not retrieve Site with ID: %s, DB error", stID), nil) + } + + if site.Status != cdbm.SiteStatusRegistered { + logger.Warn().Msg(fmt.Sprintf("Unable to associate Operating System to Site: %s. Site is not in Registered state", site.ID.String())) + return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Failed to create Operating System, Site: %s specified in request is not in Registered state", site.ID.String()), nil) + } - rdbst = append(rdbst, *site) + // Validate the TenantSite exists for current tenant and this site + _, ok := sttsmap[site.ID] + if !ok { + return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, fmt.Sprintf("Unable to associate Operating System with Site: %s, Tenant does not have access to Site", stID), nil) + } + + // Validate the Site has the ImageBasedOperatingSystem capability enabled for Image based Operating Systems + if osType == cdbm.OperatingSystemTypeImage && (site.Config == nil || !site.Config.ImageBasedOperatingSystem) { + logger.Warn().Str("siteId", stID).Msg("Image based Operating System is not supported for Site, ImageBasedOperatingSystem capability is not enabled") + return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Creation of Image based Operating Systems is not supported. Site must have ImageBasedOperatingSystem capability enabled.", nil) + } + + rdbst = append(rdbst, *site) + } } // A Templated iPXE Operating System is rendered on-Site from its iPXE template, @@ -456,6 +486,18 @@ func (csh CreateOperatingSystemHandler) Handle(c echo.Context) error { osStatusMessage = "received Operating System creation request, syncing" } + // Assign ownership: provider-owned OSes carry InfrastructureProviderID + // (tenant_id=nil); tenant-owned OSes carry TenantID + // (infrastructure_provider_id=nil). When pushed to nico-core, tenant-owned + // OSes carry tenant_organization_id while provider-owned OSes omit it. + var ownerTenantID *uuid.UUID + var ownerProviderID *uuid.UUID + if allowedByProvider { + ownerProviderID = &ip.ID + } else { + ownerTenantID = &tenant.ID + } + // Values needed after the transaction closure var os *cdbm.OperatingSystem var dbossa []cdbm.OperatingSystemSiteAssociation @@ -467,28 +509,29 @@ func (csh CreateOperatingSystemHandler) Handle(c echo.Context) error { err = cdb.WithTx(ctx, csh.dbSession, func(tx *cdb.Tx) error { // Create the db record for Operating System osInput := cdbm.OperatingSystemCreateInput{ - Name: apiRequest.Name, - Description: apiRequest.Description, - Org: org, - TenantID: &tenant.ID, - OsType: osType, - ImageURL: apiRequest.ImageURL, - ImageSHA: apiRequest.ImageSHA, - ImageAuthType: apiRequest.ImageAuthType, - ImageAuthToken: apiRequest.ImageAuthToken, - ImageDisk: apiRequest.ImageDisk, - RootFsId: apiRequest.RootFsID, - RootFsLabel: apiRequest.RootFsLabel, - IpxeScript: apiRequest.IpxeScript, - IpxeTemplateId: apiRequest.IpxeTemplateId, - IpxeTemplateParameters: apiRequest.IpxeTemplateParameters.ToDBModel(), - IpxeTemplateArtifacts: apiRequest.IpxeTemplateArtifacts.ToDBModel(), - UserData: apiRequest.UserData, - AllowOverride: apiRequest.AllowOverride, - EnableBlockStorage: apiRequest.EnableBlockStorage, - PhoneHomeEnabled: phoneHomeEnabled, - Status: osStatus, - CreatedBy: dbUser.ID, + Name: apiRequest.Name, + Description: apiRequest.Description, + Org: org, + TenantID: ownerTenantID, + InfrastructureProviderID: ownerProviderID, + OsType: osType, + ImageURL: apiRequest.ImageURL, + ImageSHA: apiRequest.ImageSHA, + ImageAuthType: apiRequest.ImageAuthType, + ImageAuthToken: apiRequest.ImageAuthToken, + ImageDisk: apiRequest.ImageDisk, + RootFsId: apiRequest.RootFsID, + RootFsLabel: apiRequest.RootFsLabel, + IpxeScript: apiRequest.IpxeScript, + IpxeTemplateId: apiRequest.IpxeTemplateId, + IpxeTemplateParameters: apiRequest.IpxeTemplateParameters.ToDBModel(), + IpxeTemplateArtifacts: apiRequest.IpxeTemplateArtifacts.ToDBModel(), + UserData: apiRequest.UserData, + AllowOverride: apiRequest.AllowOverride, + EnableBlockStorage: apiRequest.EnableBlockStorage, + PhoneHomeEnabled: phoneHomeEnabled, + Status: osStatus, + CreatedBy: dbUser.ID, } createdOs, derr := osDAO.Create(ctx, tx, osInput) if derr != nil { @@ -747,27 +790,14 @@ func (gash GetAllOperatingSystemHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve current user", nil) } - // Validate org - ok, err := auth.ValidateOrgMembership(dbUser, org) - if !ok { - if err != nil { - logger.Error().Err(err).Msg("error validating org membership for User in request") - } else { - logger.Warn().Msg("could not validate org membership for user, access denied") - } - return cutil.NewAPIErrorResponse(c, http.StatusForbidden, fmt.Sprintf("Failed to validate membership for org: %s", org), nil) - } - - // Validate role, only Tenant Admins are allowed to retrieve OperatingSystems - ok = auth.ValidateUserRoles(dbUser, org, nil, auth.TenantAdminRole) - if !ok { - logger.Warn().Msg("user does not have Tenant Admin role, access denied") - return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "User does not have Tenant Admin role with org", nil) + ip, tenant, apiError := common.IsProviderOrTenant(ctx, logger, gash.dbSession, org, dbUser, false, false) + if apiError != nil { + return cutil.NewAPIErrorResponse(c, apiError.Code, apiError.Message, apiError.Data) } // Validate pagination request pageRequest := pagination.PageRequest{} - err = c.Bind(&pageRequest) + err := c.Bind(&pageRequest) if err != nil { logger.Warn().Err(err).Msg("error binding pagination request data into API model") return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Failed to parse request pagination data", nil) @@ -780,20 +810,32 @@ func (gash GetAllOperatingSystemHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Failed to validate pagination request data", err) } - // Validate the tenant associated with the org - tenant, err := common.GetTenantForOrg(ctx, nil, gash.dbSession, org) - if err != nil { - if err == common.ErrOrgTenantNotFound { - logger.Warn().Err(err).Msg("Org does not have a Tenant associated") - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Org does not have a Tenant associated", nil) + // Visibility rules: + // Provider admin: sees only provider-created entries (no tenant entries). + // Tenant admin: sees own entries + provider entries at tenant-accessible sites. + // Dual-role: visibility is the union of both (own tenant + own provider). + filter := cdbm.OperatingSystemFilterInput{} + + switch { + case ip != nil && tenant == nil: + // Provider admin only: sees only provider-created entries. + filter.InfrastructureProviderID = &ip.ID + case tenant != nil && ip == nil: + // Tenant admin only: own entries + provider entries at tenant-accessible sites. + filter.TenantIDs = []uuid.UUID{tenant.ID} + if providerIP, iperr := common.GetInfrastructureProviderForOrg(ctx, nil, gash.dbSession, org); iperr == nil { + filter.InfrastructureProviderID = &providerIP.ID + tenantSiteIDs, tsErr := getTenantSiteIDs(ctx, gash.dbSession, tenant.ID) + if tsErr != nil { + logger.Error().Err(tsErr).Msg("error retrieving tenant site IDs for visibility filter") + return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to determine site access for tenant", nil) + } + filter.ProviderOSVisibleAtSiteIDs = &tenantSiteIDs } - logger.Error().Err(err).Msg("unable to retrieve tenant for org") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve tenant for org", nil) - } - - filter := cdbm.OperatingSystemFilterInput{ - TenantIDs: []uuid.UUID{tenant.ID}, - Orgs: []string{org}, + case tenant != nil && ip != nil: + // Dual-role: own tenant + own provider entries, no site restriction. + filter.TenantIDs = []uuid.UUID{tenant.ID} + filter.InfrastructureProviderID = &ip.ID } // Get and validate includeRelation params @@ -816,14 +858,22 @@ func (gash GetAllOperatingSystemHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Failed to retrieve Site specified in query", nil) } - // Determine if tenant has access to requested site - _, err = tsDAO.GetByTenantIDAndSiteID(ctx, nil, tenant.ID, site.ID, nil) - if err != nil { - if err == cdb.ErrDoesNotExist { - return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Tenant is not associated with Site specified in query", nil) + // Determine if caller has access to the requested site. + // Tenant path: TenantSite association exists. + // Provider path: site belongs to the caller's infrastructure provider. + tenantHasAccess := false + if tenant != nil { + _, tsErr := tsDAO.GetByTenantIDAndSiteID(ctx, nil, tenant.ID, site.ID, nil) + if tsErr == nil { + tenantHasAccess = true + } else if tsErr != cdb.ErrDoesNotExist { + logger.Warn().Err(tsErr).Msg("error retrieving Tenant Site association from DB") + return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Failed to determine if Tenant has access to Site specified in query, DB error", nil) } - logger.Warn().Err(err).Msg("error retrieving Tenant Site association from DB") - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Failed to determine if Tenant has access to Site specified in query, DB error", nil) + } + providerHasAccess := ip != nil && site.InfrastructureProviderID == ip.ID + if !tenantHasAccess && !providerHasAccess { + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Caller is not associated with Site specified in query", nil) } filter.SiteIDs = append(filter.SiteIDs, site.ID) } @@ -934,30 +984,33 @@ func (gash GetAllOperatingSystemHandler) Handle(c echo.Context) error { dbossaMap[dbossa.OperatingSystemID] = append(dbossaMap[dbossa.OperatingSystemID], curVal) } - // Get all TenantSite records for the Tenant + // Get all TenantSite records for the Tenant (only relevant when the caller + // is acting as a Tenant; provider-only admins have no tenant-site context). sttsmap := map[uuid.UUID]*cdbm.TenantSite{} - tsDAO = cdbm.NewTenantSiteDAO(gash.dbSession) - tss, _, err := tsDAO.GetAll( - ctx, - nil, - cdbm.TenantSiteFilterInput{ - TenantIDs: []uuid.UUID{tenant.ID}, - SiteIDs: siteIDs, - }, - cdbp.PageInput{ - Limit: cutil.GetPtr(cdbp.TotalLimit), - }, - nil, - ) - if err != nil { - logger.Error().Err(err).Msg("db error retrieving TenantSite records for Tenant") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Site associations for Tenant, DB error", nil) - } + if tenant != nil { + tsDAO = cdbm.NewTenantSiteDAO(gash.dbSession) + tss, _, tserr := tsDAO.GetAll( + ctx, + nil, + cdbm.TenantSiteFilterInput{ + TenantIDs: []uuid.UUID{tenant.ID}, + SiteIDs: siteIDs, + }, + cdbp.PageInput{ + Limit: cutil.GetPtr(cdbp.TotalLimit), + }, + nil, + ) + if tserr != nil { + logger.Error().Err(tserr).Msg("db error retrieving TenantSite records for Tenant") + return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Site associations for Tenant, DB error", nil) + } - for _, ts := range tss { - curVal := ts - sttsmap[ts.SiteID] = &curVal + for _, ts := range tss { + curVal := ts + sttsmap[ts.SiteID] = &curVal + } } // Create response @@ -1026,22 +1079,9 @@ func (gsh GetOperatingSystemHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve current user", nil) } - // Validate org - ok, err := auth.ValidateOrgMembership(dbUser, org) - if !ok { - if err != nil { - logger.Error().Err(err).Msg("error validating org membership for User in request") - } else { - logger.Warn().Msg("could not validate org membership for user, access denied") - } - return cutil.NewAPIErrorResponse(c, http.StatusForbidden, fmt.Sprintf("Failed to validate membership for org: %s", org), nil) - } - - // Validate role, only Tenant Admins are allowed to retrieve OperatingSystem - ok = auth.ValidateUserRoles(dbUser, org, nil, auth.TenantAdminRole) - if !ok { - logger.Warn().Msg("user does not have Tenant Admin role, access denied") - return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "User does not have Tenant Admin role with org", nil) + ip, tenant, apiError := common.IsProviderOrTenant(ctx, logger, gsh.dbSession, org, dbUser, false, false) + if apiError != nil { + return cutil.NewAPIErrorResponse(c, apiError.Code, apiError.Message, apiError.Data) } // Get and validate includeRelation params @@ -1065,17 +1105,6 @@ func (gsh GetOperatingSystemHandler) Handle(c echo.Context) error { osDAO := cdbm.NewOperatingSystemDAO(gsh.dbSession) - // Validate the tenant for which this OperatingSystem is being retrieved - tenant, err := common.GetTenantForOrg(ctx, nil, gsh.dbSession, org) - if err != nil { - if err == common.ErrOrgTenantNotFound { - logger.Warn().Err(err).Msg("Org does not have a Tenant associated") - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Org does not have a Tenant associated", nil) - } - logger.Error().Err(err).Msg("unable to retrieve tenant for org") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve tenant for org", nil) - } - // Check that operating system exists os, err := osDAO.GetByID(ctx, nil, sID, qIncludeRelations) if err != nil { @@ -1086,10 +1115,68 @@ func (gsh GetOperatingSystemHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Could not retrieve OperatingSystem to update", nil) } - // verify tenant matches - if os.TenantID == nil || tenant.ID != *os.TenantID { - logger.Warn().Msg("tenant in org does not match tenant in operating system") - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Tenant for OperatingSystem in request does not match tenant in org", nil) + // Visibility check with role-based rules: + // Provider admin: can only see provider-owned entries. + // Tenant admin: can see own entries + provider entries at accessible sites. + // Dual-role: can see both tenant and provider entries. + ownedByTenant := tenant != nil && os.TenantID != nil && *os.TenantID == tenant.ID + ownedByProvider := ip != nil && os.InfrastructureProviderID != nil && *os.InfrastructureProviderID == ip.ID + + // A tenant-only caller may also view provider-owned OSes belonging to the org's + // provider (subject to site-scoped visibility checked below). Lazy-fetch the + // org's provider to evaluate that case. + if !ownedByProvider && ip == nil && os.InfrastructureProviderID != nil { + if providerIP, iperr := common.GetInfrastructureProviderForOrg(ctx, nil, gsh.dbSession, org); iperr == nil { + ownedByProvider = *os.InfrastructureProviderID == providerIP.ID + } + } + + if !ownedByTenant && !ownedByProvider { + logger.Warn().Msg("operating system does not belong to the tenant or provider in org") + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Operating System does not belong to the tenant or infrastructure provider in org", nil) + } + + // If caller has dual role (Tenant+Provider) we already know we can go forward. + // Otherwise we need additional checks: + if !(tenant != nil && ip != nil) { + if ip != nil && !ownedByProvider { + logger.Warn().Msg("provider admin cannot view tenant-owned operating system") + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Operating System does not belong to the infrastructure provider in org", nil) + } + if tenant != nil && !ownedByTenant && ownedByProvider { + // Tenant admin seeing a provider-owned entry: verify site-scoped visibility. + ossaDAO := cdbm.NewOperatingSystemSiteAssociationDAO(gsh.dbSession) + ossas, _, ossaErr := ossaDAO.GetAll(ctx, nil, + cdbm.OperatingSystemSiteAssociationFilterInput{OperatingSystemIDs: []uuid.UUID{os.ID}}, + cdbp.PageInput{Limit: cutil.GetPtr(cdbp.TotalLimit)}, + nil, + ) + if ossaErr != nil { + logger.Error().Err(ossaErr).Msg("error retrieving OS site associations for visibility check") + return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to verify site access for Operating System", nil) + } + + tenantSiteIDs, tsErr := getTenantSiteIDs(ctx, gsh.dbSession, tenant.ID) + if tsErr != nil { + logger.Error().Err(tsErr).Msg("error retrieving tenant site IDs for visibility check") + return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to determine site access for tenant", nil) + } + tsSet := make(map[uuid.UUID]struct{}, len(tenantSiteIDs)) + for _, sid := range tenantSiteIDs { + tsSet[sid] = struct{}{} + } + visible := false + for _, ossa := range ossas { + if _, ok := tsSet[ossa.SiteID]; ok { + visible = true + break + } + } + if !visible { + logger.Warn().Msg("provider-owned OS has no site associations at sites accessible to the tenant") + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Operating System is not associated with any site accessible to the caller", nil) + } + } } // get status details for the response @@ -1194,22 +1281,9 @@ func (ush UpdateOperatingSystemHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve current user", nil) } - // Validate org - ok, err := auth.ValidateOrgMembership(dbUser, org) - if !ok { - if err != nil { - logger.Error().Err(err).Msg("error validating org membership for User in request") - } else { - logger.Warn().Msg("could not validate org membership for user, access denied") - } - return cutil.NewAPIErrorResponse(c, http.StatusForbidden, fmt.Sprintf("Failed to validate membership for org: %s", org), nil) - } - - // Validate role, only Tenant Admins are allowed to update OperatingSystem - ok = auth.ValidateUserRoles(dbUser, org, nil, auth.TenantAdminRole) - if !ok { - logger.Warn().Msg("user does not have Tenant Admin role, access denied") - return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "User does not have Tenant Admin role with org", nil) + ip, tenant, apiError := common.IsProviderOrTenant(ctx, logger, ush.dbSession, org, dbUser, false, false) + if apiError != nil { + return cutil.NewAPIErrorResponse(c, apiError.Code, apiError.Message, apiError.Data) } // Get os ID from URL param @@ -1258,41 +1332,45 @@ func (ush UpdateOperatingSystemHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Error validating user data in Operating System creation request", verr) } - // Validate the tenant for which this OperatingSystem is being updated - tenant, err := common.GetTenantForOrg(ctx, nil, ush.dbSession, org) - if err != nil { - if err == common.ErrOrgTenantNotFound { - logger.Warn().Err(err).Msg("Org does not have a Tenant associated") - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Org does not have a Tenant associated", nil) + // Enforce ownership: both roles are evaluated independently so a dual-role + // caller is permitted if either role authorizes the operation. + ownedByTenant := tenant != nil && os.TenantID != nil && *os.TenantID == tenant.ID && os.InfrastructureProviderID == nil + ownedByProvider := false + if ip != nil && os.InfrastructureProviderID != nil { + if *os.InfrastructureProviderID != ip.ID { + logger.Warn().Msg("provider admin cannot update operating system owned by a different provider") + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Provider Admin can only update Operating Systems owned by their own provider", nil) } - logger.Error().Err(err).Msg("unable to retrieve tenant for org") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve tenant for org", nil) + ownedByProvider = true } - - // verify tenant matches - if os.TenantID == nil || tenant.ID != *os.TenantID { - logger.Warn().Msg("tenant in os does not belong to tenant in org") - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Tenant for OperatingSystem in request does not match tenant in org", nil) + if !ownedByProvider && !ownedByTenant { + if ip != nil && tenant == nil { + logger.Warn().Msg("provider admin cannot update tenant-owned operating system") + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Provider Admin can only update provider-owned Operating Systems", nil) + } + if tenant != nil && ip == nil { + logger.Warn().Msg("tenant admin cannot update provider-owned operating system") + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Tenant Admin can only update their own Operating Systems", nil) + } + logger.Warn().Msg("user does not have permission to update this operating system") + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Operating System does not belong to your tenant or infrastructure provider", nil) } - // check for name uniqueness for the tenant, ie, tenant cannot have another os with same name + // Check for name uniqueness within the owner's scope (provider or tenant). if apiRequest.Name != nil && *apiRequest.Name != os.Name { - oss, tot, serr := osDAO.GetAll( - ctx, - nil, - cdbm.OperatingSystemFilterInput{ - TenantIDs: []uuid.UUID{tenant.ID}, - Names: []string{*apiRequest.Name}, - }, - cdbp.PageInput{}, - nil, - ) + uniquenessFilter := cdbm.OperatingSystemFilterInput{Names: []string{*apiRequest.Name}} + if os.InfrastructureProviderID != nil { + uniquenessFilter.InfrastructureProviderID = os.InfrastructureProviderID + } else { + uniquenessFilter.TenantIDs = []uuid.UUID{tenant.ID} + } + oss, tot, serr := osDAO.GetAll(ctx, nil, uniquenessFilter, cdbp.PageInput{}, nil) if serr != nil { - logger.Error().Err(serr).Msg("db error checking for name uniqueness of tenant os") + logger.Error().Err(serr).Msg("db error checking for name uniqueness of os") return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to update OperatingSystem due to DB error", nil) } if tot > 0 { - return cutil.NewAPIErrorResponse(c, http.StatusConflict, "Another Operating System with specified name already exists for Tenant", validation.Errors{ + return cutil.NewAPIErrorResponse(c, http.StatusConflict, "Another Operating System with specified name already exists", validation.Errors{ "id": errors.New(oss[0].ID.String()), }) } @@ -1701,22 +1779,9 @@ func (dsh DeleteOperatingSystemHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve current user", nil) } - // Validate org - ok, err := auth.ValidateOrgMembership(dbUser, org) - if !ok { - if err != nil { - logger.Error().Err(err).Msg("error validating org membership for User in request") - } else { - logger.Warn().Msg("could not validate org membership for user, access denied") - } - return cutil.NewAPIErrorResponse(c, http.StatusForbidden, fmt.Sprintf("Failed to validate membership for org: %s", org), nil) - } - - // Validate role, only Tenant Admins are allowed to delete OperatingSystem - ok = auth.ValidateUserRoles(dbUser, org, nil, auth.TenantAdminRole) - if !ok { - logger.Warn().Msg("user does not have Tenant Admin role, access denied") - return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "User does not have Tenant Admin role with org", nil) + ip, tenant, apiError := common.IsProviderOrTenant(ctx, logger, dsh.dbSession, org, dbUser, false, false) + if apiError != nil { + return cutil.NewAPIErrorResponse(c, apiError.Code, apiError.Message, apiError.Data) } // Get operating system ID from URL param @@ -1730,17 +1795,6 @@ func (dsh DeleteOperatingSystemHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Invalid Operating System ID in URL", nil) } - // Validate the tenant for which this OperatingSystem is being updated - tenant, err := common.GetTenantForOrg(ctx, nil, dsh.dbSession, org) - if err != nil { - if err == common.ErrOrgTenantNotFound { - logger.Warn().Err(err).Msg("Org does not have a Tenant associated") - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Org does not have a Tenant associated", nil) - } - logger.Error().Err(err).Msg("unable to retrieve tenant for org") - return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve tenant for org", nil) - } - // Check that operating system exists osDAO := cdbm.NewOperatingSystemDAO(dsh.dbSession) os, err := osDAO.GetByID(ctx, nil, osID, nil) @@ -1752,10 +1806,28 @@ func (dsh DeleteOperatingSystemHandler) Handle(c echo.Context) error { return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Could not retrieve Operating System to delete", nil) } - // verify tenant matches - if os.TenantID == nil || tenant.ID != *os.TenantID { - logger.Warn().Msg("tenant in os does not belong to tenant in org") - return cutil.NewAPIErrorResponse(c, http.StatusBadRequest, "Tenant for Operating System in request does not match tenant in org", nil) + // Enforce ownership: both roles are evaluated independently so a dual-role + // caller is permitted if either role authorizes the operation. + ownedByTenant := tenant != nil && os.TenantID != nil && *os.TenantID == tenant.ID && os.InfrastructureProviderID == nil + ownedByProvider := false + if ip != nil && os.InfrastructureProviderID != nil { + if *os.InfrastructureProviderID != ip.ID { + logger.Warn().Msg("provider admin cannot delete operating system owned by a different provider") + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Provider Admin can only delete Operating Systems owned by their own provider", nil) + } + ownedByProvider = true + } + if !ownedByProvider && !ownedByTenant { + if ip != nil && tenant == nil { + logger.Warn().Msg("provider admin cannot delete tenant-owned operating system") + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Provider Admin can only delete provider-owned Operating Systems", nil) + } + if tenant != nil && ip == nil { + logger.Warn().Msg("tenant admin cannot delete provider-owned operating system") + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Tenant Admin can only delete their own Operating Systems", nil) + } + logger.Warn().Msg("user does not have permission to delete this operating system") + return cutil.NewAPIErrorResponse(c, http.StatusForbidden, "Operating System does not belong to your tenant or infrastructure provider", nil) } // Verify if tenant associated with Site in case of Image based OS @@ -1795,7 +1867,11 @@ func (dsh DeleteOperatingSystemHandler) Handle(c echo.Context) error { // verify no instances are using the os isDAO := cdbm.NewInstanceDAO(dsh.dbSession) - instances, _, err := isDAO.GetAll(ctx, nil, cdbm.InstanceFilterInput{TenantIDs: []uuid.UUID{tenant.ID}, OperatingSystemIDs: []uuid.UUID{os.ID}}, paginator.PageInput{}, nil) + instanceFilter := cdbm.InstanceFilterInput{OperatingSystemIDs: []uuid.UUID{os.ID}} + if tenant != nil { + instanceFilter.TenantIDs = []uuid.UUID{tenant.ID} + } + instances, _, err := isDAO.GetAll(ctx, nil, instanceFilter, paginator.PageInput{}, nil) if err != nil { logger.Error().Err(err).Msg("error retrieving Instances for Operating System from DB") return cutil.NewAPIErrorResponse(c, http.StatusInternalServerError, "Failed to retrieve Instances for deleting operatingsystem", nil) diff --git a/rest-api/api/pkg/api/handler/operatingsystem_templated_proxy_test.go b/rest-api/api/pkg/api/handler/operatingsystem_templated_proxy_test.go index ca86d851ba..6ea18a2496 100644 --- a/rest-api/api/pkg/api/handler/operatingsystem_templated_proxy_test.go +++ b/rest-api/api/pkg/api/handler/operatingsystem_templated_proxy_test.go @@ -48,6 +48,7 @@ type templatedProxyFixture struct { ipOrg string tnOrg string + ipu *cdbm.User tnu *cdbm.User site *cdbm.Site tmpl *cdbm.IpxeTemplate @@ -66,7 +67,6 @@ func buildTemplatedProxyFixture(t *testing.T) *templatedProxyFixture { tnOrg := "tmpl-proxy-tn-org" ipu := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{ipOrg}, []string{authz.ProviderAdminRole}) - _ = ipu tnu := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{tnOrg}, []string{authz.TenantAdminRole}) ip := testMachineBuildInfrastructureProvider(t, dbSession, ipOrg, "tmpl-proxy-provider") @@ -103,6 +103,7 @@ func buildTemplatedProxyFixture(t *testing.T) *templatedProxyFixture { tracer: tracer, ipOrg: ipOrg, tnOrg: tnOrg, + ipu: ipu, tnu: tnu, site: site, tmpl: tmpl, @@ -147,6 +148,10 @@ func (f *templatedProxyFixture) bindProxyClient(psc *proxySiteClient) { } func (f *templatedProxyFixture) newEchoContext(method, body string, params map[string]string) (echo.Context, *httptest.ResponseRecorder) { + return f.newEchoContextForUser(method, body, params, f.tnu) +} + +func (f *templatedProxyFixture) newEchoContextForUser(method, body string, params map[string]string, user *cdbm.User) (echo.Context, *httptest.ResponseRecorder) { e := echo.New() req := httptest.NewRequest(method, "/", strings.NewReader(body)) req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) @@ -160,7 +165,7 @@ func (f *templatedProxyFixture) newEchoContext(method, body string, params map[s } ec.SetParamNames(names...) ec.SetParamValues(values...) - ec.Set("user", f.tnu) + ec.Set("user", user) reqCtx := context.WithValue(f.ctx, otelecho.TracerKey, f.tracer) ec.SetRequest(ec.Request().WithContext(reqCtx)) return ec, rec @@ -328,6 +333,49 @@ func TestOperatingSystemHandler_TemplatedIPXE_Proxy(t *testing.T) { }) } +func TestOperatingSystemHandler_TemplatedIPXE_ProviderCreateOmitsTenantOrganizationID(t *testing.T) { + f := buildTemplatedProxyFixture(t) + psc := newProxySiteClient(t, createOperatingSystemMethod, nil, nil) + f.bindProxyClient(psc) + + createReq := model.APIOperatingSystemCreateRequest{ + Name: "tmpl-proxy-provider-os", + Description: cutil.GetPtr("provider-owned templated OS"), + IpxeTemplateId: cutil.GetPtr(f.tmpl.ID.String()), + SiteIDs: []string{f.site.ID.String()}, + } + body, err := json.Marshal(createReq) + require.NoError(t, err) + + ec, rec := f.newEchoContextForUser( + http.MethodPost, + string(body), + map[string]string{"orgName": f.ipOrg}, + f.ipu, + ) + h := CreateOperatingSystemHandler{dbSession: f.dbSession, tc: f.tc, scp: f.scp, cfg: f.cfg} + require.NoError(t, h.Handle(ec)) + require.Equal(t, http.StatusCreated, rec.Code, "body: %s", rec.Body.String()) + + psc.client.AssertExpectations(t) + psc.workflow.AssertExpectations(t) + var coreReq corev1.CreateOperatingSystemRequest + require.NoError(t, protojson.Unmarshal(psc.captured.RequestJSON, &coreReq)) + assert.Equal(t, "tmpl-proxy-provider-os", coreReq.Name) + assert.Nil(t, coreReq.TenantOrganizationId, "provider-owned OS must omit tenant_organization_id") + + var rsp model.APIOperatingSystem + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &rsp)) + osID, err := uuid.Parse(rsp.ID) + require.NoError(t, err) + osDAO := cdbm.NewOperatingSystemDAO(f.dbSession) + persisted, err := osDAO.GetByID(f.ctx, nil, osID, nil) + require.NoError(t, err) + require.NotNil(t, persisted.InfrastructureProviderID) + assert.Equal(t, f.site.InfrastructureProviderID, *persisted.InfrastructureProviderID) + assert.Nil(t, persisted.TenantID) +} + func TestOperatingSystemHandler_TemplatedIPXE_ProxyCreateExecuteError(t *testing.T) { f := buildTemplatedProxyFixture(t) diff --git a/rest-api/api/pkg/api/handler/operatingsystem_test.go b/rest-api/api/pkg/api/handler/operatingsystem_test.go index 87d8182edf..8c37ef0192 100644 --- a/rest-api/api/pkg/api/handler/operatingsystem_test.go +++ b/rest-api/api/pkg/api/handler/operatingsystem_test.go @@ -1607,7 +1607,7 @@ func TestOperatingSystemHandler_Update(t *testing.T) { user: user, osID: os2.ID.String(), expectedErr: true, - expectedStatus: http.StatusBadRequest, + expectedStatus: http.StatusForbidden, }, { name: "error when req body doesnt bind", @@ -2158,7 +2158,7 @@ func TestOperatingSystemHandler_Delete(t *testing.T) { user: tnu, osID: os3.ID.String(), expectedErr: true, - expectedStatus: http.StatusBadRequest, + expectedStatus: http.StatusForbidden, }, { name: "error when instance present for os", @@ -2278,3 +2278,489 @@ func TestOperatingSystemHandler_Delete(t *testing.T) { }) } } + +// buildRawIpxeProviderOS creates a provider-owned raw iPXE OS (no site +// associations) via the DAO. Raw iPXE avoids any post-commit site sync, so the +// write handlers exercise ownership enforcement without Temporal/proxy mocks. +func buildRawIpxeProviderOS(t *testing.T, ctx context.Context, osDAO cdbm.OperatingSystemDAO, org string, providerID uuid.UUID, name string, createdBy uuid.UUID) *cdbm.OperatingSystem { + os, err := osDAO.Create(ctx, nil, cdbm.OperatingSystemCreateInput{ + Name: name, + Description: cutil.GetPtr("test"), + Org: org, + InfrastructureProviderID: &providerID, + OsType: cdbm.OperatingSystemTypeIPXE, + IpxeScript: cutil.GetPtr("ipxe"), + Status: cdbm.OperatingSystemStatusReady, + CreatedBy: createdBy, + }) + require.NoError(t, err) + require.NotNil(t, os) + return os +} + +func buildRawIpxeTenantOS(t *testing.T, ctx context.Context, osDAO cdbm.OperatingSystemDAO, org string, tenantID uuid.UUID, name string, createdBy uuid.UUID) *cdbm.OperatingSystem { + os, err := osDAO.Create(ctx, nil, cdbm.OperatingSystemCreateInput{ + Name: name, + Description: cutil.GetPtr("test"), + Org: org, + TenantID: &tenantID, + OsType: cdbm.OperatingSystemTypeIPXE, + IpxeScript: cutil.GetPtr("ipxe"), + Status: cdbm.OperatingSystemStatusReady, + CreatedBy: createdBy, + }) + require.NoError(t, err) + require.NotNil(t, os) + return os +} + +// TestOperatingSystemHandler_Create_Ownership asserts that a Provider Admin may +// only create iPXE Template-based Operating Systems. +func TestOperatingSystemHandler_Create_Ownership(t *testing.T) { + ctx := context.Background() + dbSession := testMachineInitDB(t) + defer dbSession.Close() + common.TestSetupSchema(t, dbSession) + + cfg := common.GetTestConfig() + tcfg, _ := cfg.GetTemporalConfig() + scp := sc.NewClientPool(tcfg) + tempClient := &tmocks.Client{} + + provOrg := "own-provider-org" + provUser := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{provOrg}, []string{authz.ProviderAdminRole}) + testMachineBuildInfrastructureProvider(t, dbSession, provOrg, "own-ip") + + tracer, _, ctx := common.TestCommonTraceProviderSetup(t, ctx) + + tests := []struct { + name string + reqBody model.APIOperatingSystemCreateRequest + expectedStatus int + }{ + { + name: "provider admin cannot create image OS", + reqBody: model.APIOperatingSystemCreateRequest{Name: "prov-image", Description: cutil.GetPtr("test"), ImageURL: cutil.GetPtr("https://example.com/img.iso")}, + expectedStatus: http.StatusForbidden, + }, + { + name: "provider admin cannot create raw ipxe OS", + reqBody: model.APIOperatingSystemCreateRequest{Name: "prov-ipxe", Description: cutil.GetPtr("test"), IpxeScript: cutil.GetPtr("ipxe")}, + expectedStatus: http.StatusForbidden, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + body, err := json.Marshal(tc.reqBody) + require.NoError(t, err) + + e := echo.New() + req := httptest.NewRequest(http.MethodPost, "/", strings.NewReader(string(body))) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + rec := httptest.NewRecorder() + + ec := e.NewContext(req, rec) + ec.SetParamNames("orgName") + ec.SetParamValues(provOrg) + ec.Set("user", provUser) + ec.SetRequest(ec.Request().WithContext(context.WithValue(ctx, otelecho.TracerKey, tracer))) + + ch := CreateOperatingSystemHandler{dbSession: dbSession, tc: tempClient, cfg: cfg, scp: scp} + require.NoError(t, ch.Handle(ec)) + require.Equal(t, tc.expectedStatus, rec.Code) + }) + } +} + +// TestOperatingSystemHandler_Update_Ownership asserts ownership enforcement for +// updates across provider and tenant roles. +func TestOperatingSystemHandler_Update_Ownership(t *testing.T) { + ctx := context.Background() + dbSession := testMachineInitDB(t) + defer dbSession.Close() + common.TestSetupSchema(t, dbSession) + + cfg := common.GetTestConfig() + tcfg, _ := cfg.GetTemporalConfig() + scp := sc.NewClientPool(tcfg) + tempClient := &tmocks.Client{} + osDAO := cdbm.NewOperatingSystemDAO(dbSession) + + provOrg := "own-provider-org" + provUser := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{provOrg}, []string{authz.ProviderAdminRole}) + ip := testMachineBuildInfrastructureProvider(t, dbSession, provOrg, "own-ip") + provOS := buildRawIpxeProviderOS(t, ctx, osDAO, provOrg, ip.ID, "prov-os-update", provUser.ID) + + sharedOrg := "own-shared-org" + tnUser := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{sharedOrg}, []string{authz.TenantAdminRole}) + sharedProvUser := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{sharedOrg}, []string{authz.ProviderAdminRole}) + ip2 := testMachineBuildInfrastructureProvider(t, dbSession, sharedOrg, "own-ip-2") + tn := testMachineBuildTenant(t, dbSession, sharedOrg, "own-tenant") + provOSShared := buildRawIpxeProviderOS(t, ctx, osDAO, sharedOrg, ip2.ID, "prov-os-shared-update", sharedProvUser.ID) + tnOS := buildRawIpxeTenantOS(t, ctx, osDAO, sharedOrg, tn.ID, "tenant-os-update", tnUser.ID) + + tracer, _, ctx := common.TestCommonTraceProviderSetup(t, ctx) + + tests := []struct { + name string + reqOrgName string + user *cdbm.User + os *cdbm.OperatingSystem + expectedStatus int + }{ + { + name: "provider admin updates own provider OS", + reqOrgName: provOrg, + user: provUser, + os: provOS, + expectedStatus: http.StatusOK, + }, + { + name: "tenant admin cannot update provider-owned OS", + reqOrgName: sharedOrg, + user: tnUser, + os: provOSShared, + expectedStatus: http.StatusForbidden, + }, + { + name: "provider admin cannot update tenant-owned OS", + reqOrgName: sharedOrg, + user: sharedProvUser, + os: tnOS, + expectedStatus: http.StatusForbidden, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + body, err := json.Marshal(model.APIOperatingSystemUpdateRequest{Description: cutil.GetPtr("updated description")}) + require.NoError(t, err) + + e := echo.New() + req := httptest.NewRequest(http.MethodPut, "/", strings.NewReader(string(body))) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + rec := httptest.NewRecorder() + + ec := e.NewContext(req, rec) + ec.SetParamNames("orgName", "id") + ec.SetParamValues(tc.reqOrgName, tc.os.ID.String()) + ec.Set("user", tc.user) + ec.SetRequest(ec.Request().WithContext(context.WithValue(ctx, otelecho.TracerKey, tracer))) + + uh := UpdateOperatingSystemHandler{dbSession: dbSession, tc: tempClient, cfg: cfg, scp: scp} + require.NoError(t, uh.Handle(ec)) + require.Equal(t, tc.expectedStatus, rec.Code) + }) + } +} + +// TestOperatingSystemHandler_Delete_Ownership asserts ownership enforcement for +// deletes across provider and tenant roles. +func TestOperatingSystemHandler_Delete_Ownership(t *testing.T) { + ctx := context.Background() + dbSession := testMachineInitDB(t) + defer dbSession.Close() + common.TestSetupSchema(t, dbSession) + + cfg := common.GetTestConfig() + tcfg, _ := cfg.GetTemporalConfig() + scp := sc.NewClientPool(tcfg) + tempClient := &tmocks.Client{} + osDAO := cdbm.NewOperatingSystemDAO(dbSession) + + provOrg := "own-provider-org" + provUser := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{provOrg}, []string{authz.ProviderAdminRole}) + ip := testMachineBuildInfrastructureProvider(t, dbSession, provOrg, "own-ip") + provOS := buildRawIpxeProviderOS(t, ctx, osDAO, provOrg, ip.ID, "prov-os-delete", provUser.ID) + + sharedOrg := "own-shared-org" + tnUser := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{sharedOrg}, []string{authz.TenantAdminRole}) + sharedProvUser := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{sharedOrg}, []string{authz.ProviderAdminRole}) + ip2 := testMachineBuildInfrastructureProvider(t, dbSession, sharedOrg, "own-ip-2") + tn := testMachineBuildTenant(t, dbSession, sharedOrg, "own-tenant") + provOSShared := buildRawIpxeProviderOS(t, ctx, osDAO, sharedOrg, ip2.ID, "prov-os-shared-delete", sharedProvUser.ID) + tnOS := buildRawIpxeTenantOS(t, ctx, osDAO, sharedOrg, tn.ID, "tenant-os-delete", tnUser.ID) + + tracer, _, ctx := common.TestCommonTraceProviderSetup(t, ctx) + + tests := []struct { + name string + reqOrgName string + user *cdbm.User + os *cdbm.OperatingSystem + expectedStatus int + }{ + { + name: "tenant admin cannot delete provider-owned OS", + reqOrgName: sharedOrg, + user: tnUser, + os: provOSShared, + expectedStatus: http.StatusForbidden, + }, + { + name: "provider admin cannot delete tenant-owned OS", + reqOrgName: sharedOrg, + user: sharedProvUser, + os: tnOS, + expectedStatus: http.StatusForbidden, + }, + { + name: "provider admin deletes own provider OS", + reqOrgName: provOrg, + user: provUser, + os: provOS, + expectedStatus: http.StatusAccepted, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodDelete, "/", nil) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + rec := httptest.NewRecorder() + + ec := e.NewContext(req, rec) + ec.SetParamNames("orgName", "id") + ec.SetParamValues(tc.reqOrgName, tc.os.ID.String()) + ec.Set("user", tc.user) + ec.SetRequest(ec.Request().WithContext(context.WithValue(ctx, otelecho.TracerKey, tracer))) + + dh := DeleteOperatingSystemHandler{dbSession: dbSession, tc: tempClient, cfg: cfg, scp: scp} + require.NoError(t, dh.Handle(ec)) + require.Equal(t, tc.expectedStatus, rec.Code) + assert.NotEqual(t, "", tc.name) + }) + } +} + +// buildProviderOS creates a provider-owned OS (no tenant) for the given provider. +func buildProviderOS(t *testing.T, ctx context.Context, osDAO cdbm.OperatingSystemDAO, org string, providerID uuid.UUID, name string, createdBy uuid.UUID) *cdbm.OperatingSystem { + os, err := osDAO.Create(ctx, nil, cdbm.OperatingSystemCreateInput{ + Name: name, + Description: cutil.GetPtr("test"), + Org: org, + InfrastructureProviderID: &providerID, + OsType: cdbm.OperatingSystemTypeIPXE, + IpxeScript: cutil.GetPtr("ipxe"), + Status: cdbm.OperatingSystemStatusReady, + CreatedBy: createdBy, + }) + require.NoError(t, err) + require.NotNil(t, os) + return os +} + +// buildTenantOS creates a tenant-owned OS for the given tenant. +func buildTenantOS(t *testing.T, ctx context.Context, osDAO cdbm.OperatingSystemDAO, org string, tenantID uuid.UUID, name string, createdBy uuid.UUID) *cdbm.OperatingSystem { + os, err := osDAO.Create(ctx, nil, cdbm.OperatingSystemCreateInput{ + Name: name, + Description: cutil.GetPtr("test"), + Org: org, + TenantID: &tenantID, + OsType: cdbm.OperatingSystemTypeIPXE, + IpxeScript: cutil.GetPtr("ipxe"), + Status: cdbm.OperatingSystemStatusReady, + CreatedBy: createdBy, + AllowOverride: false, + }) + require.NoError(t, err) + require.NotNil(t, os) + return os +} + +// TestOperatingSystemHandler_GetAll_Visibility exercises provider-admin listing +// and tenant cross-visibility of provider-owned OSes at accessible sites. +func TestOperatingSystemHandler_GetAll_Visibility(t *testing.T) { + ctx := context.Background() + dbSession := testMachineInitDB(t) + defer dbSession.Close() + common.TestSetupSchema(t, dbSession) + + cfg := common.GetTestConfig() + tempClient := &tmocks.Client{} + osDAO := cdbm.NewOperatingSystemDAO(dbSession) + + // Provider-only org. + provOrg := "vis-provider-org" + provUser := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{provOrg}, []string{authz.ProviderAdminRole}) + ip := testMachineBuildInfrastructureProvider(t, dbSession, provOrg, "vis-ip") + siteA := testMachineBuildSite(t, dbSession, ip, "vis-site-a", cdbm.SiteStatusRegistered) + siteB := testMachineBuildSite(t, dbSession, ip, "vis-site-b", cdbm.SiteStatusRegistered) + + provOSA := buildProviderOS(t, ctx, osDAO, provOrg, ip.ID, "prov-os-a", provUser.ID) + common.TestBuildOperatingSystemSiteAssociation(t, dbSession, provOSA.ID, siteA.ID, cutil.GetPtr("test"), cdbm.OperatingSystemSiteAssociationStatusSynced, provUser) + provOSB := buildProviderOS(t, ctx, osDAO, provOrg, ip.ID, "prov-os-b", provUser.ID) + common.TestBuildOperatingSystemSiteAssociation(t, dbSession, provOSB.ID, siteB.ID, cutil.GetPtr("test"), cdbm.OperatingSystemSiteAssociationStatusSynced, provUser) + + // Shared org that owns both an infrastructure provider and a tenant. The + // user is only a tenant admin; provider-owned entries become visible only + // when associated with a site the tenant can access. + sharedOrg := "vis-shared-org" + tnUser := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{sharedOrg}, []string{authz.TenantAdminRole}) + ip2 := testMachineBuildInfrastructureProvider(t, dbSession, sharedOrg, "vis-ip-2") + siteC := testMachineBuildSite(t, dbSession, ip2, "vis-site-c", cdbm.SiteStatusRegistered) + siteD := testMachineBuildSite(t, dbSession, ip2, "vis-site-d", cdbm.SiteStatusRegistered) + tn := testMachineBuildTenant(t, dbSession, sharedOrg, "vis-tenant") + tsC := testBuildTenantSiteAssociation(t, dbSession, sharedOrg, tn.ID, siteC.ID, tnUser.ID) + assert.NotNil(t, tsC) + + buildTenantOS(t, ctx, osDAO, sharedOrg, tn.ID, "tenant-os-1", tnUser.ID) + buildTenantOS(t, ctx, osDAO, sharedOrg, tn.ID, "tenant-os-2", tnUser.ID) + provC := buildProviderOS(t, ctx, osDAO, sharedOrg, ip2.ID, "prov-os-c", tnUser.ID) + common.TestBuildOperatingSystemSiteAssociation(t, dbSession, provC.ID, siteC.ID, cutil.GetPtr("test"), cdbm.OperatingSystemSiteAssociationStatusSynced, tnUser) + provD := buildProviderOS(t, ctx, osDAO, sharedOrg, ip2.ID, "prov-os-d", tnUser.ID) + common.TestBuildOperatingSystemSiteAssociation(t, dbSession, provD.ID, siteD.ID, cutil.GetPtr("test"), cdbm.OperatingSystemSiteAssociationStatusSynced, tnUser) + + tracer, _, ctx := common.TestCommonTraceProviderSetup(t, ctx) + + tests := []struct { + name string + reqOrgName string + user *cdbm.User + expectedNames []string + }{ + { + name: "provider admin sees only provider-owned OSes", + reqOrgName: provOrg, + user: provUser, + expectedNames: []string{"prov-os-a", "prov-os-b"}, + }, + { + name: "tenant admin sees own OSes plus provider OSes at accessible sites", + reqOrgName: sharedOrg, + user: tnUser, + expectedNames: []string{"tenant-os-1", "tenant-os-2", "prov-os-c"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + rec := httptest.NewRecorder() + + ec := e.NewContext(req, rec) + ec.SetParamNames("orgName") + ec.SetParamValues(tc.reqOrgName) + ec.Set("user", tc.user) + + reqCtx := context.WithValue(ctx, otelecho.TracerKey, tracer) + ec.SetRequest(ec.Request().WithContext(reqCtx)) + + mh := GetAllOperatingSystemHandler{dbSession: dbSession, tc: tempClient, cfg: cfg} + err := mh.Handle(ec) + assert.Nil(t, err) + require.Equal(t, http.StatusOK, rec.Code) + + rsp := []model.APIOperatingSystem{} + require.NoError(t, json.Unmarshal(rec.Body.Bytes(), &rsp)) + gotNames := make([]string, len(rsp)) + for i, os := range rsp { + gotNames[i] = os.Name + } + assert.ElementsMatch(t, tc.expectedNames, gotNames) + }) + } +} + +// TestOperatingSystemHandler_GetByID_Visibility exercises role-based access to a +// single OS: provider admins may only read provider-owned entries, tenant admins +// may read own entries plus provider entries at accessible sites. +func TestOperatingSystemHandler_GetByID_Visibility(t *testing.T) { + ctx := context.Background() + dbSession := testMachineInitDB(t) + defer dbSession.Close() + common.TestSetupSchema(t, dbSession) + + cfg := common.GetTestConfig() + tempClient := &tmocks.Client{} + osDAO := cdbm.NewOperatingSystemDAO(dbSession) + + provOrg := "vis-provider-org" + provUser := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{provOrg}, []string{authz.ProviderAdminRole}) + ip := testMachineBuildInfrastructureProvider(t, dbSession, provOrg, "vis-ip") + siteA := testMachineBuildSite(t, dbSession, ip, "vis-site-a", cdbm.SiteStatusRegistered) + provOSA := buildProviderOS(t, ctx, osDAO, provOrg, ip.ID, "prov-os-a", provUser.ID) + common.TestBuildOperatingSystemSiteAssociation(t, dbSession, provOSA.ID, siteA.ID, cutil.GetPtr("test"), cdbm.OperatingSystemSiteAssociationStatusSynced, provUser) + + sharedOrg := "vis-shared-org" + tnUser := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{sharedOrg}, []string{authz.TenantAdminRole}) + sharedProvUser := testMachineBuildUser(t, dbSession, uuid.NewString(), []string{sharedOrg}, []string{authz.ProviderAdminRole}) + ip2 := testMachineBuildInfrastructureProvider(t, dbSession, sharedOrg, "vis-ip-2") + siteC := testMachineBuildSite(t, dbSession, ip2, "vis-site-c", cdbm.SiteStatusRegistered) + siteD := testMachineBuildSite(t, dbSession, ip2, "vis-site-d", cdbm.SiteStatusRegistered) + tn := testMachineBuildTenant(t, dbSession, sharedOrg, "vis-tenant") + testBuildTenantSiteAssociation(t, dbSession, sharedOrg, tn.ID, siteC.ID, tnUser.ID) + + tnOS := buildTenantOS(t, ctx, osDAO, sharedOrg, tn.ID, "tenant-os-1", tnUser.ID) + provC := buildProviderOS(t, ctx, osDAO, sharedOrg, ip2.ID, "prov-os-c", tnUser.ID) + common.TestBuildOperatingSystemSiteAssociation(t, dbSession, provC.ID, siteC.ID, cutil.GetPtr("test"), cdbm.OperatingSystemSiteAssociationStatusSynced, tnUser) + provD := buildProviderOS(t, ctx, osDAO, sharedOrg, ip2.ID, "prov-os-d", tnUser.ID) + common.TestBuildOperatingSystemSiteAssociation(t, dbSession, provD.ID, siteD.ID, cutil.GetPtr("test"), cdbm.OperatingSystemSiteAssociationStatusSynced, tnUser) + + tracer, _, ctx := common.TestCommonTraceProviderSetup(t, ctx) + + tests := []struct { + name string + reqOrgName string + user *cdbm.User + os *cdbm.OperatingSystem + expectedStatus int + }{ + { + name: "provider admin can read provider-owned OS", + reqOrgName: provOrg, + user: provUser, + os: provOSA, + expectedStatus: http.StatusOK, + }, + { + name: "tenant admin can read provider OS at accessible site", + reqOrgName: sharedOrg, + user: tnUser, + os: provC, + expectedStatus: http.StatusOK, + }, + { + name: "tenant admin cannot read provider OS at inaccessible site", + reqOrgName: sharedOrg, + user: tnUser, + os: provD, + expectedStatus: http.StatusForbidden, + }, + { + name: "tenant admin can read own OS", + reqOrgName: sharedOrg, + user: tnUser, + os: tnOS, + expectedStatus: http.StatusOK, + }, + { + name: "provider admin cannot read tenant-owned OS", + reqOrgName: sharedOrg, + user: sharedProvUser, + os: tnOS, + expectedStatus: http.StatusForbidden, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + e := echo.New() + req := httptest.NewRequest(http.MethodGet, "/", nil) + req.Header.Set(echo.HeaderContentType, echo.MIMEApplicationJSON) + rec := httptest.NewRecorder() + + ec := e.NewContext(req, rec) + ec.SetParamNames("orgName", "id") + ec.SetParamValues(tc.reqOrgName, tc.os.ID.String()) + ec.Set("user", tc.user) + + reqCtx := context.WithValue(ctx, otelecho.TracerKey, tracer) + ec.SetRequest(ec.Request().WithContext(reqCtx)) + + gh := GetOperatingSystemHandler{dbSession: dbSession, tc: tempClient, cfg: cfg} + err := gh.Handle(ec) + assert.Nil(t, err) + require.Equal(t, tc.expectedStatus, rec.Code) + }) + } +} diff --git a/rest-api/api/pkg/api/model/operatingsystem.go b/rest-api/api/pkg/api/model/operatingsystem.go index a6b52f91b8..be31de7937 100644 --- a/rest-api/api/pkg/api/model/operatingsystem.go +++ b/rest-api/api/pkg/api/model/operatingsystem.go @@ -1088,7 +1088,7 @@ func BuildCreateOperatingSystemRequest(os *cdbm.OperatingSystem) *corev1.CreateO Id: &corev1.OperatingSystemId{Value: os.ID.String()}, Name: os.Name, Description: os.Description, - TenantOrganizationId: tenantOrganizationIDProto(os.Org), + TenantOrganizationId: tenantOrganizationIDProto(os), IsActive: os.IsActive, AllowOverride: os.AllowOverride, PhoneHomeEnabled: os.PhoneHomeEnabled, @@ -1127,13 +1127,15 @@ func BuildDeleteOperatingSystemRequest(os *cdbm.OperatingSystem) *corev1.DeleteO } } -// tenantOrganizationIDProto maps a persisted org string onto the optional Core -// field. Empty means provider-owned and must be omitted (Core rejects ""). -func tenantOrganizationIDProto(org string) *string { - if org == "" { +// tenantOrganizationIDProto maps a tenant-owned REST Operating System onto +// Core's optional ownership field. Provider-owned rows also carry their +// provider org in REST, so ownership must be determined from TenantID rather +// than from whether Org is populated. +func tenantOrganizationIDProto(os *cdbm.OperatingSystem) *string { + if os.TenantID == nil || os.Org == "" { return nil } - return &org + return &os.Org } func ipxeTemplateIDProto(id *string) *corev1.IpxeTemplateId { diff --git a/rest-api/api/pkg/api/model/operatingsystem_templated_test.go b/rest-api/api/pkg/api/model/operatingsystem_templated_test.go index ed4f7278ea..cb7e7d6241 100644 --- a/rest-api/api/pkg/api/model/operatingsystem_templated_test.go +++ b/rest-api/api/pkg/api/model/operatingsystem_templated_test.go @@ -127,12 +127,14 @@ func TestOperatingSystemUpdateRequest_Validate_Template(t *testing.T) { func TestBuildOperatingSystemRequests(t *testing.T) { id := uuid.New() + tenantID := uuid.New() authToken := "secret-token" os := &cdbm.OperatingSystem{ ID: id, Name: "templated-os", Description: cutil.GetPtr("desc"), Org: "org-1", + TenantID: &tenantID, Type: cdbm.OperatingSystemTypeTemplatedIPXE, IsActive: true, AllowOverride: true, @@ -148,11 +150,12 @@ func TestBuildOperatingSystemRequests(t *testing.T) { IpxeTemplateDefinitionHash: cutil.GetPtr("hash-1"), } - t.Run("create request maps all fields", func(t *testing.T) { + t.Run("tenant-owned create request maps all fields and tenant org", func(t *testing.T) { req := BuildCreateOperatingSystemRequest(os) require.NotNil(t, req) assert.Equal(t, id.String(), req.GetId().GetValue()) assert.Equal(t, "templated-os", req.Name) + require.NotNil(t, req.TenantOrganizationId) assert.Equal(t, "org-1", req.GetTenantOrganizationId()) assert.True(t, req.IsActive) assert.True(t, req.AllowOverride) @@ -168,6 +171,17 @@ func TestBuildOperatingSystemRequests(t *testing.T) { assert.Nil(t, req.IpxeTemplateArtifacts[0].CachedUrl) }) + t.Run("provider-owned create request omits tenant org", func(t *testing.T) { + providerOS := *os + providerID := uuid.New() + providerOS.TenantID = nil + providerOS.InfrastructureProviderID = &providerID + + req := BuildCreateOperatingSystemRequest(&providerOS) + require.NotNil(t, req) + assert.Nil(t, req.TenantOrganizationId) + }) + t.Run("update request maps all fields", func(t *testing.T) { req := BuildUpdateOperatingSystemRequest(os) require.NotNil(t, req) diff --git a/rest-api/db/pkg/db/model/operatingsystem.go b/rest-api/db/pkg/db/model/operatingsystem.go index 997fbad607..c415faf210 100644 --- a/rest-api/db/pkg/db/model/operatingsystem.go +++ b/rest-api/db/pkg/db/model/operatingsystem.go @@ -446,6 +446,15 @@ type OperatingSystemFilterInput struct { IsActive *bool // IncludeDeleted includes soft-deleted records (used by inventory sync to detect deletions). IncludeDeleted bool + + // ProviderOSVisibleAtSiteIDs restricts provider-owned OS visibility when + // InfrastructureProviderID is set together with TenantIDs (tenant admin view). + // Only provider-owned OSes with at least one site association at one of these + // sites are included. If nil, no cross-ownership provider entries are shown + // alongside tenant entries (default). If set to an empty slice, no provider + // entries match. This field is ignored when InfrastructureProviderID is used + // without TenantIDs (provider-only view). + ProviderOSVisibleAtSiteIDs *[]uuid.UUID } var _ bun.BeforeAppendModelHook = (*OperatingSystem)(nil) @@ -615,13 +624,39 @@ func (ossd OperatingSystemSQLDAO) GetAll(ctx context.Context, tx *db.Tx, filter query = query.Where("os.org IN (?)", bun.In(filter.Orgs)) ossd.tracerSpan.SetAttribute(operatingSystemSQLDAOSpan, "filter.org", filter.Orgs) } - if filter.InfrastructureProviderID != nil { - query = query.Where("os.infrastructure_provider_id = ?", *filter.InfrastructureProviderID) - ossd.tracerSpan.SetAttribute(operatingSystemSQLDAOSpan, "infrastructure_provider_id", filter.InfrastructureProviderID.String()) - } - if filter.TenantIDs != nil { + hasTenants := len(filter.TenantIDs) > 0 + hasProvider := filter.InfrastructureProviderID != nil + hasSiteScope := filter.ProviderOSVisibleAtSiteIDs != nil + + switch { + case hasTenants && hasProvider && hasSiteScope: + // Tenant admin view: own tenant entries + provider entries at accessible sites. + query = query.WhereGroup(" AND ", func(q *bun.SelectQuery) *bun.SelectQuery { + q = q.Where("os.tenant_id IN (?)", bun.In(filter.TenantIDs)) + if len(*filter.ProviderOSVisibleAtSiteIDs) > 0 { + q = q.WhereOr( + "(os.infrastructure_provider_id = ? AND EXISTS (SELECT 1 FROM operating_system_site_association WHERE operating_system_id = os.id AND deleted IS NULL AND site_id IN (?)))", + *filter.InfrastructureProviderID, bun.In(*filter.ProviderOSVisibleAtSiteIDs), + ) + } + return q + }) + ossd.tracerSpan.SetAttribute(operatingSystemSQLDAOSpan, "tenant_with_provider_at_sites", filter.TenantIDs) + case hasTenants && hasProvider: + // Dual-role view: own tenant entries + own provider entries, no site restriction. + query = query.WhereGroup(" AND ", func(q *bun.SelectQuery) *bun.SelectQuery { + return q. + Where("os.tenant_id IN (?)", bun.In(filter.TenantIDs)). + WhereOr("os.infrastructure_provider_id = ?", *filter.InfrastructureProviderID) + }) + ossd.tracerSpan.SetAttribute(operatingSystemSQLDAOSpan, "tenant_or_provider", filter.TenantIDs) + case hasTenants: query = query.Where("os.tenant_id IN (?)", bun.In(filter.TenantIDs)) ossd.tracerSpan.SetAttribute(operatingSystemSQLDAOSpan, "tenant_id", filter.TenantIDs) + case hasProvider: + // Provider-only view: only provider-owned entries. + query = query.Where("os.infrastructure_provider_id = ?", *filter.InfrastructureProviderID) + ossd.tracerSpan.SetAttribute(operatingSystemSQLDAOSpan, "infrastructure_provider_id", filter.InfrastructureProviderID.String()) } if filter.OsTypes != nil { query = query.Where("os.type IN (?)", bun.In(filter.OsTypes)) diff --git a/rest-api/db/pkg/db/model/operatingsystem_test.go b/rest-api/db/pkg/db/model/operatingsystem_test.go index 67c5193cf0..0c151449c2 100644 --- a/rest-api/db/pkg/db/model/operatingsystem_test.go +++ b/rest-api/db/pkg/db/model/operatingsystem_test.go @@ -2132,3 +2132,142 @@ func TestOperatingSystemSQLDAO_Delete(t *testing.T) { }) } } + +// TestOperatingSystemSQLDAO_GetAll_ProviderVisibility exercises the ownership +// visibility switch in GetAll: provider-only, tenant-only, dual-role, and the +// tenant-admin view that surfaces provider-owned OSes only when they are +// associated with one of the tenant's accessible sites +// (ProviderOSVisibleAtSiteIDs). +func TestOperatingSystemSQLDAO_GetAll_ProviderVisibility(t *testing.T) { + ctx := context.Background() + dbSession := testOperatingSystemInitDB(t) + defer dbSession.Close() + testOperatingSystemSetupSchema(t, dbSession) + + ip := testOperatingSystemBuildInfrastructureProvider(t, dbSession, "testIP") + tenant := testOperatingSystemBuildTenant(t, dbSession, "testTenant") + user := testOperatingSystemBuildUser(t, dbSession, "testUser") + siteX := TestBuildSite(t, dbSession, ip, "siteX", user) + siteY := TestBuildSite(t, dbSession, ip, "siteY", user) + + ossd := NewOperatingSystemDAO(dbSession) + ossaDAO := NewOperatingSystemSiteAssociationDAO(dbSession) + dummyUUID := uuid.New() + + buildOS := func(name string, providerID, tenantID *uuid.UUID) *OperatingSystem { + os, err := ossd.Create(ctx, nil, OperatingSystemCreateInput{ + Name: name, + Description: cutil.GetPtr("description"), + Org: "testOrg", + InfrastructureProviderID: providerID, + TenantID: tenantID, + ControllerOperatingSystemID: &dummyUUID, + Version: cutil.GetPtr("version"), + OsType: OperatingSystemTypeIPXE, + ImageURL: cutil.GetPtr("iPXE"), + IpxeScript: cutil.GetPtr("ipxeScript"), + UserData: cutil.GetPtr("userData"), + AllowOverride: true, + EnableBlockStorage: true, + PhoneHomeEnabled: false, + Status: OperatingSystemStatusPending, + CreatedBy: user.ID, + }) + require.NoError(t, err) + require.NotNil(t, os) + return os + } + + associate := func(osID, siteID uuid.UUID) { + ossa, err := ossaDAO.Create(ctx, nil, OperatingSystemSiteAssociationCreateInput{ + OperatingSystemID: osID, + SiteID: siteID, + Status: OperatingSystemSiteAssociationStatusSyncing, + CreatedBy: user.ID, + }) + require.NoError(t, err) + require.NotNil(t, ossa) + } + + // Provider-owned OSes. + provAtX := buildOS("prov-at-x", &ip.ID, nil) + associate(provAtX.ID, siteX.ID) + provAtY := buildOS("prov-at-y", &ip.ID, nil) + associate(provAtY.ID, siteY.ID) + buildOS("prov-no-site", &ip.ID, nil) + + // Tenant-owned OSes. + buildOS("tenant-1", nil, &tenant.ID) + buildOS("tenant-2", nil, &tenant.ID) + + tests := []struct { + desc string + providerID *uuid.UUID + tenantIDs []uuid.UUID + visibleSites *[]uuid.UUID + expectedNames []string + }{ + { + desc: "provider-only view returns all provider OSes", + providerID: &ip.ID, + expectedNames: []string{"prov-at-x", "prov-at-y", "prov-no-site"}, + }, + { + desc: "tenant-only view returns only tenant OSes", + tenantIDs: []uuid.UUID{tenant.ID}, + expectedNames: []string{"tenant-1", "tenant-2"}, + }, + { + desc: "dual-role view returns tenant and provider OSes", + providerID: &ip.ID, + tenantIDs: []uuid.UUID{tenant.ID}, + expectedNames: []string{"prov-at-x", "prov-at-y", "prov-no-site", "tenant-1", "tenant-2"}, + }, + { + desc: "tenant-admin view surfaces provider OS at accessible site", + providerID: &ip.ID, + tenantIDs: []uuid.UUID{tenant.ID}, + visibleSites: &[]uuid.UUID{siteX.ID}, + expectedNames: []string{"prov-at-x", "tenant-1", "tenant-2"}, + }, + { + desc: "tenant-admin view surfaces provider OSes across multiple accessible sites", + providerID: &ip.ID, + tenantIDs: []uuid.UUID{tenant.ID}, + visibleSites: &[]uuid.UUID{siteX.ID, siteY.ID}, + expectedNames: []string{"prov-at-x", "prov-at-y", "tenant-1", "tenant-2"}, + }, + { + desc: "tenant-admin view with empty accessible sites hides provider OSes", + providerID: &ip.ID, + tenantIDs: []uuid.UUID{tenant.ID}, + visibleSites: &[]uuid.UUID{}, + expectedNames: []string{"tenant-1", "tenant-2"}, + }, + { + desc: "tenant-admin view excludes provider OS not associated with accessible site", + providerID: &ip.ID, + tenantIDs: []uuid.UUID{tenant.ID}, + visibleSites: &[]uuid.UUID{siteY.ID}, + expectedNames: []string{"prov-at-y", "tenant-1", "tenant-2"}, + }, + } + for _, tc := range tests { + t.Run(tc.desc, func(t *testing.T) { + filter := OperatingSystemFilterInput{ + InfrastructureProviderID: tc.providerID, + TenantIDs: tc.tenantIDs, + ProviderOSVisibleAtSiteIDs: tc.visibleSites, + } + page := paginator.PageInput{Limit: cutil.GetPtr(paginator.TotalLimit)} + got, total, err := ossd.GetAll(ctx, nil, filter, page, nil) + require.NoError(t, err) + gotNames := make([]string, len(got)) + for i, os := range got { + gotNames[i] = os.Name + } + assert.ElementsMatch(t, tc.expectedNames, gotNames) + assert.Equal(t, len(tc.expectedNames), total) + }) + } +} diff --git a/rest-api/docs/index.html b/rest-api/docs/index.html index 843a051fa3..0b684b46d4 100644 --- a/rest-api/docs/index.html +++ b/rest-api/docs/index.html @@ -6576,10 +6576,20 @@

Typical API Call Flow for Tenant

-

Retrieve all Operating Systems

List Operating Systems visible to the caller's Tenant.

-

Org must have a Tenant entity. User must have authorization role with TENANT_ADMIN suffix. Only Operating Systems whose tenantId matches the caller's Tenant are returned.

+

Retrieve all Operating Systems

List Operating Systems visible to the caller.

+

User must have an authorization role with either the PROVIDER_ADMIN or TENANT_ADMIN suffix for the org.

+
    +
  • Provider admin: returns only Operating Systems owned by the org's Infrastructure Provider.
    • +
  • Tenant admin: returns Operating Systems owned by the caller's Tenant, plus Provider-owned Operating Systems that are associated with at least one Site the Tenant can access.
    • +
  • Dual-role (both Provider and Tenant): returns the union of the Tenant's and the Provider's Operating Systems.
    • +
Authorizations:
JWTBearerToken
path Parameters
org
required
string

Name of the Org

query Parameters
siteId
string
Typical API Call Flow for Tenant

Response samples

Content type
application/json
Example
[
  • {
    }
]

Create Operating System

Create an Operating System for the org.

-

Either infrastructureProviderId or tenantId must be provided in request data. Both cannot be provided at the same time.

-

If infrastructureProviderId is provided in request data, then org must have an Infrastructure Provider entity and its ID should match the query parameter value. User must have authorization role with PROVIDER_ADMIN suffix.

-

If tenantId is provided in request data, then org must have a Tenant entity and its ID should match the query parameter value. User must have authorization role with TENANT_ADMIN suffix.

-

Only Tenants are allowed to create Operating System for MVP.

+

Ownership is derived from the caller's role:

+
    +
  • Tenant admin (TENANT_ADMIN): creates a Tenant-owned Operating System of any type (Image, iPXE, or Templated iPXE).
    • +
  • Provider admin (PROVIDER_ADMIN): may only create Provider-owned Operating Systems of type Templated iPXE.
    • +
+

The tenantId field in the request body is deprecated; if provided, it must match the org's Tenant.

Authorizations:
JWTBearerToken
path Parameters
org
required
string

Name of the Org

Request Body schema: application/json
name
required
string [ 2 .. 256 ] characters
Typical API Call Flow for Tenant
https://nico-rest-api.nico.svc.cluster.local/v2/org/{org}/nico/operating-system

Request samples

Content type
application/json
Example
{
  • "name": "ubuntu-official-22.04",
  • "description": "Official Ubuntu 22.04",
  • "tenantId": "f97df110-f4de-492e-8849-4a6af68026b0",
  • "ipxeScript": "#!ipxe\nkernel http://archive.ubuntu.com/ubuntu/dists/xenial/main/installer-amd64/current/images/netboot/ubuntu-installer/amd64/linux initrd=initrd.gz\ninitrd http://archive.ubuntu.com/ubuntu/dists/xenial/main/installer-amd64/current/images/netboot/ubuntu-installer/amd64/initrd.gz\nboot || imgfree\n shell",
  • "userData": "#cloud-config\nautoinstall:\n apt:\n geoip: true\n preserve_sources_list: false\n primary:\n - arches: [amd64, i386]\n uri: http://archive.ubuntu.com/ubuntu\n - arches: [default]\n uri: http://ports.ubuntu.com/ubuntu-ports",
  • "isCloudInit": true,
  • "phoneHomeEnabled": true,
  • "allowOverride": false
}

Response samples

Content type
application/json
{
  • "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08",
  • "name": "ubuntu-22.04",
  • "description": "Ubuntu 22.04",
  • "infrastructureProviderId": null,
  • "tenantId": "f97df110-f4de-492e-8849-4a6af68026b0",
  • "type": "iPXE",
  • "ipxeScript": "#!ipxe\nkernel http://archive.ubuntu.com/ubuntu/dists/xenial/main/installer-amd64/current/images/netboot/ubuntu-installer/amd64/linux initrd=initrd.gz\ninitrd http://archive.ubuntu.com/ubuntu/dists/xenial/main/installer-amd64/current/images/netboot/ubuntu-installer/amd64/initrd.gz\nboot || imgfree\n shell",
  • "userData": "#cloud-config\nautoinstall:\n apt:\n geoip: true\n preserve_sources_list: false\n primary:\n - arches: [amd64, i386]\n uri: http://archive.ubuntu.com/ubuntu\n - arches: [default]\n uri: http://ports.ubuntu.com/ubuntu-ports",
  • "isCloudInit": true,
  • "phoneHomeEnabled": false,
  • "allowOverride": false,
  • "imageAuthToken": null,
  • "imageAuthType": null,
  • "imageDisk": null,
  • "imageSha": null,
  • "imageUrl": null,
  • "rootFsId": null,
  • "rootFsLabel": null,
  • "siteAssociations": [ ],
  • "isActive": true,
  • "deactivationNote": null,
  • "status": "Pending",
  • "statusHistory": [
    ],
  • "created": "2019-08-24T14:15:22Z",
  • "updated": "2019-08-24T14:15:22Z"
}

Retrieve Operating System

Get an Operating System by ID

If the Operating System has infrastructureProviderId set, then org must have an Infrastructure Provider entity and its ID should match the Operating System Infrastructure Provider ID. User must have authorization role with PROVIDER_ADMIN suffix.

If the Operating System has tenantId set, then org must have a Tenant entity and its ID should match the Operating System Tenant ID. User must have authorization role with TENANT_ADMIN suffix.

+

A Tenant admin may also read a Provider-owned Operating System when it is associated with at least one Site the Tenant can access.

Authorizations:
JWTBearerToken
path Parameters
org
required
string

Name of the Org

operatingSystemId
required
string
Typical API Call Flow for Tenant