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
