Send the "resource" ID in each resource action

This is an intermediate CL, part of the Registrar Console cleanup.

TL;DR:
- the current state: resource.js points to a resource TYPE on the server (only registrars can be resources right now), but the specific resource is selected based on the user (we select the "first resource of this type that the user has access to)
- new state: resource.js points to a SPECIFIC resource (TYPE + ID).

In this CL the server still chooses the resource like before (first one that user has access to) but we make sure the returned resource is the same one we requested.

In a subsequent CL we will use the requested ID to load the resource, and then make sure the user has access to that resource.

---------------------------

When loading the RegistrarConsole HTML page, the server determines which clientId belongs to the user ("guesses" it by looking for the first registrar that has this user as contact). It sends the relevant clientId back with the page load.

However, this information isn't currently used in the JS requests to read / update the registrar. Instead, currently the client ID is guessed again for each JS access to the server. It is also saved again in the client's "session" cookie.

As a result, it is theoretically possible to have the JS access a different clientID than the original page load (not likely, since it requires a single user registered for multiple registrars AND that the contacts change for the original registrar).

So our goal is to only have a single clientID "value" instead of the 3 we currently have for JS requests (the one from the initial page load, the one saved in the session cookie, the one guessed on the JS request)

As a first step, we send over the "initial page load" clientId on every JS request, and make sure the "session + guessed" value is equal to that one. Later we will remove the "session+guessed" values from the RegistrarSettings, using the "initial page load" clientID instead.

In addition to the "nicer code" implications, having the clientID from the initial page load always used means it'll be easy to have a clientID selection option for users who have access to multiple clientIDs (such as admins)

SECURITY NOTE:the choice of clientID has no security implication since we make sure the user has access to the clientID no matter how we actually choose the clientID on every single server request.

-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=214459506

java/google/registry/ui/js/registrar/console.js

@@ -139,7 +139,8 @@ registry.registrar.Console.prototype.handleHashChange = function() {
   }
   var oldComponent = this.component_;
   const resource = new registry.Resource(
-      new goog.Uri('/registrar-settings'), this.params.xsrfToken);
+      new goog.Uri('/registrar-settings'), this.params.clientId,
+      this.params.xsrfToken);
   this.component_ = new componentCtor(this, resource);
   this.registerDisposable(this.component_);
   this.component_.basePath = type;

java/google/registry/ui/js/resource.js

@@ -25,13 +25,16 @@ goog.forwardDeclare('goog.Uri');
  * Provide a CRUD view of a server resource.
  *
  * @param {!goog.Uri} baseUri Target RESTful resource.
+ * @param {string} id the ID of the target resource
  * @param {string} xsrfToken Security token to pass back to the server.
  * @extends {registry.Session}
  * @constructor
  */
-registry.Resource = function(baseUri, xsrfToken) {
+registry.Resource = function(baseUri, id, xsrfToken) {
   registry.Resource.base(this, 'constructor', baseUri, xsrfToken,
                          registry.Session.ContentType.JSON);
+  /** @const @private {string} the ID of the target resource. */
+  this.id_ = id;
 };
 goog.inherits(registry.Resource, registry.Session);
 
@@ -73,5 +76,6 @@ registry.Resource.prototype.send_ =
   var req = {};
   req['op'] = opCode;
   req['args'] = argsObj;
+  req['id'] = this.id_;
   this.sendXhrIo(goog.json.serialize(req), callback);
 };

java/google/registry/ui/server/registrar/RegistrarSettingsAction.java

@@ -74,6 +74,7 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
 
   static final String OP_PARAM = "op";
   static final String ARGS_PARAM = "args";
+  static final String ID_PARAM = "id";
 
   @Inject HttpServletRequest request;
   @Inject JsonActionRunner jsonActionRunner;
@@ -100,6 +101,20 @@ public class RegistrarSettingsAction implements Runnable, JsonActionRunner.JsonA
     }
 
     Registrar initialRegistrar = sessionUtils.getRegistrarForAuthResult(request, authResult);
+    // Check that the clientId requested is the same as the one we get in the
+    // getRegistrarForAuthResult.
+    // TODO(b/113925293): remove this check, and instead use the requested clientId to select the
+    // registrar (in a secure way making sure authResult has access to that registrar!)
+    String clientId = (String) input.get(ID_PARAM);
+    if (Strings.isNullOrEmpty(clientId)) {
+      throw new BadRequestException(String.format("Missing key for resource ID: %s", ID_PARAM));
+    }
+    if (!clientId.equals(initialRegistrar.getClientId())) {
+      throw new BadRequestException(
+          String.format(
+              "User's clientId changed from %s to %s. Please reload page",
+              clientId, initialRegistrar.getClientId()));
+    }
     // Process the operation.  Though originally derived from a CRUD
     // handler, registrar-settings really only supports read and update.
     String op = Optional.ofNullable((String) input.get(OP_PARAM)).orElse("read");

javatests/google/registry/ui/js/registrar/contact_settings_test.js

@@ -82,7 +82,7 @@ function testCollectionView() {
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'read', args: {}},
+      {op: 'read', id: 'testClientId', args: {}},
       {
         status: 'SUCCESS',
         message: 'OK',
@@ -106,7 +106,7 @@ function testItemView() {
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'read', args: {}},
+      {op: 'read', id: 'testClientId', args: {}},
       {
         status: 'SUCCESS',
         message: 'OK',
@@ -149,6 +149,7 @@ function testItemEdit() {
       '/registrar-settings',
       {
         op: 'update',
+        id: 'testClientId',
         args: {
           contacts: [testContact],
           readonly: false
@@ -181,6 +182,7 @@ function testChangeContactTypes() {
       '/registrar-settings',
       {
         op: 'update',
+        id: 'testClientId',
         args: {
           contacts: [testContact],
           readonly: false
@@ -204,7 +206,7 @@ function testOneOfManyUpdate() {
   registry.registrar.ConsoleTestUtil.visit(test, {
     path: 'contact-settings/test@example.com',
     xsrfToken: test.testXsrfToken,
-    testClientId: test.testClientId
+    clientId: test.testClientId
   });
   var testContacts = [
     createTestContact('new1@asdf.com'),
@@ -214,7 +216,7 @@ function testOneOfManyUpdate() {
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'read', args: {}},
+      {op: 'read', id: 'testClientId', args: {}},
       {
         status: 'SUCCESS',
         message: 'OK',
@@ -235,7 +237,14 @@ function testOneOfManyUpdate() {
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'update', args: {contacts: testContacts, readonly: false}},
+      {
+        op: 'update',
+        id: 'testClientId',
+        args: {
+          contacts: testContacts,
+          readonly: false,
+        },
+      },
       {
         status: 'SUCCESS',
         message: 'OK',
@@ -251,13 +260,15 @@ function testDomainWhoisAbuseContactOverride() {
   registry.registrar.ConsoleTestUtil.visit(test, {
     path: 'contact-settings/test@example.com',
     xsrfToken: test.testXsrfToken,
-    testClientId: test.testClientId
+    clientId: test.testClientId
   });
   var oldDomainWhoisAbuseContact = createTestContact('old@asdf.com');
   oldDomainWhoisAbuseContact.visibleInDomainWhoisAsAbuse = true;
   var testContacts = [oldDomainWhoisAbuseContact, testContact];
   registry.testing.assertReqMockRsp(
-      test.testXsrfToken, '/registrar-settings', {op: 'read', args: {}},
+      test.testXsrfToken,
+      '/registrar-settings',
+      {op: 'read', id: 'testClientId', args: {}},
       {status: 'SUCCESS', message: 'OK', results: [{contacts: testContacts}]});
   // Edit testContact.
   registry.testing.click($('reg-app-btn-edit'));
@@ -271,8 +282,13 @@ function testDomainWhoisAbuseContactOverride() {
   testContact.visibleInDomainWhoisAsAbuse = true;
   oldDomainWhoisAbuseContact.visibleInDomainWhoisAsAbuse = false;
   registry.testing.assertReqMockRsp(
-      test.testXsrfToken, '/registrar-settings',
-      {op: 'update', args: {contacts: testContacts, readonly: false}},
+      test.testXsrfToken,
+      '/registrar-settings',
+      {
+        op: 'update',
+        id: 'testClientId',
+        args: {contacts: testContacts, readonly: false},
+      },
       {status: 'SUCCESS', message: 'OK', results: [{contacts: testContacts}]});
 }
 
@@ -281,7 +297,7 @@ function testDelete() {
   registry.registrar.ConsoleTestUtil.visit(test, {
     path: 'contact-settings/test@example.com',
     xsrfToken: test.testXsrfToken,
-    testClientId: test.testClientId
+    clientId: test.testClientId
   });
   var testContacts = [
     createTestContact('new1@asdf.com'),
@@ -291,7 +307,7 @@ function testDelete() {
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'read', args: {}},
+      {op: 'read', id: 'testClientId', args: {}},
       {
         status: 'SUCCESS',
         message: 'OK',
@@ -309,7 +325,11 @@ function testDelete() {
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'update', args: {contacts: testContacts, readonly: false}},
+      {
+        op: 'update',
+        id: 'testClientId',
+        args: {contacts: testContacts, readonly: false},
+      },
       {
         status: 'SUCCESS',
         message: 'OK',

javatests/google/registry/ui/js/registrar/security_settings_test.js

@@ -80,12 +80,12 @@ function testView() {
   registry.registrar.ConsoleTestUtil.visit(test, {
     path: 'security-settings',
     xsrfToken: test.testXsrfToken,
-    testClientId: test.testClientId
+    clientId: test.testClientId
   });
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'read', args: {}},
+      {op: 'read', id: 'testClientId', args: {}},
       {
         status: 'SUCCESS',
         message: 'OK',
@@ -116,7 +116,7 @@ function testEdit() {
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'update', args: {
+      {op: 'update', id: 'testClientId', args: {
         clientCertificate: exampleCert,
         clientCertificateHash: null,
         failoverClientCertificate: 'bourgeois blues',
@@ -137,7 +137,7 @@ function testEdit() {
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'read', args: {}},
+      {op: 'read', id: 'testClientId', args: {}},
       {status: 'SUCCESS',
         message: 'OK',
         results: [expectedRegistrar]});

javatests/google/registry/ui/js/registrar/whois_settings_test.js

@@ -98,13 +98,13 @@ function testView() {
   registry.registrar.ConsoleTestUtil.visit(test, {
     path: 'whois-settings',
     xsrfToken: test.testXsrfToken,
-    testClientId: test.testClientId
+    clientId: test.testClientId
   });
   var testRegistrar = createTestRegistrar();
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'read', args: {}},
+      {op: 'read', id: 'testClientId', args: {}},
       {
         status: 'SUCCESS',
         message: 'OK',
@@ -129,7 +129,7 @@ function testEdit() {
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'update', args: parsed},
+      {op: 'update', id: 'testClientId', args: parsed},
       {
         status: 'SUCCESS',
         message: 'OK',
@@ -149,7 +149,7 @@ function testEditFieldError_insertsError() {
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'update', args: parsed},
+      {op: 'update', id: 'testClientId', args: parsed},
       {
         status: 'ERROR',
         field: 'phoneNumber',
@@ -173,7 +173,7 @@ function testEditNonFieldError_showsButterBar() {
   registry.testing.assertReqMockRsp(
       test.testXsrfToken,
       '/registrar-settings',
-      {op: 'update', args: parsed},
+      {op: 'update', id: 'testClientId', args: parsed},
       {
         status: 'ERROR',
         message: errMsg

javatests/google/registry/ui/js/testing.js

@@ -106,11 +106,10 @@ registry.testing.assertObjectEqualsPretty = function(a, b) {
   try {
     assertObjectEquals(a, b);
   } catch (e) {
-    throw Error(e.message + '\n' +
-        'expected: ' +
-        registry.testing.pretty_.format(a) + '\n' +
-        'got: ' +
-        registry.testing.pretty_.format(b));
+    e.message = e.message + '\n' +
+        'expected: ' + registry.testing.pretty_.format(a) + '\n' +
+        'got: ' + registry.testing.pretty_.format(b);
+    throw e;
   }
 };
 

javatests/google/registry/ui/server/registrar/ContactSettingsTest.java

@@ -45,6 +45,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
   public void testPost_readContacts_success() {
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "read",
+        "id", CLIENT_ID,
         "args", ImmutableMap.of()));
     @SuppressWarnings("unchecked")
     List<Map<String, ?>> results = (List<Map<String, ?>>) response.get("results");
@@ -56,6 +57,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
   public void testPost_loadSaveRegistrar_success() {
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
+        "id", CLIENT_ID,
         "args", loadRegistrar(CLIENT_ID).toJsonMap()));
     assertThat(response).containsEntry("status", "SUCCESS");
   }
@@ -75,7 +77,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
     Map<String, Object> regMap = registrar.toJsonMap();
     regMap.put("contacts", ImmutableList.of(adminContact1));
     Map<String, Object> response =
-        action.handleJsonRequest(ImmutableMap.of("op", "update", "args", regMap));
+        action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", regMap));
     assertThat(response).containsEntry("status", "SUCCESS");
 
     RegistrarContact newContact = new RegistrarContact.Builder()
@@ -98,6 +100,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
             .build().toJsonMap()));
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
+        "id", CLIENT_ID,
         "args", reqJson));
     assertThat(response).containsEntry("status", "ERROR");
     assertThat(response).containsEntry("message", "Must have at least one "
@@ -123,6 +126,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
     reqJson.put("contacts", ImmutableList.of(rc.toJsonMap()));
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
+        "id", CLIENT_ID,
         "args", reqJson));
     assertThat(response).containsEntry("status", "ERROR");
     assertThat(response).containsEntry("message", "Please provide a phone number for at least one "
@@ -148,7 +152,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
     Map<String, Object> reqJson = registrar.toJsonMap();
     reqJson.put("contacts", ImmutableList.of(rc.toJsonMap()));
     Map<String, Object> response =
-        action.handleJsonRequest(ImmutableMap.of("op", "update", "args", reqJson));
+        action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", reqJson));
     assertThat(response).containsEntry("status", "ERROR");
     assertThat(response)
         .containsEntry(
@@ -174,7 +178,7 @@ public class ContactSettingsTest extends RegistrarSettingsActionTestCase {
     Map<String, Object> reqJson = registrar.toJsonMap();
     reqJson.put("contacts", ImmutableList.of(rc.toJsonMap()));
     Map<String, Object> response =
-        action.handleJsonRequest(ImmutableMap.of("op", "update", "args", reqJson));
+        action.handleJsonRequest(ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", reqJson));
     assertThat(response).containsEntry("status", "ERROR");
     assertThat(response)
         .containsEntry(

javatests/google/registry/ui/server/registrar/RegistrarSettingsActionTest.java

@@ -32,6 +32,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import google.registry.export.sheet.SyncRegistrarsSheetAction;
 import google.registry.model.registrar.Registrar;
+import google.registry.request.HttpException.BadRequestException;
 import google.registry.request.HttpException.ForbiddenException;
 import google.registry.request.auth.AuthResult;
 import google.registry.testing.CertificateSamples;
@@ -94,18 +95,33 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
    * This is the default read test for the registrar settings actions.
    */
   @Test
-  public void testRead_authorized_returnsRegistrarJson() {
-    Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of());
+  public void testSuccess_readRegistrarInfo_authorized() {
+    Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of("id", CLIENT_ID));
     assertThat(response).containsExactly(
         "status", "SUCCESS",
         "message", "Success",
         "results", asList(loadRegistrar(CLIENT_ID).toJsonMap()));
   }
 
+  /**
+   * We got a different CLIENT_ID from the JS than the one we find ourself.
+   *
+   * <p>This might happen if the user's "guessed" registrar changes after the initial page load. For
+   * example, if the user was added as contact to a different registrar, or removed as contact from
+   * the current registrar (but is still a contact of a different one, so the "guessing" works).
+   */
+  @Test
+  public void testFailure_readRegistrarInfo_differentClientId() {
+    assertThrows(
+        BadRequestException.class,
+        () -> action.handleJsonRequest(ImmutableMap.of("id", "different")));
+  }
+
   @Test
   public void testUpdate_emptyJsonObject_errorLastUpdateTimeFieldRequired() {
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
+        "id", CLIENT_ID,
         "args", ImmutableMap.of()));
     assertThat(response).containsExactly(
         "status", "ERROR",
@@ -119,6 +135,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
   public void testUpdate_noEmail_errorEmailFieldRequired() {
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
+        "id", CLIENT_ID,
         "args", ImmutableMap.of("lastUpdateTime", getLastUpdateTime())));
     assertThat(response).containsExactly(
         "status", "ERROR",
@@ -134,6 +151,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
 
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
+        "id", CLIENT_ID,
         "args", ImmutableMap.of("lastUpdateTime", getLastUpdateTime())));
     assertThat(response).containsExactly(
         "status", "SUCCESS",
@@ -145,6 +163,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
   public void testUpdate_badEmail_errorEmailField() {
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
+        "id", CLIENT_ID,
         "args", ImmutableMap.of(
             "lastUpdateTime", getLastUpdateTime(),
             "emailAddress", "lolcat")));
@@ -160,6 +179,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
   public void testPost_nonParsableTime_getsAngry() {
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
+        "id", CLIENT_ID,
         "args", ImmutableMap.of("lastUpdateTime", "cookies")));
     assertThat(response).containsExactly(
         "status", "ERROR",
@@ -173,6 +193,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
   public void testPost_nonAsciiCharacters_getsAngry() {
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
+        "id", CLIENT_ID,
         "args", ImmutableMap.of(
             "lastUpdateTime", getLastUpdateTime(),
             "emailAddress", "ヘ(◕。◕ヘ)@example.com")));
@@ -195,6 +216,7 @@ public class RegistrarSettingsActionTest extends RegistrarSettingsActionTestCase
         action.handleJsonRequest(
             ImmutableMap.of(
                 "op", "update",
+                "id", CLIENT_ID,
                 "args", setter.apply(registrar.asBuilder(), newValue).build().toJsonMap()));
 
     registrar = loadRegistrar(CLIENT_ID);

javatests/google/registry/ui/server/registrar/SecuritySettingsTest.java

@@ -51,6 +51,7 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
             .build();
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
+        "id", CLIENT_ID,
         "args", modified.toJsonMap()));
     // Empty whoisServer field should be set to default by server.
     modified =
@@ -69,6 +70,7 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
     reqJson.put("clientCertificate", "BLAH");
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
         "op", "update",
+        "id", CLIENT_ID,
         "args", reqJson));
     assertThat(response).containsEntry("status", "ERROR");
     assertThat(response).containsEntry("message", "Invalid X.509 PEM certificate");
@@ -80,7 +82,7 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
     jsonMap.put("clientCertificate", SAMPLE_CERT);
     jsonMap.put("failoverClientCertificate", null);
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
-        "op", "update", "args", jsonMap));
+        "op", "update", "id", CLIENT_ID, "args", jsonMap));
     assertThat(response).containsEntry("status", "SUCCESS");
     Registrar registrar = loadRegistrar(CLIENT_ID);
     assertThat(registrar.getClientCertificate()).isEqualTo(SAMPLE_CERT);
@@ -94,7 +96,7 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
     Map<String, Object> jsonMap = loadRegistrar(CLIENT_ID).toJsonMap();
     jsonMap.put("failoverClientCertificate", SAMPLE_CERT2);
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
-        "op", "update", "args", jsonMap));
+        "op", "update", "id", CLIENT_ID, "args", jsonMap));
     assertThat(response).containsEntry("status", "SUCCESS");
     Registrar registrar = loadRegistrar(CLIENT_ID);
     assertThat(registrar.getFailoverClientCertificate()).isEqualTo(SAMPLE_CERT2);
@@ -116,7 +118,7 @@ public class SecuritySettingsTest extends RegistrarSettingsActionTestCase {
     jsonMap.put("clientCertificate", null);
     jsonMap.put("failoverClientCertificate", "");
     Map<String, Object> response = action.handleJsonRequest(ImmutableMap.of(
-        "op", "update", "args", jsonMap));
+        "op", "update", "id", CLIENT_ID, "args", jsonMap));
     assertThat(response).containsEntry("status", "SUCCESS");
     Registrar registrar = loadRegistrar(CLIENT_ID);
     assertThat(registrar.getClientCertificate()).isEqualTo(SAMPLE_CERT);

javatests/google/registry/ui/server/registrar/WhoisSettingsTest.java

@@ -56,7 +56,8 @@ public class WhoisSettingsTest extends RegistrarSettingsActionTestCase {
                     .build())
             .build();
     Map<String, Object> response =
-        action.handleJsonRequest(ImmutableMap.of("op", "update", "args", modified.toJsonMap()));
+        action.handleJsonRequest(
+            ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", modified.toJsonMap()));
     assertThat(response.get("status")).isEqualTo("SUCCESS");
     assertThat(response.get("results")).isEqualTo(asList(modified.toJsonMap()));
     assertThat(loadRegistrar(CLIENT_ID)).isEqualTo(modified);
@@ -80,7 +81,8 @@ public class WhoisSettingsTest extends RegistrarSettingsActionTestCase {
                     .build())
             .build();
     Map<String, Object> response =
-        action.handleJsonRequest(ImmutableMap.of("op", "update", "args", modified.toJsonMap()));
+        action.handleJsonRequest(
+            ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", modified.toJsonMap()));
     assertThat(response.get("status")).isEqualTo("ERROR");
     assertThat(response.get("field")).isEqualTo("localizedAddress.state");
     assertThat(response.get("message")).isEqualTo("Unknown US state code.");
@@ -105,7 +107,8 @@ public class WhoisSettingsTest extends RegistrarSettingsActionTestCase {
                     .build())
             .build();
     Map<String, Object> response =
-        action.handleJsonRequest(ImmutableMap.of("op", "update", "args", modified.toJsonMap()));
+        action.handleJsonRequest(
+            ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", modified.toJsonMap()));
     assertThat(response.get("status")).isEqualTo("ERROR");
     assertThat(response.get("field")).isEqualTo("localizedAddress.street[1]");
     assertThat((String) response.get("message"))
@@ -118,7 +121,8 @@ public class WhoisSettingsTest extends RegistrarSettingsActionTestCase {
     Registrar modified =
         loadRegistrar(CLIENT_ID).asBuilder().setWhoisServer("tears@dry.tragical.lol").build();
     Map<String, Object> response =
-        action.handleJsonRequest(ImmutableMap.of("op", "update", "args", modified.toJsonMap()));
+        action.handleJsonRequest(
+            ImmutableMap.of("op", "update", "id", CLIENT_ID, "args", modified.toJsonMap()));
     assertThat(response.get("status")).isEqualTo("ERROR");
     assertThat(response.get("field")).isEqualTo("whoisServer");
     assertThat(response.get("message")).isEqualTo("Not a valid hostname.");

javatests/google/registry/ui/server/registrar/testdata/update_registrar.json

@@ -1,5 +1,6 @@
 {
   "op": "update",
+  "id": "TheRegistrar",
   "args": {
     "clientIdentifier": "theregistrar",
     "driveFolderId": null,

javatests/google/registry/ui/server/registrar/testdata/update_registrar_duplicate_contacts.json

@@ -1,5 +1,6 @@
 {
   "op": "update",
+  "id": "TheRegistrar",
   "args": {
     "clientIdentifier": "theregistrar",
     "driveFolderId": null,