Merge pull request #1435 from stgraber/openfga

Openfga improvements

Author: tych0

Makefile

@@ -331,4 +331,4 @@ tags: */*.go
 # OpenFGA Syntax Transformer: https://github.com/openfga/syntax-transformer
 .PHONY: update-openfga
 update-openfga:
-	@printf 'package auth\n\n// Code generated by Makefile; DO NOT EDIT.\n\nvar authModel = `%s`\n' '$(shell npx --yes @openfga/syntax-transformer transform --from=dsl --inputFile=./internal/server/auth/driver_openfga_model.openfga | jq -c)' > ./internal/server/auth/driver_openfga_model.go
+	@printf 'package auth\n\n// Code generated by Makefile; DO NOT EDIT.\n\nvar authModel = `%s`\n' '$(shell fga model transform --file=./internal/server/auth/driver_openfga_model.openfga | jq -c)' > ./internal/server/auth/driver_openfga_model.go

cmd/incusd/api_1.0.go

@@ -394,7 +394,7 @@ func api10Get(d *Daemon, r *http.Request) response.Response {
 	fullSrv.AuthUserName = requestor.Username
 	fullSrv.AuthUserMethod = requestor.Protocol
 
-	err = s.Authorizer.CheckPermission(r.Context(), r, auth.ObjectServer(), auth.EntitlementCanEdit)
+	err = s.Authorizer.CheckPermission(r.Context(), r, auth.ObjectServer(), auth.EntitlementCanViewSensitive)
 	if err == nil {
 		fullSrv.Config = fullSrvConfig
 	} else if !api.StatusErrorCheck(err, http.StatusForbidden) {

cmd/incusd/patches.go

@@ -85,6 +85,7 @@ var patches = []patch{
 	{name: "storage_zfs_unset_invalid_block_settings_v2", stage: patchPostDaemonStorage, run: patchStorageZfsUnsetInvalidBlockSettingsV2},
 	{name: "runtime_directory", stage: patchPostDaemonStorage, run: patchRuntimeDirectory},
 	{name: "lvm_node_force_reuse", stage: patchPostDaemonStorage, run: patchLvmForceReuseKey},
+	{name: "auth_openfga_viewer", stage: patchPostNetworks, run: patchGenericAuthorization},
 }
 
 type patch struct {
@@ -135,6 +136,10 @@ func patchesApply(d *Daemon, stage patchStage) error {
 			return fmt.Errorf("Patch %q has no stage set: %d", patch.name, patch.stage)
 		}
 
+		if patch.stage != stage {
+			continue
+		}
+
 		if slices.Contains(appliedPatches, patch.name) {
 			continue
 		}
@@ -680,6 +685,29 @@ func patchMoveBackupsInstances(name string, d *Daemon) error {
 	return nil
 }
 
+func patchGenericAuthorization(name string, d *Daemon) error {
+	// Only run authorization patches on the leader.
+	isLeader := false
+
+	leaderAddress, err := d.gateway.LeaderAddress()
+	if err != nil {
+		if !errors.Is(err, cluster.ErrNodeIsNotClustered) {
+			return err
+		}
+
+		isLeader = true
+	} else if leaderAddress == d.localConfig.ClusterAddress() {
+		isLeader = true
+	}
+
+	// If clustered and not running on a leader, skip the resource update.
+	if !isLeader {
+		return nil
+	}
+
+	return d.authorizer.ApplyPatch(d.shutdownCtx, name)
+}
+
 func patchGenericStorage(name string, d *Daemon) error {
 	return storagePools.Patch(d.State(), name)
 }

doc/authorization.md

@@ -39,18 +39,14 @@ Incus will connect to the OpenFGA server, write the {ref}`openfga-model`, and qu
 With OpenFGA, access to a particular API resource is determined by the user's relationship to it.
 These relationships are determined by an [OpenFGA authorization model](https://openfga.dev/docs/concepts#what-is-an-authorization-model).
 The Incus OpenFGA authorization model describes API resources in terms of their relationship to other resources, and a relationship a user or group might have with that resource.
-Some convenient relations have also been built into the model:
-
-- `server -> admin`: Full access to Incus.
-- `server -> operator`: Full access to Incus, without edit access on server configuration, certificates, or storage pools.
-- `server -> viewer`: Can view all server level configuration but cannot edit. Cannot view projects or their contents.
-- `project -> manager`: Full access to a single project, including edit access.
-- `project -> operator`: Full access to a single project, without edit access.
-- `project -> viewer`: View access for a single project.
-- `instance -> manager`: Full access to a single instance, including edit access.
-- `instance -> operator`: Full access to a single  instance, without edit access.
-- `instance -> user`: View access to a single instance, plus permissions for `exec`, `console`, and `file` APIs.
-- `instance -> viewer`: View access to a single instance.
+
+The full Incus OpenFGA authorization model is defined in `internal/server/auth/driver_openfga_model.openfga`:
+
+```{literalinclude} ../internal/server/auth/driver_openfga_model.openfga
+---
+language: none
+---
+```
 
 ```{important}
 Users that you do not trust with root access to the host should not be granted the following relations:
@@ -68,11 +64,3 @@ Users that you do not trust with root access to the host should not be granted t
 The remaining relations may be granted.
 However, you must apply appropriate {ref}`project-restrictions`.
 ```
-
-The full Incus OpenFGA authorization model is defined in `internal/server/auth/driver_openfga_model.openfga`:
-
-```{literalinclude} ../internal/server/auth/driver_openfga_model.openfga
----
-language: none
----
-```

internal/server/auth/authorization.go

@@ -41,6 +41,7 @@ type PermissionChecker func(object Object) bool
 type Authorizer interface {
 	Driver() string
 	StopService(ctx context.Context) error
+	ApplyPatch(ctx context.Context, name string) error
 
 	CheckPermission(ctx context.Context, r *http.Request, object Object, entitlement Entitlement) error
 	GetPermissionChecker(ctx context.Context, r *http.Request, entitlement Entitlement, objectType ObjectType) (PermissionChecker, error)

internal/server/auth/authorization_types.go

@@ -17,6 +17,7 @@ const (
 	EntitlementCanViewMetrics                      Entitlement = "can_view_metrics"
 	EntitlementCanViewPrivilegedEvents             Entitlement = "can_view_privileged_events"
 	EntitlementCanViewResources                    Entitlement = "can_view_resources"
+	EntitlementCanViewSensitive                    Entitlement = "can_view_sensitive"
 
 	// Project entitlements.
 	EntitlementCanCreateImageAliases   Entitlement = "can_create_image_aliases"

internal/server/auth/driver_common.go

@@ -134,6 +134,11 @@ func (c *commonAuthorizer) StopService(ctx context.Context) error {
 	return nil
 }
 
+// ApplyPatch is a no-op.
+func (c *commonAuthorizer) ApplyPatch(ctx context.Context, name string) error {
+	return nil
+}
+
 // AddProject is a no-op.
 func (c *commonAuthorizer) AddProject(ctx context.Context, projectID int64, name string) error {
 	return nil

internal/server/auth/driver_openfga.go

@@ -144,76 +144,41 @@ func (f *fga) StopService(ctx context.Context) error {
 	return nil
 }
 
-func (f *fga) connect(ctx context.Context, certificateCache *certificate.Cache, opts Opts) error {
-	var builtinAuthorizationModel client.ClientWriteAuthorizationModelRequest
+// ApplyPatch is called when an applicable server patch is run, this triggers a model re-upload.
+func (f *fga) ApplyPatch(ctx context.Context, name string) error {
+	// Upload a new model.
+	logger.Info("Refreshing the OpenFGA model")
+	return f.refreshModel(ctx)
+}
 
+func (f *fga) refreshModel(ctx context.Context) error {
+	var builtinAuthorizationModel client.ClientWriteAuthorizationModelRequest
 	err := json.Unmarshal([]byte(authModel), &builtinAuthorizationModel)
 	if err != nil {
-		return err
+		return fmt.Errorf("Failed to unmarshal built in authorization model: %w", err)
 	}
 
-	// Load current authorization model.
-	readModelResponse, err := f.client.ReadLatestAuthorizationModel(ctx).Execute()
+	_, err = f.client.WriteAuthorizationModel(ctx).Body(builtinAuthorizationModel).Execute()
 	if err != nil {
-		return fmt.Errorf("Failed to read pre-existing OpenFGA model: %w", err)
+		return fmt.Errorf("Failed to write the authorization model: %w", err)
 	}
 
-	// Check if we need to upload a new model.
-	upload := readModelResponse.AuthorizationModel == nil
-
-	if !upload {
-		// Make sure we're not dealing with different schemas.
-		if readModelResponse.AuthorizationModel.SchemaVersion != builtinAuthorizationModel.SchemaVersion {
-			return fmt.Errorf("Existing OpenFGA model has schema version %q, but our model has version %q", readModelResponse.AuthorizationModel.SchemaVersion, builtinAuthorizationModel.SchemaVersion)
-		}
-
-		// Clear condition field from older servers.
-		for _, entry := range readModelResponse.AuthorizationModel.TypeDefinitions {
-			if entry.Metadata == nil || entry.Metadata.Relations == nil {
-				continue
-			}
-
-			for _, relation := range *entry.Metadata.Relations {
-				if relation.DirectlyRelatedUserTypes == nil {
-					continue
-				}
-
-				for i, reference := range *relation.DirectlyRelatedUserTypes {
-					if reference.Condition != nil && *reference.Condition == "" {
-						rel := *relation.DirectlyRelatedUserTypes
-						rel[i].Condition = nil
-					}
-				}
-			}
-		}
-
-		// Serialize the models to JSON.
-		existingTypeDefinitions, err := json.Marshal(readModelResponse.AuthorizationModel.TypeDefinitions)
-		if err != nil {
-			return fmt.Errorf("Failed to compare OpenFGA model type definitions: %w", err)
-		}
-
-		builtinTypeDefinitions, err := json.Marshal(builtinAuthorizationModel.TypeDefinitions)
-		if err != nil {
-			return fmt.Errorf("Failed to compare OpenFGA model type definitions: %w", err)
-		}
+	return nil
+}
 
-		// Compare them.
-		if string(existingTypeDefinitions) != string(builtinTypeDefinitions) {
-			logger.Info("The OpenFGA model has changed, uploading new model")
-			upload = true
-		}
+func (f *fga) connect(ctx context.Context, certificateCache *certificate.Cache, opts Opts) error {
+	// Load current authorization model.
+	readModelResponse, err := f.client.ReadLatestAuthorizationModel(ctx).Execute()
+	if err != nil {
+		return fmt.Errorf("Failed to read pre-existing OpenFGA model: %w", err)
 	}
 
-	if upload {
-		err = json.Unmarshal([]byte(authModel), &builtinAuthorizationModel)
-		if err != nil {
-			return fmt.Errorf("Failed to unmarshal built in authorization model: %w", err)
-		}
-
-		_, err := f.client.WriteAuthorizationModel(ctx).Body(builtinAuthorizationModel).Execute()
+	// Check if we need to upload an initial model.
+	if readModelResponse.AuthorizationModel == nil {
+		logger.Info("Upload initial OpenFGA model")
+		err := f.refreshModel(ctx)
 		if err != nil {
-			return fmt.Errorf("Failed to write the authorization model: %w", err)
+			return fmt.Errorf("Failed to load initial model: %w", err)
 		}
 	}
 
@@ -951,27 +916,41 @@ func (f *fga) projectObjects(ctx context.Context, projectName string) ([]string,
 	return allObjects, nil
 }
 
-func (f *fga) syncResources(ctx context.Context, resources Resources) error {
+func (f *fga) applyPatches(ctx context.Context) ([]client.ClientTupleKey, []client.ClientTupleKeyWithoutCondition, error) {
 	var writes []client.ClientTupleKey
 	var deletions []client.ClientTupleKeyWithoutCondition
 
-	// Check if the type-bound public access is set.
+	// Add the public access permission if not set.
 	resp, err := f.client.Check(ctx).Body(client.ClientCheckRequest{
 		User:     "user:*",
-		Relation: "viewer",
+		Relation: "authenticated",
 		Object:   ObjectServer().String(),
 	}).Execute()
 	if err != nil {
-		return err
+		return nil, nil, err
 	}
 
-	// If not, set it.
 	if !resp.GetAllowed() {
 		writes = append(writes, client.ClientTupleKey{
 			User:     "user:*",
-			Relation: "viewer",
+			Relation: "authenticated",
 			Object:   ObjectServer().String(),
 		})
+
+		// Attempt to clear the former version of this permission.
+		_ = f.updateTuples(ctx, nil, []client.ClientTupleKeyWithoutCondition{
+			{User: "user:*", Relation: "viewer", Object: ObjectServer().String()},
+		})
+	}
+
+	return writes, deletions, nil
+}
+
+func (f *fga) syncResources(ctx context.Context, resources Resources) error {
+	// Apply model patches.
+	writes, deletions, err := f.applyPatches(ctx)
+	if err != nil {
+		return err
 	}
 
 	// Helper function for diffing local objects with those in OpenFGA. These are appended to the writes and deletions

internal/server/auth/driver_openfga_model.go

@@ -77,7 +77,7 @@ type project
     define admin: [user, group#member] or admin from server
     define operator: [user, group#member] or admin or operator from server
     define user: [user, group#member] or operator or user from server
-    define viewer: [user, group#member] or user
+    define viewer: [user, group#member] or user or viewer from server
     define can_create_image_aliases: [user, group#member] or operator
     define can_create_images: [user, group#member] or operator
     define can_create_instances: [user, group#member] or operator
@@ -97,17 +97,19 @@ type server
     define admin: [user, group#member]
     define operator: [user, group#member] or admin
     define user: [user, group#member] or operator
-    define viewer: [user:*] or user
+    define viewer: [user, group#member] or user
+    define authenticated: [user:*]
     define can_create_certificates: [user, group#member] or admin
     define can_create_network_integrations: [user, group#member] or admin
     define can_create_projects: [user, group#member] or admin
     define can_create_storage_pools: [user, group#member] or admin
     define can_edit: admin
     define can_override_cluster_target_restriction: [user, group#member] or admin
-    define can_view_metrics: [user, group#member] or viewer
     define can_view_privileged_events: [user, group#member] or admin
-    define can_view_resources: [user, group#member] or viewer
-    define can_view: viewer
+    define can_view_metrics: authenticated
+    define can_view_resources: authenticated
+    define can_view_sensitive: [user, group#member] or viewer
+    define can_view: authenticated
 
 type storage_bucket
   relations
@@ -119,7 +121,7 @@ type storage_pool
   relations
     define server: [server]
     define can_edit: [user, group#member] or admin from server
-    define can_view: viewer from server
+    define can_view: authenticated from server
 
 type storage_volume
   relations

internal/server/auth/driver_openfga_model.openfga

@@ -63,7 +63,7 @@ func (t *tls) CheckPermission(ctx context.Context, r *http.Request, object Objec
 	// Check server level object types
 	switch object.Type() {
 	case ObjectTypeServer:
-		if entitlement == EntitlementCanView || entitlement == EntitlementCanViewResources || entitlement == EntitlementCanViewMetrics {
+		if entitlement == EntitlementCanView || entitlement == EntitlementCanViewResources || entitlement == EntitlementCanViewMetrics || entitlement == EntitlementCanViewSensitive {
 			return nil
 		}
 
@@ -139,7 +139,7 @@ func (t *tls) GetPermissionChecker(ctx context.Context, r *http.Request, entitle
 	// Check server level object types
 	switch objectType {
 	case ObjectTypeServer:
-		if entitlement == EntitlementCanView || entitlement == EntitlementCanViewResources || entitlement == EntitlementCanViewMetrics {
+		if entitlement == EntitlementCanView || entitlement == EntitlementCanViewResources || entitlement == EntitlementCanViewMetrics || entitlement == EntitlementCanViewSensitive {
 			return allowFunc(true), nil
 		}
 

internal/server/auth/driver_tls.go

@@ -35,6 +35,7 @@ test_openfga() {
   echo "==> Checking permissions for unknown user..."
   user_is_not_server_admin
   user_is_not_server_operator
+  user_is_not_server_viewer
   user_is_not_project_admin
   user_is_not_project_operator
 
@@ -105,12 +106,17 @@ test_openfga() {
   shutdown_openfga
 }
 
-user_is_not_server_admin() {
-  # Can always see server info (type-bound public access https://openfga.dev/docs/modeling/public-access).
-  incus info oidc-openfga: > /dev/null
+user_is_not_server_viewer() {
+  # Should still be able to list certificates.
+  [ "$(incus config trust list oidc-openfga: -f csv -cf | wc -l)" = 0 ]
 
   # Cannot see any config.
   ! incus info oidc-openfga: | grep -Fq 'core.https_address' || false
+}
+
+user_is_not_server_admin() {
+  # Can always see server info (type-bound public access https://openfga.dev/docs/modeling/public-access).
+  incus info oidc-openfga: > /dev/null
 
   # Cannot set any config.
   ! incus config set oidc-openfga: core.proxy_https=https://example.com || false
@@ -125,13 +131,6 @@ user_is_not_server_admin() {
 
   # Should not be able to create a storage pool.
   ! incus storage create oidc-openfga:test dir || false
-
-  # Should still be able to list certificates.
-  [ "$(incus config trust list oidc-openfga: -f csv -cf | wc -l)" = 1 ]
-
-  # Cannot edit certificates.
-  fingerprint="$(incus config trust list -f csv -cf)"
-  ! incus config trust show "${fingerprint}" | sed -e "s/restricted: false/restricted: true/" | incus config trust edit "oidc-openfga:${fingerprint}" || false
 }
 
 user_is_not_server_operator() {
@@ -204,7 +203,6 @@ user_is_project_operator() {
 }
 
 user_is_not_project_operator() {
-
   # Project list will not fail but there will be no output.
   [ "$(incus project list oidc-openfga: -f csv | wc -l)" = 0 ]
   ! incus project show oidc-openfga:default || false