Allow admin to set AllowedTlds in RegistrarSettingsAction

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=218508076
This commit is contained in:
jianglai 2018-10-24 07:19:06 -07:00
parent 6f155ed3d2
commit 85d971c943
6 changed files with 122 additions and 16 deletions

View file

@ -37,6 +37,7 @@ import google.registry.model.registrar.RegistrarContact;
import google.registry.model.registrar.RegistrarContact.Type;
import google.registry.request.Action;
import google.registry.request.HttpException.BadRequestException;
import google.registry.request.HttpException.ForbiddenException;
import google.registry.request.JsonActionRunner;
import google.registry.request.auth.Auth;
import google.registry.request.auth.AuthResult;
@ -44,6 +45,7 @@ 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.ui.server.registrar.AuthenticatedRegistrarAccessor.Role;
import google.registry.util.AppEngineServiceUtils;
import google.registry.util.CollectionUtils;
import google.registry.util.DiffUtils;
@ -160,8 +162,8 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
"registrar changed since reading the data! "
+ " Last updated at %s, but args data last updated at %s",
latest, latestFromArgs);
return JsonResponseHelper.create(
ERROR, "registrar has been changed by someone else. Please reload and retry.");
throw new IllegalStateException(
"registrar has been changed by someone else. Please reload and retry.");
}
// Keep the current contacts so we can later check that no required contact was
@ -170,7 +172,8 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
// Update the registrar from the request.
Registrar.Builder builder = registrar.asBuilder();
changeRegistrarFields(registrar, builder, args);
Set<Role> roles = registrarAccessor.getAllClientIdWithRoles().get(clientId);
changeRegistrarFields(registrar, roles, builder, args);
// read the contacts from the request.
ImmutableSet<RegistrarContact> updatedContacts = readContacts(registrar, args);
@ -211,11 +214,12 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
return result;
}
/**
* Updates a registrar builder with the supplied args from the http request;
*/
/** Updates a registrar builder with the supplied args from the http request; */
public static void changeRegistrarFields(
Registrar existingRegistrarObj, Registrar.Builder builder, Map<String, ?> args) {
Registrar existingRegistrarObj,
Set<Role> roles,
Registrar.Builder builder,
Map<String, ?> args) {
// BILLING
RegistrarFormFields.PREMIUM_PRICE_ACK_REQUIRED
@ -255,6 +259,18 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
.ifPresent(
certificate ->
builder.setFailoverClientCertificate(certificate, ofy().getTransactionTime()));
// Update allowed TLDs only when it is modified
Set<String> updatedAllowedTlds =
RegistrarFormFields.ALLOWED_TLDS_FIELD.extractUntyped(args).orElse(ImmutableSet.of());
if (!updatedAllowedTlds.equals(existingRegistrarObj.getAllowedTlds())) {
// Only admin is allowed to update allowed TLDs
if (roles.contains(Role.ADMIN)) {
builder.setAllowedTlds(updatedAllowedTlds);
} else {
throw new ForbiddenException("Only admin can update allowed TLDs.");
}
}
}
/** Reads the contacts from the supplied args. */

View file

@ -454,7 +454,7 @@ public class DatastoreHelper {
registrar.asBuilder().setAllowedTlds(union(registrar.getAllowedTlds(), tld)).build());
}
private static void disallowRegistrarAccess(String clientId, String tld) {
public static void disallowRegistrarAccess(String clientId, String tld) {
Registrar registrar = loadRegistrar(clientId);
persistResource(
registrar.asBuilder().setAllowedTlds(difference(registrar.getAllowedTlds(), tld)).build());

View file

@ -27,6 +27,7 @@ import static org.mockito.Mockito.verify;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import google.registry.export.sheet.SyncRegistrarsSheetAction;
import google.registry.model.registrar.Registrar;
import google.registry.testing.CertificateSamples;
@ -136,7 +137,9 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
"op", "update",
"id", CLIENT_ID,
"args", ImmutableMap.of("lastUpdateTime", getLastUpdateTime())));
"args", ImmutableMap.of(
"allowedTlds", ImmutableList.of("currenttld"),
"lastUpdateTime", getLastUpdateTime())));
assertThat(response).containsExactly(
"status", "SUCCESS",
"message", "Saved TheRegistrar",
@ -273,6 +276,75 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
(builder, s) -> builder.setFailoverClientCertificate(s, clock.nowUtc()));
}
@Test
public void testUpdate_allowedTlds_succeedWhenUserIsAdmin() {
setUserAdmin();
doTestUpdate(
Registrar::getAllowedTlds,
ImmutableSet.of("newtld"),
(builder, s) -> builder.setAllowedTlds(s));
}
@Test
public void testUpdate_allowedTlds_failedWhenUserIsNotAdmin() {
Map<String, Object> response =
action.handleJsonRequest(
ImmutableMap.of(
"op", "update",
"id", CLIENT_ID,
"args",
ImmutableMap.of(
"lastUpdateTime", getLastUpdateTime(),
"emailAddress", "abc@def.com",
"allowedTlds", ImmutableList.of("newtld"))));
assertThat(response)
.containsExactly(
"status", "ERROR",
"results", ImmutableList.of(),
"message", "Only admin can update allowed TLDs.");
assertNoTasksEnqueued("sheet");
}
@Test
public void testUpdate_allowedTlds_failedWhenTldNotExist() {
setUserAdmin();
Map<String, Object> response =
action.handleJsonRequest(
ImmutableMap.of(
"op", "update",
"id", CLIENT_ID,
"args",
ImmutableMap.of(
"lastUpdateTime", getLastUpdateTime(),
"emailAddress", "abc@def.com",
"allowedTlds", ImmutableList.of("invalidtld"))));
assertThat(response)
.containsExactly(
"status", "ERROR",
"results", ImmutableList.of(),
"message", "TLDs do not exist: invalidtld");
assertNoTasksEnqueued("sheet");
}
@Test
public void testUpdate_allowedTlds_noChange_successWhenUserIsNotAdmin() {
Map<String, Object> response =
action.handleJsonRequest(
ImmutableMap.of(
"op", "update",
"id", CLIENT_ID,
"args",
ImmutableMap.of(
"lastUpdateTime", getLastUpdateTime(),
"emailAddress", "abc@def.com",
"allowedTlds", ImmutableList.of("currenttld"))));
assertThat(response)
.containsExactly(
"status", "SUCCESS",
"message", "Saved TheRegistrar",
"results", asList(loadRegistrar(CLIENT_ID).toJsonMap()));
}
@Test
public void testUpdate_localizedAddress_city() {
doTestUpdate(

View file

@ -17,13 +17,18 @@ package google.registry.ui.server.registrar;
import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailAddress;
import static google.registry.config.RegistryConfig.getGSuiteOutgoingEmailDisplayName;
import static google.registry.security.JsonHttpTestUtils.createJsonPayload;
import static google.registry.testing.DatastoreHelper.createTlds;
import static google.registry.testing.DatastoreHelper.disallowRegistrarAccess;
import static google.registry.testing.DatastoreHelper.loadRegistrar;
import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role.ADMIN;
import static google.registry.ui.server.registrar.AuthenticatedRegistrarAccessor.Role.OWNER;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
import com.google.appengine.api.users.User;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSetMultimap;
import google.registry.config.RegistryEnvironment;
import google.registry.model.ofy.Ofy;
import google.registry.request.HttpException.ForbiddenException;
@ -81,7 +86,13 @@ public class RegistrarSettingsActionTestCase {
@Before
public void setUp() throws Exception {
action.registrarAccessor = null;
// Create a tld and give access to registrar "TheRegistrar" for most common test case.
createTlds("currenttld");
// Create another tld but remove access for registrar "TheRegistrar". This is for the test case
// of updating allowed tld for registrar
createTlds("newtld");
disallowRegistrarAccess(CLIENT_ID, "newtld");
action.registrarAccessor = mock(AuthenticatedRegistrarAccessor.class);
action.appEngineServiceUtils = appEngineServiceUtils;
when(appEngineServiceUtils.getCurrentVersionHostname("backend")).thenReturn("backend.hostname");
action.jsonActionRunner = new JsonActionRunner(
@ -110,17 +121,24 @@ public class RegistrarSettingsActionTestCase {
/** Sets registrarAccessor.getRegistrar to succeed for all AccessTypes. */
protected void setUserWithAccess() {
action.registrarAccessor = mock(AuthenticatedRegistrarAccessor.class);
when(action.registrarAccessor.getAllClientIdWithRoles())
.thenReturn(ImmutableSetMultimap.of(CLIENT_ID, OWNER));
when(action.registrarAccessor.getRegistrar(CLIENT_ID))
.thenAnswer(x -> loadRegistrar(CLIENT_ID));
}
/** Sets registrarAccessor.getRegistrar to always fail. */
protected void setUserWithoutAccess() {
action.registrarAccessor = mock(AuthenticatedRegistrarAccessor.class);
when(action.registrarAccessor.getAllClientIdWithRoles()).thenReturn(ImmutableSetMultimap.of());
when(action.registrarAccessor.getRegistrar(CLIENT_ID))
.thenThrow(new ForbiddenException("forbidden test error"));
}
/**
* Sets registrarAccessor.getAllClientIdWithRoles to return a map with admin role for CLIENT_ID
*/
protected void setUserAdmin() {
when(action.registrarAccessor.getAllClientIdWithRoles())
.thenReturn(ImmutableSetMultimap.of(CLIENT_ID, ADMIN));
}
}

View file

@ -21,7 +21,7 @@
}
],
"allowedTlds": [
"ga"
"currenttld"
],
"clientCertificateHash": null,
"faxNumber": "",

View file

@ -31,7 +31,7 @@
}
],
"allowedTlds": [
"ga"
"currenttld"
],
"clientCertificateHash": null,
"faxNumber": "",