MGMT-18627: Align assisted authorization service with OCM authorizations

Makefile

@@ -90,6 +90,7 @@ DISABLED_HOST_VALIDATIONS := $(or ${DISABLED_HOST_VALIDATIONS}, "")
 DISABLED_STEPS := $(or ${DISABLED_STEPS}, "")
 DISABLE_TLS := $(or ${DISABLE_TLS},false)
 ENABLE_ORG_TENANCY := $(or ${ENABLE_ORG_TENANCY},False)
+ENABLE_OCM_AUTHZ := $(or ${ENABLE_OCM_AUTHZ},False)
 ALLOW_CONVERGED_FLOW := $(or ${ALLOW_CONVERGED_FLOW}, false)
 ENABLE_ORG_BASED_FEATURE_GATES := $(or ${ENABLE_ORG_BASED_FEATURE_GATES},False)
 KIND_EXPERIMENTAL_PROVIDER=podman
@@ -338,7 +339,7 @@ deploy-service-requirements: | deploy-namespace deploy-inventory-service-file
 		--storage $(STORAGE) --ipv6-support $(IPV6_SUPPORT) --enable-sno-dnsmasq $(ENABLE_SINGLE_NODE_DNSMASQ) \
 		--disk-encryption-support $(DISK_ENCRYPTION_SUPPORT) --hw-requirements '$(subst ",\",$(HW_REQUIREMENTS))' \
 		--disabled-host-validations "$(DISABLED_HOST_VALIDATIONS)" --disabled-steps "$(DISABLED_STEPS)" \
-		--enable-org-tenancy $(ENABLE_ORG_TENANCY) \
+		--enable-org-tenancy $(ENABLE_ORG_TENANCY) --enable-ocm-authz $(ENABLE_OCM_AUTHZ)\
 		--enable-org-based-feature-gate $(ENABLE_ORG_BASED_FEATURE_GATES) $(ALLOW_CONVERGED_FLOW_CMD) $(DISABLE_TLS_CMD)
 ifeq ($(MIRROR_REGISTRY_SUPPORT), True)
 	python3 ./tools/deploy_assisted_installer_configmap_registry_ca.py  --target "$(TARGET)" \

internal/bminventory/inventory.go

@@ -3138,6 +3138,7 @@ func (b *bareMetalInventory) listClustersInternal(ctx context.Context, params in
 
 	var dbClusters []*common.Cluster
 	var clusters []*models.Cluster
+	var err error
 
 	if swag.BoolValue(params.GetUnregisteredClusters) {
 		if !b.authzHandler.IsAdmin(ctx) {
@@ -3146,7 +3147,11 @@ func (b *bareMetalInventory) listClustersInternal(ctx context.Context, params in
 		db = db.Unscoped()
 	}
 
-	db = b.authzHandler.OwnedByUser(ctx, db, swag.StringValue(params.Owner))
+	db, err = b.authzHandler.OwnedByUser(ctx, db, auth.ClusterResource, swag.StringValue(params.Owner))
+	if err != nil {
+		log.WithError(err).Error("Failed to list clusters in db")
+		return nil, common.NewApiError(http.StatusInternalServerError, err)
+	}
 
 	if params.OpenshiftClusterID != nil {
 		db = db.Where("openshift_cluster_id = ?", *params.OpenshiftClusterID)
@@ -3156,7 +3161,7 @@ func (b *bareMetalInventory) listClustersInternal(ctx context.Context, params in
 		db = db.Where("ams_subscription_id IN (?)", params.AmsSubscriptionIds)
 	}
 
-	dbClusters, err := common.GetClustersFromDBWhere(db, common.UseEagerLoading,
+	dbClusters, err = common.GetClustersFromDBWhere(db, common.UseEagerLoading,
 		common.DeleteRecordsState(swag.BoolValue(params.GetUnregisteredClusters)))
 	if err != nil {
 		log.WithError(err).Error("Failed to list clusters in db")
@@ -4532,14 +4537,19 @@ func (b *bareMetalInventory) ListInfraEnvsInternal(ctx context.Context, clusterI
 	db := b.db
 	var dbInfraEnvs []*common.InfraEnv
 	var infraEnvs []*models.InfraEnv
+	var err error
 
-	db = b.authzHandler.OwnedByUser(ctx, db, swag.StringValue(owner))
+	db, err = b.authzHandler.OwnedByUser(ctx, db, auth.InfraEnvResource, swag.StringValue(owner))
+	if err != nil {
+		log.WithError(err).Error("Failed to list infraEnvs in db")
+		return nil, err
+	}
 
 	if clusterId != nil {
 		db = db.Where("cluster_id = ?", clusterId)
 	}
 
-	dbInfraEnvs, err := common.GetInfraEnvsFromDBWhere(db)
+	dbInfraEnvs, err = common.GetInfraEnvsFromDBWhere(db)
 	if err != nil {
 		log.WithError(err).Error("Failed to list infraEnvs in db")
 		return nil, err

internal/events/event.go

@@ -355,12 +355,12 @@ func (e Events) prepareEventsTable(ctx context.Context, tx *gorm.DB, clusterID *
 	//host bound to a cluster or registered to a bound infra-env) check the access permission
 	//relative to the cluster ownership
 	if clusterBoundEvents() {
-		tx = tx.Model(&common.Event{}).Select("events.*, clusters.user_name, clusters.org_id").
+		tx = tx.Model(&common.Event{}).Select("events.*, clusters.user_name, clusters.org_id, clusters.openshift_cluster_id").
 			Joins("INNER JOIN clusters ON clusters.id = events.cluster_id")
 
 		// if deleted hosts flag is true, we need to add 'deleted_at' to know whether events are related to a deleted host
 		if swag.BoolValue(deletedHosts) {
-			tx = tx.Select("events.*, clusters.user_name, clusters.org_id, hosts.deleted_at").
+			tx = tx.Select("events.*, clusters.user_name, clusters.org_id, clusters.openshift_cluster_id, hosts.deleted_at").
 				Joins("LEFT JOIN hosts ON hosts.id = events.host_id")
 		}
 		return tx
@@ -369,14 +369,16 @@ func (e Events) prepareEventsTable(ctx context.Context, tx *gorm.DB, clusterID *
 	//for unbound events that are searched with infra-env id (whether events on hosts or the
 	//infra-env level itself) check the access permission relative to the infra-env ownership
 	if nonBoundEvents() {
-		return tx.Model(&common.Event{}).Select("events.*, infra_envs.user_name, infra_envs.org_id").
+		return tx.Model(&common.Event{}).Select("events.*, infra_envs.user_name, infra_envs.org_id, clusters.openshift_cluster_id").
+			Joins("LEFT JOIN clusters ON clusters.id = events.cluster_id").
 			Joins("INNER JOIN infra_envs ON infra_envs.id = events.infra_env_id")
 	}
 
 	// Events must be linked to the infra_envs table and then to the hosts table
 	// The hosts table does not hold an org_id, so permissions related fields must be supplied by the infra_env
 	if hostOnlyEvents() {
-		return tx.Model(&common.Event{}).Select("events.*, infra_envs.user_name, infra_envs.org_id").
+		return tx.Model(&common.Event{}).Select("events.*, infra_envs.user_name, infra_envs.org_id, clusters.openshift_cluster_id").
+			Joins("LEFT JOIN clusters ON clusters.id = events.cluster_id").
 			Joins("INNER JOIN infra_envs ON infra_envs.id = events.infra_env_id").
 			Joins("INNER JOIN hosts ON hosts.id = events.host_id"). // This join is here to ensure that only events for a host that exists are fetched
 			Where("hosts.deleted_at IS NULL")                       // Only interested in active hosts
@@ -427,11 +429,6 @@ func (e Events) queryEvents(ctx context.Context, params *common.V2GetEventsParam
 
 	events := []*common.Event{}
 
-	// add authorization check to query
-	if e.authz != nil {
-		tx = e.authz.OwnedBy(ctx, tx)
-	}
-
 	tx = e.prepareEventsTable(ctx, tx, params.ClusterID, params.HostIds, params.InfraEnvID, params.Severities, params.Message, params.DeletedHosts)
 	if tx == nil {
 		return make([]*common.Event, 0), &common.EventSeverityCount{}, swag.Int64(0), nil
@@ -470,6 +467,11 @@ func (e Events) queryEvents(ctx context.Context, params *common.V2GetEventsParam
 		return make([]*common.Event, 0), eventSeverityCount, &eventCount, nil
 	}
 
+	// if we need to apply authorization check then repackage tx as a subquery
+	// this is to ensure that user_name and org_id are unambiguous
+	if e.authz != nil {
+		tx, _ = e.authz.OwnedBy(ctx, cleanQuery.Table("(?) as s", tx), auth.EventsResource)
+	}
 	err = tx.Offset(int(*params.Offset)).Limit(int(*params.Limit)).Find(&events).Error
 	if err != nil {
 		return nil, nil, nil, err

internal/events/event_test.go

@@ -51,7 +51,7 @@ var _ = Describe("Add event", func() {
 		ctrl = gomock.NewController(GinkgoT())
 		log := logrus.WithField("pkg", "events")
 		theEvents = New(db, nil, commontesting.GetDummyNotificationStream(ctrl), log)
-		cfg := &auth.Config{AuthType: auth.TypeRHSSO, EnableOrgTenancy: true}
+		cfg := &auth.Config{AuthType: auth.TypeRHSSO, EnableOrgTenancy: true, EnableOcmAuthz: false}
 		authz_handler := auth.NewAuthzHandler(cfg, nil, logrus.New(), db)
 		theEvents.(*Events).authz = authz_handler
 		api = &Api{handler: theEvents, log: log}
@@ -424,7 +424,7 @@ var _ = Describe("Events library", func() {
 
 		JustBeforeEach(func() {
 			//inject RHSSO authorizer to the event handler
-			cfg := &auth.Config{AuthType: auth.TypeRHSSO, EnableOrgTenancy: true}
+			cfg := &auth.Config{AuthType: auth.TypeRHSSO, EnableOrgTenancy: true, EnableOcmAuthz: false}
 			authz_handler := auth.NewAuthzHandler(cfg, nil, logrus.New(), db)
 			theEvents.(*Events).authz = authz_handler
 		})

openshift/template.yaml

@@ -132,6 +132,9 @@ parameters:
 - name: ENABLE_ORG_TENANCY
   value: "false"
   required: false
+- name: ENABLE_OCM_AUTHZ
+  value: "false"
+  required: false
 - name: ENABLE_ORG_BASED_FEATURE_GATES
   value: "false"
   required: false
@@ -471,6 +474,8 @@ objects:
                 value: ${CNV_SNO_INSTALL_HPP}
               - name: ENABLE_ORG_TENANCY
                 value: ${ENABLE_ORG_TENANCY}
+              - name: ENABLE_OCM_AUTHZ
+                value: ${ENABLE_OCM_AUTHZ}
               - name: ENABLE_ORG_BASED_FEATURE_GATES
                 value: ${ENABLE_ORG_BASED_FEATURE_GATES}
               - name: ISO_IMAGE_TYPE

pkg/auth/agent_local_authenticator.go

@@ -59,6 +59,10 @@ func (a *AgentLocalAuthenticator) EnableOrgTenancy() bool {
 	return false
 }
 
+func (a *AgentLocalAuthenticator) EnableOcmAuthz() bool {
+	return false
+}
+
 func (a *AgentLocalAuthenticator) EnableOrgBasedFeatureGates() bool {
 	return false
 }

pkg/auth/auth_handler_test_utils.go

@@ -89,6 +89,6 @@ func GenJSJWKS(privKey crypto.PublicKey, pubKey crypto.PublicKey) ([]byte, []byt
 
 func GetConfigRHSSO() *Config {
 	_, cert := GetTokenAndCert(false)
-	cfg := &Config{JwkCert: string(cert), AuthType: TypeRHSSO, EnableOrgTenancy: true, EnableOrgBasedFeatureGates: false}
+	cfg := &Config{JwkCert: string(cert), AuthType: TypeRHSSO, EnableOrgTenancy: true, EnableOcmAuthz: false, EnableOrgBasedFeatureGates: false}
 	return cfg
 }

pkg/auth/authenticator.go

@@ -29,6 +29,7 @@ type Authenticator interface {
 	AuthWatcherAuth(token string) (interface{}, error)
 	AuthType() AuthType
 	EnableOrgTenancy() bool
+	EnableOcmAuthz() bool
 	EnableOrgBasedFeatureGates() bool
 }
 
@@ -41,6 +42,7 @@ type Config struct {
 	AllowedDomains             string   `envconfig:"ALLOWED_DOMAINS" default:""`
 	AdminUsers                 []string `envconfig:"ADMIN_USERS" default:""`
 	EnableOrgTenancy           bool     `envconfig:"ENABLE_ORG_TENANCY" default:"false"`
+	EnableOcmAuthz             bool     `envconfig:"ENABLE_OCM_AUTHZ" default:"false"`
 	EnableOrgBasedFeatureGates bool     `envconfig:"ENABLE_ORG_BASED_FEATURE_GATES" default:"false"`
 }
 

pkg/auth/authz.go

@@ -10,24 +10,29 @@ import (
 )
 
 type Action string
+type Resource string
 
 const ReadAction Action = "read"
 const UpdateAction Action = "update"
 const DeleteAction Action = "delete"
 const NoneAction Action = "none"
 
+const ClusterResource = "Cluster"
+const InfraEnvResource = "InfraEnv"
+const EventsResource = "Event"
+
 type Authorizer interface {
 	/* Limits the database query to access records that are owned by the current user,
 	 * according to the configured access policy.
 	 */
 
-	OwnedBy(ctx context.Context, db *gorm.DB) *gorm.DB
+	OwnedBy(ctx context.Context, db *gorm.DB, resource Resource) (*gorm.DB, error)
 
 	/* Limits the database query to access records owned only by the input user,
 	 * regardless of the configured access policy. If user-based authentication
 	 * is not supported, the function effectively will not limit access.
 	 */
-	OwnedByUser(ctx context.Context, db *gorm.DB, username string) *gorm.DB
+	OwnedByUser(ctx context.Context, db *gorm.DB, resource Resource, username string) (*gorm.DB, error)
 
 	/* verify that the current user has access rights (depending on the requested)
 	 * action) to the input resource

pkg/auth/local_authenticator.go

@@ -55,6 +55,10 @@ func (a *LocalAuthenticator) EnableOrgTenancy() bool {
 	return false
 }
 
+func (a *LocalAuthenticator) EnableOcmAuthz() bool {
+	return false
+}
+
 func (a *LocalAuthenticator) EnableOrgBasedFeatureGates() bool {
 	return false
 }

pkg/auth/none_authenticator.go

@@ -27,6 +27,10 @@ func (a *NoneAuthenticator) EnableOrgTenancy() bool {
 	return false
 }
 
+func (a *NoneAuthenticator) EnableOcmAuthz() bool {
+	return false
+}
+
 func (a *NoneAuthenticator) EnableOrgBasedFeatureGates() bool {
 	return false
 }

pkg/auth/none_authz_handler.go

@@ -23,12 +23,12 @@ func (*NoneHandler) CreateAuthorizer() func(*http.Request) error {
 	}
 }
 
-func (*NoneHandler) OwnedBy(ctx context.Context, db *gorm.DB) *gorm.DB {
-	return db
+func (*NoneHandler) OwnedBy(ctx context.Context, db *gorm.DB, resource Resource) (*gorm.DB, error) {
+	return db, nil
 }
 
-func (*NoneHandler) OwnedByUser(ctx context.Context, db *gorm.DB, username string) *gorm.DB {
-	return db
+func (*NoneHandler) OwnedByUser(ctx context.Context, db *gorm.DB, resource Resource, username string) (*gorm.DB, error) {
+	return db, nil
 }
 
 func (*NoneHandler) HasAccessTo(ctx context.Context, obj interface{}, action Action) (bool, error) {

pkg/auth/rhsso_authenticator.go

@@ -26,6 +26,7 @@ type RHSSOAuthenticator struct {
 	KeyMap                       map[string]*rsa.PublicKey
 	AdminUsers                   []string
 	OrgTenancyEnabled            bool
+	OcmAuthzEnabled              bool
 	OrgBasedFunctionalityEnabled bool
 	utils                        AUtilsInteface
 	log                          logrus.FieldLogger
@@ -37,6 +38,7 @@ func NewRHSSOAuthenticator(cfg *Config, ocmCLient *ocm.Client, log logrus.FieldL
 	a := &RHSSOAuthenticator{
 		AdminUsers:                   cfg.AdminUsers,
 		OrgTenancyEnabled:            cfg.EnableOrgTenancy,
+		OcmAuthzEnabled:              cfg.EnableOcmAuthz,
 		OrgBasedFunctionalityEnabled: cfg.EnableOrgBasedFeatureGates,
 		utils:                        NewAuthUtils(cfg.JwkCert, cfg.JwkCertURL),
 		client:                       ocmCLient,
@@ -60,6 +62,10 @@ func (a *RHSSOAuthenticator) EnableOrgTenancy() bool {
 	return a.OrgTenancyEnabled
 }
 
+func (a *RHSSOAuthenticator) EnableOcmAuthz() bool {
+	return a.OcmAuthzEnabled
+}
+
 func (a *RHSSOAuthenticator) EnableOrgBasedFeatureGates() bool {
 	return a.OrgBasedFunctionalityEnabled
 }

pkg/auth/rhsso_authz_handler.go

@@ -39,6 +39,10 @@ func (a *AuthzHandler) isTenancyEnabled() bool {
 	return a.cfg.EnableOrgTenancy
 }
 
+func (a *AuthzHandler) isOcmAuthzEnabled() bool {
+	return a.cfg.EnableOcmAuthz
+}
+
 func (a *AuthzHandler) isOrgBasedFunctionalityEnabled() bool {
 	return a.cfg.EnableOrgBasedFeatureGates
 }
@@ -67,27 +71,53 @@ func handleOwnershipQueryError(err error) (bool, error) {
 	return true, nil
 }
 
-func (a *AuthzHandler) OwnedBy(ctx context.Context, db *gorm.DB) *gorm.DB {
+func (a *AuthzHandler) OwnedBy(ctx context.Context, db *gorm.DB, resource Resource) (*gorm.DB, error) {
 	if a.IsAdmin(ctx) {
-		return db
+		return db, nil
+	}
+	if a.isOcmAuthzEnabled() {
+		allowedClusterID, allowedClusterUuids, err := a.listAccessibleResource(ocm.UserNameFromContext(ctx), ocm.AMSActionGet, ClusterResource)
+		if err != nil {
+			return nil, err
+		}
+
+		query := ""
+		switch resource {
+		case InfraEnvResource:
+			query = "user_name = ? OR cluster_id IN (?) OR openshift_cluster_id in (?)"
+			db = db.Joins("LEFT JOIN clusters on infra_envs.cluster_id = clusters.id")
+		case EventsResource:
+			query = "user_name = ? OR cluster_id IN (?) OR openshift_cluster_id in (?)"
+		default:
+			query = "user_name = ? OR id IN (?) OR openshift_cluster_id in (?)"
+		}
+
+		return db.Where(query, ocm.UserNameFromContext(ctx), allowedClusterID, allowedClusterUuids), nil
 	}
 	if a.isTenancyEnabled() {
-		return db.Where("org_id = ?", ocm.OrgIDFromContext(ctx))
+		return db.Where("org_id = ?", ocm.OrgIDFromContext(ctx)), nil
 	} else {
-		return db.Where("user_name = ?", ocm.UserNameFromContext(ctx))
+		return db.Where("user_name = ?", ocm.UserNameFromContext(ctx)), nil
 	}
 }
 
-func (a *AuthzHandler) OwnedByUser(ctx context.Context, db *gorm.DB, username string) *gorm.DB {
+func (a *AuthzHandler) OwnedByUser(ctx context.Context, db *gorm.DB, resource Resource, username string) (*gorm.DB, error) {
 	// When tenancy-based access is supported, the following query returns records
 	// for the input user alone. With a user-based access policy, the query returns
 	// the user's records, provided it is the current user. Otherwise, it returns an
 	// empty set since we do not support listing objects on behalf of other users in
 	// that mode.
-	if username == "" {
-		return a.OwnedBy(ctx, db)
+	res, err := a.OwnedBy(ctx, db, resource)
+
+	if err != nil {
+		return nil, err
+	}
+
+	if username != "" {
+		res = res.Where("user_name = ?", username)
 	}
-	return a.OwnedBy(ctx, db).Where("user_name = ?", username)
+
+	return res, nil
 }
 
 func (a *AuthzHandler) isObjectOwnedByUser(id string, obj interface{}, payload *ocm.AuthPayload) (bool, error) {
@@ -130,7 +160,7 @@ func (a *AuthzHandler) hasSubscriptionAccess(clusterId string, action string, pa
 		return true, nil
 	}
 
-	if a.isTenancyEnabled() {
+	if a.isTenancyEnabled() || a.isOcmAuthzEnabled() {
 		var cluster common.Cluster
 		err = a.db.Select("ams_subscription_id", "openshift_cluster_id", "kind").
 			First(&cluster, "id = ?", clusterId).Error
@@ -379,6 +409,11 @@ func (a *AuthzHandler) allowedToUseAssistedInstaller(username string) (bool, err
 		context.Background(), username, ocm.AMSActionCreate, "", ocm.BareMetalClusterResource)
 }
 
+func (a *AuthzHandler) listAccessibleResource(username, action, resource string) ([]string, []string, error) {
+	return a.client.Authorization.ResourceReview(
+		context.Background(), username, action, resource)
+}
+
 func (a *AuthzHandler) hasClusterEditRole(payload *ocm.AuthPayload, action, subscriptionID string) (bool, error) {
 	return a.client.Authorization.AccessReview(
 		context.Background(), payload.Username, action, subscriptionID, ocm.Subscription)