On Fri, 5 May 2023 at 08:05, M Hickford via GitGitGadget <gitgitgadget@xxxxxxxxx> wrote: > > From: M Hickford <mirth.hickford@xxxxxxxxx> > > d208bfd (credential: new attribute password_expiry_utc, 2023-02-18) > introduced this attribute. > > Signed-off-by: M Hickford <mirth.hickford@xxxxxxxxx> > --- > credential/libsecret: store password_expiry_utc > > Patch v3 fixes backwards compatibility. > > Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1469%2Fhickford%2Flibsecret-v3 > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1469/hickford/libsecret-v3 > Pull-Request: https://github.com/git/git/pull/1469 > > Range-diff vs v2: > > 1: 1e27677b6f5 ! 1: b46594c8897 credential/libsecret: support password_expiry_utc > @@ Metadata > ## Commit message ## > credential/libsecret: support password_expiry_utc > > + d208bfd (credential: new attribute password_expiry_utc, 2023-02-18) > + introduced this attribute. > + > Signed-off-by: M Hickford <mirth.hickford@xxxxxxxxx> > > ## contrib/credential/libsecret/git-credential-libsecret.c ## > @@ contrib/credential/libsecret/git-credential-libsecret.c: struct credential_opera > > +static const SecretSchema schema = { > + "org.git.Password", > -+ SECRET_SCHEMA_NONE, > ++ /* Ignore schema name for backwards compatibility with previous versions */ > ++ SECRET_SCHEMA_DONT_MATCH_NAME, > + { > + { "user", SECRET_SCHEMA_ATTRIBUTE_STRING }, > + { "object", SECRET_SCHEMA_ATTRIBUTE_STRING }, > @@ contrib/credential/libsecret/git-credential-libsecret.c: static int keyring_get( > - SECRET_SCHEMA_COMPAT_NETWORK, > + &schema, > attributes, > -- SECRET_SEARCH_LOAD_SECRETS | SECRET_SEARCH_UNLOCK, > -+ SECRET_SEARCH_LOAD_SECRETS | SECRET_SEARCH_UNLOCK | > -+ // for backwards compatibility > -+ SECRET_SCHEMA_DONT_MATCH_NAME, > + SECRET_SEARCH_LOAD_SECRETS | SECRET_SEARCH_UNLOCK, > NULL, > - &error); > - g_hash_table_unref(attributes); > @@ contrib/credential/libsecret/git-credential-libsecret.c: static int keyring_get(struct credential *c) > c->username = g_strdup(s); > } > > > .../libsecret/git-credential-libsecret.c | 39 +++++++++++++++++-- > t/lib-credential.sh | 30 ++++++++++++++ > t/t0301-credential-cache.sh | 2 + > t/t0303-credential-external.sh | 2 + > 4 files changed, 69 insertions(+), 4 deletions(-) > > diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c > index 2c5d76d789f..182f0805c2b 100644 > --- a/contrib/credential/libsecret/git-credential-libsecret.c > +++ b/contrib/credential/libsecret/git-credential-libsecret.c > @@ -39,6 +39,7 @@ struct credential { > char *path; > char *username; > char *password; > + char *password_expiry_utc; > }; > > #define CREDENTIAL_INIT { 0 } > @@ -54,6 +55,21 @@ struct credential_operation { > > /* ----------------- Secret Service functions ----------------- */ > > +static const SecretSchema schema = { > + "org.git.Password", > + /* Ignore schema name for backwards compatibility with previous versions */ > + SECRET_SCHEMA_DONT_MATCH_NAME, > + { > + { "user", SECRET_SCHEMA_ATTRIBUTE_STRING }, > + { "object", SECRET_SCHEMA_ATTRIBUTE_STRING }, > + { "protocol", SECRET_SCHEMA_ATTRIBUTE_STRING }, > + { "port", SECRET_SCHEMA_ATTRIBUTE_INTEGER }, > + { "server", SECRET_SCHEMA_ATTRIBUTE_STRING }, > + { "password_expiry_utc", SECRET_SCHEMA_ATTRIBUTE_INTEGER }, I've been testing this patch with credential-generating helper git-credential-helper. It works, but because libsecret overwrites items if and only if the attributes match exactly, you end up with many items in the secret store that differ only by expiry date. This is inelegant, and confusing to users. Please hold this patch, don't merge to master. A solution might be to store the expiry date as the secret of a separate item (even though the value is not confidential) > + { NULL, 0 }, > + } > +}; > + > static char *make_label(struct credential *c) > { > if (c->port) > @@ -78,6 +94,9 @@ static GHashTable *make_attr_list(struct credential *c) > g_hash_table_insert(al, "port", g_strdup_printf("%hu", c->port)); > if (c->path) > g_hash_table_insert(al, "object", g_strdup(c->path)); > + if (c->password_expiry_utc) > + g_hash_table_insert(al, "password_expiry_utc", > + g_strdup(c->password_expiry_utc)); > > return al; > } > @@ -101,7 +120,7 @@ static int keyring_get(struct credential *c) > > attributes = make_attr_list(c); > items = secret_service_search_sync(service, > - SECRET_SCHEMA_COMPAT_NETWORK, > + &schema, > attributes, > SECRET_SEARCH_LOAD_SECRETS | SECRET_SEARCH_UNLOCK, > NULL, > @@ -128,6 +147,12 @@ static int keyring_get(struct credential *c) > c->username = g_strdup(s); > } > > + s = g_hash_table_lookup(attributes, "password_expiry_utc"); > + if (s) { > + g_free(c->password_expiry_utc); > + c->password_expiry_utc = g_strdup(s); > + } > + > s = secret_value_get_text(secret); > if (s) { > g_free(c->password); > @@ -162,7 +187,7 @@ static int keyring_store(struct credential *c) > > label = make_label(c); > attributes = make_attr_list(c); > - secret_password_storev_sync(SECRET_SCHEMA_COMPAT_NETWORK, > + secret_password_storev_sync(&schema, > attributes, > NULL, > label, > @@ -198,7 +223,7 @@ static int keyring_erase(struct credential *c) > return EXIT_FAILURE; > > attributes = make_attr_list(c); > - secret_password_clearv_sync(SECRET_SCHEMA_COMPAT_NETWORK, > + secret_password_clearv_sync(&schema, > attributes, > NULL, > &error); > @@ -238,6 +263,7 @@ static void credential_clear(struct credential *c) > g_free(c->path); > g_free(c->username); > g_free(c->password); > + g_free(c->password_expiry_utc); > > credential_init(c); > } > @@ -285,6 +311,9 @@ static int credential_read(struct credential *c) > } else if (!strcmp(key, "username")) { > g_free(c->username); > c->username = g_strdup(value); > + } else if (!strcmp(key, "password_expiry_utc")) { > + g_free(c->password_expiry_utc); > + c->password_expiry_utc = g_strdup(value); > } else if (!strcmp(key, "password")) { > g_free(c->password); > c->password = g_strdup(value); > @@ -312,9 +341,11 @@ static void credential_write_item(FILE *fp, const char *key, const char *value) > > static void credential_write(const struct credential *c) > { > - /* only write username/password, if set */ > + /* only write username/password/expiry, if set */ > credential_write_item(stdout, "username", c->username); > credential_write_item(stdout, "password", c->password); > + credential_write_item(stdout, "password_expiry_utc", > + c->password_expiry_utc); > } > > static void usage(const char *name) > diff --git a/t/lib-credential.sh b/t/lib-credential.sh > index 5ea8bc9f1dc..9ebf7eeae48 100644 > --- a/t/lib-credential.sh > +++ b/t/lib-credential.sh > @@ -43,6 +43,7 @@ helper_test_clean() { > reject $1 https example.com store-user > reject $1 https example.com user1 > reject $1 https example.com user2 > + reject $1 https example.com user3 > reject $1 http path.tld user > reject $1 https timeout.tld user > reject $1 https sso.tld > @@ -298,6 +299,35 @@ helper_test_timeout() { > ' > } > > +helper_test_password_expiry_utc() { > + HELPER=$1 > + > + test_expect_success "helper ($HELPER) stores password_expiry_utc" ' > + check approve $HELPER <<-\EOF > + protocol=https > + host=example.com > + username=user3 > + password=pass > + password_expiry_utc=9999999999 > + EOF > + ' > + > + test_expect_success "helper ($HELPER) gets password_expiry_utc" ' > + check fill $HELPER <<-\EOF > + protocol=https > + host=example.com > + username=user3 > + -- > + protocol=https > + host=example.com > + username=user3 > + password=pass > + password_expiry_utc=9999999999 > + -- > + EOF > + ' > +} > + > write_script askpass <<\EOF > echo >&2 askpass: $* > what=$(echo $1 | cut -d" " -f1 | tr A-Z a-z | tr -cd a-z) > diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh > index 698b7159f03..f5ba727e53b 100755 > --- a/t/t0301-credential-cache.sh > +++ b/t/t0301-credential-cache.sh > @@ -30,6 +30,8 @@ test_atexit 'git credential-cache exit' > # test that the daemon works with no special setup > helper_test cache > > +helper_test_password_expiry_utc cache > + > test_expect_success 'socket defaults to ~/.cache/git/credential/socket' ' > test_when_finished " > git credential-cache exit && > diff --git a/t/t0303-credential-external.sh b/t/t0303-credential-external.sh > index f028fd14182..f1478680bff 100755 > --- a/t/t0303-credential-external.sh > +++ b/t/t0303-credential-external.sh > @@ -52,6 +52,8 @@ else > helper_test_timeout "$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT" > fi > > +helper_test_password_expiry_utc "$GIT_TEST_CREDENTIAL_HELPER" > + > # clean afterwards so that we are good citizens > # and don't leave cruft in the helper's storage, which > # might be long-term system storage > > base-commit: 27d43aaaf50ef0ae014b88bba294f93658016a2e > -- > gitgitgadget
