TransportHttp: support preemptive Basic authentication
If the caller knows already HTTP Basic authentication will be needed and if it also already has the username and password, preemptive authentication is a little bit more efficient since it avoids the initial 401 response. Add a setPreemptiveBasicAuthentication(username, password) method to TransportHttp. Client code could call this for instance in a TransportConfigCallback. The method throws an IllegalStateException if it is called after an HTTP request has already been made. Additionally, a URI can include userinfo. Although it is not recommended to put passwords in URIs, JGit's URIish and also the Java URL and URI classes still allow it. The underlying HTTP connection may omit these fields though. If present, take these fields as additional source for preemptive Basic authentication if setPreemptiveBasicAuthentication() has not been called. No preemptive authentication will be done if the connection is redirected to a different host. Add tests. Bug: 541327 Change-Id: Id00b975e56a15b532de96f7bbce48106d992a22b Signed-off-by: Thomas Wolf <thomas.wolf@paranor.ch>
This commit is contained in:
parent
471ad49546
commit
58f4e6e7f8
|
@ -936,6 +936,35 @@ public void testInitialClone_RedirectForbidden() throws Exception {
|
|||
}
|
||||
}
|
||||
|
||||
private void assertFetchRequests(List<AccessEvent> requests, int index) {
|
||||
AccessEvent info = requests.get(index++);
|
||||
assertEquals("GET", info.getMethod());
|
||||
assertEquals(join(authURI, "info/refs"), info.getPath());
|
||||
assertEquals(1, info.getParameters().size());
|
||||
assertEquals("git-upload-pack", info.getParameter("service"));
|
||||
assertEquals(200, info.getStatus());
|
||||
assertEquals("application/x-git-upload-pack-advertisement",
|
||||
info.getResponseHeader(HDR_CONTENT_TYPE));
|
||||
if (!enableProtocolV2) {
|
||||
assertEquals("gzip", info.getResponseHeader(HDR_CONTENT_ENCODING));
|
||||
}
|
||||
|
||||
for (int i = index; i < requests.size(); i++) {
|
||||
AccessEvent service = requests.get(i);
|
||||
assertEquals("POST", service.getMethod());
|
||||
assertEquals(join(authURI, "git-upload-pack"), service.getPath());
|
||||
assertEquals(0, service.getParameters().size());
|
||||
assertNotNull("has content-length",
|
||||
service.getRequestHeader(HDR_CONTENT_LENGTH));
|
||||
assertNull("not chunked",
|
||||
service.getRequestHeader(HDR_TRANSFER_ENCODING));
|
||||
|
||||
assertEquals(200, service.getStatus());
|
||||
assertEquals("application/x-git-upload-pack-result",
|
||||
service.getResponseHeader(HDR_CONTENT_TYPE));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInitialClone_WithAuthentication() throws Exception {
|
||||
try (Repository dst = createBareRepository();
|
||||
|
@ -955,32 +984,161 @@ public void testInitialClone_WithAuthentication() throws Exception {
|
|||
assertEquals("GET", info.getMethod());
|
||||
assertEquals(401, info.getStatus());
|
||||
|
||||
info = requests.get(1);
|
||||
assertFetchRequests(requests, 1);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInitialClone_WithPreAuthentication() throws Exception {
|
||||
try (Repository dst = createBareRepository();
|
||||
Transport t = Transport.open(dst, authURI)) {
|
||||
assertFalse(dst.getObjectDatabase().has(A_txt));
|
||||
((TransportHttp) t).setPreemptiveBasicAuthentication(
|
||||
AppServer.username, AppServer.password);
|
||||
t.fetch(NullProgressMonitor.INSTANCE, mirror(master));
|
||||
assertTrue(dst.getObjectDatabase().has(A_txt));
|
||||
assertEquals(B, dst.exactRef(master).getObjectId());
|
||||
fsck(dst, B);
|
||||
}
|
||||
|
||||
List<AccessEvent> requests = getRequests();
|
||||
assertEquals(enableProtocolV2 ? 3 : 2, requests.size());
|
||||
|
||||
assertFetchRequests(requests, 0);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInitialClone_WithPreAuthenticationCleared()
|
||||
throws Exception {
|
||||
try (Repository dst = createBareRepository();
|
||||
Transport t = Transport.open(dst, authURI)) {
|
||||
assertFalse(dst.getObjectDatabase().has(A_txt));
|
||||
((TransportHttp) t).setPreemptiveBasicAuthentication(
|
||||
AppServer.username, AppServer.password);
|
||||
((TransportHttp) t).setPreemptiveBasicAuthentication(null, null);
|
||||
t.setCredentialsProvider(testCredentials);
|
||||
t.fetch(NullProgressMonitor.INSTANCE, mirror(master));
|
||||
assertTrue(dst.getObjectDatabase().has(A_txt));
|
||||
assertEquals(B, dst.exactRef(master).getObjectId());
|
||||
fsck(dst, B);
|
||||
}
|
||||
|
||||
List<AccessEvent> requests = getRequests();
|
||||
assertEquals(enableProtocolV2 ? 4 : 3, requests.size());
|
||||
|
||||
AccessEvent info = requests.get(0);
|
||||
assertEquals("GET", info.getMethod());
|
||||
assertEquals(join(authURI, "info/refs"), info.getPath());
|
||||
assertEquals(1, info.getParameters().size());
|
||||
assertEquals("git-upload-pack", info.getParameter("service"));
|
||||
assertEquals(200, info.getStatus());
|
||||
assertEquals("application/x-git-upload-pack-advertisement",
|
||||
info.getResponseHeader(HDR_CONTENT_TYPE));
|
||||
if (!enableProtocolV2) {
|
||||
assertEquals("gzip", info.getResponseHeader(HDR_CONTENT_ENCODING));
|
||||
assertEquals(401, info.getStatus());
|
||||
|
||||
assertFetchRequests(requests, 1);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInitialClone_PreAuthenticationTooLate() throws Exception {
|
||||
try (Repository dst = createBareRepository();
|
||||
Transport t = Transport.open(dst, authURI)) {
|
||||
assertFalse(dst.getObjectDatabase().has(A_txt));
|
||||
((TransportHttp) t).setPreemptiveBasicAuthentication(
|
||||
AppServer.username, AppServer.password);
|
||||
t.fetch(NullProgressMonitor.INSTANCE, mirror(master));
|
||||
assertTrue(dst.getObjectDatabase().has(A_txt));
|
||||
assertEquals(B, dst.exactRef(master).getObjectId());
|
||||
fsck(dst, B);
|
||||
List<AccessEvent> requests = getRequests();
|
||||
assertEquals(enableProtocolV2 ? 3 : 2, requests.size());
|
||||
assertFetchRequests(requests, 0);
|
||||
assertThrows(IllegalStateException.class,
|
||||
() -> ((TransportHttp) t).setPreemptiveBasicAuthentication(
|
||||
AppServer.username, AppServer.password));
|
||||
assertThrows(IllegalStateException.class, () -> ((TransportHttp) t)
|
||||
.setPreemptiveBasicAuthentication(null, null));
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInitialClone_WithWrongPreAuthenticationAndCredentialProvider()
|
||||
throws Exception {
|
||||
try (Repository dst = createBareRepository();
|
||||
Transport t = Transport.open(dst, authURI)) {
|
||||
assertFalse(dst.getObjectDatabase().has(A_txt));
|
||||
((TransportHttp) t).setPreemptiveBasicAuthentication(
|
||||
AppServer.username, AppServer.password + 'x');
|
||||
t.setCredentialsProvider(testCredentials);
|
||||
t.fetch(NullProgressMonitor.INSTANCE, mirror(master));
|
||||
assertTrue(dst.getObjectDatabase().has(A_txt));
|
||||
assertEquals(B, dst.exactRef(master).getObjectId());
|
||||
fsck(dst, B);
|
||||
}
|
||||
|
||||
for (int i = 2; i < requests.size(); i++) {
|
||||
AccessEvent service = requests.get(i);
|
||||
assertEquals("POST", service.getMethod());
|
||||
assertEquals(join(authURI, "git-upload-pack"), service.getPath());
|
||||
assertEquals(0, service.getParameters().size());
|
||||
assertNotNull("has content-length",
|
||||
service.getRequestHeader(HDR_CONTENT_LENGTH));
|
||||
assertNull("not chunked",
|
||||
service.getRequestHeader(HDR_TRANSFER_ENCODING));
|
||||
List<AccessEvent> requests = getRequests();
|
||||
assertEquals(enableProtocolV2 ? 4 : 3, requests.size());
|
||||
|
||||
assertEquals(200, service.getStatus());
|
||||
assertEquals("application/x-git-upload-pack-result",
|
||||
service.getResponseHeader(HDR_CONTENT_TYPE));
|
||||
AccessEvent info = requests.get(0);
|
||||
assertEquals("GET", info.getMethod());
|
||||
assertEquals(401, info.getStatus());
|
||||
|
||||
assertFetchRequests(requests, 1);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInitialClone_WithWrongPreAuthentication() throws Exception {
|
||||
try (Repository dst = createBareRepository();
|
||||
Transport t = Transport.open(dst, authURI)) {
|
||||
assertFalse(dst.getObjectDatabase().has(A_txt));
|
||||
((TransportHttp) t).setPreemptiveBasicAuthentication(
|
||||
AppServer.username, AppServer.password + 'x');
|
||||
TransportException e = assertThrows(TransportException.class,
|
||||
() -> t.fetch(NullProgressMonitor.INSTANCE,
|
||||
mirror(master)));
|
||||
String msg = e.getMessage();
|
||||
assertTrue("Unexpected exception message: " + msg,
|
||||
msg.contains("no CredentialsProvider"));
|
||||
}
|
||||
List<AccessEvent> requests = getRequests();
|
||||
assertEquals(1, requests.size());
|
||||
|
||||
AccessEvent info = requests.get(0);
|
||||
assertEquals("GET", info.getMethod());
|
||||
assertEquals(401, info.getStatus());
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInitialClone_WithUserInfo() throws Exception {
|
||||
URIish withUserInfo = authURI.setUser(AppServer.username)
|
||||
.setPass(AppServer.password);
|
||||
try (Repository dst = createBareRepository();
|
||||
Transport t = Transport.open(dst, withUserInfo)) {
|
||||
assertFalse(dst.getObjectDatabase().has(A_txt));
|
||||
t.fetch(NullProgressMonitor.INSTANCE, mirror(master));
|
||||
assertTrue(dst.getObjectDatabase().has(A_txt));
|
||||
assertEquals(B, dst.exactRef(master).getObjectId());
|
||||
fsck(dst, B);
|
||||
}
|
||||
|
||||
List<AccessEvent> requests = getRequests();
|
||||
assertEquals(enableProtocolV2 ? 3 : 2, requests.size());
|
||||
|
||||
assertFetchRequests(requests, 0);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testInitialClone_PreAuthOverridesUserInfo() throws Exception {
|
||||
URIish withUserInfo = authURI.setUser(AppServer.username)
|
||||
.setPass(AppServer.password + 'x');
|
||||
try (Repository dst = createBareRepository();
|
||||
Transport t = Transport.open(dst, withUserInfo)) {
|
||||
assertFalse(dst.getObjectDatabase().has(A_txt));
|
||||
((TransportHttp) t).setPreemptiveBasicAuthentication(
|
||||
AppServer.username, AppServer.password);
|
||||
t.fetch(NullProgressMonitor.INSTANCE, mirror(master));
|
||||
assertTrue(dst.getObjectDatabase().has(A_txt));
|
||||
assertEquals(B, dst.exactRef(master).getObjectId());
|
||||
fsck(dst, B);
|
||||
}
|
||||
|
||||
List<AccessEvent> requests = getRequests();
|
||||
assertEquals(enableProtocolV2 ? 3 : 2, requests.size());
|
||||
|
||||
assertFetchRequests(requests, 0);
|
||||
}
|
||||
|
||||
@Test
|
||||
|
|
|
@ -312,6 +312,8 @@ hoursAgo={0} hours ago
|
|||
httpConfigCannotNormalizeURL=Cannot normalize URL path {0}: too many .. segments
|
||||
httpConfigInvalidURL=Cannot parse URL from subsection http.{0} in git config; ignored.
|
||||
httpFactoryInUse=Changing the HTTP connection factory after an HTTP connection has already been opened is not allowed.
|
||||
httpPreAuthTooLate=HTTP Basic preemptive authentication cannot be set once an HTTP connection has already been opened.
|
||||
httpUserInfoDecodeError=Cannot decode user info from URL {}; ignored.
|
||||
httpWrongConnectionType=Wrong connection type: expected {0}, got {1}.
|
||||
hugeIndexesAreNotSupportedByJgitYet=Huge indexes are not supported by jgit, yet
|
||||
hunkBelongsToAnotherFile=Hunk belongs to another file
|
||||
|
|
|
@ -340,6 +340,8 @@ public static JGitText get() {
|
|||
/***/ public String httpConfigCannotNormalizeURL;
|
||||
/***/ public String httpConfigInvalidURL;
|
||||
/***/ public String httpFactoryInUse;
|
||||
/***/ public String httpPreAuthTooLate;
|
||||
/***/ public String httpUserInfoDecodeError;
|
||||
/***/ public String httpWrongConnectionType;
|
||||
/***/ public String hugeIndexesAreNotSupportedByJgitYet;
|
||||
/***/ public String hunkBelongsToAnotherFile;
|
||||
|
|
|
@ -41,6 +41,7 @@
|
|||
import java.io.InputStreamReader;
|
||||
import java.io.InterruptedIOException;
|
||||
import java.io.OutputStream;
|
||||
import java.io.UnsupportedEncodingException;
|
||||
import java.net.HttpCookie;
|
||||
import java.net.MalformedURLException;
|
||||
import java.net.Proxy;
|
||||
|
@ -49,6 +50,7 @@
|
|||
import java.net.URI;
|
||||
import java.net.URISyntaxException;
|
||||
import java.net.URL;
|
||||
import java.net.URLDecoder;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.nio.file.InvalidPathException;
|
||||
import java.nio.file.Path;
|
||||
|
@ -408,6 +410,41 @@ public HttpConnectionFactory getHttpConnectionFactory() {
|
|||
return factory;
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets preemptive Basic HTTP authentication. If the given {@code username}
|
||||
* or {@code password} is empty or {@code null}, no preemptive
|
||||
* authentication will be done. If {@code username} and {@code password} are
|
||||
* set, they will override authority information from the URI
|
||||
* ("user:password@").
|
||||
* <p>
|
||||
* If the connection encounters redirects, the pre-authentication will be
|
||||
* cleared if the redirect goes to a different host.
|
||||
* </p>
|
||||
*
|
||||
* @param username
|
||||
* to use
|
||||
* @param password
|
||||
* to use
|
||||
* @throws IllegalStateException
|
||||
* if an HTTP/HTTPS connection has already been opened on this
|
||||
* {@link TransportHttp} instance
|
||||
* @since 5.11
|
||||
*/
|
||||
public void setPreemptiveBasicAuthentication(String username,
|
||||
String password) {
|
||||
if (factoryUsed) {
|
||||
throw new IllegalStateException(JGitText.get().httpPreAuthTooLate);
|
||||
}
|
||||
if (StringUtils.isEmptyOrNull(username)
|
||||
|| StringUtils.isEmptyOrNull(password)) {
|
||||
authMethod = authFromUri(currentUri);
|
||||
} else {
|
||||
HttpAuthMethod basic = HttpAuthMethod.Type.BASIC.method(null);
|
||||
basic.authorize(username, password);
|
||||
authMethod = basic;
|
||||
}
|
||||
}
|
||||
|
||||
/** {@inheritDoc} */
|
||||
@Override
|
||||
public FetchConnection openFetch() throws TransportException,
|
||||
|
@ -563,6 +600,28 @@ private NoRemoteRepositoryException createNotFoundException(URIish u,
|
|||
return new NoRemoteRepositoryException(u, text);
|
||||
}
|
||||
|
||||
private HttpAuthMethod authFromUri(URIish u) {
|
||||
String user = u.getUser();
|
||||
String pass = u.getPass();
|
||||
if (user != null && pass != null) {
|
||||
try {
|
||||
// User/password are _not_ application/x-www-form-urlencoded. In
|
||||
// particular the "+" sign would be replaced by a space.
|
||||
user = URLDecoder.decode(user.replace("+", "%2B"), //$NON-NLS-1$ //$NON-NLS-2$
|
||||
StandardCharsets.UTF_8.name());
|
||||
pass = URLDecoder.decode(pass.replace("+", "%2B"), //$NON-NLS-1$ //$NON-NLS-2$
|
||||
StandardCharsets.UTF_8.name());
|
||||
HttpAuthMethod basic = HttpAuthMethod.Type.BASIC.method(null);
|
||||
basic.authorize(user, pass);
|
||||
return basic;
|
||||
} catch (IllegalArgumentException
|
||||
| UnsupportedEncodingException e) {
|
||||
LOG.warn(JGitText.get().httpUserInfoDecodeError, u);
|
||||
}
|
||||
}
|
||||
return HttpAuthMethod.Type.NONE.method(null);
|
||||
}
|
||||
|
||||
private HttpConnection connect(String service)
|
||||
throws TransportException, NotSupportedException {
|
||||
return connect(service, null);
|
||||
|
@ -572,6 +631,9 @@ private HttpConnection connect(String service,
|
|||
TransferConfig.ProtocolVersion protocolVersion)
|
||||
throws TransportException, NotSupportedException {
|
||||
URL u = getServiceURL(service);
|
||||
if (HttpAuthMethod.Type.NONE.equals(authMethod.getType())) {
|
||||
authMethod = authFromUri(currentUri);
|
||||
}
|
||||
int authAttempts = 1;
|
||||
int redirects = 0;
|
||||
Collection<Type> ignoreTypes = null;
|
||||
|
@ -878,7 +940,13 @@ private URIish redirect(URL currentUrl, String location, String checkFor,
|
|||
}
|
||||
try {
|
||||
URI redirectTo = new URI(location);
|
||||
// Reset authentication if the redirect has user/password info or
|
||||
// if the host is different.
|
||||
boolean resetAuth = !StringUtils
|
||||
.isEmptyOrNull(redirectTo.getUserInfo());
|
||||
String currentHost = currentUrl.getHost();
|
||||
redirectTo = currentUrl.toURI().resolve(redirectTo);
|
||||
resetAuth = resetAuth || !currentHost.equals(redirectTo.getHost());
|
||||
String redirected = redirectTo.toASCIIString();
|
||||
if (!isValidRedirect(baseUrl, redirected, checkFor)) {
|
||||
throw new TransportException(uri,
|
||||
|
@ -887,6 +955,9 @@ private URIish redirect(URL currentUrl, String location, String checkFor,
|
|||
}
|
||||
redirected = redirected.substring(0, redirected.indexOf(checkFor));
|
||||
URIish result = new URIish(redirected);
|
||||
if (resetAuth) {
|
||||
authMethod = HttpAuthMethod.Type.NONE.method(null);
|
||||
}
|
||||
if (LOG.isInfoEnabled()) {
|
||||
LOG.info(MessageFormat.format(JGitText.get().redirectHttp,
|
||||
uri.setPass(null),
|
||||
|
|
Loading…
Reference in New Issue