diff --git a/core/src/main/java/google/registry/env/common/default/WEB-INF/cloud-tasks-queue.xml b/core/src/main/java/google/registry/env/common/default/WEB-INF/cloud-tasks-queue.xml index f285121a5..833edc5ca 100644 --- a/core/src/main/java/google/registry/env/common/default/WEB-INF/cloud-tasks-queue.xml +++ b/core/src/main/java/google/registry/env/common/default/WEB-INF/cloud-tasks-queue.xml @@ -96,6 +96,14 @@ 3600s + + + console-user-group-update + 1 + 1 + 3600s + + retryable-cron-tasks diff --git a/core/src/main/java/google/registry/model/OteAccountBuilder.java b/core/src/main/java/google/registry/model/OteAccountBuilder.java index aed59edee..d0429c0d9 100644 --- a/core/src/main/java/google/registry/model/OteAccountBuilder.java +++ b/core/src/main/java/google/registry/model/OteAccountBuilder.java @@ -17,6 +17,7 @@ package google.registry.model; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkState; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.ImmutableMap.toImmutableMap; import static google.registry.model.tld.Tld.TldState.GENERAL_AVAILABILITY; import static google.registry.model.tld.Tld.TldState.START_DATE_SUNRISE; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; @@ -28,19 +29,28 @@ import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; import com.google.common.collect.Sets; import com.google.common.collect.Streams; +import google.registry.batch.CloudTasksUtils; +import google.registry.model.console.RegistrarRole; +import google.registry.model.console.User; +import google.registry.model.console.UserDao; +import google.registry.model.console.UserRoles; import google.registry.model.pricing.StaticPremiumListPricingEngine; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarAddress; -import google.registry.model.registrar.RegistrarPoc; import google.registry.model.tld.Tld; import google.registry.model.tld.Tld.TldState; import google.registry.model.tld.Tld.TldType; import google.registry.model.tld.label.PremiumList; import google.registry.model.tld.label.PremiumListDao; import google.registry.persistence.VKey; +import google.registry.tools.IamClient; import google.registry.util.CidrAddressBlock; import google.registry.util.RegistryEnvironment; +import java.util.ArrayList; import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; import java.util.Optional; import java.util.function.Function; import java.util.regex.Pattern; @@ -75,8 +85,8 @@ public final class OteAccountBuilder { * Validation regex for registrar base client IDs (3-14 lowercase alphanumeric characters). * *

The base client ID is appended with numbers to create four different test registrar accounts - * (e.g. reg-1, reg-3, reg-4, reg-5). Registrar client IDs are of type clIDType in eppcom.xsd - * which is limited to 16 characters, hence the limit of 14 here to account for the dash and + * (e.g., reg-1, reg-3, reg-4, reg-5). Registrar client IDs are of type clIDType in eppcom.xsd + * that is limited to 16 characters, hence the limit of 14 here to account for the dash and * numbers. * *

The base client ID is also used to generate the OT&E TLDs, hence the restriction to @@ -113,7 +123,7 @@ public final class OteAccountBuilder { * The default billing account map applied to all OT&E registrars. * *

This contains dummy values for USD and JPY so that OT&E registrars can be granted access - * to all existing TLDs in sandbox. Note that OT&E is only on sandbox and thus these dummy + * to all existing TLDs in sandbox. Note that OT&E is only on sandbox, and thus these dummy * values will never be used in production (the only environment where real invoicing takes * place). */ @@ -124,7 +134,7 @@ public final class OteAccountBuilder { private final Tld sunriseTld; private final Tld gaTld; private final Tld eapTld; - private final ImmutableList.Builder contactsBuilder = new ImmutableList.Builder<>(); + private final List users = new ArrayList<>(); private ImmutableList registrars; private boolean replaceExisting = false; @@ -172,16 +182,28 @@ public final class OteAccountBuilder { } /** - * Adds a RegistrarContact with Web Console access. + * Adds a {@link User} with Web Console access. * - *

NOTE: can be called more than once, adding multiple contacts. Each contact will have access - * to all OT&E Registrars. + *

NOTE: can be called more than once, adding multiple users. Each user will have access to all + * OT&E Registrars. * - * @param email the contact/login email that will have web-console access to all the Registrars. - * Must be from "our G Suite domain". + * @param email the login email that will have web-console access to all the Registrars. Must be + * from "our Google Workspace domain". */ - public OteAccountBuilder addContact(String email) { - registrars.forEach(registrar -> contactsBuilder.add(createRegistrarContact(email, registrar))); + public OteAccountBuilder addUser(String email) { + users.add( + new User.Builder() + .setEmailAddress(email) + .setUserRoles( + new UserRoles.Builder() + .setRegistrarRoles( + registrars.stream() + .collect( + toImmutableMap( + Registrar::getRegistrarId, + registrar -> RegistrarRole.ACCOUNT_MANAGER))) + .build()) + .build()); return this; } @@ -217,7 +239,7 @@ public final class OteAccountBuilder { return transformRegistrars(builder -> builder.setClientCertificate(asciiCert, now)); } - /** Sets the IP allow list to all the OT&E Registrars. */ + /** Sets the IP allowlist to all the OT&E Registrars. */ public OteAccountBuilder setIpAllowList(Collection ipAllowList) { ImmutableList ipAddressAllowList = ipAllowList.stream().map(CidrAddressBlock::create).collect(toImmutableList()); @@ -237,18 +259,37 @@ public final class OteAccountBuilder { } /** - * Return map from the OT&E registrarIds we will create to the new TLDs they will have access - * to. + * Return the map from the OT&E registrarIds we will create to the new TLDs they will have + * access to. */ public ImmutableMap getRegistrarIdToTldMap() { return registrarIdToTld; } + /** Grants the users permission to pass IAP. */ + public void grantIapPermission( + Optional groupEmailAddress, CloudTasksUtils cloudTasksUtils, IamClient iamClient) { + for (User user : users) { + User.grantIapPermission( + user.getEmailAddress(), groupEmailAddress, cloudTasksUtils, iamClient); + } + } + /** Saves all the OT&E entities we created. */ private void saveAllEntities() { - // use ImmutableObject instead of Registry so that the Key generation doesn't break ImmutableList registries = ImmutableList.of(sunriseTld, gaTld, eapTld); - ImmutableList contacts = contactsBuilder.build(); + Map existingUsers = new HashMap<>(); + + users.forEach( + user -> + UserDao.loadUser(user.getEmailAddress()) + .ifPresent( + existingUser -> + existingUsers.put(existingUser.getEmailAddress(), existingUser))); + + if (!replaceExisting) { + checkState(existingUsers.isEmpty(), "Found existing users: %s", existingUsers); + } tm().transact( () -> { @@ -256,8 +297,7 @@ public final class OteAccountBuilder { ImmutableList> keys = Streams.concat( registries.stream().map(tld -> Tld.createVKey(tld.getTldStr())), - registrars.stream().map(Registrar::createVKey), - contacts.stream().map(RegistrarPoc::createVKey)) + registrars.stream().map(Registrar::createVKey)) .collect(toImmutableList()); ImmutableMap, ImmutableObject> existingObjects = tm().loadByKeysIfPresent(keys); @@ -275,8 +315,18 @@ public final class OteAccountBuilder { registrars = registrars.stream().map(this::addAllowedTld).collect(toImmutableList()); // and we can save the registrars and contacts! tm().putAll(registrars); - tm().putAll(contacts); }); + + for (User user : users) { + String email = user.getEmailAddress(); + if (existingUsers.containsKey(email)) { + // Note that other roles for the existing user are reset. We do this instead of simply + // saving the new user is that UserDao does not allow us to save the new user with the same + // email as the existing user. + user = existingUsers.get(email).asBuilder().setUserRoles(user.getUserRoles()).build(); + } + UserDao.saveUser(user); + } } private Registrar addAllowedTld(Registrar registrar) { @@ -336,15 +386,6 @@ public final class OteAccountBuilder { .build(); } - private static RegistrarPoc createRegistrarContact(String email, Registrar registrar) { - return new RegistrarPoc.Builder() - .setRegistrar(registrar) - .setName(email) - .setEmailAddress(email) - .setLoginEmailAddress(email) - .build(); - } - /** Returns the registrar IDs of the OT&E, with the TLDs each has access to. */ public static ImmutableMap createRegistrarIdToTldMap(String baseRegistrarId) { checkArgument( diff --git a/core/src/main/java/google/registry/model/console/User.java b/core/src/main/java/google/registry/model/console/User.java index f94e8d297..7d2eead60 100644 --- a/core/src/main/java/google/registry/model/console/User.java +++ b/core/src/main/java/google/registry/model/console/User.java @@ -14,7 +14,19 @@ package google.registry.model.console; +import static google.registry.tools.server.UpdateUserGroupAction.GROUP_UPDATE_QUEUE; + +import com.google.cloud.tasks.v2.Task; +import com.google.common.collect.ImmutableMultimap; +import com.google.common.flogger.FluentLogger; +import google.registry.batch.CloudTasksUtils; import google.registry.persistence.VKey; +import google.registry.request.Action.Service; +import google.registry.tools.IamClient; +import google.registry.tools.server.UpdateUserGroupAction; +import google.registry.tools.server.UpdateUserGroupAction.Mode; +import google.registry.util.RegistryEnvironment; +import java.util.Optional; import javax.persistence.Access; import javax.persistence.AccessType; import javax.persistence.Embeddable; @@ -31,6 +43,76 @@ import javax.persistence.Table; @Table(indexes = {@Index(columnList = "emailAddress", name = "user_email_address_idx")}) public class User extends UserBase { + public static final String IAP_SECURED_WEB_APP_USER_ROLE = "roles/iap.httpsResourceAccessor"; + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + + /** + * Grants the user permission to pass IAP. + * + *

Depending on if a console user group is set up, the permission is granted either + * individually or via group membership. + */ + public static void grantIapPermission( + String emailAddress, + Optional groupEmailAddress, + CloudTasksUtils cloudTasksUtils, + IamClient iamClient) { + if (RegistryEnvironment.isInTestServer()) { + return; + } + if (groupEmailAddress.isEmpty()) { + logger.atInfo().log("Granting IAP role to user %s", emailAddress); + iamClient.addBinding(emailAddress, IAP_SECURED_WEB_APP_USER_ROLE); + } else { + logger.atInfo().log("Adding %s to group %s", emailAddress, groupEmailAddress.get()); + modifyGroupMembershipAsync( + emailAddress, groupEmailAddress.get(), cloudTasksUtils, UpdateUserGroupAction.Mode.ADD); + } + } + + /** + * Revoke the user's permission to pass IAP. + * + *

Depending on if a console user group is set up, the permission is revoked either + * individually or via group membership. + */ + public static void revokeIapPermission( + String emailAddress, + Optional groupEmailAddress, + CloudTasksUtils cloudTasksUtils, + IamClient iamClient) { + if (RegistryEnvironment.isInTestServer()) { + return; + } + if (groupEmailAddress.isEmpty()) { + logger.atInfo().log("Removing IAP role from user %s", emailAddress); + iamClient.removeBinding(emailAddress, IAP_SECURED_WEB_APP_USER_ROLE); + } else { + logger.atInfo().log("Removing %s from group %s", emailAddress, groupEmailAddress.get()); + modifyGroupMembershipAsync( + emailAddress, groupEmailAddress.get(), cloudTasksUtils, Mode.REMOVE); + } + } + + private static void modifyGroupMembershipAsync( + String userEmailAddress, + String groupEmailAddress, + CloudTasksUtils cloudTasksUtils, + Mode mode) { + Task task = + cloudTasksUtils.createPostTask( + UpdateUserGroupAction.PATH, + Service.TOOLS, + ImmutableMultimap.of( + "userEmailAddress", + userEmailAddress, + "groupEmailAddress", + groupEmailAddress, + "groupUpdateMode", + mode.name())); + cloudTasksUtils.enqueue(GROUP_UPDATE_QUEUE, task); + } + @Override @Id @GeneratedValue(strategy = GenerationType.IDENTITY) diff --git a/core/src/main/java/google/registry/tools/CreateUserCommand.java b/core/src/main/java/google/registry/tools/CreateUserCommand.java index b157c3ca5..063cdcb3d 100644 --- a/core/src/main/java/google/registry/tools/CreateUserCommand.java +++ b/core/src/main/java/google/registry/tools/CreateUserCommand.java @@ -15,30 +15,25 @@ package google.registry.tools; import static com.google.common.base.Preconditions.checkArgument; +import static google.registry.model.console.User.grantIapPermission; import com.beust.jcommander.Parameters; -import com.google.common.collect.ImmutableMap; -import com.google.common.flogger.FluentLogger; -import com.google.common.net.MediaType; +import google.registry.batch.CloudTasksUtils; import google.registry.config.RegistryConfig.Config; import google.registry.model.console.User; import google.registry.model.console.UserDao; -import google.registry.tools.server.UpdateUserGroupAction; import java.util.Optional; import javax.annotation.Nullable; import javax.inject.Inject; /** Command to create a new User. */ @Parameters(separators = " =", commandDescription = "Update a user account") -public class CreateUserCommand extends CreateOrUpdateUserCommand implements CommandWithConnection { - - static final String IAP_SECURED_WEB_APP_USER_ROLE = "roles/iap.httpsResourceAccessor"; - static final FluentLogger logger = FluentLogger.forEnclosingClass(); - - private ServiceConnection connection; +public class CreateUserCommand extends CreateOrUpdateUserCommand { @Inject IamClient iamClient; + @Inject CloudTasksUtils cloudTasksUtils; + @Inject @Config("gSuiteConsoleUserGroupEmailAddress") Optional maybeGroupEmailAddress; @@ -53,29 +48,7 @@ public class CreateUserCommand extends CreateOrUpdateUserCommand implements Comm @Override protected String execute() throws Exception { String ret = super.execute(); - String groupEmailAddress = maybeGroupEmailAddress.orElse(null); - if (groupEmailAddress != null) { - logger.atInfo().log("Adding %s to group %s", email, groupEmailAddress); - connection.sendPostRequest( - UpdateUserGroupAction.PATH, - ImmutableMap.of( - "userEmailAddress", - email, - "groupEmailAddress", - groupEmailAddress, - "groupUpdateMode", - "ADD"), - MediaType.PLAIN_TEXT_UTF_8, - new byte[0]); - } else { - logger.atInfo().log("Granting IAP role to user %s", email); - iamClient.addBinding(email, IAP_SECURED_WEB_APP_USER_ROLE); - } + grantIapPermission(email, maybeGroupEmailAddress, cloudTasksUtils, iamClient); return ret; } - - @Override - public void setConnection(ServiceConnection connection) { - this.connection = connection; - } } diff --git a/core/src/main/java/google/registry/tools/DeleteUserCommand.java b/core/src/main/java/google/registry/tools/DeleteUserCommand.java index c026a5d11..a128ebf13 100644 --- a/core/src/main/java/google/registry/tools/DeleteUserCommand.java +++ b/core/src/main/java/google/registry/tools/DeleteUserCommand.java @@ -15,32 +15,27 @@ package google.registry.tools; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; -import static google.registry.tools.CreateUserCommand.IAP_SECURED_WEB_APP_USER_ROLE; import static google.registry.util.PreconditionsUtils.checkArgumentNotNull; import static google.registry.util.PreconditionsUtils.checkArgumentPresent; import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; -import com.google.common.collect.ImmutableMap; -import com.google.common.flogger.FluentLogger; -import com.google.common.net.MediaType; +import google.registry.batch.CloudTasksUtils; import google.registry.config.RegistryConfig.Config; import google.registry.model.console.User; import google.registry.model.console.UserDao; -import google.registry.tools.server.UpdateUserGroupAction; import java.util.Optional; import javax.annotation.Nullable; import javax.inject.Inject; /** Deletes a {@link User}. */ @Parameters(separators = " =", commandDescription = "Delete a user account") -public class DeleteUserCommand extends ConfirmingCommand implements CommandWithConnection { +public class DeleteUserCommand extends ConfirmingCommand { - private static final FluentLogger logger = FluentLogger.forEnclosingClass(); - - private ServiceConnection connection; @Inject IamClient iamClient; + @Inject CloudTasksUtils cloudTasksUtils; + @Inject @Config("gSuiteConsoleUserGroupEmailAddress") Optional maybeGroupEmailAddress; @@ -49,11 +44,6 @@ public class DeleteUserCommand extends ConfirmingCommand implements CommandWithC @Parameter(names = "--email", description = "Email address of the user", required = true) String email; - @Override - public void setConnection(ServiceConnection connection) { - this.connection = connection; - } - @Override protected String prompt() { checkArgumentNotNull(email, "Email must be provided"); @@ -69,24 +59,7 @@ public class DeleteUserCommand extends ConfirmingCommand implements CommandWithC checkArgumentPresent(optionalUser, "Email no longer corresponds to a valid user"); tm().delete(optionalUser.get()); }); - String groupEmailAddress = maybeGroupEmailAddress.orElse(null); - if (groupEmailAddress != null) { - logger.atInfo().log("Removing %s from group %s", email, groupEmailAddress); - connection.sendPostRequest( - UpdateUserGroupAction.PATH, - ImmutableMap.of( - "userEmailAddress", - email, - "groupEmailAddress", - groupEmailAddress, - "groupUpdateMode", - "REMOVE"), - MediaType.PLAIN_TEXT_UTF_8, - new byte[0]); - } else { - logger.atInfo().log("Removing IAP role from user %s", email); - iamClient.removeBinding(email, IAP_SECURED_WEB_APP_USER_ROLE); - } + User.revokeIapPermission(email, maybeGroupEmailAddress, cloudTasksUtils, iamClient); return String.format("Deleted user with email %s", email); } } diff --git a/core/src/main/java/google/registry/tools/IamClient.java b/core/src/main/java/google/registry/tools/IamClient.java index 202ad5d24..1cdb6c6c4 100644 --- a/core/src/main/java/google/registry/tools/IamClient.java +++ b/core/src/main/java/google/registry/tools/IamClient.java @@ -20,7 +20,7 @@ import com.google.api.services.cloudresourcemanager.model.GetIamPolicyRequest; import com.google.api.services.cloudresourcemanager.model.Policy; import com.google.api.services.cloudresourcemanager.model.SetIamPolicyRequest; import com.google.common.base.Ascii; -import google.registry.config.CredentialModule.LocalCredential; +import google.registry.config.CredentialModule.ApplicationDefaultCredential; import google.registry.config.RegistryConfig.Config; import google.registry.util.GoogleCredentialsBundle; import java.io.IOException; @@ -38,7 +38,7 @@ public class IamClient { @Inject public IamClient( - @LocalCredential GoogleCredentialsBundle credentialsBundle, + @ApplicationDefaultCredential GoogleCredentialsBundle credentialsBundle, @Config("projectId") String projectId) { this( new CloudResourceManager.Builder( diff --git a/core/src/main/java/google/registry/tools/RegistryToolDataflowModule.java b/core/src/main/java/google/registry/tools/RegistryToolDataflowModule.java index 8a3f4fa44..8ecdda87b 100644 --- a/core/src/main/java/google/registry/tools/RegistryToolDataflowModule.java +++ b/core/src/main/java/google/registry/tools/RegistryToolDataflowModule.java @@ -17,7 +17,7 @@ package google.registry.tools; import com.google.api.services.dataflow.Dataflow; import dagger.Module; import dagger.Provides; -import google.registry.config.CredentialModule.LocalCredential; +import google.registry.config.CredentialModule.ApplicationDefaultCredential; import google.registry.config.RegistryConfig.Config; import google.registry.util.GoogleCredentialsBundle; @@ -27,7 +27,7 @@ public class RegistryToolDataflowModule { @Provides static Dataflow provideDataflow( - @LocalCredential GoogleCredentialsBundle credentialsBundle, + @ApplicationDefaultCredential GoogleCredentialsBundle credentialsBundle, @Config("projectId") String projectId) { return new Dataflow.Builder( credentialsBundle.getHttpTransport(), diff --git a/core/src/main/java/google/registry/tools/SetupOteCommand.java b/core/src/main/java/google/registry/tools/SetupOteCommand.java index 48077f409..40d3a3f2b 100644 --- a/core/src/main/java/google/registry/tools/SetupOteCommand.java +++ b/core/src/main/java/google/registry/tools/SetupOteCommand.java @@ -22,6 +22,8 @@ import com.beust.jcommander.Parameter; import com.beust.jcommander.Parameters; import com.google.common.collect.ImmutableMap; import com.google.common.io.MoreFiles; +import google.registry.batch.CloudTasksUtils; +import google.registry.config.RegistryConfig.Config; import google.registry.model.OteAccountBuilder; import google.registry.tools.params.PathParameter; import google.registry.util.Clock; @@ -30,6 +32,7 @@ import google.registry.util.StringGenerator; import java.nio.file.Path; import java.util.ArrayList; import java.util.List; +import java.util.Optional; import javax.inject.Inject; import javax.inject.Named; @@ -47,7 +50,7 @@ final class SetupOteCommand extends ConfirmingCommand { @Parameter( names = {"-a", "--ip_allow_list"}, - description = "Comma-separated list of IP addreses or CIDR ranges.", + description = "Comma-separated list of IP addresses or CIDR ranges.", required = true) private List ipAllowList = new ArrayList<>(); @@ -55,7 +58,7 @@ final class SetupOteCommand extends ConfirmingCommand { names = {"--email"}, description = "The registrar's account to use for console access. " - + "Must be on the registry's G Suite domain.", + + "Must be on the registry's Google Workspace domain.", required = true) private String email; @@ -76,6 +79,14 @@ final class SetupOteCommand extends ConfirmingCommand { @Inject Clock clock; + @Inject CloudTasksUtils cloudTasksUtils; + + @Inject IamClient iamClient; + + @Inject + @Config("gSuiteConsoleUserGroupEmailAddress") + Optional maybeGroupEmailAddress; + OteAccountBuilder oteAccountBuilder; String password; @@ -87,7 +98,7 @@ final class SetupOteCommand extends ConfirmingCommand { password = passwordGenerator.createString(PASSWORD_LENGTH); oteAccountBuilder = OteAccountBuilder.forRegistrarId(registrar) - .addContact(email) + .addUser(email) .setPassword(password) .setIpAllowList(ipAllowList) .setReplaceExisting(overwrite); @@ -114,8 +125,11 @@ final class SetupOteCommand extends ConfirmingCommand { && RegistryEnvironment.get() != RegistryEnvironment.UNITTEST) { builder.append( String.format( - "\n\nWARNING: Running against %s environment. Are " - + "you sure you didn\'t mean to run this against sandbox (e.g. \"-e SANDBOX\")?", + """ + + + WARNING: Running against %s environment. Are \ + you sure you didn't mean to run this against sandbox (e.g. "-e SANDBOX")?""", RegistryEnvironment.get())); } @@ -125,15 +139,15 @@ final class SetupOteCommand extends ConfirmingCommand { @Override public String execute() { ImmutableMap clientIdToTld = oteAccountBuilder.buildAndPersist(); + oteAccountBuilder.grantIapPermission(maybeGroupEmailAddress, cloudTasksUtils, iamClient); StringBuilder output = new StringBuilder(); output.append("Copy these usernames/passwords back into the onboarding bug:\n\n"); clientIdToTld.forEach( - (clientId, tld) -> { - output.append( - String.format("Login: %s\nPassword: %s\nTLD: %s\n\n", clientId, password, tld)); - }); + (clientId, tld) -> + output.append( + String.format("Login: %s\nPassword: %s\nTLD: %s\n\n", clientId, password, tld))); return output.toString(); } diff --git a/core/src/main/java/google/registry/tools/server/UpdateUserGroupAction.java b/core/src/main/java/google/registry/tools/server/UpdateUserGroupAction.java index 38ffb6df2..7f0b1aa70 100644 --- a/core/src/main/java/google/registry/tools/server/UpdateUserGroupAction.java +++ b/core/src/main/java/google/registry/tools/server/UpdateUserGroupAction.java @@ -34,6 +34,7 @@ import javax.inject.Inject; public class UpdateUserGroupAction implements Runnable { public static final String PATH = "/_dr/admin/updateUserGroup"; + public static final String GROUP_UPDATE_QUEUE = "console-user-group-update"; private static final FluentLogger logger = FluentLogger.forEnclosingClass(); @@ -53,7 +54,7 @@ public class UpdateUserGroupAction implements Runnable { @Inject UpdateUserGroupAction() {} - enum Mode { + public enum Mode { ADD, REMOVE } diff --git a/core/src/main/java/google/registry/ui/server/registrar/ConsoleOteSetupAction.java b/core/src/main/java/google/registry/ui/server/registrar/ConsoleOteSetupAction.java index af36acae5..4a57bf85c 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/ConsoleOteSetupAction.java +++ b/core/src/main/java/google/registry/ui/server/registrar/ConsoleOteSetupAction.java @@ -24,6 +24,8 @@ import com.google.common.base.Supplier; import com.google.common.collect.ImmutableMap; import com.google.common.flogger.FluentLogger; import com.google.template.soy.tofu.SoyTofu; +import google.registry.batch.CloudTasksUtils; +import google.registry.config.RegistryConfig.Config; import google.registry.model.OteAccountBuilder; import google.registry.request.Action; import google.registry.request.Action.Method; @@ -31,6 +33,7 @@ import google.registry.request.HttpException.BadRequestException; import google.registry.request.Parameter; import google.registry.request.auth.Auth; import google.registry.request.auth.AuthenticatedRegistrarAccessor; +import google.registry.tools.IamClient; import google.registry.ui.server.SendEmailUtils; import google.registry.ui.server.SoyTemplateUtils; import google.registry.ui.soy.registrar.OteSetupConsoleSoyInfo; @@ -87,6 +90,14 @@ public final class ConsoleOteSetupAction extends HtmlAction { @Parameter("password") Optional optionalPassword; + @Inject CloudTasksUtils cloudTasksUtils; + + @Inject IamClient iamClient; + + @Inject + @Config("gSuiteConsoleUserGroupEmailAddress") + Optional maybeGroupEmailAddress; + @Inject ConsoleOteSetupAction() {} @@ -107,16 +118,11 @@ public final class ConsoleOteSetupAction extends HtmlAction { return; } switch (method) { - case POST -> { - runPost(data); - } - case GET -> { - runGet(data); - } - default -> { - throw new BadRequestException( - String.format("Action cannot be called with method %s", method)); - } + case POST -> runPost(data); + case GET -> runGet(data); + default -> + throw new BadRequestException( + String.format("Action cannot be called with method %s", method)); } } @@ -133,11 +139,13 @@ public final class ConsoleOteSetupAction extends HtmlAction { data.put("contactEmail", email.get()); String password = optionalPassword.orElse(passwordGenerator.createString(PASSWORD_LENGTH)); - ImmutableMap clientIdToTld = + OteAccountBuilder oteAccountBuilder = OteAccountBuilder.forRegistrarId(clientId.get()) - .addContact(email.get()) - .setPassword(password) - .buildAndPersist(); + .addUser(email.get()) + .setPassword(password); + ImmutableMap clientIdToTld = oteAccountBuilder.buildAndPersist(); + + oteAccountBuilder.grantIapPermission(maybeGroupEmailAddress, cloudTasksUtils, iamClient); sendExternalUpdates(clientIdToTld); diff --git a/core/src/main/java/google/registry/ui/server/registrar/ConsoleRegistrarCreatorAction.java b/core/src/main/java/google/registry/ui/server/registrar/ConsoleRegistrarCreatorAction.java index 7122b94c9..67f8cab8a 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/ConsoleRegistrarCreatorAction.java +++ b/core/src/main/java/google/registry/ui/server/registrar/ConsoleRegistrarCreatorAction.java @@ -26,10 +26,15 @@ import com.google.common.base.Splitter; import com.google.common.collect.ImmutableMap; import com.google.common.flogger.FluentLogger; import com.google.template.soy.tofu.SoyTofu; +import google.registry.batch.CloudTasksUtils; +import google.registry.config.RegistryConfig.Config; +import google.registry.model.console.RegistrarRole; +import google.registry.model.console.User; +import google.registry.model.console.UserDao; +import google.registry.model.console.UserRoles; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarAddress; import google.registry.model.registrar.RegistrarBase.State; -import google.registry.model.registrar.RegistrarPoc; import google.registry.request.Action; import google.registry.request.Action.Method; import google.registry.request.Action.Service; @@ -37,6 +42,7 @@ import google.registry.request.HttpException.BadRequestException; import google.registry.request.Parameter; import google.registry.request.auth.Auth; import google.registry.request.auth.AuthenticatedRegistrarAccessor; +import google.registry.tools.IamClient; import google.registry.ui.server.SendEmailUtils; import google.registry.ui.server.SoyTemplateUtils; import google.registry.ui.soy.registrar.AnalyticsSoyInfo; @@ -95,6 +101,14 @@ public final class ConsoleRegistrarCreatorAction extends HtmlAction { @Parameter("consoleName") Optional name; + @Inject CloudTasksUtils cloudTasksUtils; + + @Inject IamClient iamClient; + + @Inject + @Config("gSuiteConsoleUserGroupEmailAddress") + Optional maybeGroupEmailAddress; + @Inject @Parameter("billingAccount") Optional billingAccount; @Inject @Parameter("ianaId") Optional ianaId; @Inject @Parameter("referralEmail") Optional referralEmail; @@ -129,16 +143,11 @@ public final class ConsoleRegistrarCreatorAction extends HtmlAction { return; } switch (method) { - case POST -> { - runPost(data); - } - case GET -> { - runGet(data); - } - default -> { - throw new BadRequestException( - String.format("Action cannot be called with method %s", method)); - } + case POST -> runPost(data); + case GET -> runGet(data); + default -> + throw new BadRequestException( + String.format("Action cannot be called with method %s", method)); } } @@ -169,7 +178,8 @@ public final class ConsoleRegistrarCreatorAction extends HtmlAction { list)) .collect( toImmutableMap( - list -> CurrencyUnit.of(Ascii.toUpperCase(list.get(0))), list -> list.get(1))); + list -> CurrencyUnit.of(Ascii.toUpperCase(list.getFirst())), + list -> list.get(1))); } catch (Throwable e) { throw new RuntimeException("Error parsing billing accounts - " + e.getMessage(), e); } @@ -233,12 +243,15 @@ public final class ConsoleRegistrarCreatorAction extends HtmlAction { .setZip(optionalZip.orElse(null)) .build()) .build(); - RegistrarPoc contact = - new RegistrarPoc.Builder() - .setRegistrar(registrar) - .setName(consoleUserEmail.get()) + User user = + new User.Builder() .setEmailAddress(consoleUserEmail.get()) - .setLoginEmailAddress(consoleUserEmail.get()) + .setUserRoles( + new UserRoles.Builder() + .setRegistrarRoles( + ImmutableMap.of( + registrar.getRegistrarId(), RegistrarRole.ACCOUNT_MANAGER)) + .build()) .build(); tm().transact( () -> { @@ -246,8 +259,11 @@ public final class ConsoleRegistrarCreatorAction extends HtmlAction { Registrar.loadByRegistrarId(registrar.getRegistrarId()).isEmpty(), "Registrar with client ID %s already exists", registrar.getRegistrarId()); - tm().putAll(registrar, contact); + tm().put(registrar); }); + UserDao.saveUser(user); + User.grantIapPermission( + user.getEmailAddress(), maybeGroupEmailAddress, cloudTasksUtils, iamClient); data.put("password", password); data.put("passcode", phonePasscode); diff --git a/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java b/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java index 79fa179bc..48c4efc80 100644 --- a/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java +++ b/core/src/main/java/google/registry/ui/server/registrar/RegistrarSettingsAction.java @@ -88,22 +88,6 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA static final String ARGS_PARAM = "args"; static final String ID_PARAM = "id"; - /** - * Allows task enqueueing to be disabled when executing registrar console test cases. - * - *

The existing workflow in UI test cases triggers task enqueueing, which was not an issue with - * Task Queue since it's a native App Engine feature simulated by the App Engine SDK's - * environment. However, with Cloud Tasks, the server enqueues and fails to deliver to the actual - * Cloud Tasks endpoint due to lack of permission. - * - *

One way to allow enqueuing in backend test and avoid enqueuing in UI test is to disable - * enqueuing when the test server starts and enable enqueueing once the test server stops. This - * can be done by utilizing a ThreadLocal variable isInTestDriver, which is set to false - * by default. Enqueuing is allowed only if the value of isInTestDriver is false. It's set to true - * in start() and set to false in stop() inside TestDriver.java, a class used in testing. - */ - private static final ThreadLocal isInTestDriver = ThreadLocal.withInitial(() -> false); - @Inject JsonActionRunner jsonActionRunner; @Inject RegistrarConsoleMetrics registrarConsoleMetrics; @Inject SendEmailUtils sendEmailUtils; @@ -118,14 +102,6 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA return contact.getPhoneNumber() != null; } - public static void setIsInTestDriverToFalse() { - isInTestDriver.set(false); - } - - public static void setIsInTestDriverToTrue() { - isInTestDriver.set(true); - } - @Override public void run() { jsonActionRunner.run(this); @@ -623,7 +599,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA if (CollectionUtils.difference(changedKeys, "lastUpdateTime").isEmpty()) { return; } - if (!isInTestDriver.get()) { + if (!RegistryEnvironment.isInTestServer()) { // Enqueues a sync registrar sheet task if enqueuing is not triggered by console tests and // there's an update besides the lastUpdateTime cloudTasksUtils.enqueue( diff --git a/core/src/test/java/google/registry/model/OteAccountBuilderTest.java b/core/src/test/java/google/registry/model/OteAccountBuilderTest.java index 471ba6b03..5747c139c 100644 --- a/core/src/test/java/google/registry/model/OteAccountBuilderTest.java +++ b/core/src/test/java/google/registry/model/OteAccountBuilderTest.java @@ -15,6 +15,7 @@ package google.registry.model; import static com.google.common.truth.Truth.assertThat; +import static google.registry.model.console.User.IAP_SECURED_WEB_APP_USER_ROLE; import static google.registry.model.tld.Tld.TldState.GENERAL_AVAILABILITY; import static google.registry.model.tld.Tld.TldState.START_DATE_SUNRISE; import static google.registry.persistence.transaction.JpaTransactionManagerExtension.makeRegistrar1; @@ -26,16 +27,28 @@ import static google.registry.testing.DatabaseHelper.persistSimpleResource; import static google.registry.util.DateTimeUtils.START_OF_TIME; import static org.joda.money.CurrencyUnit.USD; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import com.google.cloud.tasks.v2.HttpMethod; import com.google.common.collect.ImmutableList; +import google.registry.batch.CloudTasksUtils; +import google.registry.model.console.RegistrarRole; +import google.registry.model.console.User; +import google.registry.model.console.UserDao; import google.registry.model.registrar.Registrar; -import google.registry.model.registrar.RegistrarPoc; import google.registry.model.tld.Tld; import google.registry.model.tld.Tld.TldState; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; +import google.registry.testing.CloudTasksHelper; +import google.registry.testing.CloudTasksHelper.TaskMatcher; +import google.registry.tools.IamClient; import google.registry.util.CidrAddressBlock; import google.registry.util.SystemClock; +import java.util.Optional; +import javax.annotation.Nullable; import org.joda.money.Money; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; @@ -50,6 +63,9 @@ public final class OteAccountBuilderTest { final JpaIntegrationTestExtension jpa = new JpaTestExtensions.Builder().buildIntegrationTestExtension(); + private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(); + private final IamClient iamClient = mock(IamClient.class); + @Test void testGetRegistrarToTldMap() { assertThat(OteAccountBuilder.forRegistrarId("myclientid").getRegistrarIdToTldMap()) @@ -89,23 +105,63 @@ public final class OteAccountBuilderTest { assertThat(registrar.getAllowedTlds()).containsExactly(tld); } - private static void assertContactExists(String registrarId, String email) { - Registrar registrar = Registrar.loadByRegistrarId(registrarId).get(); - assertThat(registrar.getContacts().stream().map(RegistrarPoc::getEmailAddress)).contains(email); - RegistrarPoc contact = - registrar.getContacts().stream() - .filter(c -> email.equals(c.getEmailAddress())) - .findAny() - .get(); - assertThat(contact.getEmailAddress()).isEqualTo(email); - assertThat(contact.getLoginEmailAddress()).isEqualTo(email); + public static void verifyUser(String registrarId, String email) { + Optional maybeUser = UserDao.loadUser(email); + assertThat(maybeUser).isPresent(); + assertThat(maybeUser.get().getUserRoles().getRegistrarRoles().get(registrarId)) + .isEqualTo(RegistrarRole.ACCOUNT_MANAGER); + } + + public static void verifyIapPermission( + @Nullable String emailAddress, + Optional maybeGroupEmailAddress, + CloudTasksHelper cloudTasksHelper, + IamClient iamClient) { + if (emailAddress == null) { + cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); + verifyNoInteractions(iamClient); + } else { + String groupEmailAddress = maybeGroupEmailAddress.orElse(null); + if (groupEmailAddress == null) { + cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); + verify(iamClient).addBinding(emailAddress, IAP_SECURED_WEB_APP_USER_ROLE); + } else { + cloudTasksHelper.assertTasksEnqueued( + "console-user-group-update", + new TaskMatcher() + .service("TOOLS") + .method(HttpMethod.POST) + .path("/_dr/admin/updateUserGroup") + .param("userEmailAddress", emailAddress) + .param("groupEmailAddress", groupEmailAddress) + .param("groupUpdateMode", "ADD")); + verifyNoInteractions(iamClient); + } + } + } + + @Test + void testUpdateUserGroup() { + CloudTasksUtils cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); + OteAccountBuilder.forRegistrarId("myclientid") + .addUser("email@example.com") + .grantIapPermission(Optional.of("console@example.com"), cloudTasksUtils, iamClient); + verifyIapPermission( + "email@example.com", Optional.of("console@example.com"), cloudTasksHelper, iamClient); + } + + @Test + void testGrantIndividualPermission() { + CloudTasksUtils cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); + OteAccountBuilder.forRegistrarId("myclientid") + .addUser("email@example.com") + .grantIapPermission(Optional.empty(), cloudTasksUtils, iamClient); + verifyIapPermission("email@example.com", Optional.empty(), cloudTasksHelper, iamClient); } @Test void testCreateOteEntities_success() { - OteAccountBuilder.forRegistrarId("myclientid") - .addContact("email@example.com") - .buildAndPersist(); + OteAccountBuilder.forRegistrarId("myclientid").addUser("email@example.com").buildAndPersist(); assertTldExists("myclientid-sunrise", START_DATE_SUNRISE, Money.zero(USD)); assertTldExists("myclientid-ga", GENERAL_AVAILABILITY, Money.zero(USD)); @@ -114,18 +170,18 @@ public final class OteAccountBuilderTest { assertRegistrarExists("myclientid-3", "myclientid-ga"); assertRegistrarExists("myclientid-4", "myclientid-ga"); assertRegistrarExists("myclientid-5", "myclientid-eap"); - assertContactExists("myclientid-1", "email@example.com"); - assertContactExists("myclientid-3", "email@example.com"); - assertContactExists("myclientid-4", "email@example.com"); - assertContactExists("myclientid-5", "email@example.com"); + verifyUser("myclientid-1", "email@example.com"); + verifyUser("myclientid-3", "email@example.com"); + verifyUser("myclientid-4", "email@example.com"); + verifyUser("myclientid-5", "email@example.com"); } @Test void testCreateOteEntities_multipleContacts_success() { OteAccountBuilder.forRegistrarId("myclientid") - .addContact("email@example.com") - .addContact("other@example.com") - .addContact("someone@example.com") + .addUser("email@example.com") + .addUser("other@example.com") + .addUser("someone@example.com") .buildAndPersist(); assertTldExists("myclientid-sunrise", START_DATE_SUNRISE, Money.zero(USD)); @@ -135,18 +191,18 @@ public final class OteAccountBuilderTest { assertRegistrarExists("myclientid-3", "myclientid-ga"); assertRegistrarExists("myclientid-4", "myclientid-ga"); assertRegistrarExists("myclientid-5", "myclientid-eap"); - assertContactExists("myclientid-1", "email@example.com"); - assertContactExists("myclientid-3", "email@example.com"); - assertContactExists("myclientid-4", "email@example.com"); - assertContactExists("myclientid-5", "email@example.com"); - assertContactExists("myclientid-1", "other@example.com"); - assertContactExists("myclientid-3", "other@example.com"); - assertContactExists("myclientid-4", "other@example.com"); - assertContactExists("myclientid-5", "other@example.com"); - assertContactExists("myclientid-1", "someone@example.com"); - assertContactExists("myclientid-3", "someone@example.com"); - assertContactExists("myclientid-4", "someone@example.com"); - assertContactExists("myclientid-5", "someone@example.com"); + verifyUser("myclientid-1", "email@example.com"); + verifyUser("myclientid-3", "email@example.com"); + verifyUser("myclientid-4", "email@example.com"); + verifyUser("myclientid-5", "email@example.com"); + verifyUser("myclientid-1", "other@example.com"); + verifyUser("myclientid-3", "other@example.com"); + verifyUser("myclientid-4", "other@example.com"); + verifyUser("myclientid-5", "other@example.com"); + verifyUser("myclientid-1", "someone@example.com"); + verifyUser("myclientid-3", "someone@example.com"); + verifyUser("myclientid-4", "someone@example.com"); + verifyUser("myclientid-5", "someone@example.com"); } @Test @@ -223,7 +279,7 @@ public final class OteAccountBuilderTest { OteAccountBuilder oteSetupHelper = OteAccountBuilder.forRegistrarId("myclientid"); IllegalStateException thrown = - assertThrows(IllegalStateException.class, () -> oteSetupHelper.buildAndPersist()); + assertThrows(IllegalStateException.class, oteSetupHelper::buildAndPersist); assertThat(thrown) .hasMessageThat() .contains("Found existing object(s) conflicting with OT&E objects"); @@ -236,7 +292,7 @@ public final class OteAccountBuilderTest { OteAccountBuilder oteSetupHelper = OteAccountBuilder.forRegistrarId("myclientid"); IllegalStateException thrown = - assertThrows(IllegalStateException.class, () -> oteSetupHelper.buildAndPersist()); + assertThrows(IllegalStateException.class, oteSetupHelper::buildAndPersist); assertThat(thrown) .hasMessageThat() .contains("Found existing object(s) conflicting with OT&E objects"); @@ -261,7 +317,7 @@ public final class OteAccountBuilderTest { void testCreateOteEntities_doubleCreation_actuallyReplaces() { OteAccountBuilder.forRegistrarId("myclientid") .setPassword("oldPassword") - .addContact("email@example.com") + .addUser("email@example.com") .buildAndPersist(); assertThat(Registrar.loadByRegistrarId("myclientid-3").get().verifyPassword("oldPassword")) @@ -269,7 +325,7 @@ public final class OteAccountBuilderTest { OteAccountBuilder.forRegistrarId("myclientid") .setPassword("newPassword") - .addContact("email@example.com") + .addUser("email@example.com") .setReplaceExisting(true) .buildAndPersist(); @@ -281,19 +337,17 @@ public final class OteAccountBuilderTest { @Test void testCreateOteEntities_doubleCreation_keepsOldContacts() { - OteAccountBuilder.forRegistrarId("myclientid") - .addContact("email@example.com") - .buildAndPersist(); + OteAccountBuilder.forRegistrarId("myclientid").addUser("email@example.com").buildAndPersist(); - assertContactExists("myclientid-3", "email@example.com"); + verifyUser("myclientid-3", "email@example.com"); OteAccountBuilder.forRegistrarId("myclientid") - .addContact("other@example.com") + .addUser("other@example.com") .setReplaceExisting(true) .buildAndPersist(); - assertContactExists("myclientid-3", "other@example.com"); - assertContactExists("myclientid-3", "email@example.com"); + verifyUser("myclientid-3", "other@example.com"); + verifyUser("myclientid-3", "email@example.com"); } @Test diff --git a/core/src/test/java/google/registry/model/OteStatsTestHelper.java b/core/src/test/java/google/registry/model/OteStatsTestHelper.java index bf515a175..892ceb794 100644 --- a/core/src/test/java/google/registry/model/OteStatsTestHelper.java +++ b/core/src/test/java/google/registry/model/OteStatsTestHelper.java @@ -79,9 +79,7 @@ public final class OteStatsTestHelper { public static void setupIncompleteOte(String baseClientId) throws IOException { createTld("tld"); persistPremiumList("default_sandbox_list", USD, "sandbox,USD 1000"); - OteAccountBuilder.forRegistrarId(baseClientId) - .addContact("email@example.com") - .buildAndPersist(); + OteAccountBuilder.forRegistrarId(baseClientId).buildAndPersist(); String oteAccount1 = String.format("%s-1", baseClientId); DateTime now = DateTime.now(DateTimeZone.UTC); persistResource( diff --git a/core/src/test/java/google/registry/model/console/UserTest.java b/core/src/test/java/google/registry/model/console/UserTest.java index 68a640381..a72947dfb 100644 --- a/core/src/test/java/google/registry/model/console/UserTest.java +++ b/core/src/test/java/google/registry/model/console/UserTest.java @@ -16,11 +16,21 @@ package google.registry.model.console; import static com.google.common.truth.Truth.assertThat; import static google.registry.model.ImmutableObjectSubject.assertAboutImmutableObjects; +import static google.registry.model.console.User.IAP_SECURED_WEB_APP_USER_ROLE; import static google.registry.persistence.transaction.TransactionManagerFactory.tm; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoMoreInteractions; +import com.google.cloud.tasks.v2.HttpMethod; +import google.registry.batch.CloudTasksUtils; import google.registry.model.EntityTestCase; +import google.registry.testing.CloudTasksHelper; +import google.registry.testing.CloudTasksHelper.TaskMatcher; import google.registry.testing.DatabaseHelper; +import google.registry.tools.IamClient; +import java.util.Optional; import org.junit.jupiter.api.Test; /** Tests for {@link User}. */ @@ -104,4 +114,56 @@ public class UserTest extends EntityTestCase { assertThat(user.hasRegistryLockPassword()).isFalse(); assertThat(user.verifyRegistryLockPassword("foobar")).isFalse(); } + + @Test + void testGrantIapPermission() { + CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(); + IamClient iamClient = mock(IamClient.class); + CloudTasksUtils cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); + + // Individual permission. + User.grantIapPermission("email@example.com", Optional.empty(), cloudTasksUtils, iamClient); + cloudTasksHelper.assertNoTasksEnqueued(); + verify(iamClient).addBinding("email@example.com", IAP_SECURED_WEB_APP_USER_ROLE); + + // Group membership. + User.grantIapPermission( + "email@example.com", Optional.of("console@example.com"), cloudTasksUtils, iamClient); + cloudTasksHelper.assertTasksEnqueued( + "console-user-group-update", + new TaskMatcher() + .service("TOOLS") + .method(HttpMethod.POST) + .path("/_dr/admin/updateUserGroup") + .param("userEmailAddress", "email@example.com") + .param("groupEmailAddress", "console@example.com") + .param("groupUpdateMode", "ADD")); + verifyNoMoreInteractions(iamClient); + } + + @Test + void testRevokeIapPermission() { + CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(); + IamClient iamClient = mock(IamClient.class); + CloudTasksUtils cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); + + // Individual permission. + User.revokeIapPermission("email@example.com", Optional.empty(), cloudTasksUtils, iamClient); + cloudTasksHelper.assertNoTasksEnqueued(); + verify(iamClient).removeBinding("email@example.com", IAP_SECURED_WEB_APP_USER_ROLE); + + // Group membership. + User.revokeIapPermission( + "email@example.com", Optional.of("console@example.com"), cloudTasksUtils, iamClient); + cloudTasksHelper.assertTasksEnqueued( + "console-user-group-update", + new TaskMatcher() + .service("TOOLS") + .method(HttpMethod.POST) + .path("/_dr/admin/updateUserGroup") + .param("userEmailAddress", "email@example.com") + .param("groupEmailAddress", "console@example.com") + .param("groupUpdateMode", "REMOVE")); + verifyNoMoreInteractions(iamClient); + } } diff --git a/core/src/test/java/google/registry/server/RegistryTestServerMain.java b/core/src/test/java/google/registry/server/RegistryTestServerMain.java index 4c862d39b..3b67eed0e 100644 --- a/core/src/test/java/google/registry/server/RegistryTestServerMain.java +++ b/core/src/test/java/google/registry/server/RegistryTestServerMain.java @@ -135,7 +135,7 @@ public final class RegistryTestServerMain { final RegistryTestServer server = new RegistryTestServer(address); - System.out.printf("%sLoading SQL fixtures and User service...%s\n", BLUE, RESET); + System.out.printf("%sLoading SQL fixtures setting User for authentication...%s\n", BLUE, RESET); UserRoles userRoles = new UserRoles.Builder().setIsAdmin(loginIsAdmin).setGlobalRole(GlobalRole.FTE).build(); User user = @@ -151,7 +151,7 @@ public final class RegistryTestServerMain { for (Fixture fixture : fixtures) { fixture.load(); } - System.out.printf("%sStarting Jetty6 HTTP Server...%s\n", BLUE, RESET); + System.out.printf("%sStarting Jetty HTTP Server...%s\n", BLUE, RESET); server.start(); System.out.printf("%sListening on: %s%s\n", PURPLE, server.getUrl("/"), RESET); try { diff --git a/core/src/test/java/google/registry/server/TestServer.java b/core/src/test/java/google/registry/server/TestServer.java index 385ecba44..31033505c 100644 --- a/core/src/test/java/google/registry/server/TestServer.java +++ b/core/src/test/java/google/registry/server/TestServer.java @@ -24,7 +24,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.net.HostAndPort; import com.google.common.util.concurrent.SimpleTimeLimiter; -import google.registry.ui.server.registrar.RegistrarSettingsAction; +import google.registry.util.RegistryEnvironment; import google.registry.util.UrlChecker; import jakarta.servlet.http.HttpServlet; import java.net.MalformedURLException; @@ -91,7 +91,7 @@ public final class TestServer { /** Starts the HTTP server in a new thread and returns once it's online. */ public void start() { try { - RegistrarSettingsAction.setIsInTestDriverToTrue(); + RegistryEnvironment.setIsInTestDriver(true); server.start(); } catch (Exception e) { throwIfUnchecked(e); @@ -129,7 +129,7 @@ public final class TestServer { .callWithTimeout( () -> { server.stop(); - RegistrarSettingsAction.setIsInTestDriverToFalse(); + RegistryEnvironment.setIsInTestDriver(false); return null; }, SHUTDOWN_TIMEOUT_MS, diff --git a/core/src/test/java/google/registry/testing/CloudTasksHelper.java b/core/src/test/java/google/registry/testing/CloudTasksHelper.java index 839acd1b7..66736fdbc 100644 --- a/core/src/test/java/google/registry/testing/CloudTasksHelper.java +++ b/core/src/test/java/google/registry/testing/CloudTasksHelper.java @@ -80,12 +80,12 @@ import org.joda.time.DateTime; * to the same test task container that the original instance pushes to, so that we can make * assertions on them by accessing the original instance. We cannot make the test task container * itself static because we do not want tasks enqueued in previous tests to interfere with latter - * tests, when they run on the same JVM (and therefore share the same static class members). To - * solve this we put the test container in a static map whose keys are the instance IDs. An - * explicitly created new {@link CloudTasksHelper} (as would be created for a new test method) would - * have a new ID allocated to it, and therefore stores its tasks in a distinct container. A - * deserialized {@link CloudTasksHelper}, on the other hand, will have the same instance ID and - * share the same test class container with its progenitor. + * tests when they run on the same JVM (and therefore share the same static class members). To solve + * this, we put the test container in a static map whose keys are the instance IDs. An explicitly + * created new {@link CloudTasksHelper} (as would be created for a new test method) would have a new + * ID allocated to it, and therefore stores its tasks in a distinct container. A deserialized {@link + * CloudTasksHelper}, on the other hand, will have the same instance ID and share the same test + * class container with its progenitor. */ public class CloudTasksHelper implements Serializable { @@ -131,7 +131,7 @@ public class CloudTasksHelper implements Serializable { */ public void assertTasksEnqueuedWithProperty( String queueName, Function propertyGetter, String... expectedTaskProperties) { - // Ordering is irrelevant but duplicates should be considered independently. + // Ordering is irrelevant, but duplicates should be considered independently. assertThat(getTestTasksFor(queueName).stream().map(propertyGetter)) .containsExactly((Object[]) expectedTaskProperties); } @@ -285,7 +285,7 @@ public class CloudTasksHelper implements Serializable { }); headers = headerBuilder.build(); ImmutableMultimap.Builder paramBuilder = new ImmutableMultimap.Builder<>(); - // Note that UriParameters.parse() does not throw an IAE on a bad query string (e.g. one + // Note that UriParameters.parse() does not throw an IAE on a bad query string (e.g., one // where parameters are not properly URL-encoded); it always does a best-effort parse. if (method == HttpMethod.GET && uri.getQuery() != null) { paramBuilder.putAll(UriParameters.parse(uri.getQuery())); @@ -382,7 +382,7 @@ public class CloudTasksHelper implements Serializable { * the same contract as {@link #equals}, since it will ignore null fields. * *

Match fails if any headers or params expected on the TaskMatcher are not found on the - * Task. Note that the inverse is not true (i.e. there may be extra headers on the Task). + * Task. Note that the inverse is not true (i.e., there may be extra headers on the Task). * *

Schedule time by default is Timestamp.getDefaultInstance() or null. */ diff --git a/core/src/test/java/google/registry/tools/CreateUserCommandTest.java b/core/src/test/java/google/registry/tools/CreateUserCommandTest.java index 09280f9a5..c322bf795 100644 --- a/core/src/test/java/google/registry/tools/CreateUserCommandTest.java +++ b/core/src/test/java/google/registry/tools/CreateUserCommandTest.java @@ -15,20 +15,22 @@ package google.registry.tools; import static com.google.common.truth.Truth.assertThat; -import static google.registry.tools.CreateUserCommand.IAP_SECURED_WEB_APP_USER_ROLE; +import static google.registry.model.console.User.IAP_SECURED_WEB_APP_USER_ROLE; import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; +import com.google.cloud.tasks.v2.HttpMethod; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; -import com.google.common.net.MediaType; import google.registry.model.console.GlobalRole; import google.registry.model.console.RegistrarRole; import google.registry.model.console.User; import google.registry.model.console.UserDao; +import google.registry.testing.CloudTasksHelper; +import google.registry.testing.CloudTasksHelper.TaskMatcher; import google.registry.testing.DatabaseHelper; import java.util.Optional; import org.junit.jupiter.api.BeforeEach; @@ -38,13 +40,13 @@ import org.junit.jupiter.api.Test; public class CreateUserCommandTest extends CommandTestCase { private final IamClient iamClient = mock(IamClient.class); - private final ServiceConnection connection = mock(ServiceConnection.class); + private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(); @BeforeEach void beforeEach() { command.iamClient = iamClient; command.maybeGroupEmailAddress = Optional.empty(); - command.setConnection(connection); + command.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); } @Test @@ -57,7 +59,7 @@ public class CreateUserCommandTest extends CommandTestCase { assertThat(onlyUser.getUserRoles().getRegistrarRoles()).isEmpty(); verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE); verifyNoMoreInteractions(iamClient); - verifyNoInteractions(connection); + cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); } @Test @@ -69,20 +71,16 @@ public class CreateUserCommandTest extends CommandTestCase { assertThat(onlyUser.getUserRoles().isAdmin()).isFalse(); assertThat(onlyUser.getUserRoles().getGlobalRole()).isEqualTo(GlobalRole.NONE); assertThat(onlyUser.getUserRoles().getRegistrarRoles()).isEmpty(); - verify(connection) - .sendPostRequest( - "/_dr/admin/updateUserGroup", - ImmutableMap.of( - "userEmailAddress", - "user@example.test", - "groupEmailAddress", - "group@example.test", - "groupUpdateMode", - "ADD"), - MediaType.PLAIN_TEXT_UTF_8, - new byte[0]); + cloudTasksHelper.assertTasksEnqueued( + "console-user-group-update", + new TaskMatcher() + .method(HttpMethod.POST) + .service("TOOLS") + .path("/_dr/admin/updateUserGroup") + .param("userEmailAddress", "user@example.test") + .param("groupEmailAddress", "group@example.test") + .param("groupUpdateMode", "ADD")); verifyNoInteractions(iamClient); - verifyNoMoreInteractions(connection); } @Test @@ -102,7 +100,7 @@ public class CreateUserCommandTest extends CommandTestCase { assertThat(UserDao.loadUser("user@example.test").get().getUserRoles().isAdmin()).isTrue(); verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE); verifyNoMoreInteractions(iamClient); - verifyNoInteractions(connection); + cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); } @Test @@ -112,7 +110,7 @@ public class CreateUserCommandTest extends CommandTestCase { .isEqualTo(GlobalRole.FTE); verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE); verifyNoMoreInteractions(iamClient); - verifyNoInteractions(connection); + cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); } @Test @@ -131,7 +129,7 @@ public class CreateUserCommandTest extends CommandTestCase { RegistrarRole.PRIMARY_CONTACT)); verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE); verifyNoMoreInteractions(iamClient); - verifyNoInteractions(connection); + cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); } @Test @@ -146,7 +144,7 @@ public class CreateUserCommandTest extends CommandTestCase { .hasMessageThat() .isEqualTo("A user with email user@example.test already exists"); verifyNoMoreInteractions(iamClient); - verifyNoInteractions(connection); + cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); } @Test diff --git a/core/src/test/java/google/registry/tools/DeleteUserCommandTest.java b/core/src/test/java/google/registry/tools/DeleteUserCommandTest.java index 509a08061..39cb2733f 100644 --- a/core/src/test/java/google/registry/tools/DeleteUserCommandTest.java +++ b/core/src/test/java/google/registry/tools/DeleteUserCommandTest.java @@ -15,16 +15,17 @@ package google.registry.tools; import static com.google.common.truth.Truth.assertThat; -import static google.registry.tools.CreateUserCommand.IAP_SECURED_WEB_APP_USER_ROLE; +import static google.registry.model.console.User.IAP_SECURED_WEB_APP_USER_ROLE; import static org.junit.Assert.assertThrows; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.verifyNoMoreInteractions; -import com.google.common.collect.ImmutableMap; -import com.google.common.net.MediaType; +import com.google.cloud.tasks.v2.HttpMethod; import google.registry.model.console.UserDao; +import google.registry.testing.CloudTasksHelper; +import google.registry.testing.CloudTasksHelper.TaskMatcher; import google.registry.testing.DatabaseHelper; import java.util.Optional; import org.junit.jupiter.api.BeforeEach; @@ -34,13 +35,13 @@ import org.junit.jupiter.api.Test; public class DeleteUserCommandTest extends CommandTestCase { private final IamClient iamClient = mock(IamClient.class); - private final ServiceConnection connection = mock(ServiceConnection.class); + private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(); @BeforeEach void beforeEach() { command.iamClient = iamClient; - command.setConnection(connection); command.maybeGroupEmailAddress = Optional.empty(); + command.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); } @Test @@ -51,7 +52,7 @@ public class DeleteUserCommandTest extends CommandTestCase { assertThat(UserDao.loadUser("email@example.test")).isEmpty(); verify(iamClient).removeBinding("email@example.test", IAP_SECURED_WEB_APP_USER_ROLE); verifyNoMoreInteractions(iamClient); - verifyNoInteractions(connection); + cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); } @Test @@ -61,20 +62,16 @@ public class DeleteUserCommandTest extends CommandTestCase { assertThat(UserDao.loadUser("email@example.test")).isPresent(); runCommandForced("--email", "email@example.test"); assertThat(UserDao.loadUser("email@example.test")).isEmpty(); - verify(connection) - .sendPostRequest( - "/_dr/admin/updateUserGroup", - ImmutableMap.of( - "userEmailAddress", - "email@example.test", - "groupEmailAddress", - "group@example.test", - "groupUpdateMode", - "REMOVE"), - MediaType.PLAIN_TEXT_UTF_8, - new byte[0]); + cloudTasksHelper.assertTasksEnqueued( + "console-user-group-update", + new TaskMatcher() + .method(HttpMethod.POST) + .service("TOOLS") + .path("/_dr/admin/updateUserGroup") + .param("userEmailAddress", "email@example.test") + .param("groupEmailAddress", "group@example.test") + .param("groupUpdateMode", "REMOVE")); verifyNoInteractions(iamClient); - verifyNoMoreInteractions(connection); } @Test @@ -86,6 +83,6 @@ public class DeleteUserCommandTest extends CommandTestCase { .hasMessageThat() .isEqualTo("Email does not correspond to a valid user"); verifyNoInteractions(iamClient); - verifyNoInteractions(connection); + cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); } } diff --git a/core/src/test/java/google/registry/tools/SetupOteCommandTest.java b/core/src/test/java/google/registry/tools/SetupOteCommandTest.java index b19d31d3f..294d299b9 100644 --- a/core/src/test/java/google/registry/tools/SetupOteCommandTest.java +++ b/core/src/test/java/google/registry/tools/SetupOteCommandTest.java @@ -15,6 +15,8 @@ package google.registry.tools; import static com.google.common.truth.Truth.assertThat; +import static google.registry.model.OteAccountBuilderTest.verifyUser; +import static google.registry.model.console.User.IAP_SECURED_WEB_APP_USER_ROLE; import static google.registry.model.registrar.RegistrarBase.State.ACTIVE; import static google.registry.model.tld.Tld.TldState.GENERAL_AVAILABILITY; import static google.registry.model.tld.Tld.TldState.START_DATE_SUNRISE; @@ -26,19 +28,30 @@ import static google.registry.testing.DatabaseHelper.persistResource; import static org.joda.money.CurrencyUnit.USD; import static org.joda.time.DateTimeZone.UTC; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import com.beust.jcommander.ParameterException; +import com.google.cloud.tasks.v2.HttpMethod; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedMap; +import google.registry.model.console.GlobalRole; +import google.registry.model.console.User; +import google.registry.model.console.UserDao; +import google.registry.model.console.UserRoles; import google.registry.model.registrar.Registrar; -import google.registry.model.registrar.RegistrarPoc; import google.registry.model.tld.Tld; import google.registry.model.tld.Tld.TldState; +import google.registry.testing.CloudTasksHelper; +import google.registry.testing.CloudTasksHelper.TaskMatcher; import google.registry.testing.DeterministicStringGenerator; import google.registry.testing.FakeClock; import google.registry.util.CidrAddressBlock; import java.security.cert.CertificateParsingException; +import java.util.Optional; +import javax.annotation.Nullable; import org.joda.money.Money; import org.joda.time.DateTime; import org.joda.time.Duration; @@ -50,22 +63,24 @@ class SetupOteCommandTest extends CommandTestCase { private static final String PASSWORD = "abcdefghijklmnop"; - private DeterministicStringGenerator passwordGenerator = + private final IamClient iamClient = mock(IamClient.class); + private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(); + private final DeterministicStringGenerator passwordGenerator = new DeterministicStringGenerator("abcdefghijklmnopqrstuvwxyz"); @BeforeEach void beforeEach() { command.passwordGenerator = passwordGenerator; command.clock = new FakeClock(DateTime.parse("2018-07-07TZ")); + command.maybeGroupEmailAddress = Optional.of("group@example.com"); + command.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); + command.iamClient = iamClient; persistPremiumList("default_sandbox_list", USD, "sandbox,USD 1000"); } /** Verify TLD creation. */ private void verifyTldCreation( - String tldName, - String roidSuffix, - TldState tldState, - boolean isEarlyAccess) { + String tldName, String roidSuffix, TldState tldState, boolean isEarlyAccess) { Tld registry = Tld.get(tldName); assertThat(registry).isNotNull(); assertThat(registry.getRoidSuffix()).isEqualTo(roidSuffix); @@ -107,13 +122,28 @@ class SetupOteCommandTest extends CommandTestCase { assertThat(registrar.getClientCertificateHash()).hasValue(SAMPLE_CERT_HASH); } - private void verifyRegistrarContactCreation(String registrarName, String email) { - ImmutableSet registrarPocs = loadRegistrar(registrarName).getContacts(); - assertThat(registrarPocs).hasSize(1); - RegistrarPoc registrarPoc = registrarPocs.stream().findAny().get(); - assertThat(registrarPoc.getEmailAddress()).isEqualTo(email); - assertThat(registrarPoc.getName()).isEqualTo(email); - assertThat(registrarPoc.getLoginEmailAddress()).isEqualTo(email); + private void verifyIapPermission(@Nullable String emailAddress) { + if (emailAddress == null) { + cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); + verifyNoInteractions(iamClient); + } else { + String groupEmailAddress = command.maybeGroupEmailAddress.orElse(null); + if (groupEmailAddress == null) { + cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update"); + verify(iamClient).addBinding(emailAddress, IAP_SECURED_WEB_APP_USER_ROLE); + } else { + cloudTasksHelper.assertTasksEnqueued( + "console-user-group-update", + new TaskMatcher() + .service("TOOLS") + .method(HttpMethod.POST) + .path("/_dr/admin/updateUserGroup") + .param("userEmailAddress", emailAddress) + .param("groupEmailAddress", groupEmailAddress) + .param("groupUpdateMode", "ADD")); + verifyNoInteractions(iamClient); + } + } } @Test @@ -136,10 +166,12 @@ class SetupOteCommandTest extends CommandTestCase { verifyRegistrarCreation("blobio-4", "blobio-ga", PASSWORD, ipAddress); verifyRegistrarCreation("blobio-5", "blobio-eap", PASSWORD, ipAddress); - verifyRegistrarContactCreation("blobio-1", "contact@email.com"); - verifyRegistrarContactCreation("blobio-3", "contact@email.com"); - verifyRegistrarContactCreation("blobio-4", "contact@email.com"); - verifyRegistrarContactCreation("blobio-5", "contact@email.com"); + verifyUser("blobio-1", "contact@email.com"); + verifyUser("blobio-3", "contact@email.com"); + verifyUser("blobio-4", "contact@email.com"); + verifyUser("blobio-5", "contact@email.com"); + + verifyIapPermission("contact@email.com"); } @Test @@ -162,10 +194,12 @@ class SetupOteCommandTest extends CommandTestCase { verifyRegistrarCreation("abc-4", "abc-ga", PASSWORD, ipAddress); verifyRegistrarCreation("abc-5", "abc-eap", PASSWORD, ipAddress); - verifyRegistrarContactCreation("abc-1", "abc@email.com"); - verifyRegistrarContactCreation("abc-3", "abc@email.com"); - verifyRegistrarContactCreation("abc-4", "abc@email.com"); - verifyRegistrarContactCreation("abc-5", "abc@email.com"); + verifyUser("abc-1", "abc@email.com"); + verifyUser("abc-3", "abc@email.com"); + verifyUser("abc-4", "abc@email.com"); + verifyUser("abc-5", "abc@email.com"); + + verifyIapPermission("abc@email.com"); } @Test @@ -180,19 +214,20 @@ class SetupOteCommandTest extends CommandTestCase { verifyTldCreation("blobio-ga", "BLOBIOG2", GENERAL_AVAILABILITY, false); verifyTldCreation("blobio-eap", "BLOBIOE3", GENERAL_AVAILABILITY, true); - ImmutableList ipAddresses = ImmutableList.of( - CidrAddressBlock.create("1.1.1.1"), - CidrAddressBlock.create("2.2.2.2")); + ImmutableList ipAddresses = + ImmutableList.of(CidrAddressBlock.create("1.1.1.1"), CidrAddressBlock.create("2.2.2.2")); verifyRegistrarCreation("blobio-1", "blobio-sunrise", PASSWORD, ipAddresses); verifyRegistrarCreation("blobio-3", "blobio-ga", PASSWORD, ipAddresses); verifyRegistrarCreation("blobio-4", "blobio-ga", PASSWORD, ipAddresses); verifyRegistrarCreation("blobio-5", "blobio-eap", PASSWORD, ipAddresses); - verifyRegistrarContactCreation("blobio-1", "contact@email.com"); - verifyRegistrarContactCreation("blobio-3", "contact@email.com"); - verifyRegistrarContactCreation("blobio-4", "contact@email.com"); - verifyRegistrarContactCreation("blobio-5", "contact@email.com"); + verifyUser("blobio-1", "contact@email.com"); + verifyUser("blobio-3", "contact@email.com"); + verifyUser("blobio-4", "contact@email.com"); + verifyUser("blobio-5", "contact@email.com"); + + verifyIapPermission("contact@email.com"); } @Test @@ -206,6 +241,7 @@ class SetupOteCommandTest extends CommandTestCase { "--email=contact@email.com", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("option is required: [-a | --ip_allow_list]"); + verifyIapPermission(null); } @Test @@ -219,6 +255,7 @@ class SetupOteCommandTest extends CommandTestCase { "--email=contact@email.com", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("option is required: [-r | --registrar]"); + verifyIapPermission(null); } @Test @@ -232,6 +269,7 @@ class SetupOteCommandTest extends CommandTestCase { assertThat(thrown) .hasMessageThat() .contains("Must specify exactly one client certificate file."); + verifyIapPermission(null); } @Test @@ -245,6 +283,36 @@ class SetupOteCommandTest extends CommandTestCase { "--certfile=" + getCertFilename(), "--registrar=blobio")); assertThat(thrown).hasMessageThat().contains("option is required: [--email]"); + verifyIapPermission(null); + } + + @Test + void testSuccess_noConsoleUserGroup() throws Exception { + command.maybeGroupEmailAddress = Optional.empty(); + runCommandForced( + "--ip_allow_list=1.1.1.1", + "--registrar=blobio", + "--email=contact@email.com", + "--certfile=" + getCertFilename()); + + verifyTldCreation("blobio-sunrise", "BLOBIOS0", START_DATE_SUNRISE, false); + verifyTldCreation("blobio-ga", "BLOBIOG2", GENERAL_AVAILABILITY, false); + verifyTldCreation("blobio-eap", "BLOBIOE3", GENERAL_AVAILABILITY, true); + + ImmutableList ipAddress = + ImmutableList.of(CidrAddressBlock.create("1.1.1.1")); + + verifyRegistrarCreation("blobio-1", "blobio-sunrise", PASSWORD, ipAddress); + verifyRegistrarCreation("blobio-3", "blobio-ga", PASSWORD, ipAddress); + verifyRegistrarCreation("blobio-4", "blobio-ga", PASSWORD, ipAddress); + verifyRegistrarCreation("blobio-5", "blobio-eap", PASSWORD, ipAddress); + + verifyUser("blobio-1", "contact@email.com"); + verifyUser("blobio-3", "contact@email.com"); + verifyUser("blobio-4", "contact@email.com"); + verifyUser("blobio-5", "contact@email.com"); + + verifyIapPermission("contact@email.com"); } @Test @@ -259,6 +327,7 @@ class SetupOteCommandTest extends CommandTestCase { "--email=contact@email.com", "--certfile=/dev/null")); assertThat(thrown).hasMessageThat().contains("No X509Certificate found"); + verifyIapPermission(null); } @Test @@ -273,6 +342,7 @@ class SetupOteCommandTest extends CommandTestCase { "--email=contact@email.com", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("Invalid registrar name: 3blo-bio"); + verifyIapPermission(null); } @Test @@ -287,6 +357,7 @@ class SetupOteCommandTest extends CommandTestCase { "--email=contact@email.com", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("Invalid registrar name: bl"); + verifyIapPermission(null); } @Test @@ -301,6 +372,7 @@ class SetupOteCommandTest extends CommandTestCase { "--email=contact@email.com", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("Invalid registrar name: blobiotoooolong"); + verifyIapPermission(null); } @Test @@ -315,6 +387,7 @@ class SetupOteCommandTest extends CommandTestCase { "--email=contact@email.com", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("Invalid registrar name: blo#bio"); + verifyIapPermission(null); } @Test @@ -330,6 +403,7 @@ class SetupOteCommandTest extends CommandTestCase { "--email=contact@email.com", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("VKey(sql:blobio-sunrise)"); + verifyIapPermission(null); } @Test @@ -345,6 +419,8 @@ class SetupOteCommandTest extends CommandTestCase { verifyTldCreation("blobio-sunrise", "BLOBIOS0", START_DATE_SUNRISE, false); verifyTldCreation("blobio-ga", "BLOBIOG2", GENERAL_AVAILABILITY, false); + + verifyIapPermission("contact@email.com"); } @Test @@ -366,6 +442,28 @@ class SetupOteCommandTest extends CommandTestCase { "--email=contact@email.com", "--certfile=" + getCertFilename())); assertThat(thrown).hasMessageThat().contains("VKey(sql:blobio-1)"); + verifyIapPermission(null); + } + + @Test + void testFailure_userExists() { + User user = + new User.Builder() + .setEmailAddress("contact@email.com") + .setUserRoles(new UserRoles.Builder().setGlobalRole(GlobalRole.FTE).build()) + .build(); + UserDao.saveUser(user); + IllegalStateException thrown = + assertThrows( + IllegalStateException.class, + () -> + runCommandForced( + "--ip_allow_list=1.1.1.1", + "--registrar=blobio", + "--email=contact@email.com", + "--certfile=" + getCertFilename())); + assertThat(thrown).hasMessageThat().contains("Found existing users: {contact@email.com"); + verifyIapPermission(null); } @Test @@ -385,10 +483,46 @@ class SetupOteCommandTest extends CommandTestCase { "--email=contact@email.com", "--certfile=" + getCertFilename()); - ImmutableList ipAddress = ImmutableList.of( - CidrAddressBlock.create("1.1.1.1")); + ImmutableList ipAddress = + ImmutableList.of(CidrAddressBlock.create("1.1.1.1")); verifyRegistrarCreation("blobio-1", "blobio-sunrise", PASSWORD, ipAddress); verifyRegistrarCreation("blobio-3", "blobio-ga", PASSWORD, ipAddress); + + verifyIapPermission("contact@email.com"); + } + + @Test + void testSuccess_userExists_replaceExisting() throws Exception { + User user = + new User.Builder() + .setEmailAddress("contact@email.com") + .setUserRoles(new UserRoles.Builder().setGlobalRole(GlobalRole.FTE).build()) + .build(); + UserDao.saveUser(user); + + runCommandForced( + "--overwrite", + "--ip_allow_list=1.1.1.1", + "--registrar=blobio", + "--email=contact@email.com", + "--certfile=" + getCertFilename()); + + ImmutableList ipAddress = + ImmutableList.of(CidrAddressBlock.create("1.1.1.1")); + + verifyRegistrarCreation("blobio-1", "blobio-sunrise", PASSWORD, ipAddress); + verifyRegistrarCreation("blobio-3", "blobio-ga", PASSWORD, ipAddress); + + verifyUser("blobio-1", "contact@email.com"); + verifyUser("blobio-3", "contact@email.com"); + verifyUser("blobio-4", "contact@email.com"); + verifyUser("blobio-5", "contact@email.com"); + + // verify that the role is completely replaced, e.g., the global role is gone. + assertThat(UserDao.loadUser("contact@email.com").get().getUserRoles().getGlobalRole()) + .isEqualTo(GlobalRole.NONE); + + verifyIapPermission("contact@email.com"); } } diff --git a/core/src/test/java/google/registry/ui/server/registrar/ConsoleOteSetupActionTest.java b/core/src/test/java/google/registry/ui/server/registrar/ConsoleOteSetupActionTest.java index ff4a28dac..e04973299 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/ConsoleOteSetupActionTest.java +++ b/core/src/test/java/google/registry/ui/server/registrar/ConsoleOteSetupActionTest.java @@ -15,11 +15,14 @@ package google.registry.ui.server.registrar; import static com.google.common.truth.Truth.assertThat; +import static google.registry.model.OteAccountBuilderTest.verifyIapPermission; +import static google.registry.model.OteAccountBuilderTest.verifyUser; import static google.registry.model.registrar.Registrar.loadByRegistrarId; import static google.registry.testing.DatabaseHelper.persistPremiumList; import static jakarta.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; import static org.joda.money.CurrencyUnit.USD; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import com.google.common.collect.ImmutableList; @@ -35,10 +38,12 @@ import google.registry.request.Action.Method; import google.registry.request.auth.AuthResult; import google.registry.request.auth.AuthenticatedRegistrarAccessor; import google.registry.security.XsrfTokenManager; +import google.registry.testing.CloudTasksHelper; import google.registry.testing.DeterministicStringGenerator; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.SystemPropertyExtension; +import google.registry.tools.IamClient; import google.registry.ui.server.SendEmailUtils; import google.registry.util.EmailMessage; import google.registry.util.RegistryEnvironment; @@ -65,6 +70,8 @@ public final class ConsoleOteSetupActionTest { @Order(value = Integer.MAX_VALUE) final SystemPropertyExtension systemPropertyExtension = new SystemPropertyExtension(); + private final IamClient iamClient = mock(IamClient.class); + private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(); private final FakeResponse response = new FakeResponse(); private final ConsoleOteSetupAction action = new ConsoleOteSetupAction(); private final User user = @@ -100,6 +107,9 @@ public final class ConsoleOteSetupActionTest { action.optionalPassword = Optional.empty(); action.passwordGenerator = new DeterministicStringGenerator("abcdefghijklmnopqrstuvwxyz"); + action.maybeGroupEmailAddress = Optional.of("group@example.com"); + action.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); + action.iamClient = iamClient; } @Test @@ -142,9 +152,7 @@ public final class ConsoleOteSetupActionTest { // checking that all the entities are there or that they have the correct values. assertThat(loadByRegistrarId("myclientid-3")).isPresent(); assertThat(Tld.get("myclientid-ga")).isNotNull(); - assertThat( - loadByRegistrarId("myclientid-5").get().getContacts().asList().get(0).getEmailAddress()) - .isEqualTo("contact@registry.example"); + verifyUser("myclientid-5", "contact@registry.example"); assertThat(response.getPayload()) .contains("

OT&E successfully created for registrar myclientid!

"); ArgumentCaptor contentCaptor = ArgumentCaptor.forClass(EmailMessage.class); @@ -163,6 +171,14 @@ public final class ConsoleOteSetupActionTest { Gave user contact@registry.example web access to these Registrars """); assertThat(response.getPayload()).contains("gtag('config', 'sampleId')"); + verifyIapPermission( + "contact@registry.example", action.maybeGroupEmailAddress, cloudTasksHelper, iamClient); + } + + @Test + void testPost_authorized_noConsoleGroup() { + action.maybeGroupEmailAddress = Optional.empty(); + testPost_authorized(); } @Test diff --git a/core/src/test/java/google/registry/ui/server/registrar/ConsoleRegistrarCreatorActionTest.java b/core/src/test/java/google/registry/ui/server/registrar/ConsoleRegistrarCreatorActionTest.java index 3709c093b..44100d58a 100644 --- a/core/src/test/java/google/registry/ui/server/registrar/ConsoleRegistrarCreatorActionTest.java +++ b/core/src/test/java/google/registry/ui/server/registrar/ConsoleRegistrarCreatorActionTest.java @@ -15,10 +15,13 @@ package google.registry.ui.server.registrar; import static com.google.common.truth.Truth.assertThat; +import static google.registry.model.OteAccountBuilderTest.verifyIapPermission; +import static google.registry.model.OteAccountBuilderTest.verifyUser; import static google.registry.model.registrar.Registrar.loadByRegistrarId; import static google.registry.testing.DatabaseHelper.persistPremiumList; import static jakarta.servlet.http.HttpServletResponse.SC_UNAUTHORIZED; import static org.joda.money.CurrencyUnit.USD; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; import com.google.common.collect.ImmutableList; @@ -29,17 +32,18 @@ import google.registry.model.console.User; import google.registry.model.console.UserRoles; import google.registry.model.registrar.Registrar; import google.registry.model.registrar.RegistrarAddress; -import google.registry.model.registrar.RegistrarPoc; import google.registry.persistence.transaction.JpaTestExtensions; import google.registry.persistence.transaction.JpaTestExtensions.JpaIntegrationTestExtension; import google.registry.request.Action.Method; import google.registry.request.auth.AuthResult; import google.registry.request.auth.AuthenticatedRegistrarAccessor; import google.registry.security.XsrfTokenManager; +import google.registry.testing.CloudTasksHelper; import google.registry.testing.DeterministicStringGenerator; import google.registry.testing.FakeClock; import google.registry.testing.FakeResponse; import google.registry.testing.SystemPropertyExtension; +import google.registry.tools.IamClient; import google.registry.ui.server.SendEmailUtils; import google.registry.util.EmailMessage; import google.registry.util.RegistryEnvironment; @@ -73,6 +77,8 @@ final class ConsoleRegistrarCreatorActionTest { .setEmailAddress("marla.singer@example.com") .setUserRoles(new UserRoles()) .build(); + private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper(); + private final IamClient iamClient = mock(IamClient.class); @Mock HttpServletRequest request; @Mock GmailClient gmailClient; @@ -119,6 +125,10 @@ final class ConsoleRegistrarCreatorActionTest { action.passcodeGenerator = new DeterministicStringGenerator("314159265"); action.analyticsConfig = ImmutableMap.of("googleAnalyticsId", "sampleId"); + + action.maybeGroupEmailAddress = Optional.of("group@example.com"); + action.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils(); + action.iamClient = iamClient; } @Test @@ -152,8 +162,7 @@ final class ConsoleRegistrarCreatorActionTest { assertThat(response.getPayload()).contains("gtag('config', 'sampleId')"); } - @Test - void testPost_authorized_minimalAddress() { + void runTestPost_authorized_minimalAddress() { action.clientId = Optional.of("myclientid"); action.name = Optional.of("registrar name"); action.billingAccount = Optional.of("USD=billing-account"); @@ -209,14 +218,22 @@ final class ConsoleRegistrarCreatorActionTest { .setCountryCode("CC") .build()); - assertThat(registrar.getContacts()) - .containsExactly( - new RegistrarPoc.Builder() - .setRegistrar(registrar) - .setName("myclientid@registry.example") - .setEmailAddress("myclientid@registry.example") - .setLoginEmailAddress("myclientid@registry.example") - .build()); + verifyUser("myclientid", "myclientid@registry.example"); + } + + @Test + void testPost_authorized_minimalAddress_consoleUserGroup() { + runTestPost_authorized_minimalAddress(); + verifyIapPermission( + "myclientid@registry.example", action.maybeGroupEmailAddress, cloudTasksHelper, iamClient); + } + + @Test + void testPost_authorized_minimalAddress_NoConsoleUserGroup() { + action.maybeGroupEmailAddress = Optional.empty(); + runTestPost_authorized_minimalAddress(); + verifyIapPermission( + "myclientid@registry.example", action.maybeGroupEmailAddress, cloudTasksHelper, iamClient); } @Test diff --git a/util/src/main/java/google/registry/util/RegistryEnvironment.java b/util/src/main/java/google/registry/util/RegistryEnvironment.java index 559228080..6266cc805 100644 --- a/util/src/main/java/google/registry/util/RegistryEnvironment.java +++ b/util/src/main/java/google/registry/util/RegistryEnvironment.java @@ -14,6 +14,8 @@ package google.registry.util; +import static com.google.common.base.Preconditions.checkState; + import com.google.common.base.Ascii; /** Registry environments. */ @@ -59,6 +61,20 @@ public enum RegistryEnvironment { private static final boolean ON_JETTY = Boolean.parseBoolean(System.getProperty(JETTY_PROPERTY, "false")); + /** + * A thread local boolean that can be set in tests to indicate some code is running in a local + * test server. + * + *

Certain API calls (like calls to Cloud Tasks) are hard to stub when they run in the test + * server because the test server does not allow arbitrary injection of dependencies. Instead, + * code running in the server can check this value and decide whether to skip these API calls. + * + *

The value is set to false by default and can only be set to true in unit test environment. + * It is set to {@code true} in {@code start()} and {@code false} in {@code stop()} in {@code + * TestServer}. + */ + private static final ThreadLocal IN_TEST_SERVER = ThreadLocal.withInitial(() -> false); + /** Sets this enum as the name of the registry environment. */ public RegistryEnvironment setup() { return setup(SystemPropertySetter.PRODUCTION_IMPL); @@ -81,4 +97,13 @@ public enum RegistryEnvironment { public static boolean isOnJetty() { return ON_JETTY; } + + public static void setIsInTestDriver(boolean value) { + checkState(RegistryEnvironment.get() == RegistryEnvironment.UNITTEST); + IN_TEST_SERVER.set(value); + } + + public static boolean isInTestServer() { + return IN_TEST_SERVER.get(); + } }