Add user email prefix to the console user create (#2623)

console-webapp/src/app/users/userDetails.component.html

@@ -11,31 +11,10 @@
       <mat-icon>arrow_back</mat-icon>
     </button>
   </div>
-  <form (ngSubmit)="saveEdit()">
+  <app-user-edit-form
-    <p>
+    [user]="userDetails()"
-      <mat-form-field appearance="outline">
+    (onEditComplete)="saveEdit($event)"
-        <mat-label
+  />
-          >User Role:
-          <mat-icon
-            matTooltip="Viewer role doesn't allow making updates; Editor role allows updates, like Contacts delete or SSL certificate change"
-            >help_outline</mat-icon
-          ></mat-label
-        >
-        <mat-select [(ngModel)]="userRole" name="userRole">
-          <mat-option value="PRIMARY_CONTACT">Editor</mat-option>
-          <mat-option value="ACCOUNT_MANAGER">Viewer</mat-option>
-        </mat-select>
-      </mat-form-field>
-    </p>
-    <button
-      mat-flat-button
-      color="primary"
-      aria-label="Save user"
-      type="submit"
-    >
-      Save
-    </button>
-  </form>
   } @else { @if(isNewUser) {
   <h1 class="mat-headline-4">
     {{ userDetails().emailAddress + " successfully created" }}
@@ -53,7 +32,7 @@
       mat-flat-button
       color="primary"
       aria-label="Edit User"
-      (click)="userRole = userDetails().role; isEditing = true"
+      (click)="isEditing = true"
     >
       <mat-icon>edit</mat-icon>
       Edit

console-webapp/src/app/users/userDetails.component.ts

@@ -19,13 +19,14 @@ import { SelectedRegistrarModule } from '../app.module';
 import { MaterialModule } from '../material.module';
 import { RegistrarService } from '../registrar/registrar.service';
 import { SnackBarModule } from '../snackbar.module';
-import { UsersService, roleToDescription } from './users.service';
+import { UsersService, roleToDescription, User } from './users.service';
 import { FormsModule } from '@angular/forms';
+import { UserEditFormComponent } from './userEditForm.component';
 
 @Component({
   selector: 'app-user-edit',
-  templateUrl: './userEdit.component.html',
+  templateUrl: './userDetails.component.html',
-  styleUrls: ['./userEdit.component.scss'],
+  styleUrls: ['./userDetails.component.scss'],
   standalone: true,
   imports: [
     FormsModule,
@@ -33,15 +34,15 @@ import { FormsModule } from '@angular/forms';
     SnackBarModule,
     CommonModule,
     SelectedRegistrarModule,
+    UserEditFormComponent,
   ],
   providers: [],
 })
-export class UserEditComponent {
+export class UserDetailsComponent {
   isEditing = false;
   isPasswordVisible = false;
   isNewUser = false;
   isLoading = false;
-  userRole = '';
 
   userDetails = computed(() => {
     return this.usersService
@@ -84,14 +85,9 @@ export class UserEditComponent {
     this.usersService.currentlyOpenUserEmail.set('');
   }
 
-  saveEdit() {
+  saveEdit(user: User) {
     this.isLoading = true;
-    this.usersService
+    this.usersService.updateUser(user).subscribe({
-      .updateUser({
-        role: this.userRole,
-        emailAddress: this.userDetails().emailAddress,
-      })
-      .subscribe({
       error: (err) => {
         this._snackBar.open(err.error || err.message);
         this.isLoading = false;

console-webapp/src/app/users/userEditForm.component.html

@@ -0,0 +1,39 @@
+<form (ngSubmit)="saveEdit($event)" #form>
+  <p *ngIf="isNew()">
+    <mat-form-field appearance="outline">
+      <mat-label
+        >User name prefix:
+        <mat-icon
+          matTooltip="Prefix will be combined with registrar ID to create a unique user name - {prefix}.{registrarId}@registry.google"
+          >help_outline</mat-icon
+        ></mat-label
+      >
+      <input
+        matInput
+        minlength="3"
+        maxlength="3"
+        [required]="true"
+        [(ngModel)]="user().emailAddress"
+        [ngModelOptions]="{ standalone: true }"
+      />
+    </mat-form-field>
+  </p>
+  <p>
+    <mat-form-field appearance="outline">
+      <mat-label
+        >User Role:
+        <mat-icon
+          matTooltip="Viewer role doesn't allow making updates; Editor role allows updates, like Contacts delete or SSL certificate change"
+          >help_outline</mat-icon
+        ></mat-label
+      >
+      <mat-select [(ngModel)]="user().role" name="userRole">
+        <mat-option value="PRIMARY_CONTACT">Editor</mat-option>
+        <mat-option value="ACCOUNT_MANAGER">Viewer</mat-option>
+      </mat-select>
+    </mat-form-field>
+  </p>
+  <button mat-flat-button color="primary" aria-label="Save user" type="submit">
+    Save
+  </button>
+</form>

console-webapp/src/app/users/userEditForm.component.ts

@@ -0,0 +1,58 @@
+// Copyright 2024 The Nomulus Authors. All Rights Reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+import { CommonModule } from '@angular/common';
+import {
+  Component,
+  ElementRef,
+  EventEmitter,
+  input,
+  Output,
+  ViewChild,
+} from '@angular/core';
+import { MaterialModule } from '../material.module';
+import { FormsModule } from '@angular/forms';
+import { User } from './users.service';
+
+@Component({
+  selector: 'app-user-edit-form',
+  templateUrl: './userEditForm.component.html',
+  styleUrls: ['./userEditForm.component.scss'],
+  standalone: true,
+  imports: [FormsModule, MaterialModule, CommonModule],
+  providers: [],
+})
+export class UserEditFormComponent {
+  @ViewChild('form') form!: ElementRef;
+  isNew = input<boolean>(false);
+  user = input<User>(
+    {
+      emailAddress: '',
+      role: 'ACCOUNT_MANAGER',
+    },
+    // @ts-ignore - legit option, typescript fails to match it to a proper type
+    { transform: (user: User) => structuredClone(user) }
+  );
+
+  @Output() onEditComplete = new EventEmitter<User>();
+
+  saveEdit(e: SubmitEvent) {
+    e.preventDefault();
+    if (this.form.nativeElement.checkValidity()) {
+      this.onEditComplete.emit(this.user());
+    } else {
+      this.form.nativeElement.reportValidity();
+    }
+  }
+}

console-webapp/src/app/users/users.component.html

@@ -4,10 +4,8 @@
     <mat-spinner />
   </div>
   } @else if(selectingExistingUser) {
-
   <div class="console-app__users">
     <h1 class="mat-headline-4">Add existing user</h1>
-
     <p>
       <button
         mat-icon-button
@@ -61,6 +59,19 @@
   </div>
   } @else if(usersService.currentlyOpenUserEmail()) {
   <app-user-edit></app-user-edit>
+  } @else if(isNew) {
+  <h1 class="mat-headline-4">New User Form</h1>
+  <div class="spacer"></div>
+  <p>
+    <button
+      mat-icon-button
+      aria-label="Back to users list"
+      (click)="isNew = false"
+    >
+      <mat-icon>arrow_back</mat-icon>
+    </button>
+  </p>
+  <app-user-edit-form [isNew]="true" (onEditComplete)="createNewUser($event)" />
   } @else {
   <div class="console-app__users">
     <div class="console-app__users-header">
@@ -79,11 +90,11 @@
         </button>
         <button
           mat-flat-button
-          (click)="createNewUser()"
+          (click)="isNew = true"
           aria-label="Create new user"
           color="primary"
         >
-          Create a Viewer User
+          Create New User
         </button>
       </div>
     </div>

console-webapp/src/app/users/users.component.ts

@@ -20,12 +20,13 @@ import { SelectedRegistrarModule } from '../app.module';
 import { MaterialModule } from '../material.module';
 import { RegistrarService } from '../registrar/registrar.service';
 import { SnackBarModule } from '../snackbar.module';
-import { UserEditComponent } from './userEdit.component';
+import { UserDetailsComponent } from './userDetails.component';
 import { User, UsersService } from './users.service';
 import { UserDataService } from '../shared/services/userData.service';
 import { FormsModule } from '@angular/forms';
 import { UsersListComponent } from './usersList.component';
 import { MatSelectChange } from '@angular/material/select';
+import { UserEditFormComponent } from './userEditForm.component';
 
 @Component({
   selector: 'app-users',
@@ -39,12 +40,14 @@ import { MatSelectChange } from '@angular/material/select';
     CommonModule,
     SelectedRegistrarModule,
     UsersListComponent,
-    UserEditComponent,
+    UserEditFormComponent,
+    UserDetailsComponent,
   ],
   providers: [UsersService],
 })
 export class UsersComponent {
   isLoading = false;
+  isNew = false;
   selectingExistingUser = false;
   selectedRegistrarId = '';
   usersSelection: User[] = [];
@@ -87,9 +90,9 @@ export class UsersComponent {
     });
   }
 
-  createNewUser() {
+  createNewUser(user: User) {
     this.isLoading = true;
-    this.usersService.createOrAddNewUser(null).subscribe({
+    this.usersService.createOrAddNewUser(user).subscribe({
       error: (err: HttpErrorResponse) => {
         this._snackBar.open(err.error || err.message);
         this.isLoading = false;

console-webapp/src/app/users/users.service.ts

@@ -60,9 +60,9 @@ export class UsersService {
       );
   }
 
-  createOrAddNewUser(maybeExistingUser: User | null) {
+  createOrAddNewUser(user: User) {
     return this.backendService
-      .createUser(this.registrarService.registrarId(), maybeExistingUser)
+      .createUser(this.registrarService.registrarId(), user)
       .pipe(
         tap((newUser: User) => {
           if (newUser) {

core/src/main/java/google/registry/ui/server/console/ConsoleUsersAction.java

@@ -29,7 +29,6 @@ import static jakarta.servlet.http.HttpServletResponse.SC_OK;
 
 import com.google.api.services.directory.Directory;
 import com.google.api.services.directory.model.UserName;
-import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.gson.annotations.Expose;
@@ -52,7 +51,6 @@ import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.stream.Collectors;
-import java.util.stream.IntStream;
 import javax.annotation.Nullable;
 import javax.inject.Inject;
 import javax.inject.Named;
@@ -67,7 +65,6 @@ public class ConsoleUsersAction extends ConsoleApiAction {
   static final String PATH = "/console-api/users";
 
   private static final int PASSWORD_LENGTH = 16;
-  private static final Splitter EMAIL_SPLITTER = Splitter.on('@').trimResults();
 
   private final String registrarId;
   private final Directory directory;
@@ -102,12 +99,7 @@ public class ConsoleUsersAction extends ConsoleApiAction {
     // Temporary flag while testing
     if (user.getUserRoles().isAdmin()) {
       checkPermission(user, registrarId, ConsolePermission.MANAGE_USERS);
-      if (userData.isPresent()) { // Adding existing user to registrar
+      tm().transact(this::runPostInTransaction);
-        tm().transact(this::runAppendUserInTransaction);
-      } else { // Adding new user to registrar
-        tm().transact(this::runCreateInTransaction);
-      }
-
     } else {
       consoleApiParams.response().setStatus(SC_FORBIDDEN);
     }
@@ -152,10 +144,16 @@ public class ConsoleUsersAction extends ConsoleApiAction {
     }
   }
 
-  private void runAppendUserInTransaction() {
+  private void runPostInTransaction() throws IOException {
-    if (!isModifyingRequestValid(false)) {
+    validateRequestParams();
-      return;
+    if (!tm().exists(VKey.create(User.class, this.userData.get().emailAddress))) {
+      this.runCreate();
+    } else {
+      this.runAppendUserToExistingRegistrar();
     }
+  }
+
+  private void runAppendUserToExistingRegistrar() {
     ImmutableList<User> allRegistrarUsers = getAllRegistrarUsers(registrarId);
     if (allRegistrarUsers.size() >= 4) {
       throw new BadRequestException("Total users amount per registrar is limited to 4");
@@ -164,20 +162,17 @@ public class ConsoleUsersAction extends ConsoleApiAction {
     updateUserRegistrarRoles(
         this.userData.get().emailAddress,
         registrarId,
-        RegistrarRole.valueOf(this.userData.get().role),
+        RegistrarRole.valueOf(this.userData.get().role));
-        false);
     consoleApiParams.response().setStatus(SC_OK);
   }
 
   private void runDeleteInTransaction() throws IOException {
-    if (!isModifyingRequestValid(true)) {
+    if (!isModifyingRequestValid()) {
       return;
     }
 
     String email = this.userData.get().emailAddress;
-    User updatedUser =
+    User updatedUser = updateUserRegistrarRoles(email, registrarId, null);
-        updateUserRegistrarRoles(
-            email, registrarId, RegistrarRole.valueOf(this.userData.get().role), true);
 
     // User has no registrars assigned
     if (updatedUser.getUserRoles().getRegistrarRoles().size() == 0) {
@@ -193,34 +188,33 @@ public class ConsoleUsersAction extends ConsoleApiAction {
       User.revokeIapPermission(email, maybeGroupEmailAddress, cloudTasksUtils, null, iamClient);
     }
 
-
     consoleApiParams.response().setStatus(SC_OK);
   }
 
-  private void runCreateInTransaction() throws IOException {
+  private void runCreate() throws IOException {
     ImmutableList<User> allRegistrarUsers = getAllRegistrarUsers(registrarId);
     if (allRegistrarUsers.size() >= 4) {
       throw new BadRequestException("Total users amount per registrar is limited to 4");
     }
 
-    String nextAvailableEmail =
+    String newEmailPrefix = userData.get().emailAddress.trim();
-        IntStream.range(1, 5)
+
-            .mapToObj(i -> String.format("%s-user%s@%s", registrarId, i, gSuiteDomainName))
+    if (!newEmailPrefix.matches("^[a-zA-Z0-9]{3}$")) {
-            .filter(email -> tm().loadByKeyIfPresent(VKey.create(User.class, email)).isEmpty())
+      throw new BadRequestException("Email prefix is invalid");
-            .findFirst()
+    }
-            // Can only happen if registrar cycled through 20 users, which is unlikely
+
-            .orElseThrow(
+    String newEmail = String.format("%s.%s@%s", newEmailPrefix, registrarId, gSuiteDomainName);
-                () -> new BadRequestException("Failed to find available increment for new user"));
+    if (tm().loadByKeyIfPresent(VKey.create(User.class, newEmail)).isPresent()) {
+      throw new BadRequestException("Email prefix is not available");
+    }
 
     com.google.api.services.directory.model.User newUser =
         new com.google.api.services.directory.model.User();
 
     newUser.setName(
-        new UserName()
+        new UserName().setFamilyName(registrarId).setGivenName(newEmailPrefix + "." + registrarId));
-            .setFamilyName(registrarId)
-            .setGivenName(EMAIL_SPLITTER.splitToList(nextAvailableEmail).get(0)));
     newUser.setPassword(passwordGenerator.createString(PASSWORD_LENGTH));
-    newUser.setPrimaryEmail(nextAvailableEmail);
+    newUser.setPrimaryEmail(newEmail);
 
     try {
       directory.users().insert(newUser).execute();
@@ -234,11 +228,9 @@ public class ConsoleUsersAction extends ConsoleApiAction {
             .setRegistrarRoles(ImmutableMap.of(registrarId, ACCOUNT_MANAGER))
             .build();
 
-    User.Builder builder =
+    User.Builder builder = new User.Builder().setUserRoles(userRoles).setEmailAddress(newEmail);
-        new User.Builder().setUserRoles(userRoles).setEmailAddress(newUser.getPrimaryEmail());
     tm().put(builder.build());
-    User.grantIapPermission(
+    User.grantIapPermission(newEmail, maybeGroupEmailAddress, cloudTasksUtils, null, iamClient);
-        nextAvailableEmail, maybeGroupEmailAddress, cloudTasksUtils, null, iamClient);
 
     consoleApiParams.response().setStatus(SC_CREATED);
     consoleApiParams
@@ -246,72 +238,69 @@ public class ConsoleUsersAction extends ConsoleApiAction {
         .setPayload(
             consoleApiParams
                 .gson()
-                .toJson(
+                .toJson(new UserData(newEmail, ACCOUNT_MANAGER.toString(), newUser.getPassword())));
-                    new UserData(
-                        newUser.getPrimaryEmail(),
-                        ACCOUNT_MANAGER.toString(),
-                        newUser.getPassword())));
   }
 
   private void runUpdateInTransaction() {
-    if (!isModifyingRequestValid(true)) {
+    if (!isModifyingRequestValid()) {
       return;
     }
 
     updateUserRegistrarRoles(
         this.userData.get().emailAddress,
         registrarId,
-        RegistrarRole.valueOf(this.userData.get().role),
+        RegistrarRole.valueOf(this.userData.get().role));
-        false);
     consoleApiParams.response().setStatus(SC_OK);
   }
 
-  private boolean isModifyingRequestValid(boolean verifyAccess) {
+  private boolean isModifyingRequestValid() {
+    validateRequestParams();
+    User userToUpdate = verifyUserExists(userData.get().emailAddress);
+    return validateUserRegistrarAssociation(userToUpdate);
+  }
+
+  private void validateRequestParams() {
     if (userData.isEmpty()
         || isNullOrEmpty(userData.get().emailAddress)
         || isNullOrEmpty(userData.get().role)) {
       throw new BadRequestException("User data is missing or incomplete");
     }
-    String email = userData.get().emailAddress;
+  }
-    User userToUpdate =
-        tm().loadByKeyIfPresent(VKey.create(User.class, email))
-            .orElseThrow(
-                () -> new BadRequestException(String.format("User %s doesn't exist", email)));
 
-    if (verifyAccess && !userToUpdate.getUserRoles().getRegistrarRoles().containsKey(registrarId)) {
+  private User verifyUserExists(String email) {
+    return tm().loadByKeyIfPresent(VKey.create(User.class, email))
+        .orElseThrow(() -> new BadRequestException(String.format("User %s doesn't exist", email)));
+  }
+
+  private boolean validateUserRegistrarAssociation(User user) {
+    if (user.getUserRoles().getRegistrarRoles().containsKey(registrarId)) {
+      return true;
+    }
     setFailedResponse(
         String.format("Can't update user not associated with registrarId %s", registrarId),
         SC_FORBIDDEN);
     return false;
   }
-    return true;
-  }
 
-  private User updateUserRegistrarRoles(
+  private User updateUserRegistrarRoles(String email, String registrarId, RegistrarRole newRole) {
-      String email, String registrarId, RegistrarRole newRole, boolean isDelete) {
-    User userToUpdate = tm().loadByKeyIfPresent(VKey.create(User.class, email)).get();
     Map<String, RegistrarRole> updatedRegistrarRoles;
-    if (isDelete) {
+    User user = verifyUserExists(email);
+    if (newRole == null) {
       updatedRegistrarRoles =
-          userToUpdate.getUserRoles().getRegistrarRoles().entrySet().stream()
+          user.getUserRoles().getRegistrarRoles().entrySet().stream()
               .filter(entry -> !Objects.equals(entry.getKey(), registrarId))
               .collect(ImmutableMap.toImmutableMap(Map.Entry::getKey, Map.Entry::getValue));
     } else {
       updatedRegistrarRoles =
           ImmutableMap.<String, RegistrarRole>builder()
-              .putAll(userToUpdate.getUserRoles().getRegistrarRoles())
+              .putAll(user.getUserRoles().getRegistrarRoles())
               .put(registrarId, newRole)
               .buildKeepingLast();
     }
     var updatedUser =
-        userToUpdate
+        user.asBuilder()
-            .asBuilder()
             .setUserRoles(
-                userToUpdate
+                user.getUserRoles().asBuilder().setRegistrarRoles(updatedRegistrarRoles).build())
-                    .getUserRoles()
-                    .asBuilder()
-                    .setRegistrarRoles(updatedRegistrarRoles)
-                    .build())
             .build();
     tm().put(updatedUser);
     return updatedUser;

core/src/test/java/google/registry/ui/server/console/ConsoleUsersActionTest.java

@@ -159,6 +159,24 @@ class ConsoleUsersActionTest {
     assertThat(response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
   }
 
+  @Test
+  void testFailure_invalidPrefix() throws IOException {
+    User user = DatabaseHelper.createAdminUser("email@email.com");
+    AuthResult authResult = AuthResult.createUser(user);
+    ConsoleUsersAction action =
+        createAction(
+            Optional.of(ConsoleApiParamsUtils.createFake(authResult)),
+            Optional.of("POST"),
+            Optional.of(new UserData("a@d", RegistrarRole.ACCOUNT_MANAGER.toString(), null)));
+    action.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils();
+    when(directory.users()).thenReturn(users);
+    when(users.insert(any(com.google.api.services.directory.model.User.class))).thenReturn(insert);
+    action.run();
+    var response = ((FakeResponse) consoleApiParams.response());
+    assertThat(response.getStatus()).isEqualTo(SC_BAD_REQUEST);
+    assertThat(response.getPayload()).contains("Email prefix is invalid");
+  }
+
   @Test
   void testSuccess_createsUser() throws IOException {
     User user = DatabaseHelper.createAdminUser("email@email.com");
@@ -167,7 +185,7 @@ class ConsoleUsersActionTest {
         createAction(
             Optional.of(ConsoleApiParamsUtils.createFake(authResult)),
             Optional.of("POST"),
-            Optional.empty());
+            Optional.of(new UserData("lol", RegistrarRole.ACCOUNT_MANAGER.toString(), null)));
     action.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils();
     when(directory.users()).thenReturn(users);
     when(users.insert(any(com.google.api.services.directory.model.User.class))).thenReturn(insert);
@@ -176,7 +194,7 @@ class ConsoleUsersActionTest {
     assertThat(response.getStatus()).isEqualTo(SC_CREATED);
     assertThat(response.getPayload())
         .contains(
-            "{\"emailAddress\":\"TheRegistrar-user1@email.com\",\"role\":\"ACCOUNT_MANAGER\",\"password\":\"abcdefghijklmnop\"}");
+            "{\"emailAddress\":\"lol.TheRegistrar@email.com\",\"role\":\"ACCOUNT_MANAGER\",\"password\":\"abcdefghijklmnop\"}");
   }
 
   @Test
@@ -319,7 +337,8 @@ class ConsoleUsersActionTest {
         createAction(
             Optional.of(ConsoleApiParamsUtils.createFake(authResult)),
             Optional.of("POST"),
-            Optional.empty());
+            Optional.of(
+                new UserData("test3@test.com", RegistrarRole.ACCOUNT_MANAGER.toString(), null)));
     action.cloudTasksUtils = cloudTasksHelper.getTestCloudTasksUtils();
     when(directory.users()).thenReturn(users);
     when(users.insert(any(com.google.api.services.directory.model.User.class))).thenReturn(insert);