Subject: Re: [PATCH V4 1/1] Replace SID with domain/username

From: Junio C Hamano <[email-masked]>
Date: Wed, 03 Jan 2024 14:22:58 -0800

Matthias Aßhauer <[email-masked]> writes:

> This patch only changes the output of our error message, though.
> It does not change what ownership information we actually compare.
> So if we had a hypothetical user Bob that was part of the  domain
> example.com (SID S-1-5-21-100000001-1000000001-10000001-1001) and
> had been moved over from the example.org domain (old SID S-1-5-21-
> 2000000002-2000000002-20000002-2002) and we would detect a repository
> owned by bobs old SID, we would now lookup the old SID, find it
> attached to a user named example.com\Bob, look up Bobs  current SID,
> find it belongs to a user named example.com\Bob and print a confusing
> error message.

Yup, that is exactly the kind of breakage I was worried about.

Perhaps we should do something along the lines of ...

 - The erroring out should be done purely by SID comparison, as that
   is what we have been doing to protect the users.

 - When creating a message, use LookupAccountSidA() to come up with
   a pair of domain\user strings for the directory and the process
   to be used in the error message:

   - If they are different (which is expected to be the normal
     case), we just use the pair of strings.

   - If they are the same, show old and new SID in stringified form
     (hopefully different SIDs would strigify to different
     strings?), and optionally we give the domain\user string next
     to it.

... then?  Then we would emit an error message (in the best case)

    'directory' is owned by:
    [email-masked]'
    but the current user is:
    [email-masked]'

and in a bad case we would instead see something like:

    'directory' is owned by:
    SID S-1-5-21-100000001-1000000001-10000001-1001 ([email-masked]')
    but the current user is:
    SID S-1-5-21-200000002-2000000002-20000002-2002 ([email-masked]')

which may still be serviceable.  I dunno.