git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3] credential/libsecret: support password_expiry_utc

From
M Hickford <mirth.hickford@gmail.com>
Date
May 15, 2023, 10:50 UTC
Message-ID
<CAGJzqskMwOJkriH6serqdwAVYi+fftEL8ohJd-suP6v+OxB_bg@mail.gmail.com>
In-Reply-To
<pull.1469.v3.git.git.1683270298313.gitgitgadget@gmail.com>

On Fri, 5 May 2023 at 08:05, M Hickford via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 89 quoted lines
>
> From: M Hickford <mirth.hickford@gmail.com>
>
> d208bfd (credential: new attribute password_expiry_utc, 2023-02-18)
> introduced this attribute.
>
> Signed-off-by: M Hickford <mirth.hickford@gmail.com>
> ---
>     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@gmail.com>
>
>        ## 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)

Show 166 quoted lines
> +               {  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
Previous: M Hickford via GitGitGadgetNext: Junio C Hamano
Message 6 of 11 in “credential/libsecret: support password_expiry_utc”
  1. credential/libsecret: support password_expiry_utcM Hickford via GitGitGadget, Mar 14, 2023
  2. credential/libsecret: support password_expiry_utcM Hickford via GitGitGadget, Mar 25, 2023
  3. Junio C HamanoMay 4, 2023
  4. M HickfordMay 5, 2023
  5. credential/libsecret: support password_expiry_utcM Hickford via GitGitGadget, May 5, 2023
  6. M HickfordMay 15, 2023
  7. Junio C HamanoMay 15, 2023
  8. M HickfordMay 16, 2023
  9. Junio C HamanoMay 16, 2023
  10. credential/libsecret: store new attributesM Hickford via GitGitGadget, May 17, 2023
  11. credential/libsecret: store new attributesM Hickford via GitGitGadget, Jun 16, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.