Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion GEMINI.md
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ Do not wait for the user to tell you to improve the skills; it is your responsib

# Gemini Engineering Guide: Nomulus Codebase

This document captures high-level architectural patterns, lessons learned from large-scale refactorings (like the Joda-Time to `java.time` migration), and specific instructions to avoid common pitfalls in this environment.
This document captures high-level architectural patterns, lessons learned from large-scale refactorings, and specific instructions to avoid common pitfalls in this environment.

## 🏛 Architecture Overview

Expand Down
10 changes: 6 additions & 4 deletions console-webapp/src/app/domains/domainList.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,13 @@ export class DomainListComponent {
this.dialog.open(ResponseDialogComponent, {
data: {
title: 'Domain Deletion Results',
content: `Successfully deleted - ${successCount} domain(s)<br/>Failed to delete - ${failureCount} domain(s)<br/>${
content: [
`Successfully deleted - ${successCount} domain(s)`,
`Failed to delete - ${failureCount} domain(s)`,
failureCount
? 'Some domains could not be deleted due to ongoing processes or server errors. '
: ''
}Please check the table for more information.`,
? 'Some domains could not be deleted due to ongoing processes or server errors. Please check the table for more information.'
: 'Please check the table for more information.',
],
},
});
this.selection.clear();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,9 @@ <h2>Registrar details</h2>
@for (column of columns; track column.columnDef) {
<mat-list-item role="listitem">
<span class="console-app__list-key">{{ column.header }} </span>
<span
class="console-app__list-value"
[innerHTML]="column.cell(registrarInEdit).replace('<br/>', ' ')"
></span>
<span class="console-app__list-value">{{
column.cell(registrarInEdit)
}}</span>
</mat-list-item>
<mat-divider></mat-divider>
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,10 +49,7 @@ <h1 class="mat-headline-4" forceFocus>Registrars</h1>
<mat-header-cell *matHeaderCellDef>
{{ column.header }}
</mat-header-cell>
<mat-cell
*matCellDef="let row"
[innerHTML]="column.cell(row)"
></mat-cell>
<mat-cell *matCellDef="let row">{{ column.cell(row) }}</mat-cell>
</ng-container>
}
<mat-header-row *matHeaderRowDef="displayedColumns"></mat-header-row>
Expand Down
9 changes: 3 additions & 6 deletions console-webapp/src/app/registrar/registrarsTable.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,9 @@ export const columns = [
columnDef: 'billingAccountMap',
header: 'Billing Accounts',
cell: (record: Registrar) =>
`${Object.entries(record.billingAccountMap || {}).reduce(
(acc, [key, val]) => {
return `${acc}${key}=${val}<br/>`;
},
''
)}`,
`${Object.entries(record.billingAccountMap || {})
.map(([key, val]) => `${key}=${val}`)
.join(', ')}`,
},
{
columnDef: 'registryLockAllowed',
Expand Down
13 changes: 12 additions & 1 deletion console-webapp/src/app/settings/contact/contact.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,18 @@ <h1>No contacts found</h1>
@for (column of columns; track column) {
<ng-container [matColumnDef]="column.columnDef">
<mat-header-cell *matHeaderCellDef> {{ column.header }} </mat-header-cell>
<mat-cell *matCellDef="let row" [innerHTML]="column.cell(row)"></mat-cell>
<mat-cell *matCellDef="let row">
@if (column.columnDef === 'name') {
<div class="contact__name-column">
<div class="contact__name-column-title">{{ row.name }}</div>
<div class="contact__name-column-roles">
{{ row.userFriendlyTypes.join(" • ") }}
</div>
</div>
} @else {
{{ column.cell(row) }}
}
</mat-cell>
</ng-container>
}
<mat-header-row *matHeaderRowDef="displayedColumns"></mat-header-row>
Expand Down
9 changes: 1 addition & 8 deletions console-webapp/src/app/settings/contact/contact.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,7 @@ export default class ContactComponent {
{
columnDef: 'name',
header: 'Name',
cell: (contact: ViewReadyContact) => `
<div class="contact__name-column">
<div class="contact__name-column-title">${contact.name}</div>
<div class="contact__name-column-roles">${contact.userFriendlyTypes.join(
' • '
)}</div>
</div>
`,
cell: (contact: ViewReadyContact) => `${contact.name}`,
},
{
columnDef: 'emailAddress',
Expand Down
2 changes: 1 addition & 1 deletion console-webapp/src/app/users/usersList.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
<mat-header-cell *matHeaderCellDef>
{{ column.header }}
</mat-header-cell>
<mat-cell *matCellDef="let row" [innerHTML]="column.cell(row)"></mat-cell>
<mat-cell *matCellDef="let row">{{ column.cell(row) }}</mat-cell>
</ng-container>
}
<mat-header-row *matHeaderRowDef="displayedColumns"></mat-header-row>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.gson.annotations.Expose;
import google.registry.flows.EppException.AuthenticationErrorException;
import google.registry.flows.PasswordOnlyTransportCredentials;
import google.registry.model.console.ConsolePermission;
import google.registry.model.console.ConsoleUpdateHistory;
import google.registry.model.console.User;
import google.registry.model.registrar.Registrar;
Expand Down Expand Up @@ -84,6 +85,8 @@ protected void postHandler(User user) {
eppRequestBody.newPassword().equals(eppRequestBody.newPasswordRepeat()),
"New password fields don't match");

checkPermission(user, eppRequestBody.registrarId(), ConsolePermission.CONFIGURE_EPP_CONNECTION);

Registrar registrar;
try {
registrar = registrarAccessor.getRegistrar(eppRequestBody.registrarId());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ private void historyByRegistrarId(User user, String registrarId) {
.createNativeQuery(SQL_REGISTRAR_HISTORY, ConsoleUpdateHistory.class)
.setParameter("registrarId", registrarId)
.setHint("org.hibernate.fetchSize", 1000)
.setMaxResults(500)
.getResultList());

List<ConsoleUpdateHistory> formattedHistoryList =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ private void performRequest(User user) {
"Must provide registry lock email to reset");
requiredPermission = ConsolePermission.MANAGE_USERS;
destinationEmail = passwordResetRequestData.registryLockEmail;
checkUserExistsWithRegistryLockEmail(destinationEmail);
checkUserExistsWithRegistryLockEmail(destinationEmail, registrarId);
emailSubject = "Registry lock password reset request";
}
default -> throw new IllegalArgumentException("Unknown type " + type);
Expand Down Expand Up @@ -121,12 +121,24 @@ private void performRequest(User user) {
.sendEmail(EmailMessage.create(emailSubject, body, destinationAddress));
}

static User checkUserExistsWithRegistryLockEmail(String destinationEmail) {
return tm().createQueryComposer(User.class)
.where("registryLockEmailAddress", QueryComposer.Comparator.EQ, destinationEmail)
.first()
.orElseThrow(
() -> new IllegalArgumentException("Unknown user with lock email " + destinationEmail));
static User checkUserExistsWithRegistryLockEmail(String destinationEmail, String registrarId) {
User targetUser =
tm().createQueryComposer(User.class)
.where("registryLockEmailAddress", QueryComposer.Comparator.EQ, destinationEmail)
.first()
.orElseThrow(
() ->
new IllegalArgumentException(
"Unknown user with lock email " + destinationEmail));

// Prevent IDOR: Ensure the resolved user actually belongs to the registrar the requester
// has permissions for, or is a global admin.
if (!targetUser.getUserRoles().isAdmin()
&& !targetUser.getUserRoles().getRegistrarRoles().containsKey(registrarId)) {
throw new IllegalArgumentException(
"User with lock email " + destinationEmail + " is not associated with " + registrarId);
}
return targetUser;
}

private String getAdminPocEmail(String registrarId) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,9 @@ private void handleEppPasswordReset(PasswordResetRequest request) {
}

private void handleRegistryLockPasswordReset(PasswordResetRequest request) {
User affectedUser = checkUserExistsWithRegistryLockEmail(request.getDestinationEmail());
User affectedUser =
checkUserExistsWithRegistryLockEmail(
request.getDestinationEmail(), request.getRegistrarId());
tm().put(
affectedUser
.asBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package google.registry.ui.server.console.domains;

import static com.google.common.base.Preconditions.checkArgument;
import static com.google.common.collect.ImmutableMap.toImmutableMap;
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
import static jakarta.servlet.http.HttpServletResponse.SC_OK;
Expand Down Expand Up @@ -85,6 +86,12 @@ protected void postHandler(User user) {
optionalJsonPayload.orElseThrow(
() -> new IllegalArgumentException("Bulk action payload must be present"));
BulkDomainList domainList = consoleApiParams.gson().fromJson(jsonPayload, BulkDomainList.class);
checkArgument(
domainList.domainList != null && !domainList.domainList.isEmpty(),
"Domain list cannot be empty");
checkArgument(
domainList.domainList.size() <= 500,
"Cannot process more than 500 domains in a single bulk action");
ConsoleDomainActionType actionType =
ConsoleDomainActionType.parseActionType(bulkDomainAction, jsonPayload);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

import com.google.common.collect.ImmutableMap;
import com.google.gson.Gson;
import com.google.gson.JsonArray;
import com.google.gson.JsonParser;
import google.registry.model.console.ConsoleUpdateHistory;
import google.registry.model.console.RegistrarRole;
import google.registry.model.console.User;
Expand Down Expand Up @@ -162,6 +164,27 @@ void testSuccess_noResults() {
assertThat(response.getPayload()).isEqualTo("[]");
}

@Test
void testSuccess_limitsResultsTo500() {
for (int i = 0; i < 505; i++) {
DatabaseHelper.persistResource(
new ConsoleUpdateHistory.Builder()
.setType(ConsoleUpdateHistory.Type.REGISTRAR_UPDATE)
.setDescription("TheRegistrar|some detail " + i)
.setActingUser(fteUser)
.setUrl("https://test.com")
.setMethod("GET")
.setModificationTime(clock.now())
.build());
}
ConsoleHistoryDataAction action =
createAction(AuthResult.createUser(fteUser), "TheRegistrar", Optional.empty());
action.run();
assertThat(response.getStatus()).isEqualTo(SC_OK);
JsonArray payload = JsonParser.parseString(response.getPayload()).getAsJsonArray();
assertThat(payload.size()).isEqualTo(500);
}

@Test
void testFailure_getByRegistrar_noPermission() {
ConsoleHistoryDataAction action =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import google.registry.testing.FakeResponse;
import google.registry.ui.server.console.ConsoleActionBaseTestCase;
import google.registry.ui.server.console.ConsoleApiParams;
import java.util.Collections;
import java.util.Optional;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
Expand Down Expand Up @@ -173,7 +174,9 @@ void testHalfSuccess_halfNonexistent() throws Exception {

@Test
void testFailure_badActionString() {
ConsoleBulkDomainAction action = createAction("bad", GSON.toJsonTree(ImmutableMap.of()));
ConsoleBulkDomainAction action =
createAction(
"bad", GSON.toJsonTree(ImmutableMap.of("domainList", ImmutableList.of("domain.tld"))));
action.run();
assertThat(response.getStatus()).isEqualTo(SC_BAD_REQUEST);
assertThat(response.getPayload())
Expand All @@ -190,6 +193,29 @@ void testFailure_emptyBody() {
assertThat(response.getPayload()).isEqualTo("Bulk action payload must be present");
}

@Test
void testFailure_listTooLarge() {
JsonElement payload =
GSON.toJsonTree(
ImmutableMap.of(
"domainList", Collections.nCopies(501, "domain.tld"), "reason", "reason"));
ConsoleBulkDomainAction action = createAction("DELETE", payload);
action.run();
assertThat(response.getStatus()).isEqualTo(SC_BAD_REQUEST);
assertThat(response.getPayload())
.isEqualTo("Cannot process more than 500 domains in a single bulk action");
}

@Test
void testFailure_emptyList() {
JsonElement payload =
GSON.toJsonTree(ImmutableMap.of("domainList", ImmutableList.of(), "reason", "reason"));
ConsoleBulkDomainAction action = createAction("DELETE", payload);
action.run();
assertThat(response.getStatus()).isEqualTo(SC_BAD_REQUEST);
assertThat(response.getPayload()).isEqualTo("Domain list cannot be empty");
}

@Test
void testFailure_noPermission() {
JsonElement payload =
Expand Down
Loading