Code Quality: Introduced IStorageSecurityService

[0x5bfa]

Summary

• Removal of Vanara reference (4 out of 37 files):
  • SecurityViewModel
  • SecurityAdvancedViewModel
  • FileSecurityHelpers (to IStorageSecurityService)
  • AccessControlPrincipal
• Removal of Utils namespace (9 out of 132 files):
  • Utils/Storage/Security/*
  • Utils/Storage/Helpers/FileSecurityHelpers

Resolved / Related Issues

• Code Quality: Reduce duplicated functionality classes #14572
• Code Quality: Replace Vanara with CsWin32 #15000

Steps used to test these changes

1. Open Files
2. Go to security and advanced security pages
3. Confirm no crashes and problems occur
4. Add a new entry
5. Delete the entry

[Josh65-2201]

Compared to 3.5.4, I found

• Doesn't show principles/users name in both normal and advanced pages when opening properties.
• Doesn't save principles/users that are added
• Crashes when changing the owner (Before it shows a UAC for PowerShell admin)

[0x5bfa]

@Josh65-2201 I fixed everything. Would you mind check on your end as well?

[Josh65-2201]

I found that it won't save my other local user when added but does add system principles (Administrator and system) but makes them separate entries for the folder I'm applying to and one for subfolders/files. File explorer will include them as one entry.

[0x5bfa]

You can't add an entry via their name such as '0x5bfa' or 'Josh'? It's weird though I tested.

I'm not sure I get what you meant but merging isn't available for now because it requires additional implementation (this lower layer's Windows API doesn't provide such rich features...)
For the future reference, this is called 'effective access' and ACLUI.DLL which File Explorer shows has this feature with their own implementation checking if the new one can be merged and its target is not inherited one before adding actual one into the DACL.

If you can add/remove an entry and change owner without crash and weird behavior (and try them when you have insufficient permission such as 'Program Files/WindowsApps'), it's enough for this PR.

[Josh65-2201]

I'm using the security principles find now menu to try add a local account but it doesn't save it after clicking save and reopening the properties windows. This works on 3.5.6

As for the duplicate entries I found preview only add permission targeting the folder I have selected where now it's adding an entry for sub folders/files too. File Explorer set them all in the same permission but I'd say it fine for now as it better then 3.5.6.

Add/removing entries and changing ownership work with no crash or other things.

[0x5bfa]

1. I'll check again
2. Yeah I tried to make it look alike.

Before:

Files/src/Files.App/Utils/Storage/Helpers/FileSecurityHelpers.cs

Line 194 in aa05b1a

grfInheritance = INHERIT_FLAGS.NO_INHERITANCE,

After:

Files/src/Files.App/Services/Storage/StorageSecurityService.cs

Line 224 in b159fe4

isFolder ? ACE_FLAGS.CONTAINER_INHERIT_ACE | ACE_FLAGS.OBJECT_INHERIT_ACE : ACE_FLAGS.NO_INHERITANCE,

Thank you for the testing, it've helped me a lot!

[yaira2]

@Josh65-2201 is your testing complete?

[0x5bfa]

They found an issue with adding a new entry via user logon names. I’ll check later but everything else should be fine now.

[0x5bfa]

All done.

[0x5bfa]

@Josh65-2201 is it fine for you to review again?

[Josh65-2201]

Working as expected

[0x5bfa]

Thank you guys!

As a side note for the future reference, I maybe should've used 'Get(Set)SecurityInfo' (File Explorer's dialog ACLUI.dll uses it iirc) but and 'Get(Set)NamedSeucurityInfo' doesn't require a handle of the specified file, which means this function would fail when access denied (not having READ_CONTROL), while CreateFileW to get a handle would fail before GetSecurityInfo.

[hishitetsu]

The Access for the entry I added is "Read and execute" at first, but when I open Advanced permissions again, it is "Special". I think "Special" is correct from the beginning.

[0x5bfa]

Is it something different from the stable build?

[Josh65-2201]

It always showing as read and execute for me in Preview and this branch

[0x5bfa]

The Access for the entry I added is "Read and execute" at first, but when I open Advanced permissions again, it is "Special". I think "Special" is correct from the beginning.

Please open that entry in File Explorer's dialog and expand permissions list for advanced ones to be shown and send a screenshot of it.

[hishitetsu]

I added Administrator. (Sorry for Japanese)

The second time I open Advanced permissions, it says Special.

[0x5bfa]

I mean this window. Make sure to click ‘Show advanced permissions’.

[hishitetsu]

Here it is.

[0x5bfa]

😩 so this is basically I didn’t include SYNCHRONIZE access mask (?)
It very weird that Josh’s PC shows it Read & execute even after opening again.

[0x5bfa]

@hishitetsu when you have time could you try to change this magic code

Files/src/Files.App/Services/Storage/StorageSecurityService.cs

Line 225 in ad03647

0x2000A9 /* READ & EXECUTE */,

to

0x20000000 | 0x80000000 /* GENERIC_EXECUTE and GENERIC_READ */

These generic access masks covers SYNCHRONIZE access mask.

---

(for me)

• The permission editor window is included in rshx32.dll as a shell extension.
• The security property sheet is included in aclui.dll
• The advanced security window is included in aclui.dll

[hishitetsu]

@hishitetsu when you have time could you try to change this magic code

It works for me!

[hishitetsu]

LGTM