1
0
mirror of https://github.com/google/nomulus synced 2026-01-03 11:45:39 +00:00

Call Workspace Groups API directly from nomulus tool (#2515)

When creating/deleting users, we need to add/remove the emails in
question to/from the console email group (if it exists). This used to be
done synchronously by calling the Groups API directly from the nomulus
tool. However #2488 made it so that in all cases where group membership
is modified, a Cloud Tasks task is created to execute the change on
the server side asynchronously (because there are multiple places where
this change needs to be done, and it is easier to make it all happen on the
server side).

Alas, as it turns out, Cloud Tasks tasks need to be created with a
service account's credential (which is trivially done on the server side
because the ADC is a service account). Nomulus command runs with a user
credential, and we need to grant the relevant user permission to
masquerade as a service account, in order to enqueue tasks from the
nomulus tool. It is therefore easier to just revert to the old behavior.
This commit is contained in:
Lai Jiang
2024-08-01 11:29:57 -04:00
committed by GitHub
parent 45331be166
commit 71ea16ff69
9 changed files with 216 additions and 59 deletions

View File

@@ -77,6 +77,7 @@ public class CloudTasksUtils implements Serializable {
@Config("projectId") String projectId,
@Config("locationId") String locationId,
@Config("oauthClientId") String oauthClientId,
// Note that this has to be a service account, due to limitations of the Cloud Tasks API.
@ApplicationDefaultCredential GoogleCredentialsBundle credential,
SerializableCloudTasksClient client) {
this.retrier = retrier;

View File

@@ -271,7 +271,7 @@ public final class OteAccountBuilder {
Optional<String> groupEmailAddress, CloudTasksUtils cloudTasksUtils, IamClient iamClient) {
for (User user : users) {
User.grantIapPermission(
user.getEmailAddress(), groupEmailAddress, cloudTasksUtils, iamClient);
user.getEmailAddress(), groupEmailAddress, cloudTasksUtils, null, iamClient);
}
}

View File

@@ -14,22 +14,28 @@
package google.registry.model.console;
import static com.google.common.base.Preconditions.checkArgument;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static google.registry.tools.server.UpdateUserGroupAction.GROUP_UPDATE_QUEUE;
import com.google.cloud.tasks.v2.Task;
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableMultimap;
import com.google.common.flogger.FluentLogger;
import com.google.common.net.MediaType;
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.ServiceConnection;
import google.registry.tools.server.UpdateUserGroupAction;
import google.registry.tools.server.UpdateUserGroupAction.Mode;
import google.registry.util.RegistryEnvironment;
import java.io.IOException;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicLong;
import javax.annotation.Nullable;
import javax.persistence.Access;
import javax.persistence.AccessType;
import javax.persistence.Embeddable;
@@ -57,18 +63,30 @@ public class User extends UserBase {
public static void grantIapPermission(
String emailAddress,
Optional<String> groupEmailAddress,
CloudTasksUtils cloudTasksUtils,
@Nullable CloudTasksUtils cloudTasksUtils,
@Nullable ServiceConnection connection,
IamClient iamClient) {
if (RegistryEnvironment.isInTestServer()) {
return;
}
checkArgument(
cloudTasksUtils != null || connection != null,
"At least one of cloudTasksUtils or connection must be set");
checkArgument(
cloudTasksUtils == null || connection == null,
"Only one of cloudTasksUtils or connection can be set");
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);
if (cloudTasksUtils != null) {
modifyGroupMembershipAsync(
emailAddress, groupEmailAddress.get(), cloudTasksUtils, UpdateUserGroupAction.Mode.ADD);
} else {
modifyGroupMembershipSync(
emailAddress, groupEmailAddress.get(), connection, UpdateUserGroupAction.Mode.ADD);
}
}
}
@@ -81,18 +99,29 @@ public class User extends UserBase {
public static void revokeIapPermission(
String emailAddress,
Optional<String> groupEmailAddress,
CloudTasksUtils cloudTasksUtils,
@Nullable CloudTasksUtils cloudTasksUtils,
@Nullable ServiceConnection connection,
IamClient iamClient) {
if (RegistryEnvironment.isInTestServer()) {
return;
}
checkArgument(
cloudTasksUtils != null || connection != null,
"At least one of cloudTasksUtils or connection must be set");
checkArgument(
cloudTasksUtils == null || connection == null,
"Only one of cloudTasksUtils or connection can be set");
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);
if (cloudTasksUtils != null) {
modifyGroupMembershipAsync(
emailAddress, groupEmailAddress.get(), cloudTasksUtils, Mode.REMOVE);
} else {
modifyGroupMembershipSync(emailAddress, groupEmailAddress.get(), connection, Mode.REMOVE);
}
}
}
@@ -115,6 +144,25 @@ public class User extends UserBase {
cloudTasksUtils.enqueue(GROUP_UPDATE_QUEUE, task);
}
private static void modifyGroupMembershipSync(
String userEmailAddress, String groupEmailAddress, ServiceConnection connection, Mode mode) {
try {
connection.sendPostRequest(
UpdateUserGroupAction.PATH,
ImmutableMap.of(
"userEmailAddress",
userEmailAddress,
"groupEmailAddress",
groupEmailAddress,
"groupUpdateMode",
mode.name()),
MediaType.PLAIN_TEXT_UTF_8,
new byte[0]);
} catch (IOException e) {
throw new RuntimeException("Cannot send request to server", e);
}
}
@Override
@Access(AccessType.PROPERTY)
public Long getId() {

View File

@@ -18,7 +18,6 @@ import static com.google.common.base.Preconditions.checkArgument;
import static google.registry.model.console.User.grantIapPermission;
import com.beust.jcommander.Parameters;
import google.registry.batch.CloudTasksUtils;
import google.registry.config.RegistryConfig.Config;
import google.registry.model.console.User;
import google.registry.model.console.UserDao;
@@ -28,12 +27,12 @@ import javax.inject.Inject;
/** Command to create a new User. */
@Parameters(separators = " =", commandDescription = "Update a user account")
public class CreateUserCommand extends CreateOrUpdateUserCommand {
public class CreateUserCommand extends CreateOrUpdateUserCommand implements CommandWithConnection {
private ServiceConnection connection;
@Inject IamClient iamClient;
@Inject CloudTasksUtils cloudTasksUtils;
@Inject
@Config("gSuiteConsoleUserGroupEmailAddress")
Optional<String> maybeGroupEmailAddress;
@@ -48,7 +47,12 @@ public class CreateUserCommand extends CreateOrUpdateUserCommand {
@Override
protected String execute() throws Exception {
String ret = super.execute();
grantIapPermission(email, maybeGroupEmailAddress, cloudTasksUtils, iamClient);
grantIapPermission(email, maybeGroupEmailAddress, null, connection, iamClient);
return ret;
}
@Override
public void setConnection(ServiceConnection connection) {
this.connection = connection;
}
}

View File

@@ -20,7 +20,6 @@ import static google.registry.util.PreconditionsUtils.checkArgumentPresent;
import com.beust.jcommander.Parameter;
import com.beust.jcommander.Parameters;
import google.registry.batch.CloudTasksUtils;
import google.registry.config.RegistryConfig.Config;
import google.registry.model.console.User;
import google.registry.model.console.UserDao;
@@ -30,12 +29,12 @@ import javax.inject.Inject;
/** Deletes a {@link User}. */
@Parameters(separators = " =", commandDescription = "Delete a user account")
public class DeleteUserCommand extends ConfirmingCommand {
public class DeleteUserCommand extends ConfirmingCommand implements CommandWithConnection {
private ServiceConnection connection;
@Inject IamClient iamClient;
@Inject CloudTasksUtils cloudTasksUtils;
@Inject
@Config("gSuiteConsoleUserGroupEmailAddress")
Optional<String> maybeGroupEmailAddress;
@@ -59,7 +58,12 @@ public class DeleteUserCommand extends ConfirmingCommand {
checkArgumentPresent(optionalUser, "Email no longer corresponds to a valid user");
tm().delete(optionalUser.get());
});
User.revokeIapPermission(email, maybeGroupEmailAddress, cloudTasksUtils, iamClient);
User.revokeIapPermission(email, maybeGroupEmailAddress, null, connection, iamClient);
return String.format("Deleted user with email %s", email);
}
@Override
public void setConnection(ServiceConnection connection) {
this.connection = connection;
}
}

View File

@@ -263,7 +263,7 @@ public final class ConsoleRegistrarCreatorAction extends HtmlAction {
});
UserDao.saveUser(user);
User.grantIapPermission(
user.getEmailAddress(), maybeGroupEmailAddress, cloudTasksUtils, iamClient);
user.getEmailAddress(), maybeGroupEmailAddress, cloudTasksUtils, null, iamClient);
data.put("password", password);
data.put("passcode", phonePasscode);

View File

@@ -21,15 +21,20 @@ import static google.registry.persistence.transaction.TransactionManagerFactory.
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 static org.mockito.Mockito.verifyNoMoreInteractions;
import com.google.cloud.tasks.v2.HttpMethod;
import com.google.common.collect.ImmutableMap;
import com.google.common.net.MediaType;
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 google.registry.tools.ServiceConnection;
import google.registry.tools.server.UpdateUserGroupAction;
import java.util.Optional;
import org.junit.jupiter.api.Test;
@@ -57,6 +62,34 @@ public class UserTest extends EntityTestCase {
});
}
@Test
void testFailure_asyncAndSyncModeConflict() {
assertThrows(
IllegalArgumentException.class,
() -> User.grantIapPermission("email@example.com", Optional.empty(), null, null, null));
assertThrows(
IllegalArgumentException.class,
() ->
User.grantIapPermission(
"email@example.com",
Optional.empty(),
mock(CloudTasksUtils.class),
mock(ServiceConnection.class),
null));
assertThrows(
IllegalArgumentException.class,
() -> User.revokeIapPermission("email@example.com", Optional.empty(), null, null, null));
assertThrows(
IllegalArgumentException.class,
() ->
User.revokeIapPermission(
"email@example.com",
Optional.empty(),
mock(CloudTasksUtils.class),
mock(ServiceConnection.class),
null));
}
@Test
void testFailure_badInputs() {
User.Builder builder = new User.Builder();
@@ -116,19 +149,20 @@ public class UserTest extends EntityTestCase {
}
@Test
void testGrantIapPermission() {
void testGrantIapPermissionAsync() {
CloudTasksHelper cloudTasksHelper = new CloudTasksHelper();
IamClient iamClient = mock(IamClient.class);
CloudTasksUtils cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils();
// Individual permission.
User.grantIapPermission("email@example.com", Optional.empty(), cloudTasksUtils, iamClient);
User.grantIapPermission(
"email@example.com", Optional.empty(), cloudTasksUtils, null, 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);
"email@example.com", Optional.of("console@example.com"), cloudTasksUtils, null, iamClient);
cloudTasksHelper.assertTasksEnqueued(
"console-user-group-update",
new TaskMatcher()
@@ -142,19 +176,49 @@ public class UserTest extends EntityTestCase {
}
@Test
void testRevokeIapPermission() {
void testGrantIapPermissionSync() throws Exception {
ServiceConnection connection = mock(ServiceConnection.class);
IamClient iamClient = mock(IamClient.class);
// Individual permission.
User.grantIapPermission("email@example.com", Optional.empty(), null, connection, iamClient);
verifyNoInteractions(connection);
verify(iamClient).addBinding("email@example.com", IAP_SECURED_WEB_APP_USER_ROLE);
// Group membership.
User.grantIapPermission(
"email@example.com", Optional.of("console@example.com"), null, connection, iamClient);
verify(connection)
.sendPostRequest(
UpdateUserGroupAction.PATH,
ImmutableMap.of(
"userEmailAddress",
"email@example.com",
"groupEmailAddress",
"console@example.com",
"groupUpdateMode",
"ADD"),
MediaType.PLAIN_TEXT_UTF_8,
new byte[0]);
verifyNoMoreInteractions(iamClient);
verifyNoMoreInteractions(connection);
}
@Test
void testRevokeIapPermissionAsync() {
CloudTasksHelper cloudTasksHelper = new CloudTasksHelper();
IamClient iamClient = mock(IamClient.class);
CloudTasksUtils cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils();
// Individual permission.
User.revokeIapPermission("email@example.com", Optional.empty(), cloudTasksUtils, iamClient);
User.revokeIapPermission(
"email@example.com", Optional.empty(), cloudTasksUtils, null, 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);
"email@example.com", Optional.of("console@example.com"), cloudTasksUtils, null, iamClient);
cloudTasksHelper.assertTasksEnqueued(
"console-user-group-update",
new TaskMatcher()
@@ -166,4 +230,33 @@ public class UserTest extends EntityTestCase {
.param("groupUpdateMode", "REMOVE"));
verifyNoMoreInteractions(iamClient);
}
@Test
void testRevokeIapPermissionSync() throws Exception {
ServiceConnection connection = mock(ServiceConnection.class);
IamClient iamClient = mock(IamClient.class);
// Individual permission.
User.revokeIapPermission("email@example.com", Optional.empty(), null, connection, iamClient);
verifyNoInteractions(connection);
verify(iamClient).removeBinding("email@example.com", IAP_SECURED_WEB_APP_USER_ROLE);
// Group membership.
User.revokeIapPermission(
"email@example.com", Optional.of("console@example.com"), null, connection, iamClient);
verify(connection)
.sendPostRequest(
UpdateUserGroupAction.PATH,
ImmutableMap.of(
"userEmailAddress",
"email@example.com",
"groupEmailAddress",
"console@example.com",
"groupUpdateMode",
"REMOVE"),
MediaType.PLAIN_TEXT_UTF_8,
new byte[0]);
verifyNoMoreInteractions(iamClient);
verifyNoMoreInteractions(connection);
}
}

View File

@@ -22,16 +22,15 @@ 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 google.registry.tools.server.UpdateUserGroupAction;
import java.util.Optional;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -40,13 +39,13 @@ import org.junit.jupiter.api.Test;
public class CreateUserCommandTest extends CommandTestCase<CreateUserCommand> {
private final IamClient iamClient = mock(IamClient.class);
private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper();
private final ServiceConnection connection = mock(ServiceConnection.class);
@BeforeEach
void beforeEach() {
command.iamClient = iamClient;
command.maybeGroupEmailAddress = Optional.empty();
command.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils();
command.setConnection(connection);
}
@Test
@@ -59,7 +58,7 @@ public class CreateUserCommandTest extends CommandTestCase<CreateUserCommand> {
assertThat(onlyUser.getUserRoles().getRegistrarRoles()).isEmpty();
verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE);
verifyNoMoreInteractions(iamClient);
cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update");
verifyNoInteractions(connection);
}
@Test
@@ -71,16 +70,20 @@ public class CreateUserCommandTest extends CommandTestCase<CreateUserCommand> {
assertThat(onlyUser.getUserRoles().isAdmin()).isFalse();
assertThat(onlyUser.getUserRoles().getGlobalRole()).isEqualTo(GlobalRole.NONE);
assertThat(onlyUser.getUserRoles().getRegistrarRoles()).isEmpty();
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"));
verify(connection)
.sendPostRequest(
UpdateUserGroupAction.PATH,
ImmutableMap.of(
"userEmailAddress",
"user@example.test",
"groupEmailAddress",
"group@example.test",
"groupUpdateMode",
"ADD"),
MediaType.PLAIN_TEXT_UTF_8,
new byte[0]);
verifyNoInteractions(iamClient);
verifyNoMoreInteractions(connection);
}
@Test
@@ -100,7 +103,7 @@ public class CreateUserCommandTest extends CommandTestCase<CreateUserCommand> {
assertThat(UserDao.loadUser("user@example.test").get().getUserRoles().isAdmin()).isTrue();
verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE);
verifyNoMoreInteractions(iamClient);
cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update");
verifyNoInteractions(connection);
}
@Test
@@ -110,7 +113,7 @@ public class CreateUserCommandTest extends CommandTestCase<CreateUserCommand> {
.isEqualTo(GlobalRole.FTE);
verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE);
verifyNoMoreInteractions(iamClient);
cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update");
verifyNoInteractions(connection);
}
@Test
@@ -129,7 +132,7 @@ public class CreateUserCommandTest extends CommandTestCase<CreateUserCommand> {
RegistrarRole.PRIMARY_CONTACT));
verify(iamClient).addBinding("user@example.test", IAP_SECURED_WEB_APP_USER_ROLE);
verifyNoMoreInteractions(iamClient);
cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update");
verifyNoInteractions(connection);
}
@Test
@@ -144,7 +147,7 @@ public class CreateUserCommandTest extends CommandTestCase<CreateUserCommand> {
.hasMessageThat()
.isEqualTo("A user with email user@example.test already exists");
verifyNoMoreInteractions(iamClient);
cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update");
verifyNoInteractions(connection);
}
@Test

View File

@@ -22,11 +22,11 @@ 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.net.MediaType;
import google.registry.model.console.UserDao;
import google.registry.testing.CloudTasksHelper;
import google.registry.testing.CloudTasksHelper.TaskMatcher;
import google.registry.testing.DatabaseHelper;
import google.registry.tools.server.UpdateUserGroupAction;
import java.util.Optional;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -35,13 +35,13 @@ import org.junit.jupiter.api.Test;
public class DeleteUserCommandTest extends CommandTestCase<DeleteUserCommand> {
private final IamClient iamClient = mock(IamClient.class);
private final CloudTasksHelper cloudTasksHelper = new CloudTasksHelper();
private final ServiceConnection connection = mock(ServiceConnection.class);
@BeforeEach
void beforeEach() {
command.iamClient = iamClient;
command.maybeGroupEmailAddress = Optional.empty();
command.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils();
command.setConnection(connection);
}
@Test
@@ -52,7 +52,7 @@ public class DeleteUserCommandTest extends CommandTestCase<DeleteUserCommand> {
assertThat(UserDao.loadUser("email@example.test")).isEmpty();
verify(iamClient).removeBinding("email@example.test", IAP_SECURED_WEB_APP_USER_ROLE);
verifyNoMoreInteractions(iamClient);
cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update");
verifyNoInteractions(connection);
}
@Test
@@ -62,16 +62,20 @@ public class DeleteUserCommandTest extends CommandTestCase<DeleteUserCommand> {
assertThat(UserDao.loadUser("email@example.test")).isPresent();
runCommandForced("--email", "email@example.test");
assertThat(UserDao.loadUser("email@example.test")).isEmpty();
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"));
verify(connection)
.sendPostRequest(
UpdateUserGroupAction.PATH,
ImmutableMap.of(
"userEmailAddress",
"email@example.test",
"groupEmailAddress",
"group@example.test",
"groupUpdateMode",
"REMOVE"),
MediaType.PLAIN_TEXT_UTF_8,
new byte[0]);
verifyNoInteractions(iamClient);
verifyNoMoreInteractions(connection);
}
@Test
@@ -83,6 +87,6 @@ public class DeleteUserCommandTest extends CommandTestCase<DeleteUserCommand> {
.hasMessageThat()
.isEqualTo("Email does not correspond to a valid user");
verifyNoInteractions(iamClient);
cloudTasksHelper.assertNoTasksEnqueued("console-user-group-update");
verifyNoInteractions(connection);
}
}