diff --git a/java/google/registry/request/RequestHandler.java b/java/google/registry/request/RequestHandler.java index bcd2d3763..eba0bd11e 100644 --- a/java/google/registry/request/RequestHandler.java +++ b/java/google/registry/request/RequestHandler.java @@ -171,9 +171,8 @@ public class RequestHandler { Optional authResult = requestAuthenticator.authorize(route.get().action().auth(), req); if (!authResult.isPresent()) { - logger.warning("Request would not have been authorized"); - // TODO(b/28219927): Change this to call rsp.sendError(SC_FORBIDDEN) and return - authResult = Optional.of(AuthResult.NOT_AUTHENTICATED); + rsp.sendError(SC_FORBIDDEN, "Not authorized"); + return; } // Build a new request component using any modules we've constructed by this point. diff --git a/java/google/registry/tools/server/GenerateZoneFilesAction.java b/java/google/registry/tools/server/GenerateZoneFilesAction.java index 86cdcc33e..584b3e7e5 100644 --- a/java/google/registry/tools/server/GenerateZoneFilesAction.java +++ b/java/google/registry/tools/server/GenerateZoneFilesAction.java @@ -47,6 +47,8 @@ import google.registry.model.host.HostResource; import google.registry.request.Action; import google.registry.request.HttpException.BadRequestException; import google.registry.request.JsonActionRunner; +import google.registry.request.auth.Auth; +import google.registry.request.auth.AuthLevel; import google.registry.util.Clock; import java.io.IOException; import java.io.OutputStream; @@ -72,8 +74,13 @@ import org.joda.time.Duration; @Action( path = GenerateZoneFilesAction.PATH, method = POST, - xsrfProtection = true, - xsrfScope = "admin") + auth = + @Auth( + methods = {Auth.AuthMethod.INTERNAL, Auth.AuthMethod.API}, + minimumLevel = AuthLevel.APP, + userPolicy = Auth.UserPolicy.ADMIN + ) +) public class GenerateZoneFilesAction implements Runnable, JsonActionRunner.JsonAction { public static final String PATH = "/_dr/task/generateZoneFiles"; diff --git a/java/google/registry/tools/server/KillAllCommitLogsAction.java b/java/google/registry/tools/server/KillAllCommitLogsAction.java index 51a7d63a2..9012a8d51 100644 --- a/java/google/registry/tools/server/KillAllCommitLogsAction.java +++ b/java/google/registry/tools/server/KillAllCommitLogsAction.java @@ -35,7 +35,14 @@ import google.registry.request.Response; import java.util.Arrays; import javax.inject.Inject; -/** Deletes all commit logs in Datastore. */ +/** + * Deletes all commit logs in Datastore. + * + *

Because there are no auth settings in the {@link Action} annotation, this command can only be + * run internally, or by pretending to be internal by setting the X-AppEngine-QueueName header, + * which only admin users can do. That makes this command hard to use, which is appropriate, given + * the drastic consequences of accidental execution. + */ @Action(path = "/_dr/task/killAllCommitLogs", method = POST) public class KillAllCommitLogsAction implements Runnable { diff --git a/java/google/registry/tools/server/KillAllEppResourcesAction.java b/java/google/registry/tools/server/KillAllEppResourcesAction.java index 3924ab4b9..01240c25e 100644 --- a/java/google/registry/tools/server/KillAllEppResourcesAction.java +++ b/java/google/registry/tools/server/KillAllEppResourcesAction.java @@ -35,7 +35,14 @@ import google.registry.request.Action; import google.registry.request.Response; import javax.inject.Inject; -/** Deletes all {@link EppResource} objects in Datastore, including indices and descendants. */ +/** + * Deletes all {@link EppResource} objects in Datastore, including indices and descendants. + * + *

Because there are no auth settings in the {@link Action} annotation, this command can only be + * run internally, or by pretending to be internal by setting the X-AppEngine-QueueName header, + * which only admin users can do. That makes this command hard to use, which is appropriate, given + * the drastic consequences of accidental execution. + */ @Action(path = "/_dr/task/killAllEppResources", method = POST) public class KillAllEppResourcesAction implements Runnable { diff --git a/java/google/registry/tools/server/ResaveAllEppResourcesAction.java b/java/google/registry/tools/server/ResaveAllEppResourcesAction.java index 864839680..68bd149e2 100644 --- a/java/google/registry/tools/server/ResaveAllEppResourcesAction.java +++ b/java/google/registry/tools/server/ResaveAllEppResourcesAction.java @@ -34,6 +34,10 @@ import javax.inject.Inject; *

This is useful for completing data migrations on EppResource fields that are accomplished * with @OnSave or @OnLoad annotations, and also guarantees that all EppResources will get fresh * commit logs (for backup purposes). + * + *

Because there are no auth settings in the {@link Action} annotation, this command can only be + * run internally, or by pretending to be internal by setting the X-AppEngine-QueueName header, + * which only admin users can do. */ @Action(path = "/_dr/task/resaveAllEppResources") public class ResaveAllEppResourcesAction implements Runnable { diff --git a/java/google/registry/tools/server/javascrap/RefreshAllDomainsAction.java b/java/google/registry/tools/server/javascrap/RefreshAllDomainsAction.java index b9773dc86..c11a50559 100644 --- a/java/google/registry/tools/server/javascrap/RefreshAllDomainsAction.java +++ b/java/google/registry/tools/server/javascrap/RefreshAllDomainsAction.java @@ -30,7 +30,13 @@ import javax.inject.Inject; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; -/** A mapreduce that enqueues publish tasks on all active domains. */ +/** + * A mapreduce that enqueues publish tasks on all active domains. + * + *

Because there are no auth settings in the {@link Action} annotation, this command can only be + * run internally, or by pretending to be internal by setting the X-AppEngine-QueueName header, + * which only admin users can do. + */ @Action(path = "/_dr/task/refreshAllDomains") public class RefreshAllDomainsAction implements Runnable { diff --git a/javatests/google/registry/request/RequestHandlerTest.java b/javatests/google/registry/request/RequestHandlerTest.java index e07c9463a..6f65e20b4 100644 --- a/javatests/google/registry/request/RequestHandlerTest.java +++ b/javatests/google/registry/request/RequestHandlerTest.java @@ -48,7 +48,6 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import org.junit.After; import org.junit.Before; -import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -458,8 +457,6 @@ public final class RequestHandlerTest { assertThat(providedAuthResult.userAuthInfo()).isAbsent(); } - // TODO(b/28219927): turn this on once we actually do authorization - @Ignore @Test public void testAuthNeeded_notLoggedIn() throws Exception { when(req.getMethod()).thenReturn("GET"); @@ -467,26 +464,11 @@ public final class RequestHandlerTest { handler.handleRequest(req, rsp); - verify(rsp).sendError(403); + verify(rsp).sendError(403, "Not authorized"); assertThat(providedAuthResult).isNull(); assertThat(providedAuthResult).isNull(); } - // TODO(b/28219927): remove this once we actually do authorization - @Test - public void testAuthNeeded_notLoggedIn_interim() throws Exception { - when(req.getMethod()).thenReturn("GET"); - when(req.getRequestURI()).thenReturn("/auth/adminUserAnyMethod"); - - handler.handleRequest(req, rsp); - - assertThat(providedAuthResult).isNotNull(); - assertThat(providedAuthResult.authLevel()).isEqualTo(AuthLevel.NONE); - assertThat(providedAuthResult.userAuthInfo()).isAbsent(); - } - - // TODO(b/28219927): turn this on once we actually do authorization - @Ignore @Test public void testAuthNeeded_notAuthorized() throws Exception { userService.setUser(testUser, false); @@ -495,24 +477,10 @@ public final class RequestHandlerTest { handler.handleRequest(req, rsp); - verify(rsp).sendError(403); + verify(rsp).sendError(403, "Not authorized"); assertThat(providedAuthResult).isNull(); } - // TODO(b/28219927): remove this once we actually do authorization - @Test - public void testAuthNeeded_notAuthorized_interim() throws Exception { - userService.setUser(testUser, false); - when(req.getMethod()).thenReturn("GET"); - when(req.getRequestURI()).thenReturn("/auth/adminUserAnyMethod"); - - handler.handleRequest(req, rsp); - - assertThat(providedAuthResult).isNotNull(); - assertThat(providedAuthResult.authLevel()).isEqualTo(AuthLevel.NONE); - assertThat(providedAuthResult.userAuthInfo()).isAbsent(); - } - @Test public void testAuthNeeded_success() throws Exception { userService.setUser(testUser, true);