Allow EPP password to be set during login flow (#2080)

This is part of the spec in RFC 5730 that we hadn't implemented until now. Note
that this requires changing LoginFlow to be transactional, but I don't think
that should cause any issues.

core/src/main/java/google/registry/flows/FlowRunner.java

@@ -72,22 +72,21 @@ public class FlowRunner {
     }
     eppMetricBuilder.setCommandNameFromFlow(flowClass.getSimpleName());
     if (!isTransactional) {
-      EppOutput eppOutput = EppOutput.create(flowProvider.get().run());
+      return EppOutput.create(flowProvider.get().run());
-      if (flowClass.equals(LoginFlow.class)) {
-        // In LoginFlow, registrarId isn't known until after the flow executes, so save it then.
-        eppMetricBuilder.setRegistrarId(sessionMetadata.getRegistrarId());
-      }
-      return eppOutput;
     }
     try {
-      return tm()
+      return tm().transact(
-          .transact(
               () -> {
                 try {
                   EppOutput output = EppOutput.create(flowProvider.get().run());
                   if (isDryRun) {
                     throw new DryRunException(output);
                   }
+                  if (flowClass.equals(LoginFlow.class)) {
+                    // In LoginFlow, registrarId isn't known until after the flow executes, so save
+                    // it then.
+                    eppMetricBuilder.setRegistrarId(sessionMetadata.getRegistrarId());
+                  }
                   return output;
                 } catch (EppException e) {
                   throw new EppRuntimeException(e);

core/src/main/java/google/registry/flows/session/LoginFlow.java

@@ -15,6 +15,7 @@
 package google.registry.flows.session;
 
 import static com.google.common.collect.Sets.difference;
+import static google.registry.persistence.transaction.TransactionManagerFactory.tm;
 import static google.registry.util.CollectionUtils.nullToEmpty;
 
 import com.google.common.collect.ImmutableSet;
@@ -27,11 +28,10 @@ import google.registry.flows.EppException.CommandUseErrorException;
 import google.registry.flows.EppException.ParameterValuePolicyErrorException;
 import google.registry.flows.EppException.UnimplementedExtensionException;
 import google.registry.flows.EppException.UnimplementedObjectServiceException;
-import google.registry.flows.EppException.UnimplementedOptionException;
 import google.registry.flows.ExtensionManager;
-import google.registry.flows.Flow;
 import google.registry.flows.FlowModule.RegistrarId;
 import google.registry.flows.SessionMetadata;
+import google.registry.flows.TransactionalFlow;
 import google.registry.flows.TransportCredentials;
 import google.registry.model.eppcommon.ProtocolDefinition;
 import google.registry.model.eppcommon.ProtocolDefinition.ServiceExtension;
@@ -51,6 +51,7 @@ import javax.inject.Inject;
  * @error {@link google.registry.flows.EppException.UnimplementedExtensionException}
  * @error {@link google.registry.flows.EppException.UnimplementedObjectServiceException}
  * @error {@link google.registry.flows.EppException.UnimplementedProtocolVersionException}
+ * @error {@link google.registry.flows.FlowUtils.GenericXmlSyntaxErrorException}
  * @error {@link google.registry.flows.TlsCredentials.BadRegistrarCertificateException}
  * @error {@link google.registry.flows.TlsCredentials.BadRegistrarIpAddressException}
  * @error {@link google.registry.flows.TlsCredentials.MissingRegistrarCertificateException}
@@ -58,11 +59,10 @@ import javax.inject.Inject;
  * @error {@link LoginFlow.AlreadyLoggedInException}
  * @error {@link BadRegistrarIdException}
  * @error {@link LoginFlow.TooManyFailedLoginsException}
- * @error {@link LoginFlow.PasswordChangesNotSupportedException}
  * @error {@link LoginFlow.RegistrarAccountNotActiveException}
  * @error {@link LoginFlow.UnsupportedLanguageException}
  */
-public class LoginFlow implements Flow {
+public class LoginFlow implements TransactionalFlow {
 
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
@@ -134,8 +134,13 @@ public class LoginFlow implements Flow {
     if (!registrar.get().isLive()) {
       throw new RegistrarAccountNotActiveException();
     }
-    if (login.getNewPassword() != null) {  // We don't support in-band password changes.
+    if (login.getNewPassword().isPresent()) {
-      throw new PasswordChangesNotSupportedException();
+      // Load fresh from database (bypassing the cache) to ensure we don't save stale data.
+      Optional<Registrar> freshRegistrar = Registrar.loadByRegistrarId(login.getClientId());
+      if (!freshRegistrar.isPresent()) {
+        throw new BadRegistrarIdException(login.getClientId());
+      }
+      tm().put(freshRegistrar.get().asBuilder().setPassword(login.getNewPassword().get()).build());
     }
 
     // We are in!
@@ -179,11 +184,4 @@ public class LoginFlow implements Flow {
       super("Specified language is not supported");
     }
   }
-
-  /** In-band password changes are not supported. */
-  static class PasswordChangesNotSupportedException extends UnimplementedOptionException {
-    public PasswordChangesNotSupportedException() {
-      super("In-band password changes are not supported");
-    }
-  }
 }

core/src/main/java/google/registry/model/eppinput/EppInput.java

@@ -291,8 +291,8 @@ public class EppInput extends ImmutableObject {
       return password;
     }
 
-    public String getNewPassword() {
+    public Optional<String> getNewPassword() {
-      return newPassword;
+      return Optional.ofNullable(newPassword);
     }
 
     public Options getOptions() {

core/src/test/java/google/registry/flows/session/LoginFlowTestCase.java

@@ -21,15 +21,16 @@ import static google.registry.testing.DatabaseHelper.persistResource;
 import static google.registry.testing.EppExceptionSubject.assertAboutEppExceptions;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
+import com.google.common.collect.ImmutableMap;
 import google.registry.flows.EppException;
 import google.registry.flows.EppException.UnimplementedExtensionException;
 import google.registry.flows.EppException.UnimplementedObjectServiceException;
 import google.registry.flows.EppException.UnimplementedProtocolVersionException;
 import google.registry.flows.FlowTestCase;
+import google.registry.flows.FlowUtils.GenericXmlSyntaxErrorException;
 import google.registry.flows.TransportCredentials.BadRegistrarPasswordException;
 import google.registry.flows.session.LoginFlow.AlreadyLoggedInException;
 import google.registry.flows.session.LoginFlow.BadRegistrarIdException;
-import google.registry.flows.session.LoginFlow.PasswordChangesNotSupportedException;
 import google.registry.flows.session.LoginFlow.RegistrarAccountNotActiveException;
 import google.registry.flows.session.LoginFlow.TooManyFailedLoginsException;
 import google.registry.flows.session.LoginFlow.UnsupportedLanguageException;
@@ -61,7 +62,7 @@ public abstract class LoginFlowTestCase extends FlowTestCase<LoginFlow> {
   // Also called in subclasses.
   void doSuccessfulTest(String xmlFilename) throws Exception {
     setEppInput(xmlFilename);
-    assertTransactionalFlow(false);
+    assertTransactionalFlow(true);
     runFlowAssertResponse(loadFile("generic_success_response.xml"));
   }
 
@@ -80,7 +81,7 @@ public abstract class LoginFlowTestCase extends FlowTestCase<LoginFlow> {
   @Test
   void testSuccess_setsIsLoginResponse() throws Exception {
     setEppInput("login_valid.xml");
-    assertTransactionalFlow(false);
+    assertTransactionalFlow(true);
     EppOutput output = runFlow();
     assertThat(output.getResponse().isLoginResponse()).isTrue();
   }
@@ -118,8 +119,52 @@ public abstract class LoginFlowTestCase extends FlowTestCase<LoginFlow> {
   }
 
   @Test
-  void testFailure_newPassword() {
+  void testSetNewPassword() throws Exception {
-    doFailingTest("login_invalid_newpw.xml", PasswordChangesNotSupportedException.class);
+    assertThat(registrar.verifyPassword("foo-BAR2")).isTrue();
+    assertThat(registrar.verifyPassword("ANewPassword")).isFalse();
+    assertThat(registrar.verifyPassword("randomstring")).isFalse();
+
+    setEppInput("login_set_new_password.xml", ImmutableMap.of("NEWPW", "ANewPassword"));
+    assertTransactionalFlow(true);
+    runFlowAssertResponse(loadFile("generic_success_response.xml"));
+
+    Registrar newRegistrar = loadRegistrar("NewRegistrar");
+    assertThat(newRegistrar.verifyPassword("foo-BAR2")).isFalse();
+    assertThat(newRegistrar.verifyPassword("ANewPassword")).isTrue();
+    assertThat(registrar.verifyPassword("randomstring")).isFalse();
+  }
+
+  @Test
+  void testFailure_invalidNewPassword_tooShort() throws Exception {
+    setEppInput("login_set_new_password.xml", ImmutableMap.of("NEWPW", "5Char"));
+    EppException thrown = assertThrows(GenericXmlSyntaxErrorException.class, this::runFlow);
+    assertAboutEppExceptions().that(thrown).marshalsToXml();
+    assertThat(thrown)
+        .hasMessageThat()
+        .contains(
+            "length = '5' is not facet-valid with respect to minLength '6' for type 'pwType'");
+  }
+
+  @Test
+  void testFailure_invalidNewPassword_tooLong() throws Exception {
+    setEppInput(
+        "login_set_new_password.xml", ImmutableMap.of("NEWPW", "ThisIsMoreThan16Characters"));
+    EppException thrown = assertThrows(GenericXmlSyntaxErrorException.class, this::runFlow);
+    assertAboutEppExceptions().that(thrown).marshalsToXml();
+    assertThat(thrown)
+        .hasMessageThat()
+        .contains(
+            "length = '26' is not facet-valid with respect to maxLength '16' for type 'pwType'");
+  }
+
+  @Test
+  void testFailure_invalidNewPassword_containsInvalidCharacter() throws Exception {
+    setEppInput("login_set_new_password.xml", ImmutableMap.of("NEWPW", "TheChar&IsNotValid"));
+    EppException thrown = assertThrows(GenericXmlSyntaxErrorException.class, this::runFlow);
+    assertAboutEppExceptions().that(thrown).marshalsToXml();
+    // Just generically assert on this error message because it's a pretty broad error owing to the
+    // overall XML simply not parsing correctly.
+    assertThat(thrown).hasMessageThat().contains("Syntax error");
   }
 
   @Test

core/src/test/resources/google/registry/flows/session/login_set_new_password.xml

@@ -3,7 +3,7 @@
     <login>
       <clID>NewRegistrar</clID>
       <pw>foo-BAR2</pw>
-      <newPW>ANewPassword</newPW>
+      <newPW>%NEWPW%</newPW>
       <options>
         <version>1.0</version>
         <lang>en</lang>

docs/flows.md

@@ -992,12 +992,12 @@ An EPP flow for login.
 
 ### Errors
 
+*   2001
+    *   Generic XML syntax error that can be thrown by any flow.
 *   2002
     *   Registrar is already logged in.
 *   2100
     *   Specified protocol version is not implemented.
-*   2102
-    *   In-band password changes are not supported.
 *   2103
     *   Specified extension is not implemented.
 *   2200