opendev/gerrit
Code Issues Proposed changes

Allow CORS to use modifying REST API

Browse Source 
This supports integrations with other web applications by making it
possible for another website to send mutation XHRs to Gerrit.

Change-Id: Ifb40f7a5ab2fa53d484af2ccbc2162275b2b02e1
This commit is contained in:
Shawn Pearce 2017-06-10 11:14:20 -07:00
parent e1b391b1f5
commit 53ebad44d4
3 changed files with 95 additions and 45 deletions
Documentation
config-gerrit.txt
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change
CorsIT.java
gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi
RestApiServlet.java

9
Documentation/config-gerrit.txt

@ -3920,9 +3920,12 @@ Defaults to an empty list, meaning no additional TLDs are allowed.
[[site.allowOriginRegex]]site.allowOriginRegex::
+
List of regular expressions matching origins that should be permitted
to use the Gerrit REST API to read content. These should be trusted
applications as the sites may be able to use the user's credentials.
Only applies to GET and HEAD requests.
to use the full Gerrit REST API. These should be trusted applications,
as the sites may be able to use the user's credentials. Applies to
all requests, including state changing methods (PUT, DELETE, POST).
+
Expressions should not require trailing slash. For example a valid
pattern might be `https://build-status[.]example[.]com`.
+
By default, unset, denying all cross-origin requests.

77
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java

@ -18,16 +18,24 @@ import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS;
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS;
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN;
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_MAX_AGE;
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS;
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD;
import static com.google.common.net.HttpHeaders.AUTHORIZATION;
import static com.google.common.net.HttpHeaders.CONTENT_TYPE;
import static com.google.common.net.HttpHeaders.ORIGIN;
import static com.google.common.net.HttpHeaders.VARY;
import static com.google.common.truth.Truth.assertThat;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.PushOneCommit.Result;
import com.google.gerrit.acceptance.RestResponse;
import com.google.gerrit.testutil.ConfigSuite;
import java.util.Locale;
import java.util.stream.Stream;
import org.apache.http.Header;
import org.apache.http.client.fluent.Request;
import org.apache.http.message.BasicHeader;
@ -65,14 +73,14 @@ public class CorsIT extends AbstractDaemonTest {
}
@Test
public void putWithOriginRefused() throws Exception {
public void putWithOriginAccepted() throws Exception {
Result change = createChange();
String origin = "http://example.com";
RestResponse r =
adminRestSession.putWithHeader(
"/changes/" + change.getChangeId() + "/topic", new BasicHeader(ORIGIN, origin), "A");
r.assertOK();
checkCors(r, false, origin);
checkCors(r, true, origin);
}
@Test
@ -88,71 +96,90 @@ public class CorsIT extends AbstractDaemonTest {
RestResponse res = adminRestSession.execute(req);
res.assertOK();
String vary = res.getHeader(VARY);
assertThat(vary).named(VARY).isNotNull();
assertThat(Splitter.on(", ").splitToList(vary))
.containsExactly(ORIGIN, ACCESS_CONTROL_REQUEST_METHOD, ACCESS_CONTROL_REQUEST_HEADERS);
checkCors(res, true, origin);
}
@Test
public void preflightBadOrigin() throws Exception {
Result change = createChange();
Request req =
Request.Options(adminRestSession.url() + "/a/changes/" + change.getChangeId() + "/detail");
req.addHeader(ORIGIN, "http://evil.attacker");
req.addHeader(ACCESS_CONTROL_REQUEST_METHOD, "GET");
adminRestSession.execute(req).assertBadRequest();
}
@Test
public void preflightBadMethod() throws Exception {
Result change = createChange();
for (String method : new String[] {"POST", "PUT", "DELETE", "PATCH"}) {
Request req =
Request.Options(
adminRestSession.url() + "/a/changes/" + change.getChangeId() + "/detail");
req.addHeader(ORIGIN, "http://example.com");
req.addHeader(ACCESS_CONTROL_REQUEST_METHOD, method);
adminRestSession.execute(req).assertBadRequest();
}
Request req =
Request.Options(adminRestSession.url() + "/a/changes/" + change.getChangeId() + "/detail");
req.addHeader(ORIGIN, "http://example.com");
req.addHeader(ACCESS_CONTROL_REQUEST_METHOD, "CALL");
adminRestSession.execute(req).assertBadRequest();
}
@Test
public void preflightBadHeader() throws Exception {
Result change = createChange();
Request req =
Request.Options(adminRestSession.url() + "/a/changes/" + change.getChangeId() + "/detail");
req.addHeader(ORIGIN, "http://example.com");
req.addHeader(ACCESS_CONTROL_REQUEST_METHOD, "GET");
req.addHeader(ACCESS_CONTROL_REQUEST_HEADERS, "X-Gerrit-Auth");
req.addHeader(ACCESS_CONTROL_REQUEST_HEADERS, "X-Secret-Auth-Token");
adminRestSession.execute(req).assertBadRequest();
}
private RestResponse check(String url, boolean accept, String origin) throws Exception {
Header hdr = new BasicHeader(ORIGIN, origin);
RestResponse r = adminRestSession.getWithHeader(url, hdr);
r.assertOK();
if (accept) {
r.assertOK();
} else {
r.assertBadRequest();
}
checkCors(r, accept, origin);
return r;
}
private void checkCors(RestResponse r, boolean accept, String origin) {
String vary = r.getHeader(VARY);
assertThat(vary).named(VARY).isNotNull();
assertThat(Splitter.on(", ").splitToList(vary)).named(VARY).contains(ORIGIN);
String allowOrigin = r.getHeader(ACCESS_CONTROL_ALLOW_ORIGIN);
String allowCred = r.getHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS);
String maxAge = r.getHeader(ACCESS_CONTROL_MAX_AGE);
String allowMethods = r.getHeader(ACCESS_CONTROL_ALLOW_METHODS);
String allowHeaders = r.getHeader(ACCESS_CONTROL_ALLOW_HEADERS);
if (accept) {
assertThat(allowOrigin).isEqualTo(origin);
assertThat(allowCred).isEqualTo("true");
assertThat(allowMethods).isEqualTo("GET, OPTIONS");
assertThat(allowHeaders).isEqualTo("X-Requested-With");
assertThat(allowOrigin).named(ACCESS_CONTROL_ALLOW_ORIGIN).isEqualTo(origin);
assertThat(allowCred).named(ACCESS_CONTROL_ALLOW_CREDENTIALS).isEqualTo("true");
assertThat(maxAge).named(ACCESS_CONTROL_MAX_AGE).isEqualTo("600");
assertThat(allowMethods).named(ACCESS_CONTROL_ALLOW_METHODS).isNotNull();
assertThat(Splitter.on(", ").splitToList(allowMethods))
.named(ACCESS_CONTROL_ALLOW_METHODS)
.containsExactly("GET", "HEAD", "POST", "PUT", "DELETE", "OPTIONS");
assertThat(allowHeaders).named(ACCESS_CONTROL_ALLOW_HEADERS).isNotNull();
assertThat(Splitter.on(", ").splitToList(allowHeaders))
.named(ACCESS_CONTROL_ALLOW_HEADERS)
.containsExactlyElementsIn(
Stream.of(AUTHORIZATION, CONTENT_TYPE, "X-Gerrit-Auth", "X-Requested-With")
.map(s -> s.toLowerCase(Locale.US))
.collect(ImmutableSet.toImmutableSet()));
} else {
assertThat(allowOrigin).isNull();
assertThat(allowCred).isNull();
assertThat(allowMethods).isNull();
assertThat(allowHeaders).isNull();
assertThat(allowOrigin).named(ACCESS_CONTROL_ALLOW_ORIGIN).isNull();
assertThat(allowCred).named(ACCESS_CONTROL_ALLOW_CREDENTIALS).isNull();
assertThat(maxAge).named(ACCESS_CONTROL_MAX_AGE).isNull();
assertThat(allowMethods).named(ACCESS_CONTROL_ALLOW_METHODS).isNull();
assertThat(allowHeaders).named(ACCESS_CONTROL_ALLOW_HEADERS).isNull();
}
}
}

54
gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java

@ -20,8 +20,11 @@ import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_CREDENTIALS
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS;
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS;
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN;
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_MAX_AGE;
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS;
import static com.google.common.net.HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD;
import static com.google.common.net.HttpHeaders.AUTHORIZATION;
import static com.google.common.net.HttpHeaders.CONTENT_TYPE;
import static com.google.common.net.HttpHeaders.ORIGIN;
import static com.google.common.net.HttpHeaders.VARY;
import static java.math.RoundingMode.CEILING;
@ -53,7 +56,6 @@ import com.google.common.collect.Iterables;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.MultimapBuilder;
import com.google.common.collect.Streams;
import com.google.common.io.BaseEncoding;
import com.google.common.io.CountingOutputStream;
import com.google.common.math.IntMath;
@ -139,11 +141,13 @@ import java.util.Collections;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
import java.util.TreeMap;
import java.util.concurrent.TimeUnit;
import java.util.regex.Pattern;
import java.util.stream.Stream;
import java.util.zip.GZIPOutputStream;
import javax.servlet.ServletException;
import javax.servlet.http.HttpServlet;
@ -169,8 +173,13 @@ public class RestApiServlet extends HttpServlet {
// TODO: Remove when HttpServletResponse.SC_UNPROCESSABLE_ENTITY is available
private static final int SC_UNPROCESSABLE_ENTITY = 422;
private static final String X_REQUESTED_WITH = "X-Requested-With";
private static final String X_GERRIT_AUTH = "X-Gerrit-Auth";
private static final ImmutableSet<String> ALLOWED_CORS_METHODS =
ImmutableSet.of("GET", "HEAD", "POST", "PUT", "DELETE");
private static final ImmutableSet<String> ALLOWED_CORS_REQUEST_HEADERS =
ImmutableSet.of(X_REQUESTED_WITH);
Stream.of(AUTHORIZATION, CONTENT_TYPE, X_GERRIT_AUTH, X_REQUESTED_WITH)
.map(s -> s.toLowerCase(Locale.US))
.collect(ImmutableSet.toImmutableSet());
private static final int HEAP_EST_SIZE = 10 * 8 * 1024; // Presize 10 blocks.
@ -499,10 +508,14 @@ public class RestApiServlet extends HttpServlet {
}
}
private void checkCors(HttpServletRequest req, HttpServletResponse res) {
private void checkCors(HttpServletRequest req, HttpServletResponse res)
throws BadRequestException {
String origin = req.getHeader(ORIGIN);
if (isRead(req) && !Strings.isNullOrEmpty(origin) && isOriginAllowed(origin)) {
if (!Strings.isNullOrEmpty(origin)) {
res.addHeader(VARY, ORIGIN);
if (!isOriginAllowed(origin)) {
throw new BadRequestException("origin not allowed");
}
setCorsHeaders(res, origin);
}
}
@ -516,8 +529,10 @@ public class RestApiServlet extends HttpServlet {
private void doCorsPreflight(HttpServletRequest req, HttpServletResponse res)
throws BadRequestException {
CacheHeaders.setNotCacheable(res);
res.setHeader(
VARY, Joiner.on(", ").join(ImmutableList.of(ORIGIN, ACCESS_CONTROL_REQUEST_METHOD)));
setHeaderList(
res,
VARY,
ImmutableList.of(ORIGIN, ACCESS_CONTROL_REQUEST_METHOD, ACCESS_CONTROL_REQUEST_HEADERS));
String origin = req.getHeader(ORIGIN);
if (Strings.isNullOrEmpty(origin) || !isOriginAllowed(origin)) {
@ -525,20 +540,17 @@ public class RestApiServlet extends HttpServlet {
}
String method = req.getHeader(ACCESS_CONTROL_REQUEST_METHOD);
if (!"GET".equals(method) && !"HEAD".equals(method)) {
if (!ALLOWED_CORS_METHODS.contains(method)) {
throw new BadRequestException(method + " not allowed in CORS");
}
String headers = req.getHeader(ACCESS_CONTROL_REQUEST_HEADERS);
if (headers != null) {
res.addHeader(VARY, ACCESS_CONTROL_REQUEST_HEADERS);
String badHeader =
Streams.stream(Splitter.on(',').trimResults().split(headers))
.filter(h -> !ALLOWED_CORS_REQUEST_HEADERS.contains(h))
.findFirst()
.orElse(null);
if (badHeader != null) {
throw new BadRequestException(badHeader + " not allowed in CORS");
for (String reqHdr : Splitter.on(',').trimResults().split(headers)) {
if (!ALLOWED_CORS_REQUEST_HEADERS.contains(reqHdr.toLowerCase(Locale.US))) {
throw new BadRequestException(reqHdr + " not allowed in CORS");
}
}
}
@ -548,11 +560,19 @@ public class RestApiServlet extends HttpServlet {
res.setContentLength(0);
}
private void setCorsHeaders(HttpServletResponse res, String origin) {
private static void setCorsHeaders(HttpServletResponse res, String origin) {
res.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, origin);
res.setHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS, "true");
res.setHeader(ACCESS_CONTROL_ALLOW_METHODS, "GET, OPTIONS");
res.setHeader(ACCESS_CONTROL_ALLOW_HEADERS, Joiner.on(", ").join(ALLOWED_CORS_REQUEST_HEADERS));
res.setHeader(ACCESS_CONTROL_MAX_AGE, "600");
setHeaderList(
res,
ACCESS_CONTROL_ALLOW_METHODS,
Iterables.concat(ALLOWED_CORS_METHODS, ImmutableList.of("OPTIONS")));
setHeaderList(res, ACCESS_CONTROL_ALLOW_HEADERS, ALLOWED_CORS_REQUEST_HEADERS);
}
private static void setHeaderList(HttpServletResponse res, String name, Iterable<String> values) {
res.setHeader(name, Joiner.on(", ").join(values));
}
private boolean isOriginAllowed(String origin) {