From bbd20ec71df631ad3a9072c639e844af93c431e2 Mon Sep 17 00:00:00 2001 From: mmuller Date: Fri, 28 Oct 2016 09:32:24 -0700 Subject: [PATCH] Convert RegistrarServlet to RegistrarAction Convert to an action and remove ResourceServlet, JsonTransportServlet and JsonTransportServlet, all of which exist only to support it. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=137519385 --- java/google/registry/config/ConfigModule.java | 8 + .../registry/config/RegistryConfig.java | 2 +- .../env/common/default/WEB-INF/web.xml | 17 +- .../frontend/FrontendRequestComponent.java | 2 + .../registry/request/JsonActionRunner.java | 10 +- java/google/registry/security/BUILD | 13 -- .../security/JsonTransportServlet.java | 74 -------- .../security/XsrfProtectedServlet.java | 83 --------- .../google/registry/ui/server/registrar/BUILD | 1 - ...strarServlet.java => RegistrarAction.java} | 105 ++++++++--- .../ui/server/registrar/ResourceServlet.java | 105 ----------- .../registry/request/RequestModuleTest.java | 6 + javatests/google/registry/security/BUILD | 18 -- .../registry/security/JsonHttpTestUtils.java | 4 +- .../security/JsonTransportServletTest.java | 156 ---------------- .../security/XsrfProtectedServletTest.java | 105 ----------- .../registry/server/RegistryTestServer.java | 2 +- .../google/registry/ui/server/registrar/BUILD | 1 + .../server/registrar/ContactSettingsTest.java | 45 ++--- .../server/registrar/RegistrarActionTest.java | 125 +++++++++++++ ...Case.java => RegistrarActionTestCase.java} | 37 +++- .../registrar/RegistrarServletTest.java | 168 ------------------ .../registrar/SecuritySettingsTest.java | 52 +++--- .../server/registrar/WhoisSettingsTest.java | 49 +++-- 24 files changed, 330 insertions(+), 858 deletions(-) delete mode 100644 java/google/registry/security/JsonTransportServlet.java delete mode 100644 java/google/registry/security/XsrfProtectedServlet.java rename java/google/registry/ui/server/registrar/{RegistrarServlet.java => RegistrarAction.java} (74%) delete mode 100644 java/google/registry/ui/server/registrar/ResourceServlet.java delete mode 100644 javatests/google/registry/security/JsonTransportServletTest.java delete mode 100644 javatests/google/registry/security/XsrfProtectedServletTest.java create mode 100644 javatests/google/registry/ui/server/registrar/RegistrarActionTest.java rename javatests/google/registry/ui/server/registrar/{RegistrarServletTestCase.java => RegistrarActionTestCase.java} (75%) delete mode 100644 javatests/google/registry/ui/server/registrar/RegistrarServletTest.java diff --git a/java/google/registry/config/ConfigModule.java b/java/google/registry/config/ConfigModule.java index a8906943b..3698b29d6 100644 --- a/java/google/registry/config/ConfigModule.java +++ b/java/google/registry/config/ConfigModule.java @@ -17,6 +17,7 @@ package google.registry.config; import static google.registry.config.ConfigUtils.makeUrl; import com.google.common.base.Optional; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import dagger.Module; import dagger.Provides; @@ -325,6 +326,13 @@ public final class ConfigModule { } } + @Provides + @Config("registrarChangesNotificationEmailAddresses") + public static ImmutableList provideRegistrarChangesNotificationEmailAddresses( + RegistryConfig config) { + return config.getRegistrarChangesNotificationEmailAddresses(); + } + /** * Returns the publicly accessible domain name for the running Google Apps instance. * diff --git a/java/google/registry/config/RegistryConfig.java b/java/google/registry/config/RegistryConfig.java index 08cdc0512..8ababb92f 100644 --- a/java/google/registry/config/RegistryConfig.java +++ b/java/google/registry/config/RegistryConfig.java @@ -163,7 +163,7 @@ public interface RegistryConfig { * Returns the email address(es) that notifications of registrar and/or registrar contact updates * should be sent to, or the empty list if updates should not be sent. * - * @see google.registry.ui.server.registrar.RegistrarServlet + * @see google.registry.ui.server.registrar.RegistrarAction */ public ImmutableList getRegistrarChangesNotificationEmailAddresses(); diff --git a/java/google/registry/env/common/default/WEB-INF/web.xml b/java/google/registry/env/common/default/WEB-INF/web.xml index ddaa6d477..a91a0e7c7 100644 --- a/java/google/registry/env/common/default/WEB-INF/web.xml +++ b/java/google/registry/env/common/default/WEB-INF/web.xml @@ -25,17 +25,6 @@ /registrar-xhr - - Registrar Self-serve Settings - registrar-settings - google.registry.ui.server.registrar.RegistrarServlet - 1 - - - registrar-settings - /registrar-settings - - frontend-servlet @@ -54,6 +43,12 @@ /registrar-payment + + + frontend-servlet + /registrar-settings + + frontend-servlet diff --git a/java/google/registry/module/frontend/FrontendRequestComponent.java b/java/google/registry/module/frontend/FrontendRequestComponent.java index c1e506191..1c36ac6ea 100644 --- a/java/google/registry/module/frontend/FrontendRequestComponent.java +++ b/java/google/registry/module/frontend/FrontendRequestComponent.java @@ -36,6 +36,7 @@ import google.registry.rdap.RdapNameserverSearchAction; import google.registry.request.RequestModule; import google.registry.request.RequestScope; import google.registry.ui.server.registrar.ConsoleUiAction; +import google.registry.ui.server.registrar.RegistrarAction; import google.registry.ui.server.registrar.RegistrarPaymentAction; import google.registry.ui.server.registrar.RegistrarPaymentSetupAction; import google.registry.ui.server.registrar.RegistrarUserModule; @@ -65,6 +66,7 @@ interface FrontendRequestComponent { RdapAutnumAction rdapAutnumAction(); RegistrarPaymentAction registrarPaymentAction(); RegistrarPaymentSetupAction registrarPaymentSetupAction(); + RegistrarAction registrarAction(); RdapDomainAction rdapDomainAction(); RdapDomainSearchAction rdapDomainSearchAction(); RdapEntityAction rdapEntityAction(); diff --git a/java/google/registry/request/JsonActionRunner.java b/java/google/registry/request/JsonActionRunner.java index 81d550741..72e7dfd7f 100644 --- a/java/google/registry/request/JsonActionRunner.java +++ b/java/google/registry/request/JsonActionRunner.java @@ -35,9 +35,13 @@ public final class JsonActionRunner { Map handleJsonRequest(Map json); } - @Inject @JsonPayload Map payload; - @Inject JsonResponse response; - @Inject JsonActionRunner() {} + @JsonPayload Map payload; + JsonResponse response; + + @Inject public JsonActionRunner(@JsonPayload Map payload, JsonResponse response) { + this.payload = payload; + this.response = response; + } /** Delegates request to {@code action}. */ public void run(JsonAction action) { diff --git a/java/google/registry/security/BUILD b/java/google/registry/security/BUILD index 1100907d5..af3c1f2c0 100644 --- a/java/google/registry/security/BUILD +++ b/java/google/registry/security/BUILD @@ -28,16 +28,3 @@ java_library( "//java/google/registry/util", ], ) - -java_library( - name = "servlets", - srcs = glob(["*Servlet.java"]), - deps = [ - ":security", - "//java/com/google/common/base", - "//third_party/java/appengine:appengine-api", - "//third_party/java/joda_time", - "//third_party/java/servlet/servlet_api", - "//java/google/registry/request", - ], -) diff --git a/java/google/registry/security/JsonTransportServlet.java b/java/google/registry/security/JsonTransportServlet.java deleted file mode 100644 index 36c9e009b..000000000 --- a/java/google/registry/security/JsonTransportServlet.java +++ /dev/null @@ -1,74 +0,0 @@ -// Copyright 2016 The Nomulus Authors. All Rights Reserved. -// -// 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 google.registry.security; - -import static com.google.common.base.Preconditions.checkNotNull; -import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST; -import static javax.servlet.http.HttpServletResponse.SC_OK; - -import google.registry.request.HttpException; -import java.io.IOException; -import java.util.Map; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -/** - * Secure servlet that speaks JSON for both input and output. - * - *

This servlet accepts only JSON inputs (using the payload) and returns only JSON - * responses, using and various security best practices such as a parser breaker, - * {@code Content-Disposition: attachment}, etc. - * - * @see JsonHttp - */ -public abstract class JsonTransportServlet extends XsrfProtectedServlet { - - protected JsonTransportServlet(String xsrfScope, boolean requireAdmin) { - super(xsrfScope, requireAdmin); - } - - /** - * Verify that this is a well-formed request and then execute it. A well-formed request will have - * either a JSON string in the "json" param that evaluates to a map, or nothing in "json". - */ - @Override - protected final void doPost(HttpServletRequest req, HttpServletResponse rsp) throws IOException { - Map input = JsonHttp.read(req); - if (input == null) { - rsp.sendError(SC_BAD_REQUEST, "Malformed JSON"); - return; - } - Map output; - try { - output = doJsonPost(req, input); - } catch (HttpException e) { - e.send(rsp); - return; - } - checkNotNull(output, "doJsonPost() returned null"); - rsp.setStatus(SC_OK); - JsonHttp.write(rsp, output); - } - - /** - * Handler for HTTP POST requests. - * - * @param req Servlet request object. - * @param input JSON request object or empty if none was provided. - * @return an arbitrary JSON object. Must not be {@code null}. - * @throws HttpException in order to send a non-200 status code / message to the client. - */ - public abstract Map doJsonPost(HttpServletRequest req, Map input); -} diff --git a/java/google/registry/security/XsrfProtectedServlet.java b/java/google/registry/security/XsrfProtectedServlet.java deleted file mode 100644 index 93a81b955..000000000 --- a/java/google/registry/security/XsrfProtectedServlet.java +++ /dev/null @@ -1,83 +0,0 @@ -// Copyright 2016 The Nomulus Authors. All Rights Reserved. -// -// 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 google.registry.security; - -import static com.google.appengine.api.users.UserServiceFactory.getUserService; -import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Strings.nullToEmpty; -import static google.registry.security.XsrfTokenManager.X_CSRF_TOKEN; -import static google.registry.security.XsrfTokenManager.validateToken; -import static javax.servlet.http.HttpServletResponse.SC_FORBIDDEN; - -import com.google.appengine.api.users.UserService; -import java.io.IOException; -import javax.servlet.ServletException; -import javax.servlet.http.HttpServlet; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; -import org.joda.time.Duration; - -/** - * Servlet with Cross-Site Request Forgery (XSRF) protection. - * - *

This servlet enforces XSRF protection on all requests by checking the value provided in the - * "X-CSRF-Token" header. It can also optionally enforce that only admin users can call it. - * - *

All servlets that handle client requests should use XSRF protection. - */ -public abstract class XsrfProtectedServlet extends HttpServlet { - - private static final Duration XSRF_VALIDITY = Duration.standardDays(1); - - /** Used to validate XSRF tokens. */ - private String xsrfScope; - - /** Whether to do a security check for admin status. */ - private boolean requireAdmin; - - /** Gets the XSRF scope for this servlet. */ - public String getScope() { - return xsrfScope; - } - - protected XsrfProtectedServlet(String xsrfScope, boolean requireAdmin) { - this.xsrfScope = checkNotNull(xsrfScope); - this.requireAdmin = requireAdmin; - } - - @Override - public final void service(HttpServletRequest req, HttpServletResponse rsp) - throws IOException, ServletException { - if (!validateToken(nullToEmpty(req.getHeader(X_CSRF_TOKEN)), xsrfScope, XSRF_VALIDITY)) { - rsp.sendError(SC_FORBIDDEN, "Invalid " + X_CSRF_TOKEN); - return; - } - if (!validateAdmin()) { - rsp.sendError(SC_FORBIDDEN, "Administrator access only"); - return; - } - doPost(req, rsp); - } - - /** - * If this is an admin-only servlet, require admin permissions or being in development mode. Such - * servlets should primarily be defended by being marked internal-only in web.xml, but it's worth - * adding a defense-in-depth. - */ - private boolean validateAdmin() { - UserService userService = getUserService(); - return requireAdmin ? (userService.isUserLoggedIn() && userService.isUserAdmin()) : true; - } -} diff --git a/java/google/registry/ui/server/registrar/BUILD b/java/google/registry/ui/server/registrar/BUILD index c54308f57..0ff786be9 100644 --- a/java/google/registry/ui/server/registrar/BUILD +++ b/java/google/registry/ui/server/registrar/BUILD @@ -26,7 +26,6 @@ java_library( "//java/google/registry/model", "//java/google/registry/request", "//java/google/registry/security", - "//java/google/registry/security:servlets", "//java/google/registry/ui/forms", "//java/google/registry/ui/server", "//java/google/registry/ui/soy:soy_java_wrappers", diff --git a/java/google/registry/ui/server/registrar/RegistrarServlet.java b/java/google/registry/ui/server/registrar/RegistrarAction.java similarity index 74% rename from java/google/registry/ui/server/registrar/RegistrarServlet.java rename to java/google/registry/ui/server/registrar/RegistrarAction.java index fc750dbf2..5fa86f61a 100644 --- a/java/google/registry/ui/server/registrar/RegistrarServlet.java +++ b/java/google/registry/ui/server/registrar/RegistrarAction.java @@ -18,23 +18,29 @@ import static com.google.common.collect.Iterables.any; import static com.google.common.collect.Iterables.concat; import static com.google.common.collect.Sets.difference; import static google.registry.model.ofy.ObjectifyService.ofy; +import static google.registry.security.JsonResponseHelper.Status.ERROR; import static google.registry.security.JsonResponseHelper.Status.SUCCESS; import com.google.common.base.Function; +import com.google.common.base.Optional; import com.google.common.base.Predicate; import com.google.common.collect.FluentIterable; import com.google.common.collect.HashMultimap; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Multimap; import com.googlecode.objectify.Work; -import google.registry.config.RegistryConfig; -import google.registry.config.RegistryEnvironment; +import google.registry.config.ConfigModule.Config; import google.registry.export.sheet.SyncRegistrarsSheetAction; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarContact; +import google.registry.request.Action; +import google.registry.request.HttpException.BadRequestException; +import google.registry.request.JsonActionRunner; import google.registry.security.JsonResponseHelper; import google.registry.ui.forms.FormException; +import google.registry.ui.forms.FormFieldException; import google.registry.ui.server.RegistrarFormFields; import google.registry.util.CidrAddressBlock; import google.registry.util.CollectionUtils; @@ -44,15 +50,33 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; +import javax.inject.Inject; import javax.servlet.http.HttpServletRequest; /** * Admin servlet that allows creating or updating a registrar. Deletes are not allowed so as to * preserve history. */ -public class RegistrarServlet extends ResourceServlet { +@Action( + path = RegistrarAction.PATH, + requireLogin = true, + xsrfProtection = true, + xsrfScope = "console", + method = Action.Method.POST) +public class RegistrarAction implements Runnable, JsonActionRunner.JsonAction { - private static final RegistryConfig CONFIG = RegistryEnvironment.get().config(); + public static final String PATH = "/registrar-settings"; + + private static final String OP_PARAM = "op"; + private static final String ARGS_PARAM = "args"; + + @Inject HttpServletRequest request; + @Inject SessionUtils sessionUtils; + @Inject JsonActionRunner jsonActionRunner; + @Inject Registrar initialRegistrar; + @Inject @Config("registrarChangesNotificationEmailAddresses") ImmutableList + registrarChangesNotificationEmailAddresses; + @Inject RegistrarAction() {} private static final Predicate HAS_PHONE = new Predicate() { @Override @@ -71,25 +95,20 @@ public class RegistrarServlet extends ResourceServlet { } } - @Override - public Map read(HttpServletRequest req, Map args) { - String clientId = sessionUtils.getRegistrarClientId(req); - Registrar registrar = getCheckedRegistrar(clientId); + public Map read(Map args, Registrar registrar) { return JsonResponseHelper.create(SUCCESS, "Success", registrar.toJsonMap()); } - @Override - Map update(HttpServletRequest req, final Map args) { - final String clientId = sessionUtils.getRegistrarClientId(req); + Map update(final Map args, final Registrar registrar) { + final String clientId = sessionUtils.getRegistrarClientId(request); return ofy().transact(new Work>() { @Override public Map run() { - Registrar existingRegistrar = getCheckedRegistrar(clientId); - ImmutableSet oldContacts = existingRegistrar.getContacts(); + ImmutableSet oldContacts = registrar.getContacts(); Map existingRegistrarMap = - expandRegistrarWithContacts(existingRegistrar, oldContacts); - Registrar.Builder builder = existingRegistrar.asBuilder(); - ImmutableSet updatedContacts = update(existingRegistrar, builder, args); + expandRegistrarWithContacts(oldContacts, registrar); + Registrar.Builder builder = registrar.asBuilder(); + ImmutableSet updatedContacts = update(registrar, builder, args); if (!updatedContacts.isEmpty()) { builder.setContactsRequireSyncing(true); } @@ -102,7 +121,7 @@ public class RegistrarServlet extends ResourceServlet { // Update the registrar map with updated contacts to bypass Objectify caching issues that // come into play with calling getContacts(). Map updatedRegistrarMap = - expandRegistrarWithContacts(updatedRegistrar, updatedContacts); + expandRegistrarWithContacts(updatedContacts, updatedRegistrar); sendExternalUpdatesIfNecessary( updatedRegistrar.getRegistrarName(), existingRegistrarMap, @@ -114,8 +133,8 @@ public class RegistrarServlet extends ResourceServlet { }}); } - private Map expandRegistrarWithContacts( - Registrar registrar, Iterable contacts) { + private Map expandRegistrarWithContacts(Iterable contacts, + Registrar registrar) { ImmutableSet> expandedContacts = FluentIterable.from(contacts) .transform(new Function>() { @Override @@ -146,21 +165,15 @@ public class RegistrarServlet extends ResourceServlet { return; } SyncRegistrarsSheetAction.enqueueBackendTask(); - ImmutableList toEmailAddress = CONFIG.getRegistrarChangesNotificationEmailAddresses(); - if (!toEmailAddress.isEmpty()) { + if (!registrarChangesNotificationEmailAddresses.isEmpty()) { SendEmailUtils.sendEmail( - toEmailAddress, + registrarChangesNotificationEmailAddresses, String.format("Registrar %s updated", registrarName), "The following changes were made to the registrar:\n" + DiffUtils.prettyPrintDiffedMap(diffs, null)); } } - private Registrar getCheckedRegistrar(String clientId) { - return checkExists(Registrar.loadByClientId(clientId), - "No registrar exists with the given client id: " + clientId); - } - /** * Enforces business logic checks on registrar contacts. * @@ -255,4 +268,42 @@ public class RegistrarServlet extends ResourceServlet { return contacts.build(); } + + @Override + public Map handleJsonRequest(Map input) { + if (input == null) { + throw new BadRequestException("Malformed JSON"); + } + + if (!sessionUtils.checkRegistrarConsoleLogin(request)) { + return JsonResponseHelper.create(ERROR, "Not authorized to access Registrar Console"); + } + + // Process the operation. Though originally derived from a CRUD + // handlder, registrar-settings really only supports read and update. + String op = Optional.fromNullable((String) input.get(OP_PARAM)).or("read"); + @SuppressWarnings("unchecked") + Map args = (Map) + Optional.fromNullable(input.get(ARGS_PARAM)).or(ImmutableMap.of()); + try { + switch (op) { + case "update": + return update(args, initialRegistrar); + case "read": + return read(args, initialRegistrar); + default: + return JsonResponseHelper.create(ERROR, "Unknown or unsupported operation: " + op); + } + } catch (FormFieldException e) { + return JsonResponseHelper.createFormFieldError(e.getMessage(), e.getFieldName()); + } catch (FormException ee) { + return JsonResponseHelper.create(ERROR, ee.getMessage()); + } + + } + + @Override + public void run() { + jsonActionRunner.run(this); + } } diff --git a/java/google/registry/ui/server/registrar/ResourceServlet.java b/java/google/registry/ui/server/registrar/ResourceServlet.java deleted file mode 100644 index 65bae194a..000000000 --- a/java/google/registry/ui/server/registrar/ResourceServlet.java +++ /dev/null @@ -1,105 +0,0 @@ -// Copyright 2016 The Nomulus Authors. All Rights Reserved. -// -// 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 google.registry.ui.server.registrar; - -import static com.google.appengine.api.users.UserServiceFactory.getUserService; -import static google.registry.flows.EppConsoleAction.XSRF_SCOPE; -import static google.registry.security.JsonResponseHelper.Status.ERROR; - -import com.google.common.base.Optional; -import com.google.common.collect.ImmutableMap; -import google.registry.request.HttpException.NotFoundException; -import google.registry.security.JsonResponseHelper; -import google.registry.security.JsonTransportServlet; -import google.registry.ui.forms.FormException; -import google.registry.ui.forms.FormFieldException; -import google.registry.util.NonFinalForTesting; -import java.util.Map; -import javax.annotation.Nullable; -import javax.servlet.http.HttpServletRequest; - -/** A servlet for callbacks that manipulate resources. */ -public abstract class ResourceServlet extends JsonTransportServlet { - - private static final String OP_PARAM = "op"; - private static final String ARGS_PARAM = "args"; - - @NonFinalForTesting - protected static SessionUtils sessionUtils = new SessionUtils(getUserService()); - - public ResourceServlet() { - super(XSRF_SCOPE, false); - } - - @Override - public Map doJsonPost(HttpServletRequest req, Map params) { - if (!sessionUtils.isLoggedIn()) { - return JsonResponseHelper.create(ERROR, "Not logged in"); - } - if (!sessionUtils.checkRegistrarConsoleLogin(req)) { - return JsonResponseHelper.create(ERROR, "Not authorized to access Registrar Console"); - } - String op = Optional.fromNullable((String) params.get(OP_PARAM)).or("read"); - @SuppressWarnings("unchecked") - Map args = (Map) - Optional.fromNullable(params.get(ARGS_PARAM)).or(ImmutableMap.of()); - try { - switch (op) { - case "create": - return create(req, args); - case "update": - return update(req, args); - case "delete": - return delete(req, args); - case "read": - return read(req, args); - default: - return JsonResponseHelper.create(ERROR, "Unknown operation: " + op); - } - } catch (FormFieldException e) { - return JsonResponseHelper.createFormFieldError(e.getMessage(), e.getFieldName()); - } catch (FormException ee) { - return JsonResponseHelper.create(ERROR, ee.getMessage()); - } - } - - @SuppressWarnings("unused") - Map create(HttpServletRequest req, Map args) { - throw new UnsupportedOperationException(); - } - - @SuppressWarnings("unused") - Map read(HttpServletRequest req, Map args) { - throw new UnsupportedOperationException(); - } - - @SuppressWarnings("unused") - Map update(HttpServletRequest req, Map args) { - throw new UnsupportedOperationException(); - } - - @SuppressWarnings("unused") - Map delete(HttpServletRequest req, Map args) { - throw new UnsupportedOperationException(); - } - - /** Like checkNotNull, but throws NotFoundException if given arg is null. */ - protected static T checkExists(@Nullable T obj, String msg) { - if (obj == null) { - throw new NotFoundException(msg); - } - return obj; - } -} diff --git a/javatests/google/registry/request/RequestModuleTest.java b/javatests/google/registry/request/RequestModuleTest.java index d27ebed41..1d2deeda5 100644 --- a/javatests/google/registry/request/RequestModuleTest.java +++ b/javatests/google/registry/request/RequestModuleTest.java @@ -51,6 +51,12 @@ public final class RequestModuleTest { provideJsonPayload(MediaType.JSON_UTF_8, "{\"k\":"); } + @Test + public void testProvideJsonPayload_emptyInput_throws500() throws Exception { + thrown.expect(BadRequestException.class, "Malformed JSON"); + provideJsonPayload(MediaType.JSON_UTF_8, ""); + } + @Test public void testProvideJsonPayload_nonJsonContentType_throws415() throws Exception { thrown.expect(UnsupportedMediaTypeException.class); diff --git a/javatests/google/registry/security/BUILD b/javatests/google/registry/security/BUILD index dfa4a7a5f..2161b6f81 100644 --- a/javatests/google/registry/security/BUILD +++ b/javatests/google/registry/security/BUILD @@ -32,28 +32,10 @@ java_library( ], ) -java_library( - name = "servlets", - srcs = glob(["*ServletTest.java"]), - deps = [ - "//java/com/google/common/collect", - "//java/com/google/common/net", - "//third_party/java/junit", - "//third_party/java/mockito", - "//third_party/java/servlet/servlet_api", - "//third_party/java/truth", - "//java/google/registry/request", - "//java/google/registry/security", - "//java/google/registry/security:servlets", - "//javatests/google/registry/testing", - ], -) - GenTestRules( name = "GeneratedTestRules", test_files = glob(["*Test.java"]), deps = [ ":security", - ":servlets", ], ) diff --git a/javatests/google/registry/security/JsonHttpTestUtils.java b/javatests/google/registry/security/JsonHttpTestUtils.java index 63b2272a6..1b8ba79b3 100644 --- a/javatests/google/registry/security/JsonHttpTestUtils.java +++ b/javatests/google/registry/security/JsonHttpTestUtils.java @@ -43,8 +43,8 @@ public final class JsonHttpTestUtils { } /** - * Returns JSON data parsed out of a JsonTransportServlet response stored in the given writer. - * If the data will be fetched multiple times, consider {@link #createJsonResponseSupplier}. + * Returns JSON data parsed out of the contents of the given writer. If the data will be fetched + * multiple times, consider {@link #createJsonResponseSupplier}. * *

Example Mockito usage:

  {@code
    *
diff --git a/javatests/google/registry/security/JsonTransportServletTest.java b/javatests/google/registry/security/JsonTransportServletTest.java
deleted file mode 100644
index 9ef1588bb..000000000
--- a/javatests/google/registry/security/JsonTransportServletTest.java
+++ /dev/null
@@ -1,156 +0,0 @@
-// Copyright 2016 The Nomulus Authors. All Rights Reserved.
-//
-// 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 google.registry.security;
-
-import static com.google.common.net.HttpHeaders.CONTENT_DISPOSITION;
-import static com.google.common.net.HttpHeaders.X_CONTENT_TYPE_OPTIONS;
-import static com.google.common.net.MediaType.JSON_UTF_8;
-import static com.google.common.truth.Truth.assertThat;
-import static javax.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
-import static javax.servlet.http.HttpServletResponse.SC_OK;
-import static org.mockito.Matchers.anyString;
-import static org.mockito.Matchers.eq;
-import static org.mockito.Mockito.verify;
-import static org.mockito.Mockito.when;
-
-import com.google.common.collect.ImmutableMap;
-import google.registry.request.HttpException;
-import google.registry.testing.AppEngineRule;
-import google.registry.testing.ExceptionRule;
-import java.io.BufferedReader;
-import java.io.PrintWriter;
-import java.io.StringReader;
-import java.io.StringWriter;
-import java.util.Map;
-import javax.servlet.http.HttpServletRequest;
-import javax.servlet.http.HttpServletResponse;
-import org.junit.Rule;
-import org.junit.Test;
-import org.junit.runner.RunWith;
-import org.mockito.Mock;
-import org.mockito.runners.MockitoJUnitRunner;
-
-/** Unit tests for {@link JsonTransportServlet}. */
-@RunWith(MockitoJUnitRunner.class)
-public class JsonTransportServletTest {
-
-  @Rule
-  public final AppEngineRule appEngine = AppEngineRule.builder()
-      .withDatastore()
-      .build();
-
-  @Rule
-  public final ExceptionRule thrown = new ExceptionRule();
-
-  private StringWriter writer = new StringWriter();
-
-  @Mock
-  HttpServletRequest req;
-
-  @Mock
-  HttpServletResponse rsp;
-
-  static class TestServlet extends JsonTransportServlet {
-    private Map responseMap;
-
-    TestServlet(Map responseMap) {
-      super("foo", false);
-      this.responseMap = responseMap;
-    }
-
-    @Override
-    public Map doJsonPost(HttpServletRequest req, Map input) {
-      return responseMap;
-    }
-  }
-
-  private void verifySuccess(String json) {
-    verify(rsp).setStatus(SC_OK);
-    verify(rsp).setHeader(CONTENT_DISPOSITION, "attachment");
-    verify(rsp).setHeader(X_CONTENT_TYPE_OPTIONS, "nosniff");
-    verify(rsp).setContentType(JSON_UTF_8.toString());
-    assertThat(writer.toString()).isEqualTo(")]}'\n" + json);
-  }
-
-  private void doSuccessfulTest(
-      String requestJson, Map responseMap) throws Exception {
-    when(req.getMethod()).thenReturn("POST");
-    when(req.getContentType()).thenReturn(JSON_UTF_8.toString());
-    when(req.getReader()).thenReturn(new BufferedReader(new StringReader(requestJson)));
-    when(rsp.getWriter()).thenReturn(new PrintWriter(writer));
-    new TestServlet(responseMap).doPost(req, rsp);
-    verifySuccess("{\"a\":1}");
-  }
-
-  private void verifyFailure(int error) throws Exception {
-    verify(rsp).sendError(eq(error), anyString());
-  }
-
-  @Test
-  public void testSuccess() throws Exception {
-    doSuccessfulTest("{\"key\":\"value\"}", ImmutableMap.of("a", 1));
-  }
-
-  @Test
-  public void testDoJsonPost_returnsNull_notAllowed() throws Exception {
-    when(req.getMethod()).thenReturn("POST");
-    when(req.getContentType()).thenReturn(JSON_UTF_8.toString());
-    when(req.getReader()).thenReturn(new BufferedReader(new StringReader("{\"key\":\"value\"}")));
-    when(rsp.getWriter()).thenReturn(new PrintWriter(writer));
-    thrown.expect(NullPointerException.class);
-    new TestServlet(null).doPost(req, rsp);
-  }
-
-  private void doInvalidRequestTest(String requestJson) throws Exception {
-    when(req.getMethod()).thenReturn("POST");
-    when(req.getContentType()).thenReturn(JSON_UTF_8.toString());
-    when(req.getReader()).thenReturn(new BufferedReader(new StringReader(requestJson)));
-    new TestServlet(null).doPost(req, rsp);
-    verifyFailure(SC_BAD_REQUEST);
-  }
-
-  @Test
-  public void testSuccess_emptyJsonNotAllowed() throws Exception {
-    doInvalidRequestTest("");
-  }
-
-  @Test
-  public void testFailure_badJson() throws Exception {
-    doInvalidRequestTest("{}{}");
-  }
-
-  @Test
-  public void testFailure_nonObjectJson_null() throws Exception {
-    doInvalidRequestTest("null");
-  }
-
-  @Test
-  public void testFailure_nonObjectJson_array() throws Exception {
-    doInvalidRequestTest("[]");
-  }
-
-  @Test
-  public void testErrorMessagesAreEscaped() throws Exception {
-    when(req.getMethod()).thenReturn("POST");
-    when(req.getReader()).thenReturn(new BufferedReader(new StringReader("{}")));
-    when(req.getContentType()).thenReturn(JSON_UTF_8.toString());
-    new TestServlet(null) {
-      @Override
-      public Map doJsonPost(HttpServletRequest req, Map input) {
-        throw new HttpException(123, "