* [PATCH v2 4/5] credential: use the last matching username in the config
2020-02-20 2:24 [PATCH v2 0/5] Wildcard matching for credentials brian m. carlson
` (2 preceding siblings ...)
2020-02-20 2:24 ` [PATCH v2 3/5] t0300: add tests for some additional cases brian m. carlson
@ 2020-02-20 2:24 ` brian m. carlson
2020-02-20 2:24 ` [PATCH v2 5/5] credential: allow wildcard patterns when matching config brian m. carlson
2020-02-21 6:10 ` [PATCH v2 0/5] Wildcard matching for credentials Jeff King
5 siblings, 0 replies; 8+ messages in thread
From: brian m. carlson @ 2020-02-20 2:24 UTC (permalink / raw)
To: git; +Cc: Jeff King
From: brian m. carlson <bk2204@github.com>
Everywhere else in the codebase, we use the rule that the last matching
configuration option is the one that takes effect. This is helpful
because it allows more specific configuration settings (e.g., per-repo
configuration) to override less specific settings (e.g., per-user
configuration).
However, in the credential code, we didn't honor this setting, and
instead picked the first setting we had, and stuck with it. This was
likely to ensure we picked the value from the URL, which we want to
honor over the configuration.
It's possible to do both, though, so let's check if the value is the one
we've gotten over our protocol connection, which if present will have
come from the URL, and keep it if so. Otherwise, let's overwrite the
value with the latest version we've got from the configuration, so we
keep the last configuration value.
Signed-off-by: brian m. carlson <bk2204@github.com>
---
credential.c | 9 ++++++++-
credential.h | 3 ++-
t/t0300-credentials.sh | 2 +-
3 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/credential.c b/credential.c
index 62be651b03..5701e32792 100644
--- a/credential.c
+++ b/credential.c
@@ -71,8 +71,10 @@ static int credential_config_callback(const char *var, const char *value,
else
string_list_clear(&c->helpers, 0);
} else if (!strcmp(key, "username")) {
- if (!c->username)
+ if (!c->username_from_proto) {
+ free(c->username);
c->username = xstrdup(value);
+ }
}
else if (!strcmp(key, "usehttppath"))
c->use_http_path = git_config_bool(var, value);
@@ -163,6 +165,7 @@ int credential_read(struct credential *c, FILE *fp)
if (!strcmp(key, "username")) {
free(c->username);
c->username = xstrdup(value);
+ c->username_from_proto = 1;
} else if (!strcmp(key, "password")) {
free(c->password);
c->password = xstrdup(value);
@@ -349,10 +352,14 @@ void credential_from_url(struct credential *c, const char *url)
else if (!colon || at <= colon) {
/* Case (2) */
c->username = url_decode_mem(cp, at - cp);
+ if (c->username && *c->username)
+ c->username_from_proto = 1;
host = at + 1;
} else {
/* Case (3) */
c->username = url_decode_mem(cp, colon - cp);
+ if (c->username && *c->username)
+ c->username_from_proto = 1;
c->password = url_decode_mem(colon + 1, at - (colon + 1));
host = at + 1;
}
diff --git a/credential.h b/credential.h
index a5a3ee9bb8..fec7815dd0 100644
--- a/credential.h
+++ b/credential.h
@@ -118,7 +118,8 @@ struct credential {
unsigned approved:1,
configured:1,
quit:1,
- use_http_path:1;
+ use_http_path:1,
+ username_from_proto:1;
char *username;
char *password;
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 4593a0cd3d..8f87599056 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -344,7 +344,7 @@ test_expect_success 'honors username from URL over helper (components)' '
EOF
'
-test_expect_failure 'last matching username wins' '
+test_expect_success 'last matching username wins' '
test_config credential.https://example.com/path.git.username bob &&
test_config credential.https://example.com.username alice &&
test_config credential.https://example.com.helper "verbatim \"\" bar" &&
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 5/5] credential: allow wildcard patterns when matching config
2020-02-20 2:24 [PATCH v2 0/5] Wildcard matching for credentials brian m. carlson
` (3 preceding siblings ...)
2020-02-20 2:24 ` [PATCH v2 4/5] credential: use the last matching username in the config brian m. carlson
@ 2020-02-20 2:24 ` brian m. carlson
2020-02-20 21:20 ` Junio C Hamano
2020-02-21 6:10 ` [PATCH v2 0/5] Wildcard matching for credentials Jeff King
5 siblings, 1 reply; 8+ messages in thread
From: brian m. carlson @ 2020-02-20 2:24 UTC (permalink / raw)
To: git; +Cc: Jeff King
From: brian m. carlson <bk2204@github.com>
In some cases, a user will want to use a specific credential helper for
a wildcard pattern, such as https://*.corp.example.com. We have code
that handles this already with the urlmatch code, so let's use that
instead of our custom code.
Since the urlmatch code is a superset of our current matching in terms
of capabilities, there shouldn't be any cases of things that matched
previously that don't match now. However, in addition to wildcard
matching, we now use partial path matching, which can cause slightly
different behavior in the case that a helper applies to the prefix
(considering path components) of the remote URL. While different, this
is probably the behavior people were wanting anyway.
Since we're using the urlmatch code, we need to encode the components
we've gotten into a URL to match, so add a function to percent-encode
data and format the URL with it. We now also no longer need to the
custom code to match URLs, so let's remove it.
Additionally, the urlmatch code always looks for the best match, whereas
we want all matches for credential helpers to preserve existing
behavior. Let's add an optional field, select_fn, that lets us control
which items we want (in this case, all of them) and default it to the
best-match code that already exists for other users.
Signed-off-by: brian m. carlson <bk2204@github.com>
---
Documentation/gitcredentials.txt | 4 +-
credential.c | 66 +++++++++++++++++++++++---------
strbuf.c | 15 ++++++++
strbuf.h | 6 +++
t/t0300-credentials.sh | 20 ++++++++++
urlmatch.c | 4 +-
urlmatch.h | 9 +++++
7 files changed, 103 insertions(+), 21 deletions(-)
diff --git a/Documentation/gitcredentials.txt b/Documentation/gitcredentials.txt
index 5b9d14fb1f..1814d2d23c 100644
--- a/Documentation/gitcredentials.txt
+++ b/Documentation/gitcredentials.txt
@@ -131,7 +131,9 @@ context would not match:
because the hostnames differ. Nor would it match `foo.example.com`; Git
compares hostnames exactly, without considering whether two hosts are part of
the same domain. Likewise, a config entry for `http://example.com` would not
-match: Git compares the protocols exactly.
+match: Git compares the protocols exactly. However, you may use wildcards in
+the domain name and other pattern matching techniques as with the `http.<url>.*`
+options.
If the "pattern" URL does include a path component, then this too must match
exactly: the context `https://example.com/bar/baz.git` will match a config
diff --git a/credential.c b/credential.c
index 5701e32792..77dfde44e3 100644
--- a/credential.c
+++ b/credential.c
@@ -6,6 +6,7 @@
#include "url.h"
#include "prompt.h"
#include "sigchain.h"
+#include "urlmatch.h"
void credential_init(struct credential *c)
{
@@ -40,7 +41,7 @@ static int credential_config_callback(const char *var, const char *value,
void *data)
{
struct credential *c = data;
- const char *key, *dot;
+ const char *key;
if (!skip_prefix(var, "credential.", &key))
return 0;
@@ -48,23 +49,6 @@ static int credential_config_callback(const char *var, const char *value,
if (!value)
return config_error_nonbool(var);
- dot = strrchr(key, '.');
- if (dot) {
- struct credential want = CREDENTIAL_INIT;
- char *url = xmemdupz(key, dot - key);
- int matched;
-
- credential_from_url(&want, url);
- matched = credential_match(&want, c);
-
- credential_clear(&want);
- free(url);
-
- if (!matched)
- return 0;
- key = dot + 1;
- }
-
if (!strcmp(key, "helper")) {
if (*value)
string_list_append(&c->helpers, value);
@@ -89,11 +73,38 @@ static int proto_is_http(const char *s)
return !strcmp(s, "https") || !strcmp(s, "http");
}
+static void credential_describe(struct credential *c, struct strbuf *out);
+static void credential_format(struct credential *c, struct strbuf *out);
+
+static int select_all(const struct urlmatch_item *a,
+ const struct urlmatch_item *b)
+{
+ return 0;
+}
+
static void credential_apply_config(struct credential *c)
{
+ char *normalized_url;
+ struct urlmatch_config config = { STRING_LIST_INIT_DUP };
+ struct strbuf url = STRBUF_INIT;
+
if (c->configured)
return;
- git_config(credential_config_callback, c);
+
+ config.section = "credential";
+ config.key = NULL;
+ config.collect_fn = credential_config_callback;
+ config.cascade_fn = NULL;
+ config.select_fn = select_all;
+ config.cb = c;
+
+ credential_format(c, &url);
+ normalized_url = url_normalize(url.buf, &config.url);
+
+ git_config(urlmatch_config_entry, &config);
+ free(normalized_url);
+ strbuf_release(&url);
+
c->configured = 1;
if (!c->use_http_path && proto_is_http(c->protocol)) {
@@ -114,6 +125,23 @@ static void credential_describe(struct credential *c, struct strbuf *out)
strbuf_addf(out, "/%s", c->path);
}
+static void credential_format(struct credential *c, struct strbuf *out)
+{
+ if (!c->protocol)
+ return;
+ strbuf_addf(out, "%s://", c->protocol);
+ if (c->username && *c->username) {
+ strbuf_add_percentencode(out, c->username);
+ strbuf_addch(out, '@');
+ }
+ if (c->host)
+ strbuf_addstr(out, c->host);
+ if (c->path) {
+ strbuf_addch(out, '/');
+ strbuf_add_percentencode(out, c->path);
+ }
+}
+
static char *credential_ask_one(const char *what, struct credential *c,
int flags)
{
diff --git a/strbuf.c b/strbuf.c
index f19da55b07..bb0065ccaf 100644
--- a/strbuf.c
+++ b/strbuf.c
@@ -479,6 +479,21 @@ void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src)
}
}
+#define URL_UNSAFE_CHARS " <>\"%{}|\\^`:/?#[]@!$&'()*+,;="
+
+void strbuf_add_percentencode(struct strbuf *dst, const char *src)
+{
+ size_t i, len = strlen(src);
+
+ for (i = 0; i < len; i++) {
+ unsigned char ch = src[i];
+ if (ch <= 0x1F || ch >= 0x7F || strchr(URL_UNSAFE_CHARS, ch))
+ strbuf_addf(dst, "%%%02X", (unsigned char)ch);
+ else
+ strbuf_addch(dst, ch);
+ }
+}
+
size_t strbuf_fread(struct strbuf *sb, size_t size, FILE *f)
{
size_t res;
diff --git a/strbuf.h b/strbuf.h
index aae7ac3a82..ce8e49c0b2 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -378,6 +378,12 @@ size_t strbuf_expand_dict_cb(struct strbuf *sb,
*/
void strbuf_addbuf_percentquote(struct strbuf *dst, const struct strbuf *src);
+/**
+ * Append the contents of a string to a strbuf, percent-encoding any characters
+ * that are needed to be encoded for a URL.
+ */
+void strbuf_add_percentencode(struct strbuf *dst, const char *src);
+
/**
* Append the given byte size as a human-readable string (i.e. 12.23 KiB,
* 3.50 MiB).
diff --git a/t/t0300-credentials.sh b/t/t0300-credentials.sh
index 8f87599056..39f097ea9e 100755
--- a/t/t0300-credentials.sh
+++ b/t/t0300-credentials.sh
@@ -397,6 +397,26 @@ test_expect_success 'http paths can be part of context' '
EOF
'
+test_expect_success 'context uses urlmatch' '
+ test_config "credential.https://*.org.useHttpPath" true &&
+ check fill "verbatim foo bar" <<-\EOF
+ protocol=https
+ host=example.org
+ path=foo.git
+ --
+ protocol=https
+ host=example.org
+ path=foo.git
+ username=foo
+ password=bar
+ --
+ verbatim: get
+ verbatim: protocol=https
+ verbatim: host=example.org
+ verbatim: path=foo.git
+ EOF
+'
+
test_expect_success 'helpers can abort the process' '
test_must_fail git \
-c credential.helper="!f() { echo quit=1; }; f" \
diff --git a/urlmatch.c b/urlmatch.c
index 3e42bd7504..29272a5c4f 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -557,6 +557,8 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
const char *key, *dot;
struct strbuf synthkey = STRBUF_INIT;
int retval;
+ int (*select_fn)(const struct urlmatch_item *a, const struct urlmatch_item *b) =
+ collect->select_fn ? collect->select_fn : cmp_matches;
if (!skip_prefix(var, collect->section, &key) || *(key++) != '.') {
if (collect->cascade_fn)
@@ -587,7 +589,7 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
if (!item->util) {
item->util = xcalloc(1, sizeof(matched));
} else {
- if (cmp_matches(&matched, item->util) < 0)
+ if (select_fn(&matched, item->util) < 0)
/*
* Our match is worse than the old one,
* we cannot use it.
diff --git a/urlmatch.h b/urlmatch.h
index eed5f29235..2407520731 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -50,6 +50,15 @@ struct urlmatch_config {
void *cb;
int (*collect_fn)(const char *var, const char *value, void *cb);
int (*cascade_fn)(const char *var, const char *value, void *cb);
+ /*
+ * Compare the two matches, the one just discovered and the existing
+ * best match and return a negative value if the found item is to be
+ * rejected or a non-negative value if it is to be accepted. If this
+ * field is set to NULL, use the default comparison technique, which
+ * checks to ses if found is better (according to the urlmatch
+ * specificity rules) than existing.
+ */
+ int (*select_fn)(const struct urlmatch_item *found, const struct urlmatch_item *existing);
};
int urlmatch_config_entry(const char *var, const char *value, void *cb);
^ permalink raw reply related [flat|nested] 8+ messages in thread