Skip to content

Commit 6552332

Browse files
njshah301njshah301
committed
Merge remote-tracking branch 'origin/feature/mosapi-client' into feature/mosapi-client
2 parents 2a61365 + f44103e commit 6552332

File tree

8 files changed

+162
-14
lines changed

8 files changed

+162
-14
lines changed

‎console-webapp/src/app/history/historyList.component.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ export class HistoryListComponent {
5656
return { main: 'N/A', detail: null };
5757
}
5858
const parts = description.split('|');
59-
const detail = parts.length > 1 ? parts[1].replace(/_/g, ' ') : null;
59+
const detail = parts.length > 1 ? parts[1].replace(/_/g, ' ') : parts[0];
6060

6161
return {
6262
main: parts[0],

‎console-webapp/src/app/settings/contact/contactDetails.component.html‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ <h1>Contact Details</h1>
5757
[(ngModel)]="contactService.contactInEdit.emailAddress"
5858
[ngModelOptions]="{ standalone: true }"
5959
[disabled]="emailAddressIsDisabled()"
60+
[matTooltip]="
61+
emailAddressIsDisabled()
62+
? 'Reach out to registry customer support to update email address'
63+
: ''
64+
"
6065
/>
6166
</mat-form-field>
6267

@@ -84,6 +89,7 @@ <h1>Contact Details</h1>
8489
<h1>Contact Type</h1>
8590
<p class="console-app__contact-required">
8691
<mat-icon color="accent">error</mat-icon>Required to select at least one
92+
(primary contact can't be updated)
8793
</p>
8894
<div class="">
8995
<ng-container

‎core/src/main/java/google/registry/flows/domain/DomainDeletionTimeCache.java‎

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,11 @@
2020
import com.github.benmanes.caffeine.cache.Caffeine;
2121
import com.github.benmanes.caffeine.cache.Expiry;
2222
import com.github.benmanes.caffeine.cache.LoadingCache;
23+
import com.github.benmanes.caffeine.cache.Ticker;
2324
import com.google.common.collect.ImmutableSet;
2425
import google.registry.model.ForeignKeyUtils;
2526
import google.registry.model.domain.Domain;
27+
import google.registry.util.DateTimeUtils;
2628
import java.util.Optional;
2729
import org.joda.time.DateTime;
2830

@@ -55,9 +57,9 @@
5557
public class DomainDeletionTimeCache {
5658

5759
// Max expiry time is ten minutes
58-
private static final int MAX_EXPIRY_MILLIS = 10 * 60 * 1000;
60+
private static final long NANOS_IN_ONE_MILLISECOND = 100000L;
61+
private static final long MAX_EXPIRY_NANOS = 10L * 60L * 1000L * NANOS_IN_ONE_MILLISECOND;
5962
private static final int MAX_ENTRIES = 500;
60-
private static final int NANOS_IN_ONE_MILLISECOND = 100000;
6163

6264
/**
6365
* Expire after the max duration, or after the domain is set to drop (whichever comes first).
@@ -71,9 +73,13 @@ public class DomainDeletionTimeCache {
7173
new Expiry<>() {
7274
@Override
7375
public long expireAfterCreate(String key, DateTime value, long currentTime) {
74-
long millisUntilDeletion = value.getMillis() - tm().getTransactionTime().getMillis();
75-
return NANOS_IN_ONE_MILLISECOND
76-
* Math.max(0L, Math.min(MAX_EXPIRY_MILLIS, millisUntilDeletion));
76+
// Watch out for Long overflow
77+
long deletionTimeNanos =
78+
value.equals(DateTimeUtils.END_OF_TIME)
79+
? Long.MAX_VALUE
80+
: value.getMillis() * NANOS_IN_ONE_MILLISECOND;
81+
long nanosUntilDeletion = deletionTimeNanos - currentTime;
82+
return Math.max(0L, Math.min(MAX_EXPIRY_NANOS, nanosUntilDeletion));
7783
}
7884

7985
/** Reset the time entirely on update, as if we were creating the entry anew. */
@@ -101,9 +107,14 @@ public long expireAfterRead(
101107
return mostRecentResource == null ? null : mostRecentResource.deletionTime();
102108
};
103109

110+
// Unfortunately, maintenance tasks aren't necessarily already in a transaction
111+
private static final Ticker TRANSACTION_TIME_TICKER =
112+
() -> tm().reTransact(() -> tm().getTransactionTime().getMillis() * NANOS_IN_ONE_MILLISECOND);
113+
104114
public static DomainDeletionTimeCache create() {
105115
return new DomainDeletionTimeCache(
106116
Caffeine.newBuilder()
117+
.ticker(TRANSACTION_TIME_TICKER)
107118
.expireAfter(EXPIRY_POLICY)
108119
.maximumSize(MAX_ENTRIES)
109120
.build(CACHE_LOADER));

‎core/src/main/java/google/registry/ui/server/console/ConsoleHistoryDataAction.java‎

Lines changed: 46 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,19 @@
1515
package google.registry.ui.server.console;
1616

1717
import static com.google.common.base.Preconditions.checkArgument;
18+
import static com.google.common.collect.ImmutableList.toImmutableList;
1819
import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
1920
import static google.registry.request.Action.Method.GET;
2021
import static jakarta.servlet.http.HttpServletResponse.SC_BAD_REQUEST;
2122
import static jakarta.servlet.http.HttpServletResponse.SC_OK;
2223

2324
import com.google.common.base.Strings;
25+
import google.registry.config.RegistryConfig.Config;
2426
import google.registry.model.console.ConsolePermission;
2527
import google.registry.model.console.ConsoleUpdateHistory;
28+
import google.registry.model.console.GlobalRole;
2629
import google.registry.model.console.User;
30+
import google.registry.model.console.UserRoles;
2731
import google.registry.request.Action;
2832
import google.registry.request.Action.GaeService;
2933
import google.registry.request.Action.GkeService;
@@ -43,8 +47,8 @@ public class ConsoleHistoryDataAction extends ConsoleApiAction {
4347

4448
private static final String SQL_USER_HISTORY =
4549
"""
46-
SELECT * FROM "ConsoleUpdateHistory"
47-
WHERE acting_user = :actingUser
50+
SELECT * FROM "ConsoleUpdateHistory"
51+
WHERE acting_user = :actingUser
4852
""";
4953

5054
private static final String SQL_REGISTRAR_HISTORY =
@@ -59,14 +63,18 @@ public class ConsoleHistoryDataAction extends ConsoleApiAction {
5963
private final String registrarId;
6064
private final Optional<String> consoleUserEmail;
6165

66+
private final String supportEmail;
67+
6268
@Inject
6369
public ConsoleHistoryDataAction(
6470
ConsoleApiParams consoleApiParams,
71+
@Config("supportEmail") String supportEmail,
6572
@Parameter("registrarId") String registrarId,
6673
@Parameter("consoleUserEmail") Optional<String> consoleUserEmail) {
6774
super(consoleApiParams);
6875
this.registrarId = registrarId;
6976
this.consoleUserEmail = consoleUserEmail;
77+
this.supportEmail = supportEmail;
7078
}
7179

7280
@Override
@@ -95,7 +103,9 @@ private void historyByUser(User user, String consoleUserEmail) {
95103
.setHint("org.hibernate.fetchSize", 1000)
96104
.getResultList());
97105

98-
consoleApiParams.response().setPayload(consoleApiParams.gson().toJson(queryResult));
106+
List<ConsoleUpdateHistory> formattedHistoryList =
107+
replaceActiveUserIfNecessary(queryResult, user);
108+
consoleApiParams.response().setPayload(consoleApiParams.gson().toJson(formattedHistoryList));
99109
consoleApiParams.response().setStatus(SC_OK);
100110
}
101111

@@ -110,7 +120,39 @@ private void historyByRegistrarId(User user, String registrarId) {
110120
.setParameter("registrarId", registrarId)
111121
.setHint("org.hibernate.fetchSize", 1000)
112122
.getResultList());
113-
consoleApiParams.response().setPayload(consoleApiParams.gson().toJson(queryResult));
123+
124+
List<ConsoleUpdateHistory> formattedHistoryList =
125+
replaceActiveUserIfNecessary(queryResult, user);
126+
consoleApiParams.response().setPayload(consoleApiParams.gson().toJson(formattedHistoryList));
114127
consoleApiParams.response().setStatus(SC_OK);
115128
}
129+
130+
/** Anonymizes support users in history logs if the user viewing the log is a registrar user. */
131+
private List<ConsoleUpdateHistory> replaceActiveUserIfNecessary(
132+
List<ConsoleUpdateHistory> historyList, User requestingUser) {
133+
// Check if the user *viewing* the history is a registrar user (not a support user)
134+
if (GlobalRole.NONE.equals(requestingUser.getUserRoles().getGlobalRole())) {
135+
User genericSupportUser = // Fixed typo
136+
new User.Builder()
137+
.setEmailAddress(this.supportEmail)
138+
.setUserRoles(new UserRoles.Builder().build()) // Simplified roles
139+
.build();
140+
141+
return historyList.stream()
142+
.map(
143+
history -> {
144+
// Check if the user who performed the action was a support user
145+
if (!GlobalRole.NONE.equals(
146+
history.getActingUser().getUserRoles().getGlobalRole())) {
147+
return history.asBuilder().setActingUser(genericSupportUser).build();
148+
}
149+
// If acting user was a registrar user show them as-is
150+
return history;
151+
})
152+
.collect(toImmutableList());
153+
}
154+
155+
// If the viewing user is a support user, return the list unmodified
156+
return historyList;
157+
}
116158
}

‎core/src/main/java/google/registry/ui/server/console/ConsoleUpdateRegistrarAction.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
import com.google.common.base.Strings;
2525
import com.google.common.collect.ImmutableSet;
26+
import com.google.common.flogger.FluentLogger;
2627
import google.registry.model.console.ConsolePermission;
2728
import google.registry.model.console.ConsoleUpdateHistory;
2829
import google.registry.model.console.User;
@@ -47,6 +48,8 @@
4748
method = {POST},
4849
auth = Auth.AUTH_PUBLIC_LOGGED_IN)
4950
public class ConsoleUpdateRegistrarAction extends ConsoleApiAction {
51+
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
52+
private static final String CHANGE_LOG_ENTRY = "%s updated on %s, old -> %s, new -> %s";
5053
static final String PATH = "/console-api/registrar";
5154
private final Optional<Registrar> registrar;
5255

@@ -124,6 +127,9 @@ protected void postHandler(User user) {
124127
new ConsoleUpdateHistory.Builder()
125128
.setType(ConsoleUpdateHistory.Type.REGISTRAR_UPDATE)
126129
.setDescription(updatedRegistrar.getRegistrarId()));
130+
131+
logConsoleChangesIfNecessary(updatedRegistrar, existingRegistrar.get());
132+
127133
sendExternalUpdatesIfNecessary(
128134
EmailInfo.create(
129135
existingRegistrar.get(),
@@ -134,4 +140,25 @@ protected void postHandler(User user) {
134140

135141
consoleApiParams.response().setStatus(SC_OK);
136142
}
143+
144+
private void logConsoleChangesIfNecessary(
145+
Registrar updatedRegistrar, Registrar existingRegistrar) {
146+
if (!updatedRegistrar.getAllowedTlds().containsAll(existingRegistrar.getAllowedTlds())) {
147+
logger.atInfo().log(
148+
CHANGE_LOG_ENTRY,
149+
"Allowed TLDs",
150+
updatedRegistrar.getRegistrarId(),
151+
existingRegistrar.getAllowedTlds(),
152+
updatedRegistrar.getAllowedTlds());
153+
}
154+
155+
if (updatedRegistrar.isRegistryLockAllowed() != existingRegistrar.isRegistryLockAllowed()) {
156+
logger.atInfo().log(
157+
CHANGE_LOG_ENTRY,
158+
"Registry lock",
159+
updatedRegistrar.getRegistrarId(),
160+
existingRegistrar.isRegistryLockAllowed(),
161+
updatedRegistrar.isRegistryLockAllowed());
162+
}
163+
}
137164
}

‎core/src/test/java/google/registry/flows/domain/DomainDeletionTimeCacheTest.java‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import google.registry.testing.FakeClock;
2727
import java.util.Optional;
2828
import org.joda.time.DateTime;
29+
import org.joda.time.Duration;
2930
import org.junit.jupiter.api.BeforeEach;
3031
import org.junit.jupiter.api.Test;
3132
import org.junit.jupiter.api.extension.RegisterExtension;
@@ -67,7 +68,8 @@ void testCache_returnsOldData() {
6768
Domain domain = persistActiveDomain("domain.tld");
6869
assertThat(getDeletionTimeFromCache("domain.tld")).hasValue(END_OF_TIME);
6970
persistDomainAsDeleted(domain, clock.nowUtc().plusDays(1));
70-
// Without intervention, the cache should have the old data
71+
// Without intervention, the cache should have the old data, even if a few minutes have passed
72+
clock.advanceBy(Duration.standardMinutes(5));
7173
assertThat(getDeletionTimeFromCache("domain.tld")).hasValue(END_OF_TIME);
7274
}
7375

@@ -79,6 +81,16 @@ void testCache_returnsNewDataAfterDomainCreate() {
7981
assertThat(getDeletionTimeFromCache("domain.tld")).hasValue(clock.nowUtc().plusDays(1));
8082
}
8183

84+
@Test
85+
void testCache_expires() {
86+
Domain domain = persistActiveDomain("domain.tld");
87+
assertThat(getDeletionTimeFromCache("domain.tld")).hasValue(END_OF_TIME);
88+
DateTime elevenMinutesFromNow = clock.nowUtc().plusMinutes(11);
89+
persistDomainAsDeleted(domain, elevenMinutesFromNow);
90+
clock.advanceBy(Duration.standardMinutes(30));
91+
assertThat(getDeletionTimeFromCache("domain.tld")).hasValue(elevenMinutesFromNow);
92+
}
93+
8294
private Optional<DateTime> getDeletionTimeFromCache(String domainName) {
8395
return tm().transact(() -> cache.getDeletionTimeForDomain(domainName));
8496
}

‎core/src/test/java/google/registry/ui/server/console/ConsoleHistoryDataActionTest.java‎

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,10 @@
4040

4141
class ConsoleHistoryDataActionTest extends ConsoleActionBaseTestCase {
4242

43+
private static final String SUPPORT_EMAIL = "[email protected]";
4344
private static final Gson GSON = new Gson();
4445
private User noPermissionUser;
46+
private User registrarPrimaryUser;
4547

4648
@BeforeEach
4749
void beforeEach() {
@@ -56,6 +58,17 @@ void beforeEach() {
5658
.build())
5759
.build());
5860

61+
registrarPrimaryUser =
62+
DatabaseHelper.persistResource(
63+
new User.Builder()
64+
.setEmailAddress("[email protected]")
65+
.setUserRoles(
66+
new UserRoles.Builder()
67+
.setRegistrarRoles(
68+
ImmutableMap.of("TheRegistrar", RegistrarRole.PRIMARY_CONTACT))
69+
.build())
70+
.build());
71+
5972
DatabaseHelper.persistResources(
6073
ImmutableList.of(
6174
new ConsoleUpdateHistory.Builder()
@@ -85,14 +98,43 @@ void beforeEach() {
8598
}
8699

87100
@Test
88-
void testSuccess_getByRegistrar() {
101+
void testSuccess_getByRegistrar_notAnonymizedForSupportUser() {
89102
ConsoleHistoryDataAction action =
90103
createAction(AuthResult.createUser(fteUser), "TheRegistrar", Optional.empty());
91104
action.run();
92105
assertThat(response.getStatus()).isEqualTo(SC_OK);
93106
List<Map<String, Object>> payload = GSON.fromJson(response.getPayload(), List.class);
94107
assertThat(payload.stream().map(record -> record.get("description")).collect(toImmutableList()))
95108
.containsExactly("TheRegistrar|Some change", "TheRegistrar|Another change");
109+
110+
// Assert that the support user sees the real acting users
111+
List<String> actingUserEmails =
112+
payload.stream()
113+
.map(record -> (Map<String, Object>) record.get("actingUser"))
114+
.map(userMap -> (String) userMap.get("emailAddress"))
115+
.collect(toImmutableList());
116+
117+
assertThat(actingUserEmails).containsExactly("[email protected]", "[email protected]");
118+
}
119+
120+
@Test
121+
void testSuccess_getByRegistrar_anonymizedForRegistrarUser() {
122+
ConsoleHistoryDataAction action =
123+
createAction(AuthResult.createUser(registrarPrimaryUser), "TheRegistrar", Optional.empty());
124+
action.run();
125+
assertThat(response.getStatus()).isEqualTo(SC_OK);
126+
List<Map<String, Object>> payload = GSON.fromJson(response.getPayload(), List.class);
127+
assertThat(payload.stream().map(record -> record.get("description")).collect(toImmutableList()))
128+
.containsExactly("TheRegistrar|Some change", "TheRegistrar|Another change");
129+
130+
// Assert that the registrar user sees the anonymized support user
131+
List<String> actingUserEmails =
132+
payload.stream()
133+
.map(record -> (Map<String, Object>) record.get("actingUser"))
134+
.map(userMap -> (String) userMap.get("emailAddress"))
135+
.collect(toImmutableList());
136+
137+
assertThat(actingUserEmails).containsExactly(SUPPORT_EMAIL, "[email protected]");
96138
}
97139

98140
@Test
@@ -104,6 +146,13 @@ void testSuccess_getByUser() {
104146
List<Map<String, Object>> payload = GSON.fromJson(response.getPayload(), List.class);
105147
assertThat(payload.stream().map(record -> record.get("description")).collect(toImmutableList()))
106148
.containsExactly("TheRegistrar|Some change", "OtherRegistrar|Some change");
149+
150+
List<String> actingUserEmails =
151+
payload.stream()
152+
.map(record -> (Map<String, Object>) record.get("actingUser"))
153+
.map(userMap -> (String) userMap.get("emailAddress"))
154+
.collect(toImmutableList());
155+
assertThat(actingUserEmails).containsExactly("[email protected]", "[email protected]");
107156
}
108157

109158
@Test
@@ -148,6 +197,7 @@ private ConsoleHistoryDataAction createAction(
148197
consoleApiParams = ConsoleApiParamsUtils.createFake(authResult);
149198
when(consoleApiParams.request().getMethod()).thenReturn("GET");
150199
response = (FakeResponse) consoleApiParams.response();
151-
return new ConsoleHistoryDataAction(consoleApiParams, registrarId, consoleUserEmail);
200+
return new ConsoleHistoryDataAction(
201+
consoleApiParams, SUPPORT_EMAIL, registrarId, consoleUserEmail);
152202
}
153203
}

‎db/build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -260,7 +260,7 @@ if (project.baseSchemaTag != '') {
260260

261261
// Checks if Flyway scripts can be deployed to an existing database with
262262
// an older release. Please refer to SchemaTest.java for more information.
263-
task schemaIncrementalDeployTest(dependsOn: processResources, type: Test) {
263+
task schemaIncrementalDeployTest(dependsOn: processTestResources, type: Test) {
264264
useJUnitPlatform()
265265
include 'google/registry/sql/flyway/SchemaTest.*'
266266
classpath = configurations.testRuntimeClasspath

0 commit comments

Comments
 (0)