credential/libsecret: store new attributes

d208bfd (credential: new attribute password_expiry_utc, 2023-02-18)
and a5c76569e7 (credential: new attribute oauth_refresh_token)
introduced new credential attributes.

libsecret assumes attribute values are non-confidential and
unchanging, so we encode the new attributes in the secret, separated by
newline:

    hunter2
    password_expiry_utc=1684189401
    oauth_refresh_token=xyzzy

This is extensible and backwards compatible. The credential protocol
already assumes that attribute values do not contain newlines.

Alternatives considered: store password_expiry_utc in a libsecret
attribute. This has the problem that libsecret creates new items
rather than overwrites when attribute values change.

Signed-off-by: M Hickford <mirth.hickford@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c
index ef681f29d5ba12dd5c6f7f11baa42cb1e6aab7d5..31cf32ad96937e4ad06afae7fd467ea4fbea4a2e 100644 (file)
--- a/contrib/credential/libsecret/git-credential-libsecret.c
+++ b/contrib/credential/libsecret/git-credential-libsecret.c
@@ -39,6 +39,8 @@ struct credential {
        char *path;
        char *username;
        char *password;
+       char *password_expiry_utc;
+       char *oauth_refresh_token;
 };
 
 #define CREDENTIAL_INIT { 0 }
@@ -54,6 +56,25 @@ struct credential_operation {
 
 /* ----------------- Secret Service functions ----------------- */
 
+static const SecretSchema schema = {
+       "org.git.Password",
+       /* Ignore schema name during search for backwards compatibility */
+       SECRET_SCHEMA_DONT_MATCH_NAME,
+       {
+               /*
+                * libsecret assumes attribute values are non-confidential and
+                * unchanging, so we can't include oauth_refresh_token or
+                * password_expiry_utc.
+                */
+               {  "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 },
+               {  NULL, 0 },
+       }
+};
+
 static char *make_label(struct credential *c)
 {
        if (c->port)
@@ -101,7 +122,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,
@@ -117,6 +138,7 @@ static int keyring_get(struct credential *c)
                SecretItem *item;
                SecretValue *secret;
                const char *s;
+               gchar **parts;
 
                item = items->data;
                secret = secret_item_get_secret(item);
@@ -130,8 +152,27 @@ static int keyring_get(struct credential *c)
 
                s = secret_value_get_text(secret);
                if (s) {
-                       g_free(c->password);
-                       c->password = g_strdup(s);
+                       /*
+                        * Passwords and other attributes encoded in following format:
+                        *   hunter2
+                        *   password_expiry_utc=1684189401
+                        *   oauth_refresh_token=xyzzy
+                        */
+                       parts = g_strsplit(s, "\n", 0);
+                       if (g_strv_length(parts) >= 1) {
+                               g_free(c->password);
+                               c->password = g_strdup(parts[0]);
+                       }
+                       for (int i = 1; i < g_strv_length(parts); i++) {
+                               if (g_str_has_prefix(parts[i], "password_expiry_utc=")) {
+                                       g_free(c->password_expiry_utc);
+                                       c->password_expiry_utc = g_strdup(&parts[i][20]);
+                               } else if (g_str_has_prefix(parts[i], "oauth_refresh_token=")) {
+                                       g_free(c->oauth_refresh_token);
+                                       c->oauth_refresh_token = g_strdup(&parts[i][20]);
+                               }
+                       }
+                       g_strfreev(parts);
                }
 
                g_hash_table_unref(attributes);
@@ -148,6 +189,7 @@ static int keyring_store(struct credential *c)
        char *label = NULL;
        GHashTable *attributes = NULL;
        GError *error = NULL;
+       GString *secret = NULL;
 
        /*
         * Sanity check that what we are storing is actually sensible.
@@ -162,13 +204,23 @@ 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 = g_string_new(c->password);
+       if (c->password_expiry_utc) {
+               g_string_append_printf(secret, "\npassword_expiry_utc=%s",
+                       c->password_expiry_utc);
+       }
+       if (c->oauth_refresh_token) {
+               g_string_append_printf(secret, "\noauth_refresh_token=%s",
+                       c->oauth_refresh_token);
+       }
+       secret_password_storev_sync(&schema,
                                    attributes,
                                    NULL,
                                    label,
-                                   c->password,
+                                   secret->str,
                                    NULL,
                                    &error);
+       g_string_free(secret, TRUE);
        g_free(label);
        g_hash_table_unref(attributes);
 
@@ -198,7 +250,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 +290,8 @@ 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);
+       g_free(c->oauth_refresh_token);
 
        credential_init(c);
 }
@@ -284,11 +338,19 @@ 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);
                        while (*value)
                                *value++ = '\0';
+               } else if (!strcmp(key, "oauth_refresh_token")) {
+                       g_free(c->oauth_refresh_token);
+                       c->oauth_refresh_token = g_strdup(value);
+                       while (*value)
+                               *value++ = '\0';
                }
                /*
                 * Ignore other lines; we don't know what they mean, but
@@ -314,6 +376,10 @@ static void credential_write(const struct credential *c)
        /* only write username/password, 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);
+       credential_write_item(stdout, "oauth_refresh_token",
+               c->oauth_refresh_token);
 }
 
 static void usage(const char *name)

diff --git a/t/lib-credential.sh b/t/lib-credential.sh
index f1ab92ba35c432052e1f903750795443122869ad..72f52cfedb25a7025f311c763340f14ec6697b98 100644 (file)
--- a/t/lib-credential.sh
+++ b/t/lib-credential.sh
@@ -43,6 +43,8 @@ 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 user-expiry
+       reject $1 https example.com user-expiry-overwrite
        reject $1 https example.com user4
        reject $1 http path.tld user
        reject $1 https timeout.tld user
@@ -328,6 +330,81 @@ 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=user-expiry
+               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=user-expiry
+               --
+               protocol=https
+               host=example.com
+               username=user-expiry
+               password=pass
+               password_expiry_utc=9999999999
+               --
+               EOF
+       '
+
+       test_expect_success "helper ($HELPER) overwrites when password_expiry_utc changes" '
+               check approve $HELPER <<-\EOF &&
+               protocol=https
+               host=example.com
+               username=user-expiry-overwrite
+               password=pass1
+               password_expiry_utc=9999999998
+               EOF
+               check approve $HELPER <<-\EOF &&
+               protocol=https
+               host=example.com
+               username=user-expiry-overwrite
+               password=pass2
+               password_expiry_utc=9999999999
+               EOF
+               check fill $HELPER <<-\EOF &&
+               protocol=https
+               host=example.com
+               username=user-expiry-overwrite
+               --
+               protocol=https
+               host=example.com
+               username=user-expiry-overwrite
+               password=pass2
+               password_expiry_utc=9999999999
+               EOF
+               check reject $HELPER <<-\EOF &&
+               protocol=https
+               host=example.com
+               username=user-expiry-overwrite
+               password=pass2
+               EOF
+               check fill $HELPER <<-\EOF
+               protocol=https
+               host=example.com
+               username=user-expiry-overwrite
+               --
+               protocol=https
+               host=example.com
+               username=user-expiry-overwrite
+               password=askpass-password
+               --
+               askpass: Password for '\''https://user-expiry-overwrite@example.com'\'':
+               EOF
+       '
+}
+
 helper_test_oauth_refresh_token() {
        HELPER=$1
 

diff --git a/t/t0301-credential-cache.sh b/t/t0301-credential-cache.sh
index c02a3b5969c3d2265d1552132464df7135cd522b..8300faadea9a76f19e3d2c82f5ff600f38bfe18f 100755 (executable)
--- a/t/t0301-credential-cache.sh
+++ b/t/t0301-credential-cache.sh
@@ -29,6 +29,7 @@ test_atexit 'git credential-cache exit'
 
 # test that the daemon works with no special setup
 helper_test cache
+helper_test_password_expiry_utc cache
 helper_test_oauth_refresh_token cache
 
 test_expect_success 'socket defaults to ~/.cache/git/credential/socket' '

diff --git a/t/t0303-credential-external.sh b/t/t0303-credential-external.sh
index f028fd1418285107a90e170a6ea1cd7657e31bb8..095574bfc6edf2aaf835b2ff43bb8cd35f792591 100755 (executable)
--- a/t/t0303-credential-external.sh
+++ b/t/t0303-credential-external.sh
@@ -45,6 +45,8 @@ test -z "$GIT_TEST_CREDENTIAL_HELPER_SETUP" ||
 helper_test_clean "$GIT_TEST_CREDENTIAL_HELPER"
 
 helper_test "$GIT_TEST_CREDENTIAL_HELPER"
+helper_test_password_expiry_utc "$GIT_TEST_CREDENTIAL_HELPER"
+helper_test_oauth_refresh_token "$GIT_TEST_CREDENTIAL_HELPER"
 
 if test -z "$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT"; then
        say "# skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set)"