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
> 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
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