{"thread":{"id":"59471","subject":"[PATCH] credential/wincred: store password_expiry_utc","startedAt":"2023-03-25T07:39:27Z","lastAt":"2023-05-02T18:17:08Z","messageCount":11,"participants":["M Hickford via GitGitGadget","Johannes Schindelin","M Hickford","Junio C Hamano","Johannes Sixt","Felipe Contreras"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"474153","messageId":"pull.1477.git.git.1679729956620.gitgitgadget@gmail.com","threadId":"59471","inReplyTo":null,"subject":"[PATCH] credential/wincred: store password_expiry_utc","fromName":"M Hickford via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-03-25T07:39:16Z","receivedAt":"2023-03-25T07:39:27Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"From: M Hickford <mirth.hickford@gmail.com>\n\nSigned-off-by: M Hickford <mirth.hickford@gmail.com>\n---\n    credential/wincred: store password_expiry_utc\n    \n    Help wanted from a Windows user to test. I tried testing on Linux with\n    Wine after cross-compiling [1] but Wine has incomplete support for\n    wincred.h [2]. To test:\n    \n    cd contrib/credential/wincred/\n    make\n    echo host=example.com\\nprotocol=https\\nusername=tim\\npassword=xyzzy\\npassword_expiry_utc=2000 | ./git-credential-wincred.exe store\n    echo host=example.com\\nprotocol=https | ./git-credential-wincred.exe get\n    \n    \n    Output of second command should include line password_expiry_utc=2000\n    \n    [1] make CC=\"zig cc -target x86_64-windows-gnu\" [2]\n    https://github.com/wine-mirror/wine/blob/bf9d15e3b1a29f73fedda0c34547a9b29d5e2789/dlls/advapi32/cred.c#L181-L186\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1477%2Fhickford%2Fwincred-expiry-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1477/hickford/wincred-expiry-v1\nPull-Request: https://github.com/git/git/pull/1477\n\n .../wincred/git-credential-wincred.c          | 26 +++++++++++++++++--\n 1 file changed, 24 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c\nindex ead6e267c78..9be892610d0 100644\n--- a/contrib/credential/wincred/git-credential-wincred.c\n+++ b/contrib/credential/wincred/git-credential-wincred.c\n@@ -91,7 +91,8 @@ static void load_cred_funcs(void)\n \t\tdie(\"failed to load functions\");\n }\n \n-static WCHAR *wusername, *password, *protocol, *host, *path, target[1024];\n+static WCHAR *wusername, *password, *protocol, *host, *path, target[1024],\n+\t*password_expiry_utc;\n \n static void write_item(const char *what, LPCWSTR wbuf, int wlen)\n {\n@@ -183,6 +184,7 @@ static void get_credential(void)\n \tCREDENTIALW **creds;\n \tDWORD num_creds;\n \tint i;\n+\tCREDENTIAL_ATTRIBUTEW *attr;\n \n \tif (!CredEnumerateW(L\"git:*\", 0, &num_creds, &creds))\n \t\treturn;\n@@ -195,6 +197,15 @@ static void get_credential(void)\n \t\t\twrite_item(\"password\",\n \t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n \t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n+\t\t\tattr = creds[i]->Attributes;\n+\t\t\tfor (int j = 0; j < creds[i]->AttributeCount; j++) {\n+\t\t\t\tif (wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n+\t\t\t\t\twrite_item(\"password_expiry_utc\", (LPCWSTR)attr->Value,\n+\t\t\t\t\tattr->ValueSize / sizeof(WCHAR));\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t\tattr++;\n+\t\t\t}\n \t\t\tbreak;\n \t\t}\n \n@@ -204,6 +215,7 @@ static void get_credential(void)\n static void store_credential(void)\n {\n \tCREDENTIALW cred;\n+\tCREDENTIAL_ATTRIBUTEW expiry_attr;\n \n \tif (!wusername || !password)\n \t\treturn;\n@@ -217,6 +229,14 @@ static void store_credential(void)\n \tcred.Persist = CRED_PERSIST_LOCAL_MACHINE;\n \tcred.AttributeCount = 0;\n \tcred.Attributes = NULL;\n+\tif (password_expiry_utc != NULL) {\n+\t\texpiry_attr.Keyword = L\"git_password_expiry_utc\";\n+\t\texpiry_attr.Value = (LPVOID)password_expiry_utc;\n+\t\texpiry_attr.ValueSize = (wcslen(password_expiry_utc)) * sizeof(WCHAR);\n+\t\texpiry_attr.Flags = 0;\n+\t\tcred.Attributes = &expiry_attr;\n+\t\tcred.AttributeCount = 1;\n+\t}\n \tcred.TargetAlias = NULL;\n \tcred.UserName = wusername;\n \n@@ -278,6 +298,8 @@ static void read_credential(void)\n \t\t\twusername = utf8_to_utf16_dup(v);\n \t\t} else if (!strcmp(buf, \"password\"))\n \t\t\tpassword = utf8_to_utf16_dup(v);\n+\t\telse if (!strcmp(buf, \"password_expiry_utc\"))\n+\t\t\tpassword_expiry_utc = utf8_to_utf16_dup(v);\n \t\t/*\n \t\t * Ignore other lines; we don't know what they mean, but\n \t\t * this future-proofs us when later versions of git do\n@@ -292,7 +314,7 @@ int main(int argc, char *argv[])\n \t    \"usage: git credential-wincred <get|store|erase>\\n\";\n \n \tif (!argv[1])\n-\t\tdie(usage);\n+\t\tdie(\"%s\", usage);\n \n \t/* git use binary pipes to avoid CRLF-issues */\n \t_setmode(_fileno(stdin), _O_BINARY);\n\nbase-commit: 27d43aaaf50ef0ae014b88bba294f93658016a2e\n-- \ngitgitgadget\n"},{"id":"474274","messageId":"35e1ebe6-e15b-1712-f030-70ab708740db@gmx.de","threadId":"59471","inReplyTo":"pull.1477.git.git.1679729956620.gitgitgadget@gmail.com","subject":"Re: [PATCH] credential/wincred: store password_expiry_utc","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2023-03-28T12:14:18Z","receivedAt":"2023-03-28T12:14:41Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi M,\n\nOn Sat, 25 Mar 2023, M Hickford via GitGitGadget wrote:\n\n> From: M Hickford <mirth.hickford@gmail.com>\n>\n> Signed-off-by: M Hickford <mirth.hickford@gmail.com>\n> ---\n>     credential/wincred: store password_expiry_utc\n>\n>     Help wanted from a Windows user to test. I tried testing on Linux with\n>     Wine after cross-compiling [1] but Wine has incomplete support for\n>     wincred.h [2]. To test:\n>\n>     cd contrib/credential/wincred/\n>     make\n>     echo host=example.com\\nprotocol=https\\nusername=tim\\npassword=xyzzy\\npassword_expiry_utc=2000 | ./git-credential-wincred.exe store\n>     echo host=example.com\\nprotocol=https | ./git-credential-wincred.exe get\n>\n>\n>     Output of second command should include line password_expiry_utc=2000\n\nSadly, no, it's empty:\n\n\t$ cd contrib/credential/wincred/\n\t$ make\n\tcc     git-credential-wincred.c   -o git-credential-wincred.exe\n\t$ echo host=example.com\\nprotocol=https\\nusername=tim\\npassword=xyzzy\\npassword_expiry_utc=2000 | ./git-credential-wincred.exe store\n\t$ echo host=example.com\\nprotocol=https | ./git-credential-wincred.exe get\n\nThe reason is that `echo` does not interpret the escaped `n`, it does not\neven get the backslash because Bash eats it first.\n\nBut even with `printf` it does not work:\n\n\t$ printf 'host=example.com\\nprotocol=https\\nusername=tim\\npassword=xyzzy\\npassword_expiry_utc=2000\\n' | ./git-credential-wincred.exe store\n\t$ printf 'host=example.com\\nprotocol=https\\n' | ./git-credential-wincred.exe get                                        username=tim\n\tpassword=xyzzy\n\nAnd the reason is...\n\n> @@ -195,6 +197,15 @@ static void get_credential(void)\n>  \t\t\twrite_item(\"password\",\n>  \t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n>  \t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n> +\t\t\tattr = creds[i]->Attributes;\n> +\t\t\tfor (int j = 0; j < creds[i]->AttributeCount; j++) {\n> +\t\t\t\tif (wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n\n\t\t\t\t  ^^^^^^\n\n... here. Note how the return value of `wcscmp()` needs to be non-zero to\nenter the conditional block? But `wcscmp()` returns 0 if there are no\ndifferences between the two provided strings.\n\nThat's not the only bug, though. While the loop iterates over all of the\nattributes, the `attr` variable is unchanged, and always points to the\nfirst attribute. You have to access it as `creds[i]->Attributes[j]`,\nthough, see e.g.\nhttps://github.com/sandboxie-plus/Sandboxie/blob/f2a357f9222b81e7bdc994e5d9824790a1b5d826/Sandboxie/core/dll/cred.c#L324\n\nSo with this patch on top of your patch, it works for me:\n\n-- snip --\ndiff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c\nindex 9be892610d0..1aa840e31a0 100644\n--- a/contrib/credential/wincred/git-credential-wincred.c\n+++ b/contrib/credential/wincred/git-credential-wincred.c\n@@ -197,9 +197,9 @@ static void get_credential(void)\n \t\t\twrite_item(\"password\",\n \t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n \t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n-\t\t\tattr = creds[i]->Attributes;\n \t\t\tfor (int j = 0; j < creds[i]->AttributeCount; j++) {\n-\t\t\t\tif (wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n+\t\t\t\tattr = creds[i]->Attributes + j;\n+\t\t\t\tif (!wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n \t\t\t\t\twrite_item(\"password_expiry_utc\", (LPCWSTR)attr->Value,\n \t\t\t\t\tattr->ValueSize / sizeof(WCHAR));\n \t\t\t\t\tbreak;\n-- snap --\n\nBut I have to wonder: why even bother with `git-wincred`? This credential\nhelper is so ridiculously limited in its capabilities, it does not even\nsupport any host that is remotely close to safe (no 2FA, no OAuth, just\npasswords). So I would be just as happy if I weren't asked to spend my\ntime to review changes to a credential helper I'd much rather see retired\nthan actively worked on.\n\nCiao,\nJohannes\n\n> +\t\t\t\t\twrite_item(\"password_expiry_utc\", (LPCWSTR)attr->Value,\n> +\t\t\t\t\tattr->ValueSize / sizeof(WCHAR));\n> +\t\t\t\t\tbreak;\n> +\t\t\t\t}\n> +\t\t\t\tattr++;\n> +\t\t\t}\n>  \t\t\tbreak;\n>  \t\t}\n>\n> @@ -204,6 +215,7 @@ static void get_credential(void)\n>  static void store_credential(void)\n>  {\n>  \tCREDENTIALW cred;\n> +\tCREDENTIAL_ATTRIBUTEW expiry_attr;\n>\n>  \tif (!wusername || !password)\n>  \t\treturn;\n> @@ -217,6 +229,14 @@ static void store_credential(void)\n>  \tcred.Persist = CRED_PERSIST_LOCAL_MACHINE;\n>  \tcred.AttributeCount = 0;\n>  \tcred.Attributes = NULL;\n> +\tif (password_expiry_utc != NULL) {\n> +\t\texpiry_attr.Keyword = L\"git_password_expiry_utc\";\n> +\t\texpiry_attr.Value = (LPVOID)password_expiry_utc;\n> +\t\texpiry_attr.ValueSize = (wcslen(password_expiry_utc)) * sizeof(WCHAR);\n> +\t\texpiry_attr.Flags = 0;\n> +\t\tcred.Attributes = &expiry_attr;\n> +\t\tcred.AttributeCount = 1;\n> +\t}\n>  \tcred.TargetAlias = NULL;\n>  \tcred.UserName = wusername;\n>\n> @@ -278,6 +298,8 @@ static void read_credential(void)\n>  \t\t\twusername = utf8_to_utf16_dup(v);\n>  \t\t} else if (!strcmp(buf, \"password\"))\n>  \t\t\tpassword = utf8_to_utf16_dup(v);\n> +\t\telse if (!strcmp(buf, \"password_expiry_utc\"))\n> +\t\t\tpassword_expiry_utc = utf8_to_utf16_dup(v);\n>  \t\t/*\n>  \t\t * Ignore other lines; we don't know what they mean, but\n>  \t\t * this future-proofs us when later versions of git do\n> @@ -292,7 +314,7 @@ int main(int argc, char *argv[])\n>  \t    \"usage: git credential-wincred <get|store|erase>\\n\";\n>\n>  \tif (!argv[1])\n> -\t\tdie(usage);\n> +\t\tdie(\"%s\", usage);\n>\n>  \t/* git use binary pipes to avoid CRLF-issues */\n>  \t_setmode(_fileno(stdin), _O_BINARY);\n>\n> base-commit: 27d43aaaf50ef0ae014b88bba294f93658016a2e\n> --\n> gitgitgadget\n>\n"},{"id":"474444","messageId":"CAGJzqs=D8hmcxJKGCcz-NqEQ+QDYgi_aO02fj59kQoHZgiW3OQ@mail.gmail.com","threadId":"59471","inReplyTo":"35e1ebe6-e15b-1712-f030-70ab708740db@gmx.de","subject":"Re: [PATCH] credential/wincred: store password_expiry_utc","fromName":"M Hickford","fromEmail":"mirth.hickford@gmail.com","sentAt":"2023-03-30T05:50:30Z","receivedAt":"2023-03-30T05:51:14Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"On Tue, 28 Mar 2023 at 13:14, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n> And the reason is...\n>\n> > @@ -195,6 +197,15 @@ static void get_credential(void)\n> >                       write_item(\"password\",\n> >                               (LPCWSTR)creds[i]->CredentialBlob,\n> >                               creds[i]->CredentialBlobSize / sizeof(WCHAR));\n> > +                     attr = creds[i]->Attributes;\n> > +                     for (int j = 0; j < creds[i]->AttributeCount; j++) {\n> > +                             if (wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n>\n>                                   ^^^^^^\n>\n> ... here. Note how the return value of `wcscmp()` needs to be non-zero to\n> enter the conditional block? But `wcscmp()` returns 0 if there are no\n> differences between the two provided strings.\n>\n> That's not the only bug, though. While the loop iterates over all of the\n> attributes, the `attr` variable is unchanged, and always points to the\n> first attribute. You have to access it as `creds[i]->Attributes[j]`,\n> though, see e.g.\n> https://github.com/sandboxie-plus/Sandboxie/blob/f2a357f9222b81e7bdc994e5d9824790a1b5d826/Sandboxie/core/dll/cred.c#L324\n>\n> So with this patch on top of your patch, it works for me:\n>\n\nThanks Johannes for the review and the fix. I'll include it in any patch v2.\n\n>\n> But I have to wonder: why even bother with `git-wincred`? This credential\n> helper is so ridiculously limited in its capabilities, it does not even\n> support any host that is remotely close to safe (no 2FA, no OAuth, just\n> passwords). So I would be just as happy if I weren't asked to spend my\n> time to review changes to a credential helper I'd much rather see retired\n> than actively worked on.\n\ngit-credential-wincred has the same capabilities as popular helpers\ngit-credential-cache, git-credential-store, git-credential-osxkeychain\nand git-credential-libsecret. Any of which can store OAuth credentials\ngenerated by a helper such as git-credential-oauth [1]. This is\ncompatible with 2FA (any 2FA happens in browser). Example config:\n\n    [credential]\n        helper = wincred\n        helper = oauth\n\nThis patch to store password_expiry_utc is necessary to avoid Git\ntrying to use OAuth credentials beyond expiry. See\nhttps://github.com/git/git/commit/d208bfdfef97a1e8fb746763b5057e0ad91e283b\nfor background (I'll add to commit message v2).\n\nWhat about Git Credential Manager? GCM has a similar need to store\npassword expiry, see comment\nhttps://github.com/git-ecosystem/git-credential-manager/discussions/1169#discussioncomment-5472096.\n\nI think OAuth is important enough to fix this issue in both\ngit-credential-wincred and GCM. Some users might prefer the above\nsetup over GCM to avoid .NET dependency or if they really like\ngit-credential-oauth.\n\nNote that OAuth expiry issues don't occur authenticating to GitHub\nbecause GitHub doesn't populate OAuth expiry.\n\n[1] https://github.com/hickford/git-credential-oauth\n"},{"id":"474473","messageId":"pull.1477.v2.git.git.1680200278780.gitgitgadget@gmail.com","threadId":"59471","inReplyTo":"pull.1477.git.git.1679729956620.gitgitgadget@gmail.com","subject":"[PATCH v2] credential/wincred: store password_expiry_utc","fromName":"M Hickford via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-03-30T18:17:58Z","receivedAt":"2023-03-30T18:18:07Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"From: M Hickford <mirth.hickford@gmail.com>\n\nThis attribute is important when storing OAuth credentials which may\nexpire after as little as one hour. See\nhttps://github.com/git/git/commit/d208bfdfef97a1e8fb746763b5057e0ad91e283b\n\nSigned-off-by: M Hickford <mirth.hickford@gmail.com>\n---\n    credential/wincred: store password_expiry_utc\n    \n    Help wanted from a Windows user to test. I tried testing on Linux with\n    Wine after cross-compiling [1] but Wine has incomplete support for\n    wincred.h [2]. To test:\n    \n    cd contrib/credential/wincred/\n    make\n    echo host=example.com\\nprotocol=https\\nusername=tim\\npassword=xyzzy\\npassword_expiry_utc=2000 | ./git-credential-wincred.exe store\n    echo host=example.com\\nprotocol=https | ./git-credential-wincred.exe get\n    \n    \n    Output of second command should include line password_expiry_utc=2000\n    \n    [1] make CC=\"zig cc -target x86_64-windows-gnu\" [2]\n    https://github.com/wine-mirror/wine/blob/bf9d15e3b1a29f73fedda0c34547a9b29d5e2789/dlls/advapi32/cred.c#L181-L186\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1477%2Fhickford%2Fwincred-expiry-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1477/hickford/wincred-expiry-v2\nPull-Request: https://github.com/git/git/pull/1477\n\nRange-diff vs v1:\n\n 1:  05340715ef2 ! 1:  51a9039bd15 credential/wincred: store password_expiry_utc\n     @@ Metadata\n       ## Commit message ##\n          credential/wincred: store password_expiry_utc\n      \n     +    This attribute is important when storing OAuth credentials which may\n     +    expire after as little as one hour. See\n     +    https://github.com/git/git/commit/d208bfdfef97a1e8fb746763b5057e0ad91e283b\n     +\n          Signed-off-by: M Hickford <mirth.hickford@gmail.com>\n      \n       ## contrib/credential/wincred/git-credential-wincred.c ##\n     @@ contrib/credential/wincred/git-credential-wincred.c: static void get_credential(\n       \t\t\twrite_item(\"password\",\n       \t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n       \t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n     -+\t\t\tattr = creds[i]->Attributes;\n      +\t\t\tfor (int j = 0; j < creds[i]->AttributeCount; j++) {\n     -+\t\t\t\tif (wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n     ++\t\t\t\tattr = creds[i]->Attributes + j;\n     ++\t\t\t\tif (!wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n      +\t\t\t\t\twrite_item(\"password_expiry_utc\", (LPCWSTR)attr->Value,\n      +\t\t\t\t\tattr->ValueSize / sizeof(WCHAR));\n      +\t\t\t\t\tbreak;\n      +\t\t\t\t}\n     -+\t\t\t\tattr++;\n      +\t\t\t}\n       \t\t\tbreak;\n       \t\t}\n\n\n .../wincred/git-credential-wincred.c          | 25 +++++++++++++++++--\n 1 file changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c\nindex ead6e267c78..7b4e7fae675 100644\n--- a/contrib/credential/wincred/git-credential-wincred.c\n+++ b/contrib/credential/wincred/git-credential-wincred.c\n@@ -91,7 +91,8 @@ static void load_cred_funcs(void)\n \t\tdie(\"failed to load functions\");\n }\n \n-static WCHAR *wusername, *password, *protocol, *host, *path, target[1024];\n+static WCHAR *wusername, *password, *protocol, *host, *path, target[1024],\n+\t*password_expiry_utc;\n \n static void write_item(const char *what, LPCWSTR wbuf, int wlen)\n {\n@@ -183,6 +184,7 @@ static void get_credential(void)\n \tCREDENTIALW **creds;\n \tDWORD num_creds;\n \tint i;\n+\tCREDENTIAL_ATTRIBUTEW *attr;\n \n \tif (!CredEnumerateW(L\"git:*\", 0, &num_creds, &creds))\n \t\treturn;\n@@ -195,6 +197,14 @@ static void get_credential(void)\n \t\t\twrite_item(\"password\",\n \t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n \t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n+\t\t\tfor (int j = 0; j < creds[i]->AttributeCount; j++) {\n+\t\t\t\tattr = creds[i]->Attributes + j;\n+\t\t\t\tif (!wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n+\t\t\t\t\twrite_item(\"password_expiry_utc\", (LPCWSTR)attr->Value,\n+\t\t\t\t\tattr->ValueSize / sizeof(WCHAR));\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\tbreak;\n \t\t}\n \n@@ -204,6 +214,7 @@ static void get_credential(void)\n static void store_credential(void)\n {\n \tCREDENTIALW cred;\n+\tCREDENTIAL_ATTRIBUTEW expiry_attr;\n \n \tif (!wusername || !password)\n \t\treturn;\n@@ -217,6 +228,14 @@ static void store_credential(void)\n \tcred.Persist = CRED_PERSIST_LOCAL_MACHINE;\n \tcred.AttributeCount = 0;\n \tcred.Attributes = NULL;\n+\tif (password_expiry_utc != NULL) {\n+\t\texpiry_attr.Keyword = L\"git_password_expiry_utc\";\n+\t\texpiry_attr.Value = (LPVOID)password_expiry_utc;\n+\t\texpiry_attr.ValueSize = (wcslen(password_expiry_utc)) * sizeof(WCHAR);\n+\t\texpiry_attr.Flags = 0;\n+\t\tcred.Attributes = &expiry_attr;\n+\t\tcred.AttributeCount = 1;\n+\t}\n \tcred.TargetAlias = NULL;\n \tcred.UserName = wusername;\n \n@@ -278,6 +297,8 @@ static void read_credential(void)\n \t\t\twusername = utf8_to_utf16_dup(v);\n \t\t} else if (!strcmp(buf, \"password\"))\n \t\t\tpassword = utf8_to_utf16_dup(v);\n+\t\telse if (!strcmp(buf, \"password_expiry_utc\"))\n+\t\t\tpassword_expiry_utc = utf8_to_utf16_dup(v);\n \t\t/*\n \t\t * Ignore other lines; we don't know what they mean, but\n \t\t * this future-proofs us when later versions of git do\n@@ -292,7 +313,7 @@ int main(int argc, char *argv[])\n \t    \"usage: git credential-wincred <get|store|erase>\\n\";\n \n \tif (!argv[1])\n-\t\tdie(usage);\n+\t\tdie(\"%s\", usage);\n \n \t/* git use binary pipes to avoid CRLF-issues */\n \t_setmode(_fileno(stdin), _O_BINARY);\n\nbase-commit: 8d90352acc5c855620042fdcc6092f23a276af6d\n-- \ngitgitgadget\n"},{"id":"474479","messageId":"xmqqo7oahws8.fsf@gitster.g","threadId":"59471","inReplyTo":"pull.1477.v2.git.git.1680200278780.gitgitgadget@gmail.com","subject":"Re: [PATCH v2] credential/wincred: store password_expiry_utc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-03-30T19:19:35Z","receivedAt":"2023-03-30T19:20:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"M Hickford via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n\n> From: M Hickford <mirth.hickford@gmail.com>\n>\n> This attribute is important when storing OAuth credentials which may\n> expire after as little as one hour. See\n> https://github.com/git/git/commit/d208bfdfef97a1e8fb746763b5057e0ad91e283b\n\nReaders do not have to visit GitHub at all, and proposed log message\nshouldn't force them to.  Refer to an existing commit in this\nproject like so instead:\n\n    ... as one hour.  d208bfdf (credential: new attribute\n    password_expiry_utc, 2023-02-18) added support for this\n    attribute in general so that individual credential backend like\n    wincred can use it.\n\n>     Help wanted from a Windows user to test. I tried testing on Linux with\n>     Wine after cross-compiling [1] but Wine has incomplete support for\n>     wincred.h [2]. To test:\n\nI cannot be one to help testing but ...\n\n> @@ -292,7 +313,7 @@ int main(int argc, char *argv[])\n>  \t    \"usage: git credential-wincred <get|store|erase>\\n\";\n>  \n>  \tif (!argv[1])\n> -\t\tdie(usage);\n> +\t\tdie(\"%s\", usage);\n\n... this is a nice one.  Logically it may belong to a separate\ntopic, but it is small and obvious enough that it is OK to do as a\n\"while at it\" clean-up.\n"},{"id":"474674","messageId":"CAGJzqskDmiYwoe2tdgLbxCpRHzZZMon-SxjLqiJ8OYMk3f0hcg@mail.gmail.com","threadId":"59471","inReplyTo":"xmqqo7oahws8.fsf@gitster.g","subject":"Re: [PATCH v2] credential/wincred: store password_expiry_utc","fromName":"M Hickford","fromEmail":"mirth.hickford@gmail.com","sentAt":"2023-04-03T07:00:00Z","receivedAt":"2023-04-03T07:02:55Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"On Thu, 30 Mar 2023 at 20:20, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> \"M Hickford via GitGitGadget\" <gitgitgadget@gmail.com> writes:\n>\n> > From: M Hickford <mirth.hickford@gmail.com>\n> >\n> > This attribute is important when storing OAuth credentials which may\n> > expire after as little as one hour. See\n> > https://github.com/git/git/commit/d208bfdfef97a1e8fb746763b5057e0ad91e283b\n>\n> Readers do not have to visit GitHub at all, and proposed log message\n> shouldn't force them to.  Refer to an existing commit in this\n> project like so instead:\n>\n>     ... as one hour.  d208bfdf (credential: new attribute\n>     password_expiry_utc, 2023-02-18) added support for this\n>     attribute in general so that individual credential backend like\n>     wincred can use it.\n>\n\nThanks Junio for the example. I'll update the commit message in patch v3.\n"},{"id":"474676","messageId":"pull.1477.v3.git.git.1680508028077.gitgitgadget@gmail.com","threadId":"59471","inReplyTo":"pull.1477.v2.git.git.1680200278780.gitgitgadget@gmail.com","subject":"[PATCH v3] credential/wincred: store password_expiry_utc","fromName":"M Hickford via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2023-04-03T07:47:07Z","receivedAt":"2023-04-03T07:47:14Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"From: M Hickford <mirth.hickford@gmail.com>\n\nThis attribute is important when storing OAuth credentials which may\nexpire after as little as one hour. d208bfdf (credential: new attribute\npassword_expiry_utc, 2023-02-18) added support for this attribute in\ngeneral so that individual credential backend like wincred can use it.\n\nSigned-off-by: M Hickford <mirth.hickford@gmail.com>\n---\n    credential/wincred: store password_expiry_utc\n    \n    Windows users can test with the following commands:\n    \n    cd contrib/credential/wincred/ make printf\n    'host=example.com\\nprotocol=https\\nusername=tim\\npassword=xyzzy\\npassword_expiry_utc=2000\\n'\n    | ./git-credential-wincred.exe store printf\n    'host=example.com\\nprotocol=https\\n' | ./git-credential-wincred.exe get\n\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1477%2Fhickford%2Fwincred-expiry-v3\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1477/hickford/wincred-expiry-v3\nPull-Request: https://github.com/git/git/pull/1477\n\nRange-diff vs v2:\n\n 1:  51a9039bd15 ! 1:  d2dff063eb4 credential/wincred: store password_expiry_utc\n     @@ Commit message\n          credential/wincred: store password_expiry_utc\n      \n          This attribute is important when storing OAuth credentials which may\n     -    expire after as little as one hour. See\n     -    https://github.com/git/git/commit/d208bfdfef97a1e8fb746763b5057e0ad91e283b\n     +    expire after as little as one hour. d208bfdf (credential: new attribute\n     +    password_expiry_utc, 2023-02-18) added support for this attribute in\n     +    general so that individual credential backend like wincred can use it.\n      \n          Signed-off-by: M Hickford <mirth.hickford@gmail.com>\n      \n\n\n .../wincred/git-credential-wincred.c          | 25 +++++++++++++++++--\n 1 file changed, 23 insertions(+), 2 deletions(-)\n\ndiff --git a/contrib/credential/wincred/git-credential-wincred.c b/contrib/credential/wincred/git-credential-wincred.c\nindex ead6e267c78..7b4e7fae675 100644\n--- a/contrib/credential/wincred/git-credential-wincred.c\n+++ b/contrib/credential/wincred/git-credential-wincred.c\n@@ -91,7 +91,8 @@ static void load_cred_funcs(void)\n \t\tdie(\"failed to load functions\");\n }\n \n-static WCHAR *wusername, *password, *protocol, *host, *path, target[1024];\n+static WCHAR *wusername, *password, *protocol, *host, *path, target[1024],\n+\t*password_expiry_utc;\n \n static void write_item(const char *what, LPCWSTR wbuf, int wlen)\n {\n@@ -183,6 +184,7 @@ static void get_credential(void)\n \tCREDENTIALW **creds;\n \tDWORD num_creds;\n \tint i;\n+\tCREDENTIAL_ATTRIBUTEW *attr;\n \n \tif (!CredEnumerateW(L\"git:*\", 0, &num_creds, &creds))\n \t\treturn;\n@@ -195,6 +197,14 @@ static void get_credential(void)\n \t\t\twrite_item(\"password\",\n \t\t\t\t(LPCWSTR)creds[i]->CredentialBlob,\n \t\t\t\tcreds[i]->CredentialBlobSize / sizeof(WCHAR));\n+\t\t\tfor (int j = 0; j < creds[i]->AttributeCount; j++) {\n+\t\t\t\tattr = creds[i]->Attributes + j;\n+\t\t\t\tif (!wcscmp(attr->Keyword, L\"git_password_expiry_utc\")) {\n+\t\t\t\t\twrite_item(\"password_expiry_utc\", (LPCWSTR)attr->Value,\n+\t\t\t\t\tattr->ValueSize / sizeof(WCHAR));\n+\t\t\t\t\tbreak;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\tbreak;\n \t\t}\n \n@@ -204,6 +214,7 @@ static void get_credential(void)\n static void store_credential(void)\n {\n \tCREDENTIALW cred;\n+\tCREDENTIAL_ATTRIBUTEW expiry_attr;\n \n \tif (!wusername || !password)\n \t\treturn;\n@@ -217,6 +228,14 @@ static void store_credential(void)\n \tcred.Persist = CRED_PERSIST_LOCAL_MACHINE;\n \tcred.AttributeCount = 0;\n \tcred.Attributes = NULL;\n+\tif (password_expiry_utc != NULL) {\n+\t\texpiry_attr.Keyword = L\"git_password_expiry_utc\";\n+\t\texpiry_attr.Value = (LPVOID)password_expiry_utc;\n+\t\texpiry_attr.ValueSize = (wcslen(password_expiry_utc)) * sizeof(WCHAR);\n+\t\texpiry_attr.Flags = 0;\n+\t\tcred.Attributes = &expiry_attr;\n+\t\tcred.AttributeCount = 1;\n+\t}\n \tcred.TargetAlias = NULL;\n \tcred.UserName = wusername;\n \n@@ -278,6 +297,8 @@ static void read_credential(void)\n \t\t\twusername = utf8_to_utf16_dup(v);\n \t\t} else if (!strcmp(buf, \"password\"))\n \t\t\tpassword = utf8_to_utf16_dup(v);\n+\t\telse if (!strcmp(buf, \"password_expiry_utc\"))\n+\t\t\tpassword_expiry_utc = utf8_to_utf16_dup(v);\n \t\t/*\n \t\t * Ignore other lines; we don't know what they mean, but\n \t\t * this future-proofs us when later versions of git do\n@@ -292,7 +313,7 @@ int main(int argc, char *argv[])\n \t    \"usage: git credential-wincred <get|store|erase>\\n\";\n \n \tif (!argv[1])\n-\t\tdie(usage);\n+\t\tdie(\"%s\", usage);\n \n \t/* git use binary pipes to avoid CRLF-issues */\n \t_setmode(_fileno(stdin), _O_BINARY);\n\nbase-commit: 8d90352acc5c855620042fdcc6092f23a276af6d\n-- \ngitgitgadget\n"},{"id":"476390","messageId":"xmqqa5yn3d0k.fsf@gitster.g","threadId":"59471","inReplyTo":"CAGJzqs=D8hmcxJKGCcz-NqEQ+QDYgi_aO02fj59kQoHZgiW3OQ@mail.gmail.com","subject":"Re: [PATCH] credential/wincred: store password_expiry_utc","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2023-05-01T22:25:31Z","receivedAt":"2023-05-01T22:25:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"M Hickford <mirth.hickford@gmail.com> writes:\n\n> Thanks Johannes for the review and the fix. I'll include it in any patch v2.\n>\n>> But I have to wonder: why even bother with `git-wincred`? This credential\n>> helper is so ridiculously limited in its capabilities, it does not even\n>> support any host that is remotely close to safe (no 2FA, no OAuth, just\n>> passwords). So I would be just as happy if I weren't asked to spend my\n>> time to review changes to a credential helper I'd much rather see retired\n>> than actively worked on.\n>\n> git-credential-wincred has the same capabilities as popular helpers\n> git-credential-cache, git-credential-store, git-credential-osxkeychain\n> and git-credential-libsecret. Any of which can store OAuth credentials\n> generated by a helper such as git-credential-oauth [1]. This is\n> compatible with 2FA (any 2FA happens in browser). Example config:\n>\n>     [credential]\n>         helper = wincred\n>         helper = oauth\n>\n> This patch to store password_expiry_utc is necessary to avoid Git\n> trying to use OAuth credentials beyond expiry. See\n> https://github.com/git/git/commit/d208bfdfef97a1e8fb746763b5057e0ad91e283b\n> for background (I'll add to commit message v2).\n\nSo, even though earlier Dscho sounded negative on extending wincred\nhelper, are we now on track of enhancing its capabilities?  The v3\nis now queued in my tree and nobody who knows Windows seem to have\nmade any comments on either v2 or v3---I am wondering if the lack\nof comments is a good news or no interest.\n\nThanks.\n"},{"id":"476413","messageId":"CAGJzqsm4LmpYE46v2=y4=A+Och44zaQyzTXQRteX-KNSzA_18g@mail.gmail.com","threadId":"59471","inReplyTo":"xmqqa5yn3d0k.fsf@gitster.g","subject":"Re: [PATCH] credential/wincred: store password_expiry_utc","fromName":"M Hickford","fromEmail":"mirth.hickford@gmail.com","sentAt":"2023-05-02T09:38:15Z","receivedAt":"2023-05-02T09:38:58Z","isPatch":true,"sender":{"key":"mirth.hickford@gmail.com","avatar":"https://avatars.githubusercontent.com/u/105314?v=4"},"body":"On Mon, 1 May 2023 at 23:25, Junio C Hamano <gitster@pobox.com> wrote:\n>\n> M Hickford <mirth.hickford@gmail.com> writes:\n>\n> > Thanks Johannes for the review and the fix. I'll include it in any patch v2.\n> >\n> >> But I have to wonder: why even bother with `git-wincred`? This credential\n> >> helper is so ridiculously limited in its capabilities, it does not even\n> >> support any host that is remotely close to safe (no 2FA, no OAuth, just\n> >> passwords). So I would be just as happy if I weren't asked to spend my\n> >> time to review changes to a credential helper I'd much rather see retired\n> >> than actively worked on.\n> >\n> > git-credential-wincred has the same capabilities as popular helpers\n> > git-credential-cache, git-credential-store, git-credential-osxkeychain\n> > and git-credential-libsecret. Any of which can store OAuth credentials\n> > generated by a helper such as git-credential-oauth [1]. This is\n> > compatible with 2FA (any 2FA happens in browser). Example config:\n> >\n> >     [credential]\n> >         helper = wincred\n> >         helper = oauth\n> >\n> > This patch to store password_expiry_utc is necessary to avoid Git\n> > trying to use OAuth credentials beyond expiry. See\n> > https://github.com/git/git/commit/d208bfdfef97a1e8fb746763b5057e0ad91e283b\n> > for background (I'll add to commit message v2).\n>\n> So, even though earlier Dscho sounded negative on extending wincred\n> helper, are we now on track of enhancing its capabilities?  The v3\n> is now queued in my tree and nobody who knows Windows seem to have\n> made any comments on either v2 or v3---I am wondering if the lack\n> of comments is a good news or no interest.\n>\n> Thanks.\n\nThanks to Johannes's fixes for v1, the latest patch should be correct,\nbut it would be prudent to wait for a Windows user to test.\n\nThe utility of storing password_expiry_utc is universal to all\ncredential helpers. The latest commit message references the\nintroduction of this attribute\n(d208bfdfef97a1e8fb746763b5057e0ad91e283b) for background. I repeat\nthe arguments in [1], I hope they are persuasive.\n\n[1] https://lore.kernel.org/git/CAGJzqs=D8hmcxJKGCcz-NqEQ+QDYgi_aO02fj59kQoHZgiW3OQ@mail.gmail.com/\n"},{"id":"476444","messageId":"985ff655-51a5-2a0d-db09-5a53563fcc8b@kdbg.org","threadId":"59471","inReplyTo":"xmqqa5yn3d0k.fsf@gitster.g","subject":"Re: [PATCH] credential/wincred: store password_expiry_utc","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2023-05-02T17:43:50Z","receivedAt":"2023-05-02T17:43:58Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Am 02.05.23 um 00:25 schrieb Junio C Hamano:\n> I am wondering if the lack\n> of comments is a good news or no interest.\n\nSo far, I have totally negated the existence of credential helpers. I\ncan't offer any help in this regard, I am afraid.\n\n-- Hannes\n\n"},{"id":"476446","messageId":"645153949c192_1ba2d29427@chronos.notmuch","threadId":"59471","inReplyTo":"985ff655-51a5-2a0d-db09-5a53563fcc8b@kdbg.org","subject":"Re: [PATCH] credential/wincred: store password_expiry_utc","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2023-05-02T18:16:52Z","receivedAt":"2023-05-02T18:17:08Z","isPatch":true,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Johannes Sixt wrote:\n> Am 02.05.23 um 00:25 schrieb Junio C Hamano:\n> > I am wondering if the lack\n> > of comments is a good news or no interest.\n> \n> So far, I have totally negated the existence of credential helpers. I\n> can't offer any help in this regard, I am afraid.\n\nFWIW the same here. I exclusively use ssh with gnome-keyring and that\nworks perfectly fine.\n\n-- \nFelipe Contreras\n"}]}