Fix: Fixed an issue where it wasn't possible to edit some file permissions (#12142)

Author: 0x5bfa

src/Files.App/Filesystem/Security/AccessControlEntry.cs

@@ -13,56 +13,144 @@
 namespace Files.App.Filesystem.Security
 {
 	/// <summary>
-	/// Represents an ACE.
+	/// Represents an access control entry (ACE).
 	/// </summary>
 	public class AccessControlEntry : ObservableObject
 	{
-		public bool IsFolder { get; set; }
-
+		/// <summary>
+		/// Whether the path indicates folder or not
+		/// </summary>
+		public bool IsFolder { get; private set; }
+
+		/// <summary>
+		/// The owner in the security descriptor (SD).
+		/// NULL if the security descriptor has no owner SID.
+		/// </summary>
 		public Principal Principal { get; set; }
 
-		private AccessControlType _AccessControlType;
-		public AccessControlType AccessControlType
+		/// <summary>
+		/// Whether the ACE is inherited or not
+		/// </summary>
+		public bool IsInherited { get; private set; }
+
+		/// <summary>
+		/// Whether the ACE is editable or not
+		/// </summary>
+		public bool IsEditable
+			=> IsSelected && !IsInherited && false;
+
+		/// <summary>
+		/// AccessControlTypeHumanized
+		/// </summary>
+		public string AccessControlTypeHumanized
+			=> AccessControlType switch
+			{
+				AccessControlEntryType.Allow => "Allow",
+				_ => "Deny" // AccessControlType.Deny
+			};
+
+		/// <summary>
+		/// AccessControlTypeGlyph
+		/// </summary>
+		public string AccessControlTypeGlyph
+			=> AccessControlType switch
+			{
+				AccessControlEntryType.Allow => "\xE73E",
+				_ => "\xF140" // AccessControlType.Deny
+			};
+
+		/// <summary>
+		/// AccessMaskFlagsHumanized
+		/// </summary>
+		public string AccessMaskFlagsHumanized
 		{
-			get => _AccessControlType;
-			set
+			get
 			{
-				// Update access control type glyph
-				if (SetProperty(ref _AccessControlType, value))
-					OnPropertyChanged(nameof(AccessControlTypeGlyph));
+				var accessMaskStrings = new List<string>();
+
+				if (AccessMaskFlags == AccessMaskFlags.NULL)
+					accessMaskStrings.Add("None".GetLocalizedResource());
+
+				if (FullControlAccess)
+					accessMaskStrings.Add("SecurityFullControlLabel/Text".GetLocalizedResource());
+				else if (ModifyAccess)
+					accessMaskStrings.Add("SecurityModifyLabel/Text".GetLocalizedResource());
+				else if (ReadAndExecuteAccess)
+					accessMaskStrings.Add("SecurityReadAndExecuteLabel/Text".GetLocalizedResource());
+				else if (ReadAccess)
+					accessMaskStrings.Add("SecurityReadLabel/Text".GetLocalizedResource());
+
+				if (!FullControlAccess && !ModifyAccess && WriteAccess)
+					accessMaskStrings.Add("Write".GetLocalizedResource());
+
+				if (SpecialAccess)
+					accessMaskStrings.Add("SecuritySpecialLabel/Text".GetLocalizedResource());
+
+				return string.Join(", ", accessMaskStrings);
 			}
 		}
 
-		private AccessMaskFlags _AccessMaskFlags;
-		public AccessMaskFlags AccessMaskFlags
+		/// <summary>
+		/// IsInheritedHumanized
+		/// </summary>
+		public string IsInheritedHumanized
+			=> IsInherited ? "Yes".GetLocalizedResource() : "No".GetLocalizedResource();
+
+		/// <summary>
+		/// InheritanceFlagsHumanized
+		/// </summary>
+		public string InheritanceFlagsHumanized
 		{
-			get => _AccessMaskFlags;
-			set
+			get
 			{
-				if (SetProperty(ref _AccessMaskFlags, value))
-					OnPropertyChanged(nameof(AccessMaskFlagsHumanized));
+				var inheritanceStrings = new List<string>();
+
+				if (AccessControlEntryFlags == AccessControlEntryFlags.None ||
+					AccessControlEntryFlags == AccessControlEntryFlags.NoPropagateInherit)
+					inheritanceStrings.Add("SecurityAdvancedFlagsFolderLabel".GetLocalizedResource());
+
+				if (AccessControlEntryFlags.HasFlag(AccessControlEntryFlags.ContainerInherit))
+					inheritanceStrings.Add("SecurityAdvancedFlagsSubfoldersLabel".GetLocalizedResource());
+
+				if (AccessControlEntryFlags.HasFlag(AccessControlEntryFlags.ObjectInherit))
+					inheritanceStrings.Add("SecurityAdvancedFlagsFilesLabel".GetLocalizedResource());
+
+				// Capitalize the first letter
+				if (inheritanceStrings.Any())
+					inheritanceStrings[0] = char.ToUpperInvariant(inheritanceStrings[0].First()) + inheritanceStrings[0][1..];
+
+				return string.Join(", ", inheritanceStrings);
 			}
 		}
 
-		private InheritanceFlags _InheritanceFlags;
-		public InheritanceFlags InheritanceFlags
+		/// <summary>
+		/// AccessMaskItems
+		/// </summary>
+		public ObservableCollection<AccessMaskItem> AccessMaskItems { get; set; }
+
+		private AccessControlEntryType _AccessControlType;
+		public AccessControlEntryType AccessControlType
 		{
-			get => _InheritanceFlags;
+			get => _AccessControlType;
 			set
 			{
-				if (SetProperty(ref _InheritanceFlags, value))
-					OnPropertyChanged(nameof(InheritanceFlagsHumanized));
+				if (SetProperty(ref _AccessControlType, value))
+				{
+					OnPropertyChanged(nameof(AccessControlTypeGlyph));
+					OnPropertyChanged(nameof(AccessControlTypeHumanized));
+				}
 			}
 		}
 
-		private PropagationFlags _PropagationFlags;
-		public PropagationFlags PropagationFlags
+		#region Access Mask Properties
+		private AccessMaskFlags _AccessMaskFlags;
+		public AccessMaskFlags AccessMaskFlags
 		{
-			get => _PropagationFlags;
+			get => _AccessMaskFlags;
 			set
 			{
-				if (SetProperty(ref _PropagationFlags, value))
-					OnPropertyChanged(nameof(InheritanceFlagsHumanized));
+				if (SetProperty(ref _AccessMaskFlags, value))
+					OnPropertyChanged(nameof(AccessMaskFlagsHumanized));
 			}
 		}
 
@@ -102,9 +190,21 @@ public AccessMaskFlags DeniedAccessMaskFlags
 			}
 		}
 
+		private AccessControlEntryFlags _InheritanceFlags;
+		public AccessControlEntryFlags AccessControlEntryFlags
+		{
+			get => _InheritanceFlags;
+			set
+			{
+				if (SetProperty(ref _InheritanceFlags, value))
+					OnPropertyChanged(nameof(InheritanceFlagsHumanized));
+			}
+		}
+
 		public AccessMaskFlags InheritedAllowAccessMaskFlags { get; set; }
 
 		public AccessMaskFlags InheritedDenyAccessMaskFlags { get; set; }
+		#endregion
 
 		private bool _IsSelected;
 		public bool IsSelected
@@ -114,10 +214,9 @@ public bool IsSelected
 			{
 				if (SetProperty(ref _IsSelected, value))
 				{
-					if (!value)
-						AreAdvancedPermissionsShown = false;
+					AreAdvancedPermissionsShown = false;
 
-					OnPropertyChanged(nameof(IsEditEnabled));
+					OnPropertyChanged(nameof(IsEditable));
 				}
 			}
 		}
@@ -130,86 +229,10 @@ public bool AreAdvancedPermissionsShown
 			{
 				// Reinitialize list
 				if (SetProperty(ref _AreAdvancedPermissionsShown, value))
-					AccessMaskItemList = SecurityAdvancedAccessControlItemFactory.Initialize(this, AreAdvancedPermissionsShown, IsInherited, IsFolder);
+					AccessMaskItems = SecurityAdvancedAccessControlItemFactory.Initialize(this, value, IsInherited, IsFolder);
 			}
 		}
 
-		public bool IsInherited { get; set; }
-
-		public bool IsEditEnabled
-			=> IsSelected && !IsInherited;
-
-		public string AccessControlTypeHumanized
-			=> AccessControlType switch
-			{
-				AccessControlType.Allow => "Allow",
-				_ => "Deny" // AccessControlType.Deny
-			};
-
-		public string AccessControlTypeGlyph
-			=> AccessControlType switch
-			{
-				AccessControlType.Allow => "\xE73E",
-				_ => "\xF140" // AccessControlType.Deny
-			};
-
-		public string AccessMaskFlagsHumanized
-		{
-			get
-			{
-				var accessMaskStrings = new List<string>();
-
-				if (AccessMaskFlags == AccessMaskFlags.NULL)
-					accessMaskStrings.Add("None".GetLocalizedResource());
-
-				if (FullControlAccess)
-					accessMaskStrings.Add("SecurityFullControlLabel/Text".GetLocalizedResource());
-				else if (ModifyAccess)
-					accessMaskStrings.Add("SecurityModifyLabel/Text".GetLocalizedResource());
-				else if (ReadAndExecuteAccess)
-					accessMaskStrings.Add("SecurityReadAndExecuteLabel/Text".GetLocalizedResource());
-				else if (ReadAccess)
-					accessMaskStrings.Add("SecurityReadLabel/Text".GetLocalizedResource());
-
-				if (!FullControlAccess && !ModifyAccess && WriteAccess)
-					accessMaskStrings.Add("Write".GetLocalizedResource());
-
-				if (SpecialAccess)
-					accessMaskStrings.Add("SecuritySpecialLabel/Text".GetLocalizedResource());
-
-				return string.Join(",", accessMaskStrings);
-			}
-		}
-
-		public string IsInheritedHumanized
-			=> IsInherited ? "Yes".GetLocalizedResource() : "No".GetLocalizedResource();
-
-		public string InheritanceFlagsHumanized
-		{
-			get
-			{
-				var inheritanceStrings = new List<string>();
-
-				if (PropagationFlags == PropagationFlags.None ||
-					PropagationFlags == PropagationFlags.NoPropagateInherit)
-					inheritanceStrings.Add("SecurityAdvancedFlagsFolderLabel".GetLocalizedResource());
-
-				if (InheritanceFlags.HasFlag(InheritanceFlags.ContainerInherit))
-					inheritanceStrings.Add("SecurityAdvancedFlagsSubfoldersLabel".GetLocalizedResource());
-
-				if (InheritanceFlags.HasFlag(InheritanceFlags.ObjectInherit))
-					inheritanceStrings.Add("SecurityAdvancedFlagsFilesLabel".GetLocalizedResource());
-
-				// Capitalize first letter
-				if (inheritanceStrings.Any())
-					inheritanceStrings[0] = char.ToUpperInvariant(inheritanceStrings[0].First()) + inheritanceStrings[0][1..];
-
-				return string.Join(",", inheritanceStrings);
-			}
-		}
-
-		public ObservableCollection<AccessMaskItem> AccessMaskItemList { get; set; }
-
 		#region Security page
 		public bool WriteAccess => AccessMaskFlags.HasFlag(AccessMaskFlags.Write);
 		public bool ReadAccess => AccessMaskFlags.HasFlag(AccessMaskFlags.Read);
@@ -318,40 +341,37 @@ public bool DeniedFullControlAccess
 		public IRelayCommand<string> ChangeAccessControlTypeCommand { get; set; }
 		public IRelayCommand<string> ChangeInheritanceFlagsCommand { get; set; }
 
-		public AccessControlEntry(bool isFolder, string ownerSid, AccessControlType type, AccessMaskFlags accessMaskFlags, bool isInherited, InheritanceFlags inheritanceFlags, PropagationFlags propagationFlags)
+		public AccessControlEntry(bool isFolder, string ownerSid, AccessControlEntryType type, AccessMaskFlags accessMaskFlags, bool isInherited, AccessControlEntryFlags inheritanceFlags)
 		{
-			AccessMaskItemList = SecurityAdvancedAccessControlItemFactory.Initialize(this, AreAdvancedPermissionsShown, IsInherited, IsFolder);
-
-			ChangeAccessControlTypeCommand = new RelayCommand<string>(x =>
-			{
-				AccessControlType = Enum.Parse<AccessControlType>(x);
-			});
+			AccessMaskItems = SecurityAdvancedAccessControlItemFactory.Initialize(this, AreAdvancedPermissionsShown, IsInherited, IsFolder);
 
-			ChangeInheritanceFlagsCommand = new RelayCommand<string>(x =>
-			{
-				var parts = x.Split(',');
+			//ChangeAccessControlTypeCommand = new RelayCommand<string>(x =>
+			//{
+			//	AccessControlType = Enum.Parse<AccessControlType>(x);
+			//});
 
-				InheritanceFlags = Enum.Parse<InheritanceFlags>(parts[0]);
-				PropagationFlags = Enum.Parse<PropagationFlags>(parts[1]);
-			});
+			//ChangeInheritanceFlagsCommand = new RelayCommand<string>(x =>
+			//{
+			//	var parts = x.Split(',');
+			//	InheritanceFlags = Enum.Parse<AccessControlEntryFlags>(parts[0]);
+			//});
 
 			IsFolder = isFolder;
-			Principal = Principal.FromSid(ownerSid);
+			Principal = new(ownerSid);
 			AccessControlType = type;
 			AccessMaskFlags = accessMaskFlags;
 			IsInherited = isInherited;
-			InheritanceFlags = inheritanceFlags;
-			PropagationFlags = propagationFlags;
+			AccessControlEntryFlags = inheritanceFlags;
 
 			switch (AccessControlType)
 			{
-				case AccessControlType.Allow:
+				case AccessControlEntryType.Allow:
 					if (IsInherited)
 						InheritedAllowAccessMaskFlags |= AccessMaskFlags;
 					else
 						AllowedAccessMaskFlags |= AccessMaskFlags;
 					break;
-				case AccessControlType.Deny:
+				case AccessControlEntryType.Deny:
 					if (IsInherited)
 						InheritedDenyAccessMaskFlags |= AccessMaskFlags;
 					else