Auth: Redefine identity certificate entity types to prevent overlap

lxd/auth_groups.go

@@ -855,7 +855,6 @@ func upsertPermissions(ctx context.Context, tx *sql.Tx, groupID int, permissions
 		}
 
 		authGroupPermissions = append(authGroupPermissions, dbCluster.Permission{
-			GroupID:     groupID,
 			Entitlement: entitlement,
 			EntityType:  entityType,
 			EntityID:    entityRef.EntityID,

lxd/db/cluster/auth_groups.go

@@ -287,7 +287,7 @@ func SetAuthGroupPermissions(ctx context.Context, tx *sql.Tx, groupID int, authG
 	}
 
 	for _, permission := range authGroupPermissions {
-		_, err := tx.ExecContext(ctx, `INSERT INTO auth_groups_permissions (auth_group_id, entity_type, entity_id, entitlement) VALUES (?, ?, ?, ?);`, permission.GroupID, permission.EntityType, permission.EntityID, permission.Entitlement)
+		_, err := tx.ExecContext(ctx, `INSERT INTO auth_groups_permissions (auth_group_id, entity_type, entity_id, entitlement) VALUES (?, ?, ?, ?);`, groupID, permission.EntityType, permission.EntityID, permission.Entitlement)
 		if err != nil {
 			return fmt.Errorf("Failed to write group permissions: %w", err)
 		}

lxd/db/cluster/entity_type_certificate.go

@@ -12,7 +12,16 @@ func (e entityTypeCertificate) code() int64 {
 }
 
 func (e entityTypeCertificate) allURLsQuery() string {
-	return fmt.Sprintf(`SELECT %d, identities.id, '', '', json_array(identities.identifier) FROM identities WHERE auth_method = %d`, e.code(), authMethodTLS)
+	return fmt.Sprintf(
+		`SELECT %d, identities.id, '', '', json_array(identities.identifier) FROM identities WHERE auth_method = %d AND type IN (%d, %d, %d, %d, %d)`,
+		e.code(),
+		authMethodTLS,
+		identityTypeCertificateClientRestricted,
+		identityTypeCertificateClientUnrestricted,
+		identityTypeCertificateServer,
+		identityTypeCertificateMetricsRestricted,
+		identityTypeCertificateMetricsUnrestricted,
+	)
 }
 
 func (e entityTypeCertificate) urlsByProjectQuery() string {
@@ -31,7 +40,14 @@ WHERE '' = ?
 	AND '' = ? 
 	AND identities.identifier = ? 
 	AND identities.auth_method = %d
-`, authMethodTLS)
+	AND identities.type IN (%d, %d, %d, %d, %d)
+`, authMethodTLS,
+		identityTypeCertificateClientRestricted,
+		identityTypeCertificateClientUnrestricted,
+		identityTypeCertificateServer,
+		identityTypeCertificateMetricsRestricted,
+		identityTypeCertificateMetricsUnrestricted,
+	)
 }
 
 func (e entityTypeCertificate) onDeleteTriggerSQL() (name string, sql string) {

lxd/db/cluster/entity_type_identity.go

@@ -28,10 +28,12 @@ SELECT
 		identities.identifier
 	) 
 FROM identities
+WHERE type IN (%d)
 `,
 		e.code(),
 		authMethodTLS, api.AuthenticationMethodTLS,
 		authMethodOIDC, api.AuthenticationMethodOIDC,
+		identityTypeOIDCClient,
 	)
 }
 
@@ -40,7 +42,7 @@ func (e entityTypeIdentity) urlsByProjectQuery() string {
 }
 
 func (e entityTypeIdentity) urlByIDQuery() string {
-	return fmt.Sprintf(`%s WHERE identities.id = ?`, e.allURLsQuery())
+	return fmt.Sprintf(`%s AND identities.id = ?`, e.allURLsQuery())
 }
 
 func (e entityTypeIdentity) idFromURLQuery() string {
@@ -54,8 +56,11 @@ WHERE '' = ?
 		WHEN %d THEN '%s' 
 	END = ? 
 	AND identities.identifier = ?
+	AND identities.type IN (%d)
 `, authMethodTLS, api.AuthenticationMethodTLS,
-		authMethodOIDC, api.AuthenticationMethodOIDC)
+		authMethodOIDC, api.AuthenticationMethodOIDC,
+		identityTypeOIDCClient,
+	)
 }
 
 func (e entityTypeIdentity) onDeleteTriggerSQL() (name string, sql string) {

lxd/identities.go

@@ -99,9 +99,16 @@ func identityAccessHandler(entitlement auth.Entitlement) func(d *Daemon, r *http
 			return response.SmartError(err)
 		}
 
-		err = s.Authorizer.CheckPermission(r.Context(), entity.IdentityURL(authenticationMethod, id.Identifier), entitlement)
-		if err != nil {
-			return response.SmartError(err)
+		if identity.IsFineGrainedIdentityType(string(id.Type)) {
+			err = s.Authorizer.CheckPermission(r.Context(), entity.IdentityURL(authenticationMethod, id.Identifier), entitlement)
+			if err != nil {
+				return response.SmartError(err)
+			}
+		} else {
+			err = s.Authorizer.CheckPermission(r.Context(), entity.CertificateURL(id.Identifier), entitlement)
+			if err != nil {
+				return response.SmartError(err)
+			}
 		}
 
 		request.SetCtxValue(r, ctxClusterDBIdentity, id)
@@ -292,6 +299,19 @@ func getIdentities(d *Daemon, r *http.Request) response.Response {
 		return response.SmartError(err)
 	}
 
+	canViewCertificate, err := s.Authorizer.GetPermissionChecker(r.Context(), auth.EntitlementCanView, entity.TypeCertificate)
+	if err != nil {
+		return response.SmartError(err)
+	}
+
+	canView := func(id dbCluster.Identity) bool {
+		if identity.IsFineGrainedIdentityType(string(id.Type)) {
+			return canViewIdentity(entity.IdentityURL(string(id.AuthMethod), id.Identifier))
+		}
+
+		return canViewCertificate(entity.CertificateURL(id.Identifier))
+	}
+
 	canViewGroup, err := s.Authorizer.GetPermissionChecker(r.Context(), auth.EntitlementCanView, entity.TypeAuthGroup)
 	if err != nil {
 		return response.SmartError(err)
@@ -315,7 +335,7 @@ func getIdentities(d *Daemon, r *http.Request) response.Response {
 
 		// Filter results by what the user is allowed to view.
 		for _, id := range allIdentities {
-			if canViewIdentity(entity.IdentityURL(string(id.AuthMethod), id.Identifier)) {
+			if canView(id) {
 				identities = append(identities, id)
 			}
 		}
@@ -599,8 +619,8 @@ func updateIdentity(d *Daemon, r *http.Request) response.Response {
 		return response.BadRequest(fmt.Errorf("Failed to unmarshal request body: %w", err))
 	}
 
-	if id.AuthMethod == api.AuthenticationMethodTLS && len(identityPut.Groups) > 0 {
-		return response.NotImplemented(fmt.Errorf("Adding TLS identities to groups is currently not supported"))
+	if !identity.IsFineGrainedIdentityType(string(id.Type)) {
+		return response.NotImplemented(fmt.Errorf("Identities of type %q cannot be modified via this API", id.Type))
 	}
 
 	s := d.State()
@@ -692,9 +712,8 @@ func patchIdentity(d *Daemon, r *http.Request) response.Response {
 		return response.BadRequest(fmt.Errorf("Failed to unmarshal request body: %w", err))
 	}
 
-	authenticationMethod := mux.Vars(r)["authenticationMethod"]
-	if authenticationMethod == api.AuthenticationMethodTLS && len(identityPut.Groups) > 0 {
-		return response.NotImplemented(fmt.Errorf("Adding TLS identities to groups is currently not supported"))
+	if !identity.IsFineGrainedIdentityType(string(id.Type)) {
+		return response.NotImplemented(fmt.Errorf("Identities of type %q cannot be modified via this API", id.Type))
 	}
 
 	s := d.State()

lxd/identity/util.go

@@ -8,6 +8,11 @@ import (
 	"github.com/canonical/lxd/shared/api"
 )
 
+// IsFineGrainedIdentityType returns true if permissions of the identity type are managed via group membership.
+func IsFineGrainedIdentityType(identityType string) bool {
+	return shared.ValueInSlice(identityType, []string{api.IdentityTypeOIDCClient})
+}
+
 // IsRestrictedIdentityType returns whether the given identity is restricted or not. Identity types that are not
 // restricted have full access to LXD. An error is returned if the identity type is not recognised.
 func IsRestrictedIdentityType(identityType string) (bool, error) {

lxd/patches.go

@@ -29,6 +29,7 @@ import (
 	"github.com/canonical/lxd/lxd/util"
 	"github.com/canonical/lxd/shared"
 	"github.com/canonical/lxd/shared/api"
+	"github.com/canonical/lxd/shared/entity"
 	"github.com/canonical/lxd/shared/logger"
 	"github.com/canonical/lxd/shared/revert"
 )
@@ -94,6 +95,7 @@ var patches = []patch{
 	{name: "config_remove_core_trust_password", stage: patchPreLoadClusterConfig, run: patchRemoveCoreTrustPassword},
 	{name: "entity_type_instance_snapshot_on_delete_trigger_typo_fix", stage: patchPreLoadClusterConfig, run: patchEntityTypeInstanceSnapshotOnDeleteTriggerTypoFix},
 	{name: "instance_remove_volatile_last_state_ip_addresses", stage: patchPostDaemonStorage, run: patchInstanceRemoveVolatileLastStateIPAddresses},
+	{name: "entity_type_identity_certificate_split", stage: patchPreLoadClusterConfig, run: patchSplitIdentityCertificateEntityTypes},
 }
 
 type patch struct {
@@ -1436,4 +1438,42 @@ func patchInstanceRemoveVolatileLastStateIPAddresses(_ string, d *Daemon) error
 	return nil
 }
 
+func patchSplitIdentityCertificateEntityTypes(_ string, d *Daemon) error {
+	s := d.State()
+	err := s.DB.Cluster.Transaction(s.ShutdownCtx, func(ctx context.Context, tx *db.ClusterTx) error {
+		// Notes:
+		// - We don't need to handle warnings, there are no warnings for identities.
+		// - No permissions have been created for OIDC identities against the certificate entity type, because the database
+		//   definition for the certificate entity type ensures that the authentication method is TLS.
+		// - We only need to fix permissions defined with the "identity" entity type that are certificates.
+
+		// Select all permissions with entity type = "identity", that really are certificates (auth_method = "tls")
+		// and set their entity type to "certificate" instead. Use "UPDATE OR REPLACE" in case of UNIQUE constraint violation.
+		stmt := `
+UPDATE OR REPLACE auth_groups_permissions 
+	SET entity_type = ?
+	WHERE id IN (
+	    SELECT auth_groups_permissions.id FROM auth_groups_permissions
+			JOIN identities ON auth_groups_permissions.entity_id = identities.id
+			WHERE auth_groups_permissions.entity_type = ?
+			AND identities.auth_method = ?
+	)
+`
+
+		// Set entity type to:
+		certificateEntityTypeCode, _ := dbCluster.EntityType(entity.TypeCertificate).Value()
+		// where entity type is:
+		identityEntityTypeCode, _ := dbCluster.EntityType(entity.TypeIdentity).Value()
+		// and authentication method is:
+		tlsAuthMethodCode, _ := dbCluster.AuthMethod(api.AuthenticationMethodTLS).Value()
+		_, err := tx.Tx().Exec(stmt, []any{certificateEntityTypeCode, identityEntityTypeCode, tlsAuthMethodCode}...)
+		return err
+	})
+	if err != nil {
+		return fmt.Errorf("Failed to redefine certificate and identity entity types: %w", err)
+	}
+
+	return nil
+}
+
 // Patches end here

lxd/patches_test.go

@@ -0,0 +1,116 @@
+package main
+
+import (
+	"context"
+	"encoding/json"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/canonical/lxd/lxd/auth"
+	"github.com/canonical/lxd/lxd/certificate"
+	"github.com/canonical/lxd/lxd/db"
+	dbCluster "github.com/canonical/lxd/lxd/db/cluster"
+	"github.com/canonical/lxd/shared"
+	"github.com/canonical/lxd/shared/api"
+	"github.com/canonical/lxd/shared/entity"
+)
+
+func Test_patchSplitIdentityCertificateEntityTypes(t *testing.T) {
+	// Set up test database.
+	cluster, cleanup := db.NewTestCluster(t)
+	defer cleanup()
+	ctx := context.Background()
+
+	var groupID int
+	var certificateID int
+	var identityID int
+	err := cluster.Transaction(ctx, func(ctx context.Context, tx *db.ClusterTx) error {
+		// Create a group.
+		groupIDint64, err := dbCluster.CreateAuthGroup(ctx, tx.Tx(), dbCluster.AuthGroup{
+			Name: "test-group",
+		})
+		require.NoError(t, err)
+		groupID = int(groupIDint64)
+
+		// Create a certificate
+		cert, _, err := shared.GenerateMemCert(true, shared.CertOptions{})
+		require.NoError(t, err)
+		x509Cert, err := shared.ParseCert(cert)
+		require.NoError(t, err)
+
+		certificateIDint64, err := dbCluster.CreateCertificate(ctx, tx.Tx(), dbCluster.Certificate{
+			Fingerprint: shared.CertFingerprint(x509Cert),
+			Type:        certificate.TypeClient,
+			Name:        "test-cert",
+			Certificate: string(cert),
+			Restricted:  false,
+		})
+		require.NoError(t, err)
+		certificateID = int(certificateIDint64)
+
+		// Create an OIDC identity
+		oidcMetadata, err := json.Marshal(dbCluster.OIDCMetadata{Subject: "test-subject"})
+		require.NoError(t, err)
+		identityIDint64, err := dbCluster.CreateIdentity(ctx, tx.Tx(), dbCluster.Identity{
+			AuthMethod: api.AuthenticationMethodOIDC,
+			Type:       api.IdentityTypeOIDCClient,
+			Identifier: "[email protected]",
+			Name:       "Jane Doe",
+			Metadata:   string(oidcMetadata),
+		})
+		require.NoError(t, err)
+		identityID = int(identityIDint64)
+
+		// Create three permissions for the group.
+		err = dbCluster.SetAuthGroupPermissions(ctx, tx.Tx(), groupID, []dbCluster.Permission{
+			{
+				// This permission has "identity" as the entity type but references a certificate. We expect the entity
+				// type to change to "certificate" after the patch.
+				Entitlement: auth.EntitlementCanView,
+				EntityType:  dbCluster.EntityType(entity.TypeIdentity),
+				EntityID:    certificateID,
+			},
+			{
+				// This permission has "certificate" as the entity type. We expect that this will be replaced after the patch.
+				Entitlement: auth.EntitlementCanView,
+				EntityType:  dbCluster.EntityType(entity.TypeCertificate),
+				EntityID:    certificateID,
+			},
+			{
+				// This permission also has "identity" as the entity type and this references an OIDC client. The entity
+				// type for this permission should not change.
+				Entitlement: auth.EntitlementCanView,
+				EntityType:  dbCluster.EntityType(entity.TypeIdentity),
+				EntityID:    identityID,
+			},
+		})
+		require.NoError(t, err)
+		return nil
+	})
+	require.NoError(t, err)
+
+	// Run the patch.
+	daemonDB := &db.DB{Cluster: cluster}
+	daemon := &Daemon{db: daemonDB, shutdownCtx: ctx}
+	err = patchSplitIdentityCertificateEntityTypes("", daemon)
+	require.NoError(t, err)
+
+	// Get the permissions.
+	err = cluster.Transaction(ctx, func(ctx context.Context, tx *db.ClusterTx) error {
+		permissions, err := dbCluster.GetPermissionsByAuthGroupID(ctx, tx.Tx(), groupID)
+		require.NoError(t, err)
+
+		// The second permission should have been overwritten, so there should now only be two permissions.
+		assert.Len(t, permissions, 2)
+
+		// The first permission we created should have the entity type changed to "certificate".
+		assert.Equal(t, entity.TypeCertificate, entity.Type(permissions[0].EntityType))
+
+		// The entity type of the third permission should not have changed.
+		assert.Equal(t, entity.TypeIdentity, entity.Type(permissions[1].EntityType))
+		return nil
+	})
+	require.NoError(t, err)
+}

test/suites/auth.sh

@@ -27,14 +27,15 @@ test_authorization() {
   ! lxc auth group permission remove test-group server admin || false # Permission already removed
   ! lxc auth group permission add test-group server not_a_server_entitlement || false # Invalid entitlement
 
-  # Identity permissions.
-  ! lxc auth group permission add test-group identity "${tls_user_fingerprint}" can_view || false # Missing authentication method
-  lxc auth group permission add test-group identity "tls/${tls_user_fingerprint}" can_view # Valid
-  lxc auth group permission remove test-group identity "tls/${tls_user_fingerprint}" can_view
-  ! lxc auth group permission remove test-group identity "tls/${tls_user_fingerprint}" can_view || false # Already removed
+  # Certificate permissions.
+  ! lxc auth group permission add test-group certificate notacertificate can_view || false # Not found
+  lxc auth group permission add test-group certificate "${tls_user_fingerprint}" can_view # Valid
+  lxc auth group permission remove test-group certificate "${tls_user_fingerprint}" can_view
+  ! lxc auth group permission remove test-group certificate "${tls_user_fingerprint}" can_view || false # Already removed
+  ! lxc auth group permission add test-group identity "tls/${tls_user_fingerprint}" can_view || false # The certificate is not an identity, it is a certificate.
 
   # Project permissions.
-  ! lxc auth group permission add test-group project not-found operator # Not found
+  ! lxc auth group permission add test-group project not-found operator || false # Not found
   lxc auth group permission add test-group project default operator # Valid
   lxc auth group permission remove test-group project default operator # Valid
   ! lxc auth group permission remove test-group project default operator || false # Already removed
@@ -96,6 +97,12 @@ EOF
 )
   lxc auth identity info oidc: | grep -Fz "${expected}"
 
+  # Identity permissions.
+  ! lxc auth group permission add test-group identity [email protected] can_view || false # Missing authentication method
+  lxc auth group permission add test-group identity oidc/[email protected] can_view # Valid
+  lxc auth group permission remove test-group identity oidc/[email protected] can_view
+  ! lxc auth group permission remove test-group identity oidc/[email protected] can_view || false # Already removed
+
   ### IDENTITY PROVIDER GROUP MANAGEMENT ###
   lxc auth identity-provider-group create test-idp-group
   ! lxc auth identity-provider-group group add test-idp-group not-found || false # Group not found