Looks like we do not have self-tests for smbcacls

[Noel Power]

Hi Uri
On 03/08/17 12:53, Uri Simchoni wrote:
> I have only made a shallow review of the code (will really try to make a
> proper review but I have a lot on my plate now), and have some general
> comments / suggestions:
thanks for looking!
>
> 1. The switch name. In principle, the --propagate-inheritance feature
> propagates changes made to an ACL, so it doesn't propagate just
> inheritance information, it propagates ACEs. Maybe just --propagate?
well it propagates ACES(s) that's true but only in the context of the
rules of inheritance, --propagate on it's own to me suggests something
like a recursive operation. I would have been happy with just
--inheritance (or something like that) but of course everyone has an
opinion :-) and I was convinced by someone else at some stage that
'--propagate-inheritance' If there is some sort of consensus I'll
happily change it to something else, meanwhile I'd be happy to leave it
as is
>
> 2. Usually we "do what Windows does(tm)". However in this case it seems
> like "Windows" is not the Windows OS but rather Windows Explorer. I
> think the documentation and / or commit message should make it clear
> what the reference implementation (or other reference) is, in case there
> are bug reports down the road.
ok, I can do that. But.. note that smbcacls isn't trying to behave like
Windows Explorer but rather obey what is described here
https://msdn.microsoft.com/en-us/library/windows/desktop/aa374924(v=vs.85).aspx
However if you are looking for something to compare against icacls.exe
is probably the closest thing to smbcacls that is around (and what I
mostly referenced where possible) icacls has 'grant', 'grant:r' & 'deny
that loosely perform similar operations to smbcacls --add, --set &
--delete respectively. Note: icacls.exe automatically propagates
inheritable aces according to the rules above
>
> 3. Assuming this *is* a Windows Explorer look-alike, Windows
> Explorer pops up a message if it fails to set the ACL of a file,
> allowing the user to continue or abort. IMHO that would be useful here,
> because changing a large tree without the option to continue would be
> difficult. The program can output messages on files which failed.
see above, smbcacls like icacls is a power tool, you can shoot yourself
in the foot royally, I can't recall whether an icacls failure is
reported and it continues on or not. My gut feeling is we should do as
icacls does (where possible)
I'll look into what it does
>
> 4. In the docs, there's a modification saying the flags serve smbcacls -
> well they serve the SMB server too when creating new files and folders
> (I think they are an artifact of the NTFS file system, and Samba
> emulates that behavior).
Not sure what modification you mean, can you elaborate

Thanks again
Noel
>
> Thanks,
> Uri.
>
> On 08/03/2017 02:41 PM, Noel Power via samba-technical wrote:
>> And another version.....
>>
>> On 31/07/17 20:34, Andrew Bartlett wrote:
>>> On Mon, 2017-07-31 at 13:51 +0100, Noel Power via samba-technical
>>> wrote:
>>>> Please find a new version of the patchset (with tests re-written in python)
>>>> Noel
>>> Thanks for writing these tests.  It would have been better however if
>>> the tests used the standard test classes like BlackboxTestCase and the
>>> assertions that go with it.  
>>>
>>> I know it will be a pile of pain to re-work the test (again!), but the
>>> consistency really helps as others will copy and paste from the first
>>> example they find.
>>>
>>> See for example python/samba/tests/blackbox/ndrdump.py
>>>
>>> (We used to have more of those, but SambaToolCmdTest took over for
>>> Samba tests).
>>>
>>> Sorry,
>>>
>>> Andrew Bartlett
>>
>