diff --git a/lxd/auth_groups.go b/lxd/auth_groups.go index a64ba60957e8..c5c27a432cf5 100644 --- a/lxd/auth_groups.go +++ b/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, diff --git a/lxd/db/cluster/auth_groups.go b/lxd/db/cluster/auth_groups.go index 011b8a968159..3e8d59e1827b 100644 --- a/lxd/db/cluster/auth_groups.go +++ b/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) } diff --git a/lxd/db/cluster/entity_type_certificate.go b/lxd/db/cluster/entity_type_certificate.go index 129beb8e6385..9c352bc22b95 100644 --- a/lxd/db/cluster/entity_type_certificate.go +++ b/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) { diff --git a/lxd/db/cluster/entity_type_identity.go b/lxd/db/cluster/entity_type_identity.go index 21f403153e6c..281674558a20 100644 --- a/lxd/db/cluster/entity_type_identity.go +++ b/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) { diff --git a/lxd/identities.go b/lxd/identities.go index 13e8fa84d086..2c842113246e 100644 --- a/lxd/identities.go +++ b/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() diff --git a/lxd/identity/util.go b/lxd/identity/util.go index f60b5dbd26ff..4aec2452cc2f 100644 --- a/lxd/identity/util.go +++ b/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) { diff --git a/lxd/patches.go b/lxd/patches.go index d82df58381f5..9190e7d63cce 100644 --- a/lxd/patches.go +++ b/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 diff --git a/lxd/patches_test.go b/lxd/patches_test.go new file mode 100644 index 000000000000..c06cdfbf8426 --- /dev/null +++ b/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: "jane.doe@example.com", + 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) +} diff --git a/test/suites/auth.sh b/test/suites/auth.sh index c227c6de1a83..2f807ec64030 100644 --- a/test/suites/auth.sh +++ b/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 test-user@example.com can_view || false # Missing authentication method + lxc auth group permission add test-group identity oidc/test-user@example.com can_view # Valid + lxc auth group permission remove test-group identity oidc/test-user@example.com can_view + ! lxc auth group permission remove test-group identity oidc/test-user@example.com 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