Support faster cross-domain XHR calls
CORS preflight for POST, PUT, DELETE makes every mutation operation require 2 round trips with the server. This can increase latency for any application running on a different origin. There is a workaround available in modern browsers: use POST with Content-Type: text/plain. This does not require CORS preflight, as servers should already be using XSRF protection strategies. Unfortunately this is incompatible with the current REST API, as many operations require PUT or DELETE methods, and a Content-Type of application/json. Support the requester to select a different method using query parameter '$m' and Content-Type with '$ct' in the URL, mocking the request with those. Using this style of request still requires the user session to be valid for access. Accept identity through the query parameters as 'access_token'. The XSRF token isn't necessary in this type of request as only permitted websites would be allowed to read cookie content to obtain the GerritAccount cookie value and include it in the URL. Change-Id: Ic7bc5ad2e57eef27b0d2e13523be78e8a2d0a65c
This commit is contained in:
parent
53ebad44d4
commit
d79dcef4a6
Documentation
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change
gerrit-httpd/src
main/java/com/google/gerrit/httpd
test/java/com/google/gerrit/httpd/restapi
gerrit-pgm
gerrit-util-http/src/test/java/com/google/gerrit/util/http/testutil
@ -32,12 +32,41 @@ By default all REST endpoints assume anonymous access and filter
|
||||
results to correspond to what anonymous users can read (which may
|
||||
be nothing at all).
|
||||
|
||||
Users (and programs) may authenticate by prefixing the endpoint URL with
|
||||
`/a/`. For example to authenticate to `/projects/`, request the URL
|
||||
`/a/projects/`.
|
||||
Users (and programs) can authenticate with HTTP passwords by prefixing
|
||||
the endpoint URL with `/a/`. For example to authenticate to
|
||||
`/projects/`, request the URL `/a/projects/`. Gerrit will use HTTP basic
|
||||
authentication with the HTTP password from the user's account settings
|
||||
page. This form of authentication bypasses the need for XSRF tokens.
|
||||
|
||||
Gerrit uses HTTP basic authentication with the HTTP password from the
|
||||
user's account settings page.
|
||||
An authorization cookie may be presented in the request URL inside the
|
||||
`access_token` query parameter. XSRF tokens are not required when a
|
||||
valid `access_token` is used in the URL.
|
||||
|
||||
[[cors]]
|
||||
=== CORS
|
||||
|
||||
Cross-site scripting may be supported if the administrator has configured
|
||||
link:config-gerrit.html#site.allowOriginRegex[site.allowOriginRegex].
|
||||
|
||||
Approved web applications running from an allowed origin can rely on
|
||||
CORS preflight to authorize requests requiring cookie based
|
||||
authentication, or mutations (POST, PUT, DELETE). Mutations require a
|
||||
valid XSRF token in the `X-Gerrit-Auth` request header.
|
||||
|
||||
Alternatively applications can use `access_token` in the URL (see
|
||||
above) to authorize requests. Mutations sent as POST with a request
|
||||
content type of `text/plain` can skip CORS preflight. Gerrit accepts
|
||||
additional query parameters `$m` to override the correct method (PUT,
|
||||
POST, DELETE) and `$ct` to specify the actual content type, such as
|
||||
`application/json; charset=UTF-8`. Example:
|
||||
|
||||
----
|
||||
POST /changes/42/topic?$m=PUT&$ct=application/json%3B%20charset%3DUTF-8&access_token=secret HTTP/1.1
|
||||
Content-Type: text/plain
|
||||
Content-Length: 23
|
||||
|
||||
{"topic": "new-topic"}
|
||||
----
|
||||
|
||||
[[preconditions]]
|
||||
=== Preconditions
|
||||
|
@ -33,11 +33,18 @@ 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.extensions.common.ChangeInfo;
|
||||
import com.google.gerrit.server.UrlEncoded;
|
||||
import com.google.gerrit.testutil.ConfigSuite;
|
||||
import java.nio.charset.StandardCharsets;
|
||||
import java.util.Locale;
|
||||
import java.util.stream.Stream;
|
||||
import org.apache.http.Header;
|
||||
import org.apache.http.HttpResponse;
|
||||
import org.apache.http.client.fluent.Executor;
|
||||
import org.apache.http.client.fluent.Request;
|
||||
import org.apache.http.cookie.Cookie;
|
||||
import org.apache.http.impl.client.BasicCookieStore;
|
||||
import org.apache.http.message.BasicHeader;
|
||||
import org.eclipse.jgit.lib.Config;
|
||||
import org.junit.Test;
|
||||
@ -46,6 +53,7 @@ public class CorsIT extends AbstractDaemonTest {
|
||||
@ConfigSuite.Default
|
||||
public static Config allowExampleDotCom() {
|
||||
Config cfg = new Config();
|
||||
cfg.setString("auth", null, "type", "DEVELOPMENT_BECOME_ANY_ACCOUNT");
|
||||
cfg.setStringList(
|
||||
"site",
|
||||
null,
|
||||
@ -135,6 +143,50 @@ public class CorsIT extends AbstractDaemonTest {
|
||||
adminRestSession.execute(req).assertBadRequest();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void crossDomainPutTopic() throws Exception {
|
||||
Result change = createChange();
|
||||
BasicCookieStore cookies = new BasicCookieStore();
|
||||
Executor http = Executor.newInstance().cookieStore(cookies);
|
||||
|
||||
Request req = Request.Get(canonicalWebUrl.get() + "/login/?account_id=" + admin.id.get());
|
||||
HttpResponse r = http.execute(req).returnResponse();
|
||||
String auth = null;
|
||||
for (Cookie c : cookies.getCookies()) {
|
||||
if ("GerritAccount".equals(c.getName())) {
|
||||
auth = c.getValue();
|
||||
}
|
||||
}
|
||||
assertThat(auth).named("GerritAccount cookie").isNotNull();
|
||||
cookies.clear();
|
||||
|
||||
UrlEncoded url =
|
||||
new UrlEncoded(canonicalWebUrl.get() + "/changes/" + change.getChangeId() + "/topic");
|
||||
url.put("$m", "PUT");
|
||||
url.put("$ct", "application/json; charset=US-ASCII");
|
||||
url.put("access_token", auth);
|
||||
|
||||
String origin = "http://example.com";
|
||||
req = Request.Post(url.toString());
|
||||
req.setHeader(CONTENT_TYPE, "text/plain");
|
||||
req.setHeader(ORIGIN, origin);
|
||||
req.bodyByteArray("{\"topic\":\"test-xd\"}".getBytes(StandardCharsets.US_ASCII));
|
||||
|
||||
r = http.execute(req).returnResponse();
|
||||
assertThat(r.getStatusLine().getStatusCode()).isEqualTo(200);
|
||||
|
||||
Header vary = r.getFirstHeader(VARY);
|
||||
assertThat(vary).named(VARY).isNotNull();
|
||||
assertThat(Splitter.on(", ").splitToList(vary.getValue())).named(VARY).contains(ORIGIN);
|
||||
|
||||
Header allowOrigin = r.getFirstHeader(ACCESS_CONTROL_ALLOW_ORIGIN);
|
||||
assertThat(allowOrigin).named(ACCESS_CONTROL_ALLOW_ORIGIN).isNotNull();
|
||||
assertThat(allowOrigin.getValue()).named(ACCESS_CONTROL_ALLOW_ORIGIN).isEqualTo(origin);
|
||||
|
||||
ChangeInfo info = gApi.changes().id(change.getChangeId()).get();
|
||||
assertThat(info.topic).named("topic").isEqualTo("test-xd");
|
||||
}
|
||||
|
||||
private RestResponse check(String url, boolean accept, String origin) throws Exception {
|
||||
Header hdr = new BasicHeader(ORIGIN, origin);
|
||||
RestResponse r = adminRestSession.getWithHeader(url, hdr);
|
||||
|
@ -19,8 +19,10 @@ import static java.util.concurrent.TimeUnit.HOURS;
|
||||
import com.google.common.base.Strings;
|
||||
import com.google.gerrit.common.Nullable;
|
||||
import com.google.gerrit.common.data.HostPageData;
|
||||
import com.google.gerrit.extensions.restapi.BadRequestException;
|
||||
import com.google.gerrit.httpd.WebSessionManager.Key;
|
||||
import com.google.gerrit.httpd.WebSessionManager.Val;
|
||||
import com.google.gerrit.httpd.restapi.ParameterParser;
|
||||
import com.google.gerrit.reviewdb.client.Account;
|
||||
import com.google.gerrit.server.AccessPath;
|
||||
import com.google.gerrit.server.AnonymousUser;
|
||||
@ -70,31 +72,50 @@ public abstract class CacheBasedWebSession implements WebSession {
|
||||
this.identified = identified;
|
||||
|
||||
if (request.getRequestURI() == null || !GitSmartHttpTools.isGitClient(request)) {
|
||||
String cookie = readCookie();
|
||||
String cookie = readCookie(request);
|
||||
if (cookie != null) {
|
||||
key = new Key(cookie);
|
||||
val = manager.get(key);
|
||||
if (val != null && val.needsCookieRefresh()) {
|
||||
// Cookie is more than half old. Send the cookie again to the
|
||||
// client with an updated expiration date.
|
||||
val = manager.createVal(key, val);
|
||||
authFromCookie(cookie);
|
||||
} else {
|
||||
String token;
|
||||
try {
|
||||
token = ParameterParser.getQueryParams(request).accessToken();
|
||||
} catch (BadRequestException e) {
|
||||
token = null;
|
||||
}
|
||||
|
||||
String token = request.getHeader(HostPageData.XSRF_HEADER_NAME);
|
||||
if (val != null && token != null && token.equals(val.getAuth())) {
|
||||
okPaths.add(AccessPath.REST_API);
|
||||
if (token != null) {
|
||||
authFromQueryParameter(token);
|
||||
}
|
||||
}
|
||||
if (val != null && val.needsCookieRefresh()) {
|
||||
// Session is more than half old; update cache entry with new expiration date.
|
||||
val = manager.createVal(key, val);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private String readCookie() {
|
||||
final Cookie[] all = request.getCookies();
|
||||
private void authFromCookie(String cookie) {
|
||||
key = new Key(cookie);
|
||||
val = manager.get(key);
|
||||
String token = request.getHeader(HostPageData.XSRF_HEADER_NAME);
|
||||
if (val != null && token != null && token.equals(val.getAuth())) {
|
||||
okPaths.add(AccessPath.REST_API);
|
||||
}
|
||||
}
|
||||
|
||||
private void authFromQueryParameter(String accessToken) {
|
||||
key = new Key(accessToken);
|
||||
val = manager.get(key);
|
||||
if (val != null) {
|
||||
okPaths.add(AccessPath.REST_API);
|
||||
}
|
||||
}
|
||||
|
||||
private static String readCookie(HttpServletRequest request) {
|
||||
Cookie[] all = request.getCookies();
|
||||
if (all != null) {
|
||||
for (final Cookie c : all) {
|
||||
for (Cookie c : all) {
|
||||
if (ACCOUNT_COOKIE.equals(c.getName())) {
|
||||
final String v = c.getValue();
|
||||
return v != null && !"".equals(v) ? v : null;
|
||||
return Strings.emptyToNull(c.getValue());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -14,16 +14,24 @@
|
||||
|
||||
package com.google.gerrit.httpd.restapi;
|
||||
|
||||
import static com.google.gerrit.httpd.restapi.RestApiServlet.ALLOWED_CORS_METHODS;
|
||||
import static com.google.gerrit.httpd.restapi.RestApiServlet.XD_AUTHORIZATION;
|
||||
import static com.google.gerrit.httpd.restapi.RestApiServlet.XD_CONTENT_TYPE;
|
||||
import static com.google.gerrit.httpd.restapi.RestApiServlet.XD_METHOD;
|
||||
import static com.google.gerrit.httpd.restapi.RestApiServlet.replyBinaryResult;
|
||||
import static com.google.gerrit.httpd.restapi.RestApiServlet.replyError;
|
||||
import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
|
||||
|
||||
import com.google.auto.value.AutoValue;
|
||||
import com.google.common.annotations.VisibleForTesting;
|
||||
import com.google.common.base.Splitter;
|
||||
import com.google.common.base.Strings;
|
||||
import com.google.common.collect.ImmutableListMultimap;
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import com.google.common.collect.Iterables;
|
||||
import com.google.common.collect.ListMultimap;
|
||||
import com.google.common.collect.MultimapBuilder;
|
||||
import com.google.gerrit.common.Nullable;
|
||||
import com.google.gerrit.extensions.registration.DynamicMap;
|
||||
import com.google.gerrit.extensions.restapi.BadRequestException;
|
||||
import com.google.gerrit.extensions.restapi.BinaryResult;
|
||||
@ -47,10 +55,97 @@ import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import org.kohsuke.args4j.CmdLineException;
|
||||
|
||||
class ParameterParser {
|
||||
public class ParameterParser {
|
||||
private static final ImmutableSet<String> RESERVED_KEYS =
|
||||
ImmutableSet.of("pp", "prettyPrint", "strict", "callback", "alt", "fields");
|
||||
|
||||
@AutoValue
|
||||
public abstract static class QueryParams {
|
||||
static final String I = QueryParams.class.getName();
|
||||
|
||||
static QueryParams create(
|
||||
@Nullable String accessToken,
|
||||
@Nullable String xdMethod,
|
||||
@Nullable String xdContentType,
|
||||
ImmutableListMultimap<String, String> config,
|
||||
ImmutableListMultimap<String, String> params) {
|
||||
return new AutoValue_ParameterParser_QueryParams(
|
||||
accessToken, xdMethod, xdContentType, config, params);
|
||||
}
|
||||
|
||||
@Nullable
|
||||
public abstract String accessToken();
|
||||
|
||||
@Nullable
|
||||
abstract String xdMethod();
|
||||
|
||||
@Nullable
|
||||
abstract String xdContentType();
|
||||
|
||||
abstract ImmutableListMultimap<String, String> config();
|
||||
|
||||
abstract ImmutableListMultimap<String, String> params();
|
||||
|
||||
boolean hasXdOverride() {
|
||||
return xdMethod() != null || xdContentType() != null;
|
||||
}
|
||||
}
|
||||
|
||||
public static QueryParams getQueryParams(HttpServletRequest req) throws BadRequestException {
|
||||
QueryParams qp = (QueryParams) req.getAttribute(QueryParams.I);
|
||||
if (qp != null) {
|
||||
return qp;
|
||||
}
|
||||
|
||||
String accessToken = null;
|
||||
String xdMethod = null;
|
||||
String xdContentType = null;
|
||||
ListMultimap<String, String> config = MultimapBuilder.hashKeys(4).arrayListValues().build();
|
||||
ListMultimap<String, String> params = MultimapBuilder.hashKeys().arrayListValues().build();
|
||||
|
||||
String queryString = req.getQueryString();
|
||||
if (!Strings.isNullOrEmpty(queryString)) {
|
||||
for (String kvPair : Splitter.on('&').split(queryString)) {
|
||||
Iterator<String> i = Splitter.on('=').limit(2).split(kvPair).iterator();
|
||||
String key = Url.decode(i.next());
|
||||
String val = i.hasNext() ? Url.decode(i.next()) : "";
|
||||
|
||||
if (XD_AUTHORIZATION.equals(key)) {
|
||||
if (accessToken != null) {
|
||||
throw new BadRequestException("duplicate " + XD_AUTHORIZATION);
|
||||
}
|
||||
accessToken = val;
|
||||
} else if (XD_METHOD.equals(key)) {
|
||||
if (xdMethod != null) {
|
||||
throw new BadRequestException("duplicate " + XD_METHOD);
|
||||
} else if (!ALLOWED_CORS_METHODS.contains(val)) {
|
||||
throw new BadRequestException("invalid " + XD_METHOD);
|
||||
}
|
||||
xdMethod = val;
|
||||
} else if (XD_CONTENT_TYPE.equals(key)) {
|
||||
if (xdContentType != null) {
|
||||
throw new BadRequestException("duplicate " + XD_CONTENT_TYPE);
|
||||
}
|
||||
xdContentType = val;
|
||||
} else if (RESERVED_KEYS.contains(key)) {
|
||||
config.put(key, val);
|
||||
} else {
|
||||
params.put(key, val);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
qp =
|
||||
QueryParams.create(
|
||||
accessToken,
|
||||
xdMethod,
|
||||
xdContentType,
|
||||
ImmutableListMultimap.copyOf(config),
|
||||
ImmutableListMultimap.copyOf(params));
|
||||
req.setAttribute(QueryParams.I, qp);
|
||||
return qp;
|
||||
}
|
||||
|
||||
private final CmdLineParser.Factory parserFactory;
|
||||
private final Injector injector;
|
||||
private final DynamicMap<DynamicOptions.DynamicBean> dynamicBeans;
|
||||
@ -98,24 +193,6 @@ class ParameterParser {
|
||||
return true;
|
||||
}
|
||||
|
||||
static void splitQueryString(
|
||||
String queryString,
|
||||
ListMultimap<String, String> config,
|
||||
ListMultimap<String, String> params) {
|
||||
if (!Strings.isNullOrEmpty(queryString)) {
|
||||
for (String kvPair : Splitter.on('&').split(queryString)) {
|
||||
Iterator<String> i = Splitter.on('=').limit(2).split(kvPair).iterator();
|
||||
String key = Url.decode(i.next());
|
||||
String val = i.hasNext() ? Url.decode(i.next()) : "";
|
||||
if (RESERVED_KEYS.contains(key)) {
|
||||
config.put(key, val);
|
||||
} else {
|
||||
params.put(key, val);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
private static Set<String> query(HttpServletRequest req) {
|
||||
Set<String> params = new HashSet<>();
|
||||
if (!Strings.isNullOrEmpty(req.getQueryString())) {
|
||||
|
@ -55,7 +55,6 @@ import com.google.common.collect.ImmutableSet;
|
||||
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.io.BaseEncoding;
|
||||
import com.google.common.io.CountingOutputStream;
|
||||
import com.google.common.math.IntMath;
|
||||
@ -94,6 +93,7 @@ import com.google.gerrit.extensions.restapi.RestView;
|
||||
import com.google.gerrit.extensions.restapi.TopLevelResource;
|
||||
import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
|
||||
import com.google.gerrit.httpd.WebSession;
|
||||
import com.google.gerrit.httpd.restapi.ParameterParser.QueryParams;
|
||||
import com.google.gerrit.server.AccessPath;
|
||||
import com.google.gerrit.server.AnonymousUser;
|
||||
import com.google.gerrit.server.CurrentUser;
|
||||
@ -152,6 +152,7 @@ import java.util.zip.GZIPOutputStream;
|
||||
import javax.servlet.ServletException;
|
||||
import javax.servlet.http.HttpServlet;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletRequestWrapper;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import org.eclipse.jgit.http.server.ServletUtils;
|
||||
import org.eclipse.jgit.lib.Config;
|
||||
@ -174,13 +175,17 @@ public class RestApiServlet extends HttpServlet {
|
||||
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 =
|
||||
static final ImmutableSet<String> ALLOWED_CORS_METHODS =
|
||||
ImmutableSet.of("GET", "HEAD", "POST", "PUT", "DELETE");
|
||||
private static final ImmutableSet<String> ALLOWED_CORS_REQUEST_HEADERS =
|
||||
Stream.of(AUTHORIZATION, CONTENT_TYPE, X_GERRIT_AUTH, X_REQUESTED_WITH)
|
||||
.map(s -> s.toLowerCase(Locale.US))
|
||||
.collect(ImmutableSet.toImmutableSet());
|
||||
|
||||
public static final String XD_AUTHORIZATION = "access_token";
|
||||
public static final String XD_CONTENT_TYPE = "$ct";
|
||||
public static final String XD_METHOD = "$m";
|
||||
|
||||
private static final int HEAP_EST_SIZE = 10 * 8 * 1024; // Presize 10 blocks.
|
||||
|
||||
/**
|
||||
@ -261,8 +266,7 @@ public class RestApiServlet extends HttpServlet {
|
||||
int status = SC_OK;
|
||||
long responseBytes = -1;
|
||||
Object result = null;
|
||||
ListMultimap<String, String> params = MultimapBuilder.hashKeys().arrayListValues().build();
|
||||
ListMultimap<String, String> config = MultimapBuilder.hashKeys().arrayListValues().build();
|
||||
QueryParams qp = null;
|
||||
Object inputRequestBody = null;
|
||||
RestResource rsrc = TopLevelResource.INSTANCE;
|
||||
ViewData viewData = null;
|
||||
@ -272,10 +276,13 @@ public class RestApiServlet extends HttpServlet {
|
||||
doCorsPreflight(req, res);
|
||||
return;
|
||||
}
|
||||
checkCors(req, res);
|
||||
checkUserSession(req);
|
||||
|
||||
ParameterParser.splitQueryString(req.getQueryString(), config, params);
|
||||
qp = ParameterParser.getQueryParams(req);
|
||||
checkCors(req, res, qp.hasXdOverride());
|
||||
if (qp.hasXdOverride()) {
|
||||
req = applyXdOverrides(req, qp);
|
||||
}
|
||||
checkUserSession(req);
|
||||
|
||||
List<IdString> path = splitPath(req);
|
||||
RestCollection<RestResource, RestResource> rc = members.get();
|
||||
@ -288,7 +295,7 @@ public class RestApiServlet extends HttpServlet {
|
||||
|
||||
if (path.isEmpty()) {
|
||||
if (rc instanceof NeedsParams) {
|
||||
((NeedsParams) rc).setParams(params);
|
||||
((NeedsParams) rc).setParams(qp.params());
|
||||
}
|
||||
|
||||
if (isRead(req)) {
|
||||
@ -381,7 +388,7 @@ public class RestApiServlet extends HttpServlet {
|
||||
return;
|
||||
}
|
||||
|
||||
if (!globals.paramParser.get().parse(viewData.view, params, req, res)) {
|
||||
if (!globals.paramParser.get().parse(viewData.view, qp.params(), req, res)) {
|
||||
return;
|
||||
}
|
||||
|
||||
@ -424,7 +431,7 @@ public class RestApiServlet extends HttpServlet {
|
||||
if (result instanceof BinaryResult) {
|
||||
responseBytes = replyBinaryResult(req, res, (BinaryResult) result);
|
||||
} else {
|
||||
responseBytes = replyJson(req, res, config, result);
|
||||
responseBytes = replyJson(req, res, qp.config(), result);
|
||||
}
|
||||
}
|
||||
} catch (MalformedJsonException e) {
|
||||
@ -499,7 +506,7 @@ public class RestApiServlet extends HttpServlet {
|
||||
globals.currentUser.get(),
|
||||
req,
|
||||
auditStartTs,
|
||||
params,
|
||||
qp != null ? qp.params() : ImmutableListMultimap.of(),
|
||||
inputRequestBody,
|
||||
status,
|
||||
result,
|
||||
@ -508,7 +515,36 @@ public class RestApiServlet extends HttpServlet {
|
||||
}
|
||||
}
|
||||
|
||||
private void checkCors(HttpServletRequest req, HttpServletResponse res)
|
||||
private static HttpServletRequest applyXdOverrides(HttpServletRequest req, QueryParams qp)
|
||||
throws BadRequestException {
|
||||
if (!"POST".equals(req.getMethod())) {
|
||||
throw new BadRequestException("POST required");
|
||||
}
|
||||
|
||||
String method = qp.xdMethod();
|
||||
String contentType = qp.xdContentType();
|
||||
if (method.equals("POST") || method.equals("PUT")) {
|
||||
if (!"text/plain".equals(req.getContentType())) {
|
||||
throw new BadRequestException("invalid " + CONTENT_TYPE);
|
||||
} else if (Strings.isNullOrEmpty(contentType)) {
|
||||
throw new BadRequestException(XD_CONTENT_TYPE + " required");
|
||||
}
|
||||
}
|
||||
|
||||
return new HttpServletRequestWrapper(req) {
|
||||
@Override
|
||||
public String getMethod() {
|
||||
return method;
|
||||
}
|
||||
|
||||
@Override
|
||||
public String getContentType() {
|
||||
return contentType;
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
private void checkCors(HttpServletRequest req, HttpServletResponse res, boolean isXd)
|
||||
throws BadRequestException {
|
||||
String origin = req.getHeader(ORIGIN);
|
||||
if (!Strings.isNullOrEmpty(origin)) {
|
||||
@ -516,7 +552,13 @@ public class RestApiServlet extends HttpServlet {
|
||||
if (!isOriginAllowed(origin)) {
|
||||
throw new BadRequestException("origin not allowed");
|
||||
}
|
||||
setCorsHeaders(res, origin);
|
||||
if (isXd) {
|
||||
res.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, origin);
|
||||
} else {
|
||||
setCorsHeaders(res, origin);
|
||||
}
|
||||
} else if (isXd) {
|
||||
throw new BadRequestException("expected " + ORIGIN);
|
||||
}
|
||||
}
|
||||
|
||||
@ -1074,7 +1116,8 @@ public class RestApiServlet extends HttpServlet {
|
||||
throw new AmbiguousViewException(
|
||||
String.format(
|
||||
"Projection %s is ambiguous: %s",
|
||||
name, r.keySet().stream().map(in -> in + "~" + projection).collect(joining(", "))));
|
||||
name,
|
||||
r.keySet().stream().map(in -> in + "~" + projection).collect(joining(", "))));
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -14,11 +14,15 @@
|
||||
|
||||
package com.google.gerrit.httpd.restapi;
|
||||
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
import static org.junit.Assert.assertEquals;
|
||||
import static org.junit.Assert.fail;
|
||||
|
||||
import com.google.common.collect.ImmutableMap;
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import com.google.gerrit.extensions.restapi.BadRequestException;
|
||||
import com.google.gerrit.httpd.restapi.ParameterParser.QueryParams;
|
||||
import com.google.gerrit.util.http.testutil.FakeHttpServletRequest;
|
||||
import com.google.gson.JsonArray;
|
||||
import com.google.gson.JsonObject;
|
||||
import com.google.gson.JsonPrimitive;
|
||||
@ -49,4 +53,91 @@ public class ParameterParserTest {
|
||||
|
||||
assertEquals(exp, obj);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void parseQuery() throws BadRequestException {
|
||||
FakeHttpServletRequest req = new FakeHttpServletRequest();
|
||||
req.setQueryString("query=status%3aopen");
|
||||
QueryParams qp = ParameterParser.getQueryParams(req);
|
||||
assertThat(qp.accessToken()).isNull();
|
||||
assertThat(qp.xdMethod()).isNull();
|
||||
assertThat(qp.xdContentType()).isNull();
|
||||
assertThat(qp.hasXdOverride()).isFalse();
|
||||
assertThat(qp.config()).isEmpty();
|
||||
assertThat(qp.params()).containsKey("query");
|
||||
assertThat(qp.params().get("query")).containsExactly("status:open");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void parseAccessToken() throws BadRequestException {
|
||||
FakeHttpServletRequest req = new FakeHttpServletRequest();
|
||||
req.setQueryString("query=status%3aopen&access_token=secr%65t");
|
||||
QueryParams qp = ParameterParser.getQueryParams(req);
|
||||
assertThat(qp.accessToken()).isEqualTo("secret");
|
||||
assertThat(qp.xdMethod()).isNull();
|
||||
assertThat(qp.xdContentType()).isNull();
|
||||
assertThat(qp.hasXdOverride()).isFalse();
|
||||
assertThat(qp.config()).isEmpty();
|
||||
assertThat(qp.params()).containsKey("query");
|
||||
assertThat(qp.params().get("query")).containsExactly("status:open");
|
||||
|
||||
req = new FakeHttpServletRequest();
|
||||
req.setQueryString("access_token=secret");
|
||||
qp = ParameterParser.getQueryParams(req);
|
||||
assertThat(qp.accessToken()).isEqualTo("secret");
|
||||
assertThat(qp.xdMethod()).isNull();
|
||||
assertThat(qp.xdContentType()).isNull();
|
||||
assertThat(qp.hasXdOverride()).isFalse();
|
||||
assertThat(qp.config()).isEmpty();
|
||||
assertThat(qp.params()).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void parseXdOverride() throws BadRequestException {
|
||||
FakeHttpServletRequest req = new FakeHttpServletRequest();
|
||||
req.setQueryString("$m=PUT&$ct=json&access_token=secret");
|
||||
QueryParams qp = ParameterParser.getQueryParams(req);
|
||||
assertThat(qp.accessToken()).isEqualTo("secret");
|
||||
assertThat(qp.xdMethod()).isEqualTo("PUT");
|
||||
assertThat(qp.xdContentType()).isEqualTo("json");
|
||||
assertThat(qp.hasXdOverride()).isTrue();
|
||||
assertThat(qp.config()).isEmpty();
|
||||
assertThat(qp.params()).isEmpty();
|
||||
}
|
||||
|
||||
@Test
|
||||
public void rejectDuplicateMethod() {
|
||||
FakeHttpServletRequest req = new FakeHttpServletRequest();
|
||||
req.setQueryString("$m=PUT&$m=DELETE");
|
||||
try {
|
||||
ParameterParser.getQueryParams(req);
|
||||
fail("expected BadRequestException");
|
||||
} catch (BadRequestException bad) {
|
||||
assertThat(bad).hasMessageThat().isEqualTo("duplicate $m");
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void rejectDuplicateContentType() {
|
||||
FakeHttpServletRequest req = new FakeHttpServletRequest();
|
||||
req.setQueryString("$ct=json&$ct=string");
|
||||
try {
|
||||
ParameterParser.getQueryParams(req);
|
||||
fail("expected BadRequestException");
|
||||
} catch (BadRequestException bad) {
|
||||
assertThat(bad).hasMessageThat().isEqualTo("duplicate $ct");
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
public void rejectInvalidMethod() {
|
||||
FakeHttpServletRequest req = new FakeHttpServletRequest();
|
||||
req.setQueryString("$m=CONNECT");
|
||||
try {
|
||||
ParameterParser.getQueryParams(req);
|
||||
fail("expected BadRequestException");
|
||||
} catch (BadRequestException bad) {
|
||||
assertThat(bad).hasMessageThat().isEqualTo("invalid $m");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -156,6 +156,7 @@ junit_tests(
|
||||
name = "pgm_tests",
|
||||
srcs = glob(["src/test/java/**/*.java"]),
|
||||
deps = [
|
||||
":http-jetty",
|
||||
":init",
|
||||
":init-api",
|
||||
":pgm",
|
||||
@ -163,6 +164,7 @@ junit_tests(
|
||||
"//gerrit-server:server",
|
||||
"//lib:guava",
|
||||
"//lib:junit",
|
||||
"//lib:truth",
|
||||
"//lib/easymock",
|
||||
"//lib/guice",
|
||||
"//lib/jgit/org.eclipse.jgit:jgit",
|
||||
|
@ -14,10 +14,18 @@
|
||||
|
||||
package com.google.gerrit.pgm.http.jetty;
|
||||
|
||||
import static com.google.gerrit.httpd.restapi.RestApiServlet.XD_AUTHORIZATION;
|
||||
|
||||
import com.google.common.annotations.VisibleForTesting;
|
||||
import com.google.common.base.Splitter;
|
||||
import com.google.common.base.Strings;
|
||||
import com.google.common.collect.ImmutableSet;
|
||||
import com.google.gerrit.common.TimeUtil;
|
||||
import com.google.gerrit.extensions.restapi.Url;
|
||||
import com.google.gerrit.httpd.GetUserFilter;
|
||||
import com.google.gerrit.server.util.SystemLog;
|
||||
import com.google.inject.Inject;
|
||||
import java.util.Iterator;
|
||||
import org.apache.log4j.AsyncAppender;
|
||||
import org.apache.log4j.Level;
|
||||
import org.apache.log4j.Logger;
|
||||
@ -31,6 +39,7 @@ import org.eclipse.jetty.util.component.AbstractLifeCycle;
|
||||
class HttpLog extends AbstractLifeCycle implements RequestLog {
|
||||
private static final Logger log = Logger.getLogger(HttpLog.class);
|
||||
private static final String LOG_NAME = "httpd_log";
|
||||
private static final ImmutableSet<String> REDACT_PARAM = ImmutableSet.of(XD_AUTHORIZATION);
|
||||
|
||||
interface HttpLogFactory {
|
||||
HttpLog get();
|
||||
@ -78,10 +87,7 @@ class HttpLog extends AbstractLifeCycle implements RequestLog {
|
||||
);
|
||||
|
||||
String uri = req.getRequestURI();
|
||||
String qs = req.getQueryString();
|
||||
if (qs != null) {
|
||||
uri = uri + "?" + qs;
|
||||
}
|
||||
uri = redactQueryString(uri, req.getQueryString());
|
||||
|
||||
String user = (String) req.getAttribute(GetUserFilter.REQ_ATTR_KEY);
|
||||
if (user != null) {
|
||||
@ -100,6 +106,31 @@ class HttpLog extends AbstractLifeCycle implements RequestLog {
|
||||
async.append(event);
|
||||
}
|
||||
|
||||
@VisibleForTesting
|
||||
static String redactQueryString(String uri, String qs) {
|
||||
if (Strings.isNullOrEmpty(qs)) {
|
||||
return uri;
|
||||
}
|
||||
|
||||
StringBuilder b = new StringBuilder(uri);
|
||||
boolean first = true;
|
||||
for (String kvPair : Splitter.on('&').split(qs)) {
|
||||
Iterator<String> i = Splitter.on('=').limit(2).split(kvPair).iterator();
|
||||
String key = i.next();
|
||||
b.append(first ? '?' : '&').append(key);
|
||||
first = false;
|
||||
if (i.hasNext()) {
|
||||
b.append('=');
|
||||
if (REDACT_PARAM.contains(Url.decode(key))) {
|
||||
b.append('*');
|
||||
} else {
|
||||
b.append(i.next());
|
||||
}
|
||||
}
|
||||
}
|
||||
return b.toString();
|
||||
}
|
||||
|
||||
private static void set(LoggingEvent event, String key, String val) {
|
||||
if (val != null && !val.isEmpty()) {
|
||||
event.setProperty(key, val);
|
||||
|
@ -0,0 +1,46 @@
|
||||
// Copyright (C) 2017 The Android Open Source Project
|
||||
//
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
package com.google.gerrit.pgm.http.jetty;
|
||||
|
||||
import static com.google.common.truth.Truth.assertThat;
|
||||
|
||||
import org.junit.Test;
|
||||
|
||||
public class HttpLogRedactTest {
|
||||
@Test
|
||||
public void includeQueryString() {
|
||||
assertThat(HttpLog.redactQueryString("/changes/", null)).isEqualTo("/changes/");
|
||||
assertThat(HttpLog.redactQueryString("/changes/", "")).isEqualTo("/changes/");
|
||||
assertThat(HttpLog.redactQueryString("/changes/", "x")).isEqualTo("/changes/?x");
|
||||
assertThat(HttpLog.redactQueryString("/changes/", "x=y")).isEqualTo("/changes/?x=y");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void redactAuth() {
|
||||
assertThat(HttpLog.redactQueryString("/changes/", "query=status:open"))
|
||||
.isEqualTo("/changes/?query=status:open");
|
||||
|
||||
assertThat(HttpLog.redactQueryString("/changes/", "query=status:open&access_token=foo"))
|
||||
.isEqualTo("/changes/?query=status:open&access_token=*");
|
||||
|
||||
assertThat(HttpLog.redactQueryString("/changes/", "access_token=foo"))
|
||||
.isEqualTo("/changes/?access_token=*");
|
||||
|
||||
assertThat(
|
||||
HttpLog.redactQueryString(
|
||||
"/changes/", "query=status:open&access_token=foo&access_token=bar"))
|
||||
.isEqualTo("/changes/?query=status:open&access_token=*&access_token=*");
|
||||
}
|
||||
}
|
@ -19,11 +19,11 @@ import static com.google.common.base.Preconditions.checkNotNull;
|
||||
import static java.nio.charset.StandardCharsets.UTF_8;
|
||||
|
||||
import com.google.common.base.Splitter;
|
||||
import com.google.common.base.Strings;
|
||||
import com.google.common.collect.Iterables;
|
||||
import com.google.common.collect.LinkedListMultimap;
|
||||
import com.google.common.collect.ListMultimap;
|
||||
import com.google.common.collect.Maps;
|
||||
import com.google.gerrit.extensions.restapi.Url;
|
||||
import java.io.BufferedReader;
|
||||
import java.io.UnsupportedEncodingException;
|
||||
import java.net.URLDecoder;
|
||||
@ -60,6 +60,7 @@ public class FakeHttpServletRequest implements HttpServletRequest {
|
||||
private final ListMultimap<String, String> headers;
|
||||
|
||||
private ListMultimap<String, String> parameters;
|
||||
private String queryString;
|
||||
private String hostName;
|
||||
private int port;
|
||||
private String contextPath;
|
||||
@ -158,6 +159,7 @@ public class FakeHttpServletRequest implements HttpServletRequest {
|
||||
}
|
||||
|
||||
public void setQueryString(String qs) {
|
||||
this.queryString = qs;
|
||||
ListMultimap<String, String> params = LinkedListMultimap.create();
|
||||
for (String entry : Splitter.on('&').split(qs)) {
|
||||
List<String> kv = Splitter.on('=').limit(2).splitToList(entry);
|
||||
@ -306,7 +308,7 @@ public class FakeHttpServletRequest implements HttpServletRequest {
|
||||
|
||||
@Override
|
||||
public String getQueryString() {
|
||||
return paramsToString(parameters);
|
||||
return queryString;
|
||||
}
|
||||
|
||||
@Override
|
||||
@ -317,8 +319,8 @@ public class FakeHttpServletRequest implements HttpServletRequest {
|
||||
@Override
|
||||
public String getRequestURI() {
|
||||
String uri = contextPath + servletPath + path;
|
||||
if (!parameters.isEmpty()) {
|
||||
uri += "?" + paramsToString(parameters);
|
||||
if (!Strings.isNullOrEmpty(queryString)) {
|
||||
uri += '?' + queryString;
|
||||
}
|
||||
return uri;
|
||||
}
|
||||
@ -379,23 +381,6 @@ public class FakeHttpServletRequest implements HttpServletRequest {
|
||||
throw new UnsupportedOperationException();
|
||||
}
|
||||
|
||||
private static String paramsToString(ListMultimap<String, String> params) {
|
||||
StringBuilder sb = new StringBuilder();
|
||||
boolean first = true;
|
||||
for (Map.Entry<String, String> e : params.entries()) {
|
||||
if (!first) {
|
||||
sb.append('&');
|
||||
} else {
|
||||
first = false;
|
||||
}
|
||||
sb.append(Url.encode(e.getKey()));
|
||||
if (!"".equals(e.getValue())) {
|
||||
sb.append('=').append(Url.encode(e.getValue()));
|
||||
}
|
||||
}
|
||||
return sb.toString();
|
||||
}
|
||||
|
||||
@Override
|
||||
public AsyncContext getAsyncContext() {
|
||||
throw new UnsupportedOperationException();
|
||||
|
Loading…
x
Reference in New Issue
Block a user