From: "M Hickford via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, M Hickford <mirth.hickford@gmail.com>,
M Hickford <mirth.hickford@gmail.com>
Subject: [PATCH v5] credential/libsecret: store new attributes
Date: Fri, 16 Jun 2023 19:55:06 +0000 [thread overview]
Message-ID: <pull.1469.v5.git.git.1686945306242.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1469.v4.git.git.1684306540947.gitgitgadget@gmail.com>
From: M Hickford <mirth.hickford@gmail.com>
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>
---
credential/libsecret: store password_expiry_utc and oauth_refresh_token
Test that storing credential with expiry overwrites any old credential
Rename test users for clarity
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-1469%2Fhickford%2Flibsecret-v5
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-1469/hickford/libsecret-v5
Pull-Request: https://github.com/git/git/pull/1469
Range-diff vs v4:
1: 07452a10372 ! 1: 38e02c5e24e credential/libsecret: store new attributes
@@ t/lib-credential.sh: 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 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
@@ t/lib-credential.sh: helper_test_timeout() {
+ check approve $HELPER <<-\EOF
+ protocol=https
+ host=example.com
-+ username=user3
++ username=user-expiry
+ password=pass
+ password_expiry_utc=9999999999
+ EOF
@@ t/lib-credential.sh: helper_test_timeout() {
+ check fill $HELPER <<-\EOF
+ protocol=https
+ host=example.com
-+ username=user3
++ username=user-expiry
+ --
+ protocol=https
+ host=example.com
-+ username=user3
++ 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() {
.../libsecret/git-credential-libsecret.c | 78 +++++++++++++++++--
t/lib-credential.sh | 77 ++++++++++++++++++
t/t0301-credential-cache.sh | 1 +
t/t0303-credential-external.sh | 2 +
4 files changed, 152 insertions(+), 6 deletions(-)
diff --git a/contrib/credential/libsecret/git-credential-libsecret.c b/contrib/credential/libsecret/git-credential-libsecret.c
index ef681f29d5b..31cf32ad969 100644
--- 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 f1ab92ba35c..72f52cfedb2 100644
--- 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 c02a3b5969c..8300faadea9 100755
--- 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 f028fd14182..095574bfc6e 100755
--- 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)"
base-commit: 9e49351c3060e1fa6e0d2de64505b7becf157f28
--
gitgitgadget
prev parent reply other threads:[~2023-06-16 19:55 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-03-14 21:32 [PATCH] credential/libsecret: support password_expiry_utc M Hickford via GitGitGadget
2023-03-25 7:36 ` [PATCH v2] " M Hickford via GitGitGadget
2023-05-04 17:42 ` Junio C Hamano
2023-05-05 7:00 ` M Hickford
2023-05-05 7:04 ` [PATCH v3] " M Hickford via GitGitGadget
2023-05-15 10:50 ` M Hickford
2023-05-15 18:14 ` Junio C Hamano
2023-05-16 8:03 ` M Hickford
2023-05-16 16:10 ` Junio C Hamano
2023-05-17 6:55 ` [PATCH v4] credential/libsecret: store new attributes M Hickford via GitGitGadget
2023-06-16 19:55 ` M Hickford via GitGitGadget [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=pull.1469.v5.git.git.1686945306242.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=git@vger.kernel.org \
--cc=mirth.hickford@gmail.com \
--cc=peff@peff.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).