Code Quality: Refactored code for security settings

[0x5bfa]

Description

I'm working on refactoring security codes.

Motivation and Context

The storage object security codes were noticeably unmaintable; it was way too difficult to understand what the class represents from the class name.

• Related Feature: Improving Performance, optimizations, reliability and codebase  #4180

Tasks

• Force to be one class per file.
• Rename all security related classes with more Win32API-like naming convention.
• Rename variables and methods too.
• Add xml comments to all classes to describe that what the class represents.
• Integrate classes that don't need to be separated.

Changes

• Rename Files.App.Filesystem.Permissions with Files.App.Filesystem.Security.
• Rename the classes in Files.App.Filesystem.Security to improve ease of understanding what the class represents.
• Refactor the classes in Files.App.Filesystem.Security.
• Add xml comments in all classes in Files.App.Filesystem.Security.
• Refactor Files.App.ViewModels.Properties.SecurityViewModel.
• Refactor code-behinds of Files.App.Views.Properties.SecurityPage and Files.App.Views.Properties.SecurityAdvancedPage.

My changes validation

Preferable terms

Name	Description	Rationale	Instead of
AccessControlList	Represents a storage object's access control list	ACL Used in Win32API	None
AccessControlEntry	Represents an entry of access control list	ACE ACCESS_ALLOWED_ACE ACCESS_DENIED_ACE Used in Win32API	None
AccessMask	Represents access control flags(enum) of an ACE	ACCESS_MASK Used in Win32API	None
Principal	Represents an owner of ACL or ACE	Used in File Explorer UI of security advanced page	None
Allow[ed]	Literally	Used in Win32API	Grant[ed|s]
Deny[ed]	Literally	Used in Win32API	None

Unpreferable terms

Name	Description	Rationale	Instead,
Grant[ed|s]	Literally	This term is not used in Win32API	Allow[ed]
Permission[s]	Literally	This term is not used in Win32API	Security or something else

Classes

Name	Previous name	Rationale
App.Filesystem.Permissions	App.Filesystem.Security	that was unpreferable term. Security is used in File Explorer UI
AccessControlEntry	FileSystemAccessRule	Used as ACE of Win32API for SecurityPage UI
Removed	FileSystemAccessRuleForUI	Used as ACE of Win32API for SecurityAdvancedPage UI
Removed	FileSystemAccessRule	No references
AccessControlEntryPrimitiveMapping	FileSystemAccessRule2	Apparently
AccessControlList	FilePermissions	Used as ACL of Win32API
Removed	FilePermissionsManager	This class was the same as AccessControlList
AccessControlType	AccessControlType	None
AccessMaskFlags	FileSystemRights	FileSystemRights is not familiar for Win32API devs. Equivalent to ACCESS_MASK of Win32API.
AccessMaskItem	GrantedPermission	Represents details of one flag of AccessMaskFlags.
InheritanceFlags	InheritanceFlags	None
Principal	UserGroup	This class can be either User or Group(UserGroup isn't accurate). Principal is used in File Explorer UI
PrincipalType	SecurityType	Apparently
PropagationFlags	PropagationFlags	None

Validation

• Built and ran the app
• Tested the changes for accessibility

Test

• Display ACL and all ACEs
• Add an ACE
• Remove an ACE

Screenshots

[ferrariofilippo]

I left some comments regarding code style. If I have any time, I'll try to make a second code-review and test this out

[0x5bfa]

region-endregion will be removed when I finish. so please ignore them:)

[ferrariofilippo]

region-endregion will be removed when I finish. so please ignore them:)

I think half the comments are about regions, I'll close them

[0x5bfa]

So sorry about that 😢

[ferrariofilippo]

So sorry about that 😢

Don't worry

[0x5bfa]

/azp run

[azure-pipelines]

Commenter does not have sufficient privileges for PR 11471 in repo files-community/Files

[yaira2]

I found some issues

• Even though you change ACE's type, Files won't change (there's no functionality)
• Even though you change access mask flags, Files won't change (there's no functionality)
• Even though you change inheritance flags, Files won't change (there's no functionality)

Those issues were not created by my refactoring; it means originally existed. So I think I should not fix(I will probably open a PR for those).

Can you open separate issues to track these?

[0x5bfa]

Each of the three?

[yaira2]

Yes

[QuaintMako]

I won't give my approval for the PR because I am not versed enough in the security to be sure at 100%, but alongside the two other reviews, I believe it's good!

Thanks for the huge refactoring and cleaning @onein528 ! (And for the others you've done as well)