yv01p/Umbraco-CMS
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove property value permissions when related content and/or property types are removed (#19778)

Browse Source 
* Removed two unnecessary delete clauses when removing content types (they are looking for user group Ids, but we are deleting a content type).

* Renamed table name constant with obsoletion to better reflect name and contents of table.

* Added granular permission for property value records to delete clauses when deleting a document type.

* Delete property value permissions for removed property types.

* Added integration tests to verify behaviour.
This commit is contained in:
Andy Butland
2025-08-07 14:22:19 +02:00
committed by GitHub
parent 8a94383262
commit a0aff9d10c
7 changed files with 124 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/Umbraco.Cms.Persistence.Sqlite/Services/SqliteSyntaxProvider.cs
View File

@@ -162,6 +162,8 @@ public class SqliteSyntaxProvider : SqlSyntaxProviderBase<SqliteSyntaxProvider>
public override string ConvertDateToOrderableString => "{0}";
public override string ConvertUniqueIdentifierToString => "{0}";
public override string RenameTable => "ALTER TABLE {0} RENAME TO {1}";
/// <inheritdoc />

5
src/Umbraco.Core/Persistence/Constants-DatabaseSchema.cs
View File

@@ -20,7 +20,12 @@ public static partial class Constants
public const string ContentType = /*TableNamePrefix*/ "cms" + "ContentType";
public const string ContentChildType = /*TableNamePrefix*/ "cms" + "ContentTypeAllowedContentType";
public const string DocumentType = /*TableNamePrefix*/ "cms" + "DocumentType";
[Obsolete("Please use ContentTypeTree instead. Scheduled for removal in Umbraco 18.")]
public const string ElementTypeTree = /*TableNamePrefix*/ "cms" + "ContentType2ContentType";
public const string ContentTypeTree = /*TableNamePrefix*/ "cms" + "ContentType2ContentType";
public const string DataType = TableNamePrefix + "DataType";
public const string Template = /*TableNamePrefix*/ "cms" + "Template";

2
src/Umbraco.Infrastructure/Persistence/Dtos/ContentType2ContentTypeDto.cs
View File

@@ -4,7 +4,7 @@ using Umbraco.Cms.Infrastructure.Persistence.DatabaseAnnotations;
namespace Umbraco.Cms.Infrastructure.Persistence.Dtos;
[TableName(Constants.DatabaseSchema.Tables.ElementTypeTree)]
[TableName(Constants.DatabaseSchema.Tables.ContentTypeTree)]
[ExplicitColumns]
internal sealed class ContentType2ContentTypeDto
{

47
src/Umbraco.Infrastructure/Persistence/Repositories/Implement/ContentTypeRepositoryBase.cs
View File

@@ -438,7 +438,7 @@ AND umbracoNode.id <> @id",
IEnumerable<int> propertyTypeToDeleteIds = dbPropertyTypeIds.Except(entityPropertyTypes);
foreach (var propertyTypeId in propertyTypeToDeleteIds)
{
DeletePropertyType(entity.Id, propertyTypeId);
DeletePropertyType(entity, propertyTypeId);
}
}
@@ -647,7 +647,7 @@ AND umbracoNode.id <> @id",
{
foreach (var id in orphanPropertyTypeIds)
{
DeletePropertyType(entity.Id, id);
DeletePropertyType(entity, id);
}
}
@@ -1410,16 +1410,27 @@ AND umbracoNode.id <> @id",
}
}
private void DeletePropertyType(int contentTypeId, int propertyTypeId)
private void DeletePropertyType(IContentTypeComposition contentType, int propertyTypeId)
{
// first clear dependencies
// First clear dependencies.
Database.Delete<TagRelationshipDto>("WHERE propertyTypeId = @Id", new { Id = propertyTypeId });
Database.Delete<PropertyDataDto>("WHERE propertyTypeId = @Id", new { Id = propertyTypeId });
// then delete the property type
// Clear the property value permissions, which aren't a hard dependency with a foreign key, but we want to ensure
// that any for removed property types are cleared.
var uniqueIdAsString = string.Format(SqlContext.SqlSyntax.ConvertUniqueIdentifierToString, "uniqueId");
var permissionSearchString = SqlContext.SqlSyntax.GetConcat(
"(SELECT " + uniqueIdAsString + " FROM " + Constants.DatabaseSchema.Tables.PropertyType + " WHERE id = @PropertyTypeId)",
"'|%'");
Database.Delete<UserGroup2GranularPermissionDto>(
"WHERE uniqueId = @ContentTypeKey AND permission LIKE " + permissionSearchString,
new { ContentTypeKey = contentType.Key, PropertyTypeId = propertyTypeId });
// Finally delete the property type.
Database.Delete<PropertyTypeDto>(
"WHERE contentTypeId = @Id AND id = @PropertyTypeId",
new { Id = contentTypeId, PropertyTypeId = propertyTypeId });
new { contentType.Id, PropertyTypeId = propertyTypeId });
}
protected void ValidateAlias(TEntity entity)
@@ -1555,20 +1566,16 @@ WHERE {Constants.DatabaseSchema.Tables.Content}.nodeId IN (@ids) AND cmsContentT
// is included here just to be 100% sure since it has a FK on cmsPropertyType.
var list = new List<string>
{
"DELETE FROM umbracoUser2NodeNotify WHERE nodeId = @id",
"DELETE FROM umbracoUserGroup2Permission WHERE userGroupKey IN (SELECT [umbracoUserGroup].[Key] FROM umbracoUserGroup WHERE Id = @id)",
"DELETE FROM umbracoUserGroup2GranularPermission WHERE userGroupKey IN (SELECT [umbracoUserGroup].[Key] FROM umbracoUserGroup WHERE Id = @id)",
"DELETE FROM cmsTagRelationship WHERE nodeId = @id",
"DELETE FROM cmsContentTypeAllowedContentType WHERE Id = @id",
"DELETE FROM cmsContentTypeAllowedContentType WHERE AllowedId = @id",
"DELETE FROM cmsContentType2ContentType WHERE parentContentTypeId = @id",
"DELETE FROM cmsContentType2ContentType WHERE childContentTypeId = @id",
"DELETE FROM " + Constants.DatabaseSchema.Tables.PropertyData +
" WHERE propertyTypeId IN (SELECT id FROM cmsPropertyType WHERE contentTypeId = @id)",
"DELETE FROM " + Constants.DatabaseSchema.Tables.PropertyType +
" WHERE contentTypeId = @id",
"DELETE FROM " + Constants.DatabaseSchema.Tables.PropertyTypeGroup +
" WHERE contenttypeNodeId = @id",
"DELETE FROM " + Constants.DatabaseSchema.Tables.User2NodeNotify + " WHERE nodeId = @id",
"DELETE FROM " + Constants.DatabaseSchema.Tables.UserGroup2GranularPermission + " WHERE uniqueId IN (SELECT uniqueId FROM umbracoNode WHERE id = @id)",
"DELETE FROM " + Constants.DatabaseSchema.Tables.TagRelationship + " WHERE nodeId = @id",
"DELETE FROM " + Constants.DatabaseSchema.Tables.ContentChildType + " WHERE Id = @id",
"DELETE FROM " + Constants.DatabaseSchema.Tables.ContentChildType + " WHERE AllowedId = @id",
"DELETE FROM " + Constants.DatabaseSchema.Tables.ContentTypeTree + " WHERE parentContentTypeId = @id",
"DELETE FROM " + Constants.DatabaseSchema.Tables.ContentTypeTree + " WHERE childContentTypeId = @id",
"DELETE FROM " + Constants.DatabaseSchema.Tables.PropertyData + " WHERE propertyTypeId IN (SELECT id FROM cmsPropertyType WHERE contentTypeId = @id)",
"DELETE FROM " + Constants.DatabaseSchema.Tables.PropertyType + " WHERE contentTypeId = @id",
"DELETE FROM " + Constants.DatabaseSchema.Tables.PropertyTypeGroup + " WHERE contenttypeNodeId = @id",
};
return list;
}

2
src/Umbraco.Infrastructure/Persistence/SqlSyntax/ISqlSyntaxProvider.cs
View File

@@ -72,6 +72,8 @@ public interface ISqlSyntaxProvider
string ConvertDecimalToOrderableString { get; }
string ConvertUniqueIdentifierToString => throw new NotImplementedException();
/// <summary>
/// Returns the default isolation level for the database
/// </summary>

2
src/Umbraco.Infrastructure/Persistence/SqlSyntax/SqlSyntaxProviderBase.cs
View File

@@ -469,6 +469,8 @@ public abstract class SqlSyntaxProviderBase<TSyntax> : ISqlSyntaxProvider
public virtual string ConvertDecimalToOrderableString => "REPLACE(STR({0}, 20, 9), SPACE(1), '0')";
public virtual string ConvertUniqueIdentifierToString => "CONVERT(nvarchar(36), {0})";
private DbTypes InitColumnTypeMap()
{
var dbTypeMap = new DbTypesFactory();

88
tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/Persistence/Repositories/ContentTypeRepositoryTest.cs
View File

@@ -1,19 +1,19 @@
// Copyright (c) Umbraco.
// See LICENSE for more details.
using System.Collections.Generic;
using System.Linq;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using Moq;
using NUnit.Framework;
using Umbraco.Cms.Api.Management.Mapping.Permissions;
using Umbraco.Cms.Core;
using Umbraco.Cms.Core.Cache;
using Umbraco.Cms.Core.Configuration.Models;
using Umbraco.Cms.Core.IO;
using Umbraco.Cms.Core.Mapping;
using Umbraco.Cms.Core.Models;
using Umbraco.Cms.Core.Models.ContentEditing;
using Umbraco.Cms.Core.Models.Membership;
using Umbraco.Cms.Core.Models.Membership.Permissions;
using Umbraco.Cms.Core.Persistence;
using Umbraco.Cms.Core.Persistence.Repositories;
using Umbraco.Cms.Core.Services;
@@ -52,8 +52,11 @@ internal sealed class ContentTypeRepositoryTest : UmbracoIntegrationTest
private IMediaTypeRepository MediaTypeRepository => GetRequiredService<IMediaTypeRepository>();
private IDocumentRepository DocumentRepository => GetRequiredService<IDocumentRepository>();
private IContentService ContentService => GetRequiredService<IContentService>();
private IUserGroupRepository UserGroupRepository => GetRequiredService<IUserGroupRepository>();
private ContentTypeRepository ContentTypeRepository =>
(ContentTypeRepository)GetRequiredService<IContentTypeRepository>();
@@ -918,4 +921,83 @@ internal sealed class ContentTypeRepositoryTest : UmbracoIntegrationTest
Assert.That(hasCulture, Is.True);
}
}
[Test]
public void Can_Remove_Property_Value_Permissions_On_Removal_Of_Property_Types()
{
var provider = ScopeProvider;
using (var scope = provider.CreateScope())
{
// Create, save and re-retrieve a content type and user group.
IContentType contentType = ContentTypeBuilder.CreateSimpleContentType(defaultTemplateId: 0);
ContentTypeRepository.Save(contentType);
contentType = ContentTypeRepository.Get(contentType.Id);
var userGroup = CreateUserGroupWithGranularPermissions(contentType);
// Remove property types and verify that the permission is removed from the user group.
contentType.RemovePropertyType("author");
ContentTypeRepository.Save(contentType);
userGroup = UserGroupRepository.Get(userGroup.Id);
Assert.AreEqual(3, userGroup.GranularPermissions.Count);
contentType.RemovePropertyType("bodyText");
ContentTypeRepository.Save(contentType);
userGroup = UserGroupRepository.Get(userGroup.Id);
Assert.AreEqual(2, userGroup.GranularPermissions.Count);
contentType.RemovePropertyType("title");
ContentTypeRepository.Save(contentType);
userGroup = UserGroupRepository.Get(userGroup.Id);
Assert.AreEqual(0, userGroup.GranularPermissions.Count);
}
}
[Test]
public void Can_Remove_Property_Value_Permissions_On_Removal_Of_Content_Type()
{
var provider = ScopeProvider;
using (var scope = provider.CreateScope())
{
// Create, save and re-retrieve a content type and user group.
IContentType contentType = ContentTypeBuilder.CreateSimpleContentType(defaultTemplateId: 0);
ContentTypeRepository.Save(contentType);
contentType = ContentTypeRepository.Get(contentType.Id);
var userGroup = CreateUserGroupWithGranularPermissions(contentType);
// Remove the content type and verify all permissions are removed from the user group.
ContentTypeRepository.Delete(contentType);
userGroup = UserGroupRepository.Get(userGroup.Id);
Assert.AreEqual(0, userGroup.GranularPermissions.Count);
}
}
private IUserGroup CreateUserGroupWithGranularPermissions(IContentType contentType)
{
DocumentPropertyValueGranularPermission CreatePermission(IPropertyType propertyType, string permission = "")
=> new()
{
Key = contentType.Key,
Permission = propertyType.Key.ToString().ToLowerInvariant() + "|" + permission,
};
var titlePropertyType = contentType.PropertyTypes.Single(x => x.Alias == "title");
var bodyTextPropertyType = contentType.PropertyTypes.Single(x => x.Alias == "bodyText");
var authorPropertyType = contentType.PropertyTypes.Single(x => x.Alias == "author");
var userGroup = new UserGroupBuilder()
.WithGranularPermissions([
CreatePermission(titlePropertyType, "Umb.Document.PropertyValue.Read"),
CreatePermission(titlePropertyType, "Umb.Document.PropertyValue.Write"),
CreatePermission(bodyTextPropertyType, "Umb.Document.PropertyValue.Read"),
CreatePermission(authorPropertyType)
])
.Build();
UserGroupRepository.Save(userGroup);
userGroup = UserGroupRepository.Get(userGroup.Id);
Assert.AreEqual(4, userGroup.GranularPermissions.Count);
return userGroup;
}
}