On Tue, Jan 8, 2013 at 5:20 PM, Karsten Blees <karsten.bl...@gmail.com> wrote:
> Am 04.01.2013 22:57, schrieb Erik Faye-Lund:
>> The only reason why I used Cred[Un]PackAuthenticationBuffer, were that
>> I wasn't aware that it was possible any other way. I didn't even know
>> there was a Windows Credential Manager in Windows XP.
> I believe the Cred* API was introduced in Win2k. The XP control panel applet 
> supports domain credentials only, but cmdkey.exe from Windows Server 2003 can 
> be used on XP to manage generic credentials.

Thanks for the background-info.

>> The credential attributes were because they were convenient, and I'm
>> not sure I understand what you mean about the Win7 credential manager
>> tools. I did test my code with it - in fact, it was a very useful tool
>> for debugging the helper.
>> Are you referring to the credentials not *looking* like normal
>> HTTP-credentials?
> No, I was referring to creating / editing git credentials with Windows tools 
> manually. For example, changing your password in control panel removes all 
> credential attributes, so the current wincred won't find them any 
> longer...same for git credentials created e.g. via 'cmdkey 
> /generic:git:http://m...@example.com /user:me /pass:secret'.
> The 'puzzling' part is that those credentials *look* exactly the same as if 
> created by wincred, but they don't work. And wincred isn't exactly verbose in 
> its error messages :-)

Right, thanks for clearing that up.

>> But, if we do any of these changes, does this mean I will lose my
>> existing credentials? It's probably not a big deal, but it's worth a
>> mention, isn't it?
> Yes, existing stored credentials are lost after this patch. Will add a note 
> to the commit message.
> We _could_ try to detect the old format, but I don't think it's worth the 
> trouble.

Nah, I don't think it's worth the trouble. It's a bit unfortunate that
people might get stale credentials clogging up the system, but I don't
really thing this is a big deal.

>>>  static int match_cred(const CREDENTIALW *cred)
>>>  {
>>> -       return (!wusername || !wcscmp(wusername, cred->UserName)) &&
>>> -           match_attr(cred, L"git_protocol", protocol) &&
>>> -           match_attr(cred, L"git_host", host) &&
>>> -           match_attr(cred, L"git_path", path);
>>> +       LPCWSTR target = cred->TargetName;
>>> +       if (wusername && wcscmp(wusername, cred->UserName))
>>> +               return 0;
>>> +
>>> +       return match_part(&target, L"git", L":") &&
>>> +               match_part(&target, protocol, L"://") &&
>>> +               match_part(&target, wusername, L"@") &&
>>> +               match_part(&target, host, L"/") &&
>>> +               match_part(&target, path, L"");
>>>  }
>> Ugh, it feels a bit wrong to store and verify the username twice. Do
>> we really have to?
>> The target needs to be unique, even if two different usernames are
>> stored for the same site under the same conditions. So probably. It
>> just doesn't feel quite right.
> I don't really see why you would need several usernames to connect to the 
> same target. I can imagine different credentials for reading / writing, but 
> then the protocol would usually be different as well, wouldn't it? (e.g. http 
> vs. ssh)

I can kind of make up some theoretical reasons, but they are a bit exotic ;)

> One of my interim solutions was to remove the username part from the URL 
> entirely. That enabled me to change credentials in control panel (including 
> the username), and wincred would use them. However, that version failed the 
> 'helper can store multiple users' test, so I ended up with storing / checking 
> username twice.

I don't think breaking this is a good idea. It just feels a bit silly,
but I see now that other applications does the same duplication. So
let's just stick to it, even if it's a bit icky ;)
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to