Skip to content

Commit

Permalink
Refactor common exception handling in ConsoleApiAction (#2443)
Browse files Browse the repository at this point in the history 
There are a bunch of cases where we want common exception handling and
it's annoying to have to deal with the common "set failed response and
make sure to return" a bunch of times.

This allows us to break up request methods more easily, since we can now
often throw exceptions that will break all the way back up to
ConsoleApiAction. Previously, any error handling had to exist in the
primary handler method so it could return.
  • Loading branch information
@gbrodman
gbrodman committed May 16, 2024
1 parent a66b9ea commit 43000a5
Show file tree
Hide file tree
Showing 9 changed files with 168 additions and 251 deletions.
44 changes: 39 additions & 5 deletions core/src/main/java/google/registry/ui/server/console/ConsoleApiAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,12 @@
import static google.registry.request.Action.Method.GET;

import com.google.api.client.http.HttpStatusCodes;
import com.google.common.base.Throwables;
import com.google.common.flogger.FluentLogger;
import google.registry.model.console.ConsolePermission;
import google.registry.model.console.GlobalRole;
import google.registry.model.console.User;
import google.registry.request.HttpException;
import google.registry.request.auth.AuthResult;
import google.registry.security.XsrfTokenManager;
import google.registry.ui.server.registrar.ConsoleApiParams;
Expand All @@ -31,6 +35,9 @@

/** Base class for handling Console API requests */
public abstract class ConsoleApiAction implements Runnable {

private static final FluentLogger logger = FluentLogger.forEnclosingClass();

protected ConsoleApiParams consoleApiParams;

public ConsoleApiAction(ConsoleApiParams consoleApiParams) {
Expand Down Expand Up @@ -60,15 +67,36 @@ public final void run() {
}
}

if (consoleApiParams.request().getMethod().equals(GET.toString())) {
getHandler(user);
} else {
if (verifyXSRF()) {
postHandler(user);
try {
if (consoleApiParams.request().getMethod().equals(GET.toString())) {
getHandler(user);
} else {
if (verifyXSRF()) {
postHandler(user);
}
}
} catch (ConsolePermissionForbiddenException e) {
logger.atWarning().withCause(e).log("Forbidden");
setFailedResponse("", HttpStatusCodes.STATUS_CODE_FORBIDDEN);
} catch (HttpException.BadRequestException | IllegalArgumentException e) {
logger.atWarning().withCause(e).log("Error in request");
setFailedResponse(
Throwables.getRootCause(e).getMessage(), HttpStatusCodes.STATUS_CODE_BAD_REQUEST);
} catch (Throwable t) {
logger.atWarning().withCause(t).log("Internal server error");
setFailedResponse(
Throwables.getRootCause(t).getMessage(), HttpStatusCodes.STATUS_CODE_SERVER_ERROR);
}
}

protected void checkPermission(User user, String registrarId, ConsolePermission permission) {
if (!user.getUserRoles().hasPermission(registrarId, permission)) {
throw new ConsolePermissionForbiddenException(
String.format(
"User %s does not have permission %s on registrar %s",
user.getEmailAddress(), permission, registrarId));
}
}

protected void postHandler(User user) {
throw new UnsupportedOperationException("Console API POST handler not implemented");
Expand Down Expand Up @@ -96,4 +124,10 @@ private boolean verifyXSRF() {
return true;
}

/** Specialized exception class used for failure when a user doesn't have the right permission. */
private static class ConsolePermissionForbiddenException extends RuntimeException {
private ConsolePermissionForbiddenException(String message) {
super(message);
}
}
}
22 changes: 6 additions & 16 deletions core/src/main/java/google/registry/ui/server/console/ConsoleDomainListAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package google.registry.ui.server.console;

import static com.google.common.base.Preconditions.checkArgument;
import static google.registry.model.console.ConsolePermission.DOWNLOAD_DOMAINS;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;

Expand Down Expand Up @@ -82,22 +83,11 @@ public ConsoleDomainListAction(

@Override
protected void getHandler(User user) {
if (!user.getUserRoles().hasPermission(registrarId, DOWNLOAD_DOMAINS)) {
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_FORBIDDEN);
return;
}
if (resultsPerPage < 1 || resultsPerPage > 500) {
setFailedResponse(
"Results per page must be between 1 and 500 inclusive",
HttpStatusCodes.STATUS_CODE_BAD_REQUEST);
return;
}
if (pageNumber < 0) {
setFailedResponse(
"Page number must be non-negative", HttpStatusCodes.STATUS_CODE_BAD_REQUEST);
return;
}

checkPermission(user, registrarId, DOWNLOAD_DOMAINS);
checkArgument(
resultsPerPage > 0 && resultsPerPage <= 500,
"Results per page must be between 1 and 500 inclusive");
checkArgument(pageNumber >= 0, "Page number must be non-negative");
tm().transact(this::runInTransaction);
}

Expand Down
8 changes: 2 additions & 6 deletions core/src/main/java/google/registry/ui/server/console/ConsoleDumDownloadAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ SELECT CONCAT(
private static final FluentLogger logger = FluentLogger.forEnclosingClass();

public static final String PATH = "/console-api/dum-download";
private Clock clock;
private final Clock clock;
private final String registrarId;
private final String dumFileName;

Expand All @@ -76,11 +76,7 @@ public ConsoleDumDownloadAction(

@Override
protected void getHandler(User user) {
if (!user.getUserRoles().hasPermission(registrarId, ConsolePermission.DOWNLOAD_DOMAINS)) {
consoleApiParams.response().setStatus(HttpServletResponse.SC_FORBIDDEN);
return;
}

checkPermission(user, registrarId, ConsolePermission.DOWNLOAD_DOMAINS);
consoleApiParams.response().setContentType(MediaType.CSV_UTF_8);
consoleApiParams
.response()
Expand Down
57 changes: 16 additions & 41 deletions core/src/main/java/google/registry/ui/server/console/ConsoleEppPasswordAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,26 +14,23 @@

package google.registry.ui.server.console;

import static com.google.common.base.Preconditions.checkArgument;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.request.Action.Method.POST;
import static google.registry.request.RequestParameters.extractRequiredParameter;

import com.google.api.client.http.HttpStatusCodes;
import com.google.common.base.Throwables;
import com.google.common.flogger.FluentLogger;
import google.registry.flows.EppException.AuthenticationErrorException;
import google.registry.flows.PasswordOnlyTransportCredentials;
import google.registry.groups.GmailClient;
import google.registry.model.console.User;
import google.registry.model.registrar.Registrar;
import google.registry.request.Action;
import google.registry.request.HttpException.BadRequestException;
import google.registry.request.auth.Auth;
import google.registry.request.auth.AuthenticatedRegistrarAccessor;
import google.registry.request.auth.AuthenticatedRegistrarAccessor.RegistrarAccessDeniedException;
import google.registry.ui.server.registrar.ConsoleApiParams;
import google.registry.util.EmailMessage;
import java.util.Optional;
import javax.inject.Inject;
import javax.mail.internet.InternetAddress;

Expand All @@ -44,8 +41,6 @@
auth = Auth.AUTH_PUBLIC_LOGGED_IN)
public class ConsoleEppPasswordAction extends ConsoleApiAction {

private static final FluentLogger logger = FluentLogger.forEnclosingClass();

protected static final String EMAIL_SUBJ = "EPP password update confirmation";
protected static final String EMAIL_BODY =
"Dear %s,\n" + "This is to confirm that your account password has been changed.";
Expand All @@ -69,25 +64,12 @@ public ConsoleEppPasswordAction(

@Override
protected void postHandler(User user) {
String registrarId;
String oldPassword;
String newPassword;
String newPasswordRepeat;

try {
registrarId = extractRequiredParameter(consoleApiParams.request(), "registrarId");
oldPassword = extractRequiredParameter(consoleApiParams.request(), "oldPassword");
newPassword = extractRequiredParameter(consoleApiParams.request(), "newPassword");
newPasswordRepeat = extractRequiredParameter(consoleApiParams.request(), "newPasswordRepeat");
} catch (BadRequestException e) {
setFailedResponse(e.getMessage(), HttpStatusCodes.STATUS_CODE_BAD_REQUEST);
return;
}

if (!newPassword.equals(newPasswordRepeat)) {
setFailedResponse("New password fields don't match", HttpStatusCodes.STATUS_CODE_BAD_REQUEST);
return;
}
String registrarId = extractRequiredParameter(consoleApiParams.request(), "registrarId");
String oldPassword = extractRequiredParameter(consoleApiParams.request(), "oldPassword");
String newPassword = extractRequiredParameter(consoleApiParams.request(), "newPassword");
String newPasswordRepeat =
extractRequiredParameter(consoleApiParams.request(), "newPasswordRepeat");
checkArgument(newPassword.equals(newPasswordRepeat), "New password fields don't match");

Registrar registrar;
try {
Expand All @@ -104,22 +86,15 @@ protected void postHandler(User user) {
return;
}

try {
tm().transact(
() -> {
tm().put(registrar.asBuilder().setPassword(newPassword).build());
this.gmailClient.sendEmail(
EmailMessage.create(
EMAIL_SUBJ,
String.format(EMAIL_BODY, registrar.getRegistrarName()),
new InternetAddress(registrar.getEmailAddress(), true)));
});
} catch (Throwable e) {
logger.atWarning().withCause(e).log("Failed to update password.");
String message =
Optional.ofNullable(Throwables.getRootCause(e).getMessage()).orElse("Unspecified error");
setFailedResponse(message, HttpStatusCodes.STATUS_CODE_SERVER_ERROR);
}
tm().transact(
() -> {
tm().put(registrar.asBuilder().setPassword(newPassword).build());
this.gmailClient.sendEmail(
EmailMessage.create(
EMAIL_SUBJ,
String.format(EMAIL_BODY, registrar.getRegistrarName()),
new InternetAddress(registrar.getEmailAddress(), true)));
});

consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_OK);
}
Expand Down
97 changes: 31 additions & 66 deletions core/src/main/java/google/registry/ui/server/console/ConsoleRegistryLockAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package google.registry.ui.server.console;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableList.toImmutableList;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.request.Action.Method.GET;
Expand All @@ -26,7 +27,6 @@

import com.google.api.client.http.HttpStatusCodes;
import com.google.common.collect.ImmutableList;
import com.google.common.flogger.FluentLogger;
import com.google.gson.Gson;
import google.registry.flows.EppException;
import google.registry.flows.domain.DomainFlowUtils;
Expand All @@ -37,7 +37,6 @@
import google.registry.model.registrar.Registrar;
import google.registry.model.tld.RegistryLockDao;
import google.registry.request.Action;
import google.registry.request.HttpException;
import google.registry.request.Parameter;
import google.registry.request.Response;
import google.registry.request.auth.Auth;
Expand Down Expand Up @@ -66,8 +65,6 @@
auth = Auth.AUTH_PUBLIC_LOGGED_IN)
public class ConsoleRegistryLockAction extends ConsoleApiAction {

private static final FluentLogger logger = FluentLogger.forEnclosingClass();

static final String PATH = "/console-api/registry-lock";

private final DomainLockUtils domainLockUtils;
Expand All @@ -91,10 +88,7 @@ public ConsoleRegistryLockAction(

@Override
protected void getHandler(User user) {
if (!user.getUserRoles().hasPermission(registrarId, ConsolePermission.REGISTRY_LOCK)) {
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_FORBIDDEN);
return;
}
checkPermission(user, registrarId, ConsolePermission.REGISTRY_LOCK);
consoleApiParams.response().setPayload(gson.toJson(getLockedDomains()));
consoleApiParams.response().setStatus(HttpStatusCodes.STATUS_CODE_OK);
}
Expand All @@ -104,84 +98,55 @@ protected void postHandler(User user) {
HttpServletRequest req = consoleApiParams.request();
Response response = consoleApiParams.response();
// User must have the proper permission on the registrar
if (!user.getUserRoles().hasPermission(registrarId, ConsolePermission.REGISTRY_LOCK)) {
setFailedResponse("", HttpStatusCodes.STATUS_CODE_FORBIDDEN);
return;
}
checkPermission(user, registrarId, ConsolePermission.REGISTRY_LOCK);

// Shouldn't happen, but double-check the registrar has registry lock enabled
Registrar registrar = Registrar.loadByRegistrarIdCached(registrarId).get();
if (!registrar.isRegistryLockAllowed()) {
setFailedResponse(
String.format("Registry lock not allowed for registrar %s", registrarId),
HttpStatusCodes.STATUS_CODE_BAD_REQUEST);
return;
}
checkArgument(
registrar.isRegistryLockAllowed(),
"Registry lock not allowed for registrar %s",
registrarId);

// Retrieve and validate the necessary params
String domainName;
boolean isLock;
Optional<String> maybePassword;
Optional<Long> relockDurationMillis;
String domainName = extractRequiredParameter(req, "domainName");
boolean isLock = extractBooleanParameter(req, "isLock");
Optional<String> maybePassword = extractOptionalParameter(req, "password");
Optional<Long> relockDurationMillis = extractOptionalLongParameter(req, "relockDurationMillis");

try {
domainName = extractRequiredParameter(req, "domainName");
isLock = extractBooleanParameter(req, "isLock");
maybePassword = extractOptionalParameter(req, "password");
relockDurationMillis = extractOptionalLongParameter(req, "relockDurationMillis");
DomainFlowUtils.validateDomainName(domainName);
} catch (HttpException.BadRequestException | EppException e) {
logger.atWarning().withCause(e).log("Bad request when attempting registry lock/unlock");
setFailedResponse(e.getMessage(), HttpStatusCodes.STATUS_CODE_BAD_REQUEST);
return;
} catch (EppException e) {
throw new IllegalArgumentException(e);
}

// Passwords aren't required for admin users, otherwise we need to validate it
boolean isAdmin = user.getUserRoles().isAdmin();
if (!isAdmin) {
if (maybePassword.isEmpty()) {
setFailedResponse("No password provided", HttpStatusCodes.STATUS_CODE_BAD_REQUEST);
return;
}
checkArgument(maybePassword.isPresent(), "No password provided");
if (!user.verifyRegistryLockPassword(maybePassword.get())) {
setFailedResponse(
"Incorrect registry lock password", HttpStatusCodes.STATUS_CODE_UNAUTHORIZED);
return;
}
}

Optional<String> maybeRegistryLockEmail = user.getRegistryLockEmailAddress();
if (maybeRegistryLockEmail.isEmpty()) {
setFailedResponse(
"User has no registry lock email address", HttpStatusCodes.STATUS_CODE_BAD_REQUEST);
return;
}
String registryLockEmail = maybeRegistryLockEmail.get();

try {
tm().transact(
() -> {
RegistryLock registryLock =
isLock
? domainLockUtils.saveNewRegistryLockRequest(
domainName, registrarId, registryLockEmail, isAdmin)
: domainLockUtils.saveNewRegistryUnlockRequest(
domainName,
registrarId,
isAdmin,
relockDurationMillis.map(Duration::new));
sendVerificationEmail(registryLock, registryLockEmail, isLock);
});
} catch (IllegalArgumentException e) {
// Catch IllegalArgumentExceptions separately to give a nicer error message and code
logger.atWarning().withCause(e).log("Failed to lock/unlock domain");
setFailedResponse(e.getMessage(), HttpStatusCodes.STATUS_CODE_BAD_REQUEST);
return;
} catch (Throwable t) {
logger.atWarning().withCause(t).log("Failed to lock/unlock domain");
setFailedResponse("Internal server error", HttpStatusCodes.STATUS_CODE_SERVER_ERROR);
return;
}
String registryLockEmail =
user.getRegistryLockEmailAddress()
.orElseThrow(
() -> new IllegalArgumentException("User has no registry lock email address"));
tm().transact(
() -> {
RegistryLock registryLock =
isLock
? domainLockUtils.saveNewRegistryLockRequest(
domainName, registrarId, registryLockEmail, isAdmin)
: domainLockUtils.saveNewRegistryUnlockRequest(
domainName,
registrarId,
isAdmin,
relockDurationMillis.map(Duration::new));
sendVerificationEmail(registryLock, registryLockEmail, isLock);
});
response.setStatus(HttpStatusCodes.STATUS_CODE_OK);
}

Expand Down

0 comments on commit 43000a5

Please sign in to comment.