From 96677009ada03747ab5bdd6d6593e909cda27bb3 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 9 Aug 2017 07:37:26 -0700 Subject: [PATCH 1/3] Remove duplicate Vary: Access-Control-Request header This is already included a few lines up in the same method and doesn't need to be sent a second time. Change-Id: I51265bddfec8167970a2da52320915314c9384ed --- .../java/com/google/gerrit/httpd/restapi/RestApiServlet.java | 1 - 1 file changed, 1 deletion(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index 8adf5f5a1f..84c1c2f0a0 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -591,7 +591,6 @@ public class RestApiServlet extends HttpServlet { String headers = req.getHeader(ACCESS_CONTROL_REQUEST_HEADERS); if (headers != null) { - res.addHeader(VARY, ACCESS_CONTROL_REQUEST_HEADERS); 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"); From 7cf423d6088ac9656fc240a904e4197dc88478cc Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 9 Aug 2017 08:09:46 -0700 Subject: [PATCH 2/3] Always setLastLoginExternalIdKey for IdentifiedUsers Anonymous users have no login external id key, so it makes no sense to set it for them in the read path. The two callers for this property are GetExternalIds and DeleteExternalIds. The latter is not a read method and should expect to have the current value to prevent the user from deleting their current external id. Change-Id: Id0671530fe649e6b0fcfd489fc384e37a2fd02e2 --- .../java/com/google/gerrit/httpd/restapi/RestApiServlet.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index 84c1c2f0a0..8f6da4f042 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -1147,7 +1147,6 @@ public class RestApiServlet extends HttpServlet { CurrentUser user = globals.currentUser.get(); if (isRead(req)) { user.setAccessPath(AccessPath.REST_API); - user.setLastLoginExternalIdKey(globals.webSession.get().getLastLoginExternalId()); } else if (user instanceof AnonymousUser) { throw new AuthException("Authentication required"); } else if (!globals.webSession.get().isAccessPathOk(AccessPath.REST_API)) { @@ -1155,6 +1154,9 @@ public class RestApiServlet extends HttpServlet { "Invalid authentication method. In order to authenticate, " + "prefix the REST endpoint URL with /a/ (e.g. http://example.com/a/projects/)."); } + if (user.isIdentifiedUser()) { + user.setLastLoginExternalIdKey(globals.webSession.get().getLastLoginExternalId()); + } } private static boolean isRead(HttpServletRequest req) { From 1b5b6ab3c862b8559179807eb1fb33fbd7a74ef1 Mon Sep 17 00:00:00 2001 From: Shawn Pearce Date: Wed, 9 Aug 2017 07:37:59 -0700 Subject: [PATCH 3/3] Allow additional cookies during xd requests Some HTTP servers sitting in front of Gerrit may have additional authentication demands beyond what Gerrit requires for the xd request format. Allow xd requests with additional cookies to support these servers to authenticate requests and forward to Gerrit. Change-Id: I11562ab19c052dda5647cccb29265368e45a1159 --- .../com/google/gerrit/acceptance/rest/change/CorsIT.java | 5 +++++ .../java/com/google/gerrit/httpd/restapi/RestApiServlet.java | 1 + 2 files changed, 6 insertions(+) diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java index bd9c98d707..861a22c344 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/CorsIT.java @@ -212,6 +212,11 @@ public class CorsIT extends AbstractDaemonTest { 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); + + Header allowAuth = r.getFirstHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS); + assertThat(allowAuth).named(ACCESS_CONTROL_ALLOW_CREDENTIALS).isNotNull(); + assertThat(allowAuth.getValue()).named(ACCESS_CONTROL_ALLOW_CREDENTIALS).isEqualTo("true"); + checkTopic(change, "test-xd"); } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java index 8f6da4f042..5b1045a8a0 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/restapi/RestApiServlet.java @@ -554,6 +554,7 @@ public class RestApiServlet extends HttpServlet { } res.addHeader(VARY, ORIGIN); res.setHeader(ACCESS_CONTROL_ALLOW_ORIGIN, origin); + res.setHeader(ACCESS_CONTROL_ALLOW_CREDENTIALS, "true"); } else if (!Strings.isNullOrEmpty(origin)) { // All other requests must be processed, but conditionally set CORS headers. if (globals.allowOrigin != null) {