[3.17] 468985: Gerrit Connector not working when using Gerrit HTTP Auth
Change-Id: Iaaab2081c713daa5c1b3cd63f2688862f44c21b2
Task-Url: https://bugs.eclipse.org/bugs/show_bug.cgi?id=468985
diff --git a/org.eclipse.mylyn.gerrit.core.tests/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClientTest.java b/org.eclipse.mylyn.gerrit.core.tests/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClientTest.java
index 8a1c232..1d578da 100644
--- a/org.eclipse.mylyn.gerrit.core.tests/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClientTest.java
+++ b/org.eclipse.mylyn.gerrit.core.tests/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClientTest.java
@@ -13,20 +13,34 @@
package org.eclipse.mylyn.internal.gerrit.core.client;
import static org.junit.Assert.assertArrayEquals;
+import static org.junit.Assert.assertEquals;
+import static org.mockito.Matchers.any;
+import static org.mockito.Matchers.isA;
+import static org.mockito.Mockito.inOrder;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.spy;
+import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
import java.io.IOException;
import org.apache.commons.httpclient.HttpMethodBase;
+import org.apache.commons.httpclient.HttpStatus;
+import org.apache.commons.httpclient.methods.GetMethod;
+import org.apache.commons.httpclient.methods.PostMethod;
import org.eclipse.core.runtime.AssertionFailedException;
import org.eclipse.core.runtime.IProgressMonitor;
+import org.eclipse.core.runtime.NullProgressMonitor;
import org.eclipse.mylyn.commons.net.AbstractWebLocation;
+import org.eclipse.mylyn.commons.net.AuthenticationCredentials;
+import org.eclipse.mylyn.commons.net.AuthenticationType;
import org.eclipse.mylyn.internal.gerrit.core.client.GerritHttpClient.JsonEntity;
import org.eclipse.mylyn.internal.gerrit.core.client.GerritHttpClient.Request;
import org.eclipse.mylyn.internal.gerrit.core.client.GerritHttpClient.Request.HttpMethod;
import org.junit.Test;
import org.junit.runner.RunWith;
+import org.mockito.InOrder;
import org.mockito.Mock;
import org.mockito.runners.MockitoJUnitRunner;
@@ -39,6 +53,30 @@
*/
@RunWith(MockitoJUnitRunner.class)
public class GerritHttpClientTest {
+ public class TestGerritHttpClient extends GerritHttpClient {
+ private final int code;
+
+ private TestGerritHttpClient(AbstractWebLocation location, int code) {
+ super(location);
+ this.code = code;
+ }
+
+ @Override
+ public int execute(org.apache.commons.httpclient.HttpMethod method, IProgressMonitor monitor)
+ throws IOException {
+ return code;
+ }
+
+ @Override
+ void requestCredentials(IProgressMonitor monitor, AuthenticationType authenticationType)
+ throws GerritLoginException {
+ }
+
+ @Override
+ String getUrl() {
+ return "http://mock";
+ }
+ }
@Mock
AbstractWebLocation abstractWebLocation;
@@ -97,4 +135,26 @@
assertArrayEquals(binary, result);
}
+ @Test
+ public void authenticateForm() throws IOException, GerritException {
+ GerritHttpClient client = spy(new TestGerritHttpClient(abstractWebLocation, HttpStatus.SC_BAD_REQUEST));
+ int result = client.authenticateForm(new AuthenticationCredentials("", ""), new NullProgressMonitor());
+ assertEquals(HttpStatus.SC_NOT_FOUND, result);
+ InOrder inOrder = inOrder(client);
+ inOrder.verify(client).execute(isA(PostMethod.class), any(IProgressMonitor.class));
+ inOrder.verify(client).execute(isA(GetMethod.class), any(IProgressMonitor.class));
+
+ client = spy(new TestGerritHttpClient(abstractWebLocation, HttpStatus.SC_METHOD_NOT_ALLOWED));
+ result = client.authenticateForm(new AuthenticationCredentials("", ""), new NullProgressMonitor());
+ assertEquals(HttpStatus.SC_NOT_FOUND, result);
+ inOrder = inOrder(client);
+ inOrder.verify(client).execute(isA(PostMethod.class), any(IProgressMonitor.class));
+ inOrder.verify(client).execute(isA(GetMethod.class), any(IProgressMonitor.class));
+
+ client = spy(new TestGerritHttpClient(abstractWebLocation, HttpStatus.SC_UNAUTHORIZED));
+ result = client.authenticateForm(new AuthenticationCredentials("", ""), new NullProgressMonitor());
+ assertEquals(-1, result);
+ verify(client).execute(isA(PostMethod.class), any(IProgressMonitor.class));
+ verify(client, never()).execute(isA(GetMethod.class), any(IProgressMonitor.class));
+ }
}
diff --git a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClient.java b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClient.java
index 02dc3cb..436e10e 100644
--- a/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClient.java
+++ b/org.eclipse.mylyn.gerrit.core/src/org/eclipse/mylyn/internal/gerrit/core/client/GerritHttpClient.java
@@ -336,7 +336,7 @@
}
try {
// Execute the method.
- WebUtil.execute(httpClient, hostConfiguration, method, monitor);
+ execute(method, monitor);
} catch (IOException e) {
WebUtil.releaseConnection(method, monitor);
throw e;
@@ -386,7 +386,7 @@
method.setFollowRedirects(false);
int code;
try {
- code = WebUtil.execute(httpClient, hostConfiguration, method, monitor);
+ code = execute(method, monitor);
if (code == HttpStatus.SC_OK) {
InputStream in = WebUtil.getResponseBodyAsStream(method, monitor);
try {
@@ -412,7 +412,7 @@
GetMethod method = new GetMethod(getUrl() + serviceUri);
try {
// Execute the method.
- WebUtil.execute(httpClient, hostConfiguration, method, monitor);
+ execute(method, monitor);
return method;
} catch (IOException e) {
WebUtil.releaseConnection(method, monitor);
@@ -497,7 +497,7 @@
JsonRequest jsonRequest = new JsonRequest(GerritConnector.GERRIT_RPC_URI + "OpenIdService", entity); //$NON-NLS-1$
PostMethod method = jsonRequest.createMethod();
try {
- int code = WebUtil.execute(httpClient, hostConfiguration, method, monitor);
+ int code = execute(method, monitor);
if (needsReauthentication(code, monitor)) {
return -1;
}
@@ -540,7 +540,7 @@
GetMethod validateMethod = new GetMethod(openIdResponse.getResponseUrl());
try {
// Execute the method.
- WebUtil.execute(httpClient, hostConfiguration, validateMethod, monitor);
+ execute(validateMethod, monitor);
} catch (IOException e) {
WebUtil.releaseConnection(method, monitor);
throw e;
@@ -574,7 +574,7 @@
JsonRequest jsonRequest = new JsonRequest(GerritConnector.GERRIT_RPC_URI + "UserPassAuthService", entity); //$NON-NLS-1$
PostMethod method = jsonRequest.createMethod();
try {
- int code = WebUtil.execute(httpClient, hostConfiguration, method, monitor);
+ int code = execute(method, monitor);
if (needsReauthentication(code, monitor)) {
return -1;
}
@@ -618,7 +618,7 @@
GetMethod method = new GetMethod(requestPath);
method.setFollowRedirects(false);
try {
- int code = WebUtil.execute(httpClient, hostConfiguration, method, monitor);
+ int code = execute(method, monitor);
if (needsReauthentication(code, monitor)) {
return -1;
}
@@ -641,7 +641,7 @@
return HttpStatus.SC_NOT_FOUND;
}
- private int authenticateForm(AuthenticationCredentials credentials, IProgressMonitor monitor) throws IOException,
+ int authenticateForm(AuthenticationCredentials credentials, IProgressMonitor monitor) throws IOException,
GerritException {
// try standard basic/digest/ntlm authentication first
String repositoryUrl = getUrl();
@@ -654,8 +654,8 @@
for (HttpMethodBase method : methods) {
int code;
try {
- code = WebUtil.execute(httpClient, hostConfiguration, method, monitor);
- if (code == HttpStatus.SC_METHOD_NOT_ALLOWED) {
+ code = execute(method, monitor);
+ if (code == HttpStatus.SC_BAD_REQUEST || code == HttpStatus.SC_METHOD_NOT_ALLOWED) {
continue; // try next http method
} else if (needsReauthentication(code, monitor)) {
return -1;
@@ -681,6 +681,10 @@
return HttpStatus.SC_NOT_FOUND;
}
+ int execute(org.apache.commons.httpclient.HttpMethod method, IProgressMonitor monitor) throws IOException {
+ return WebUtil.execute(httpClient, hostConfiguration, method, monitor);
+ }
+
private HttpMethodBase[] getFormAuthMethods(String repositoryUrl, AuthenticationCredentials credentials) {
PostMethod post = new PostMethod(WebUtil.getRequestPath(repositoryUrl + LOGIN_URL));
post.setParameter("username", credentials.getUserName()); //$NON-NLS-1$
