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>
M Hickford committed
Jun 16, 2023 at 19:55 UTC
0ce02e2feca6b0a7b09c71cf890d116de4e09f36
4 files changed
+152
-6
contrib/credential/libsecret/git-credential-libsecret.c
+72
-6
@@ -39,6 +39,8 @@ struct credential {
39
char *path;
40
char *username;
41
char *password;
42
+ char *password_expiry_utc;
43
+ char *oauth_refresh_token;
44
};
45
46
#define CREDENTIAL_INIT { 0 }
@@ -54,6 +56,25 @@ struct credential_operation {
56
57
/* ----------------- Secret Service functions ----------------- */
58
59
+static const SecretSchema schema = {
60
+ "org.git.Password",
61
+ /* Ignore schema name during search for backwards compatibility */
62
+ SECRET_SCHEMA_DONT_MATCH_NAME,
63
+ {
64
+ /*
65
+ * libsecret assumes attribute values are non-confidential and
66
+ * unchanging, so we can't include oauth_refresh_token or
67
+ * password_expiry_utc.
68
+ */
69
+ { "user", SECRET_SCHEMA_ATTRIBUTE_STRING },
70
+ { "object", SECRET_SCHEMA_ATTRIBUTE_STRING },
71
+ { "protocol", SECRET_SCHEMA_ATTRIBUTE_STRING },
72
+ { "port", SECRET_SCHEMA_ATTRIBUTE_INTEGER },
73
+ { "server", SECRET_SCHEMA_ATTRIBUTE_STRING },
74
+ { NULL, 0 },
75
+ }
76
+};
77
+
78
static char *make_label(struct credential *c)
79
{
80
if (c->port)
@@ -101,7 +122,7 @@ static int keyring_get(struct credential *c)
122
123
attributes = make_attr_list(c);
124
items = secret_service_search_sync(service,
104
- SECRET_SCHEMA_COMPAT_NETWORK,
125
+ &schema,
126
attributes,
127
SECRET_SEARCH_LOAD_SECRETS | SECRET_SEARCH_UNLOCK,
128
NULL,
@@ -117,6 +138,7 @@ static int keyring_get(struct credential *c)
138
SecretItem *item;
139
SecretValue *secret;
140
const char *s;
141
+ gchar **parts;
142
143
item = items->data;
144
secret = secret_item_get_secret(item);
@@ -130,8 +152,27 @@ static int keyring_get(struct credential *c)
152
153
s = secret_value_get_text(secret);
154
if (s) {
133
- g_free(c->password);
134
- c->password = g_strdup(s);
155
+ /*
156
+ * Passwords and other attributes encoded in following format:
157
+ * hunter2
158
+ * password_expiry_utc=1684189401
159
+ * oauth_refresh_token=xyzzy
160
+ */
161
+ parts = g_strsplit(s, "\n", 0);
162
+ if (g_strv_length(parts) >= 1) {
163
+ g_free(c->password);
164
+ c->password = g_strdup(parts[0]);
165
+ }
166
+ for (int i = 1; i < g_strv_length(parts); i++) {
167
+ if (g_str_has_prefix(parts[i], "password_expiry_utc=")) {
168
+ g_free(c->password_expiry_utc);
169
+ c->password_expiry_utc = g_strdup(&parts[i][20]);
170
+ } else if (g_str_has_prefix(parts[i], "oauth_refresh_token=")) {
171
+ g_free(c->oauth_refresh_token);
172
+ c->oauth_refresh_token = g_strdup(&parts[i][20]);
173
+ }
174
+ }
175
+ g_strfreev(parts);
176
}
177
178
g_hash_table_unref(attributes);
@@ -148,6 +189,7 @@ static int keyring_store(struct credential *c)
189
char *label = NULL;
190
GHashTable *attributes = NULL;
191
GError *error = NULL;
192
+ GString *secret = NULL;
193
194
/*
195
* Sanity check that what we are storing is actually sensible.
@@ -162,13 +204,23 @@ static int keyring_store(struct credential *c)
204
205
label = make_label(c);
206
attributes = make_attr_list(c);
165
- secret_password_storev_sync(SECRET_SCHEMA_COMPAT_NETWORK,
207
+ secret = g_string_new(c->password);
208
+ if (c->password_expiry_utc) {
209
+ g_string_append_printf(secret, "\npassword_expiry_utc=%s",
210
+ c->password_expiry_utc);
211
+ }
212
+ if (c->oauth_refresh_token) {
213
+ g_string_append_printf(secret, "\noauth_refresh_token=%s",
214
+ c->oauth_refresh_token);
215
+ }
216
+ secret_password_storev_sync(&schema,
217
attributes,
218
NULL,
219
label,
169
- c->password,
220
+ secret->str,
221
NULL,
222
&error);
223
+ g_string_free(secret, TRUE);
224
g_free(label);
225
g_hash_table_unref(attributes);
226
@@ -198,7 +250,7 @@ static int keyring_erase(struct credential *c)
250
return EXIT_FAILURE;
251
252
attributes = make_attr_list(c);
201
- secret_password_clearv_sync(SECRET_SCHEMA_COMPAT_NETWORK,
253
+ secret_password_clearv_sync(&schema,
254
attributes,
255
NULL,
256
&error);
@@ -238,6 +290,8 @@ static void credential_clear(struct credential *c)
290
g_free(c->path);
291
g_free(c->username);
292
g_free(c->password);
293
+ g_free(c->password_expiry_utc);
294
+ g_free(c->oauth_refresh_token);
295
296
credential_init(c);
297
}
@@ -284,11 +338,19 @@ static int credential_read(struct credential *c)
338
} else if (!strcmp(key, "username")) {
339
g_free(c->username);
340
c->username = g_strdup(value);
341
+ } else if (!strcmp(key, "password_expiry_utc")) {
342
+ g_free(c->password_expiry_utc);
343
+ c->password_expiry_utc = g_strdup(value);
344
} else if (!strcmp(key, "password")) {
345
g_free(c->password);
346
c->password = g_strdup(value);
347
while (*value)
348
*value++ = '\0';
349
+ } else if (!strcmp(key, "oauth_refresh_token")) {
350
+ g_free(c->oauth_refresh_token);
351
+ c->oauth_refresh_token = g_strdup(value);
352
+ while (*value)
353
+ *value++ = '\0';
354
}
355
/*
356
* Ignore other lines; we don't know what they mean, but
@@ -314,6 +376,10 @@ static void credential_write(const struct credential *c)
376
/* only write username/password, if set */
377
credential_write_item(stdout, "username", c->username);
378
credential_write_item(stdout, "password", c->password);
379
+ credential_write_item(stdout, "password_expiry_utc",
380
+ c->password_expiry_utc);
381
+ credential_write_item(stdout, "oauth_refresh_token",
382
+ c->oauth_refresh_token);
383
}
384
385
static void usage(const char *name)
t/lib-credential.sh
+77
@@ -43,6 +43,8 @@ helper_test_clean() {
43
reject $1 https example.com store-user
44
reject $1 https example.com user1
45
reject $1 https example.com user2
46
+ reject $1 https example.com user-expiry
47
+ reject $1 https example.com user-expiry-overwrite
48
reject $1 https example.com user4
49
reject $1 http path.tld user
50
reject $1 https timeout.tld user
@@ -328,6 +330,81 @@ helper_test_timeout() {
330
'
331
}
332
333
+helper_test_password_expiry_utc() {
334
+ HELPER=$1
335
+
336
+ test_expect_success "helper ($HELPER) stores password_expiry_utc" '
337
+ check approve $HELPER <<-\EOF
338
+ protocol=https
339
+ host=example.com
340
+ username=user-expiry
341
+ password=pass
342
+ password_expiry_utc=9999999999
343
+ EOF
344
+ '
345
+
346
+ test_expect_success "helper ($HELPER) gets password_expiry_utc" '
347
+ check fill $HELPER <<-\EOF
348
+ protocol=https
349
+ host=example.com
350
+ username=user-expiry
351
+ --
352
+ protocol=https
353
+ host=example.com
354
+ username=user-expiry
355
+ password=pass
356
+ password_expiry_utc=9999999999
357
+ --
358
+ EOF
359
+ '
360
+
361
+ test_expect_success "helper ($HELPER) overwrites when password_expiry_utc changes" '
362
+ check approve $HELPER <<-\EOF &&
363
+ protocol=https
364
+ host=example.com
365
+ username=user-expiry-overwrite
366
+ password=pass1
367
+ password_expiry_utc=9999999998
368
+ EOF
369
+ check approve $HELPER <<-\EOF &&
370
+ protocol=https
371
+ host=example.com
372
+ username=user-expiry-overwrite
373
+ password=pass2
374
+ password_expiry_utc=9999999999
375
+ EOF
376
+ check fill $HELPER <<-\EOF &&
377
+ protocol=https
378
+ host=example.com
379
+ username=user-expiry-overwrite
380
+ --
381
+ protocol=https
382
+ host=example.com
383
+ username=user-expiry-overwrite
384
+ password=pass2
385
+ password_expiry_utc=9999999999
386
+ EOF
387
+ check reject $HELPER <<-\EOF &&
388
+ protocol=https
389
+ host=example.com
390
+ username=user-expiry-overwrite
391
+ password=pass2
392
+ EOF
393
+ check fill $HELPER <<-\EOF
394
+ protocol=https
395
+ host=example.com
396
+ username=user-expiry-overwrite
397
+ --
398
+ protocol=https
399
+ host=example.com
400
+ username=user-expiry-overwrite
401
+ password=askpass-password
402
+ --
403
+ askpass: Password for '\''https://user-expiry-overwrite@example.com'\'':
404
+ EOF
405
+ '
406
+}
407
+
408
helper_test_oauth_refresh_token() {
409
HELPER=$1
410
t/t0301-credential-cache.sh
+1
@@ -29,6 +29,7 @@ test_atexit 'git credential-cache exit'
29
30
# test that the daemon works with no special setup
31
helper_test cache
32
+helper_test_password_expiry_utc cache
33
helper_test_oauth_refresh_token cache
34
35
test_expect_success 'socket defaults to ~/.cache/git/credential/socket' '
t/t0303-credential-external.sh
+2
@@ -45,6 +45,8 @@ test -z "$GIT_TEST_CREDENTIAL_HELPER_SETUP" ||
45
helper_test_clean "$GIT_TEST_CREDENTIAL_HELPER"
46
47
helper_test "$GIT_TEST_CREDENTIAL_HELPER"
48
+helper_test_password_expiry_utc "$GIT_TEST_CREDENTIAL_HELPER"
49
+helper_test_oauth_refresh_token "$GIT_TEST_CREDENTIAL_HELPER"
50
51
if test -z "$GIT_TEST_CREDENTIAL_HELPER_TIMEOUT"; then
52
say "# skipping timeout tests (GIT_TEST_CREDENTIAL_HELPER_TIMEOUT not set)"