Decouple SessionMetadata and TransportCredentials

TransportCredentials are per-request, not per-session, and
there's no reason to carry them within SessionMetadata.

While I'm in here, get rid of "null" credentials.
-------------
Created by MOE: https://github.com/google/moe
MOE_MIGRATED_REVID=125202213

java/google/registry/flows/EppConsoleAction.java

@@ -41,7 +41,8 @@ public class EppConsoleAction implements Runnable {
   @Override
   public void run() {
     eppRequestHandler.executeEpp(
-        new HttpSessionMetadata(new GaeUserCredentials(getUserService().getCurrentUser()), session),
+        new HttpSessionMetadata(session),
+        new GaeUserCredentials(getUserService().getCurrentUser()),
         inputXmlBytes);
   }
 }

java/google/registry/flows/EppController.java

@@ -51,7 +51,10 @@ public final class EppController {
    * Read an EPP envelope from the client, find the matching flow, execute it, and return
    * the response marshalled to a byte array.
    */
-  public byte[] handleEppCommand(SessionMetadata sessionMetadata, byte[] inputXmlBytes) {
+  public byte[] handleEppCommand(
+      SessionMetadata sessionMetadata,
+      TransportCredentials credentials,
+      byte[] inputXmlBytes) {
     Trid trid = null;
     try {
       EppInput eppInput = unmarshal(EppInput.class, inputXmlBytes);
@@ -68,6 +71,7 @@ public final class EppController {
           eppInput,
           trid,
           sessionMetadata,
+          credentials,
           inputXmlBytes,
           metrics,
           clock);

java/google/registry/flows/EppRequestHandler.java

@@ -39,10 +39,13 @@ public class EppRequestHandler {
   @Inject EppRequestHandler() {}
 
   /** Handle an EPP request and write out a servlet response. */
-  public void executeEpp(SessionMetadata sessionMetadata, byte[] inputXmlBytes) {
+  public void executeEpp(
+      SessionMetadata sessionMetadata,
+      TransportCredentials credentials,
+      byte[] inputXmlBytes) {
     try {
       response.setPayload(new String(
-          eppController.handleEppCommand(sessionMetadata, inputXmlBytes), UTF_8));
+          eppController.handleEppCommand(sessionMetadata, credentials, inputXmlBytes), UTF_8));
       response.setContentType(APPLICATION_EPP_XML);
       // Note that we always return 200 (OK) even if the EppController returns an error response.
       // This is because returning an non-OK HTTP status code will cause the proxy server to

java/google/registry/flows/EppTlsAction.java

@@ -46,7 +46,7 @@ public class EppTlsAction implements Runnable {
     if (!tlsCredentials.hasSni()) {
       logger.warning("Request did not include required SNI header.");
     }
-    eppRequestHandler.executeEpp(new HttpSessionMetadata(tlsCredentials, session), inputXmlBytes);
+    eppRequestHandler.executeEpp(new HttpSessionMetadata(session), tlsCredentials, inputXmlBytes);
   }
 }
 

java/google/registry/flows/EppToolAction.java

@@ -54,6 +54,7 @@ public class EppToolAction implements Runnable {
             dryRun,
             ProtocolDefinition.getVisibleServiceExtensionUris(),
             SessionSource.TOOL),
+        new PasswordOnlyTransportCredentials(),
         xml.getBytes(UTF_8));
   }
 
@@ -61,7 +62,7 @@ public class EppToolAction implements Runnable {
   @Module
   public static final class EppToolModule {
 
-    // TODO(b/29139545): Make parameters consistent across the graph. @Parameter("dryRun") is 
+    // TODO(b/29139545): Make parameters consistent across the graph. @Parameter("dryRun") is
     // already provided elsewhere in the graph and happens to work for us but that's just luck.
 
     @Provides

java/google/registry/flows/Flow.java

@@ -42,6 +42,7 @@ public abstract class Flow {
 
   protected EppInput eppInput;
   protected SessionMetadata sessionMetadata;
+  protected TransportCredentials credentials;
   protected Trid trid;
   protected DateTime now;
   protected byte[] inputXmlBytes;
@@ -101,11 +102,13 @@ public abstract class Flow {
       EppInput eppInput,
       Trid trid,
       SessionMetadata sessionMetadata,
+      TransportCredentials credentials,
       DateTime now,
       byte[] inputXmlBytes) throws EppException {
     this.eppInput = eppInput;
     this.trid = trid;
     this.sessionMetadata = sessionMetadata;
+    this.credentials = credentials;
     this.now = now;
     this.superuser = sessionMetadata.isSuperuser();
     this.inputXmlBytes = inputXmlBytes;

java/google/registry/flows/FlowRunner.java

@@ -36,7 +36,7 @@ import org.joda.time.DateTime;
 /** Run a flow, either transactionally or not, with logging and retrying as needed. */
 public class FlowRunner {
 
-  private static final String COMMAND_LOG_FORMAT = "EPP Command" + Strings.repeat("\n\t%s", 4);
+  private static final String COMMAND_LOG_FORMAT = "EPP Command" + Strings.repeat("\n\t%s", 5);
 
   private static final FormattingLogger logger = FormattingLogger.getLoggerForCallerClass();
 
@@ -44,6 +44,7 @@ public class FlowRunner {
   private final EppInput eppInput;
   private final Trid trid;
   private final SessionMetadata sessionMetadata;
+  private final TransportCredentials credentials;
   private final byte[] inputXmlBytes;
   private final EppMetrics metrics;
   private final Clock clock;
@@ -53,13 +54,16 @@ public class FlowRunner {
       EppInput eppInput,
       Trid trid,
       SessionMetadata sessionMetadata,
+      TransportCredentials credentials,
       byte[] inputXmlBytes,
       final EppMetrics metrics,
       Clock clock) {
+    credentials.toString();
     this.flowClass = flowClass;
     this.eppInput = eppInput;
     this.trid = trid;
     this.sessionMetadata = sessionMetadata;
+    this.credentials = credentials;
     this.inputXmlBytes = inputXmlBytes;
     this.metrics = metrics;
     this.clock = clock;
@@ -72,7 +76,8 @@ public class FlowRunner {
         trid.getServerTransactionId(),
         clientId,
         sessionMetadata,
-        prettyPrint(inputXmlBytes).replaceAll("\n", "\n\t"));
+        prettyPrint(inputXmlBytes).replaceAll("\n", "\n\t"),
+        credentials);
     if (!isTransactional()) {
       if (metrics != null) {
         metrics.incrementAttempts();
@@ -124,6 +129,7 @@ public class FlowRunner {
           eppInput,
           trid,
           sessionMetadata,
+          credentials,
           now,
           inputXmlBytes);
   }

java/google/registry/flows/GaeUserCredentials.java

@@ -38,12 +38,8 @@ public class GaeUserCredentials implements TransportCredentials {
   }
 
   @Override
-  public boolean performsLoginCheck() {
-    return true;
-  }
-
-  @Override
-  public void validate(Registrar r) throws AuthenticationErrorException {
+  public void validate(Registrar registrar, String ignoredPassword)
+      throws AuthenticationErrorException {
     if (gaeUser == null) {
       throw new UserNotLoggedInException();
     }
@@ -53,7 +49,7 @@ public class GaeUserCredentials implements TransportCredentials {
     }
     // Check Registrar's contacts to see if any are associated with this gaeUserId.
     final String gaeUserId = gaeUser.getUserId();
-    for (RegistrarContact rc : r.getContacts()) {
+    for (RegistrarContact rc : registrar.getContacts()) {
       if (gaeUserId.equals(rc.getGaeUserId())) {
         return;
       }

java/google/registry/flows/HttpSessionMetadata.java

@@ -24,9 +24,8 @@ public class HttpSessionMetadata extends SessionMetadata {
   private final HttpSession session;
   private boolean isValid = true;
 
-  public HttpSessionMetadata(TransportCredentials credentials, HttpSession session) {
+  public HttpSessionMetadata(HttpSession session) {
     this.session = session;
-    setTransportCredentials(credentials);
   }
 
   @Override

java/google/registry/flows/PasswordOnlyTransportCredentials.java

@@ -0,0 +1,28 @@
+// Copyright 2016 The Domain Registry 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.
+
+package google.registry.flows;
+
+import google.registry.flows.EppException.AuthenticationErrorException;
+import google.registry.model.registrar.Registrar;
+
+/** A transport credentials that validates the registrar's EPP password and nothing else. */
+public class PasswordOnlyTransportCredentials implements TransportCredentials {
+  @Override
+  public void validate(Registrar r, String password) throws AuthenticationErrorException {
+    if (!r.testPassword(password)) {
+      throw new BadRegistrarPasswordException();
+    }
+  }
+}

java/google/registry/flows/SessionMetadata.java

@@ -46,9 +46,7 @@ public abstract class SessionMetadata {
     NONE
   }
 
-  private TransportCredentials credentials;
-
-   /** The key used for looking up the current client id on the session object. */
+  /** The key used for looking up the current client id on the session object. */
   protected static final String CLIENT_ID_KEY = "CLIENT_ID";
 
   /** The key used for looking up the superuser bit on the session object. */
@@ -91,16 +89,6 @@ public abstract class SessionMetadata {
     return clazz.cast(getProperty(key));
   }
 
-  public TransportCredentials getTransportCredentials() {
-    checkValid();
-    return credentials;
-  }
-
-  public void setTransportCredentials(TransportCredentials credentials) {
-    checkValid();
-    this.credentials = credentials;
-  }
-
   public String getClientId() {
     return getProperty(String.class, CLIENT_ID_KEY);
   }
@@ -164,7 +152,6 @@ public abstract class SessionMetadata {
         .add("failedLoginAttempts", getFailedLoginAttempts())
         .add("sessionSource", getSessionSource())
         .add("serviceExtensionUris", Joiner.on('.').join(nullToEmpty(getServiceExtensionUris())))
-        .add("transportCredentials", getTransportCredentials())
         .toString();
   }
 }

java/google/registry/flows/StatelessRequestSessionMetadata.java

@@ -68,11 +68,6 @@ public class StatelessRequestSessionMetadata extends SessionMetadata {
     throw new UnsupportedOperationException();
   }
 
-  @Override
-  public void setTransportCredentials(TransportCredentials credentials) {
-    throw new UnsupportedOperationException();
-  }
-
   @Override
   protected void setProperty(String key, Object value) {
     throw new UnsupportedOperationException();

java/google/registry/flows/TlsCredentials.java

@@ -86,20 +86,16 @@ public class TlsCredentials implements TransportCredentials {
     }
   }
 
-  @Override
-  public boolean performsLoginCheck() {
-    return false;
-  }
-
   /** Returns {@code true} if frontend passed us the requested server name. */
   boolean hasSni() {
     return !isNullOrEmpty(sni);
   }
 
   @Override
-  public void validate(Registrar registrar) throws AuthenticationErrorException {
+  public void validate(Registrar registrar, String password) throws AuthenticationErrorException {
     validateIp(registrar);
     validateCertificate(registrar);
+    validatePassword(registrar, password);
   }
 
   /**
@@ -160,6 +156,13 @@ public class TlsCredentials implements TransportCredentials {
     }
   }
 
+  private void validatePassword(Registrar registrar, String password)
+      throws BadRegistrarPasswordException {
+    if (!registrar.testPassword(password)) {
+      throw new BadRegistrarPasswordException();
+    }
+  }
+
   @Override
   public String toString() {
     return toStringHelper(getClass())

java/google/registry/flows/TransportCredentials.java

@@ -17,23 +17,21 @@ package google.registry.flows;
 import google.registry.flows.EppException.AuthenticationErrorException;
 import google.registry.model.registrar.Registrar;
 
-/**
- * A marker interface for objects containing registrar credentials provided via an EPP transport.
- */
+/** Interface for objects containing registrar credentials provided via an EPP transport. */
 public interface TransportCredentials {
-
   /**
-   * Indicates whether the transport takes the place of EPP login checks, in which case LoginFlow
-   * will not check the password. Alternatively, if the password should be checked, it MUST match
-   * the user's and GAE's isUserAdmin should not be used to bypass this check as internal
-   * connections over RPC will have this property for all registrars.
+   * Check that these credentials are valid for the registrar and optionally check the password.
+   *
+   * Called by {@link google.registry.flows.session.LoginFlow LoginFlow} to check the transport
+   * credentials against the stored registrar's credentials. If they do not match, throw an
+   * {@link AuthenticationErrorException}.
    */
-  boolean performsLoginCheck();
+  void validate(Registrar registrar, String password) throws AuthenticationErrorException;
 
-  /**
-   * Called by {@link google.registry.flows.session.LoginFlow LoginFlow}
-   * to check the transport credentials against the stored registrar's credentials.
-   * If they do not match, throw an AuthenticationErrorException.
-   */
-  void validate(Registrar r) throws AuthenticationErrorException;
+  /** Registrar password is incorrect. */
+  static class BadRegistrarPasswordException extends AuthenticationErrorException {
+    public BadRegistrarPasswordException() {
+      super("Registrar password is incorrect");
+    }
+  }
 }

java/google/registry/flows/session/LoginFlow.java

@@ -29,7 +29,6 @@ import google.registry.flows.EppException.UnimplementedExtensionException;
 import google.registry.flows.EppException.UnimplementedObjectServiceException;
 import google.registry.flows.EppException.UnimplementedOptionException;
 import google.registry.flows.Flow;
-import google.registry.flows.TransportCredentials;
 import google.registry.model.eppcommon.ProtocolDefinition;
 import google.registry.model.eppcommon.ProtocolDefinition.ServiceExtension;
 import google.registry.model.eppinput.EppInput.Login;
@@ -55,9 +54,9 @@ import java.util.Set;
  * @error {@link google.registry.flows.TlsCredentials.BadRegistrarIpAddressException}
  * @error {@link google.registry.flows.TlsCredentials.MissingRegistrarCertificateException}
  * @error {@link google.registry.flows.TlsCredentials.NoSniException}
+ * @error {@link google.registry.flows.TransportCredentials.BadRegistrarPasswordException}
  * @error {@link LoginFlow.AlreadyLoggedInException}
  * @error {@link LoginFlow.BadRegistrarClientIdException}
- * @error {@link LoginFlow.BadRegistrarPasswordException}
  * @error {@link LoginFlow.TooManyFailedLoginsException}
  * @error {@link LoginFlow.PasswordChangesNotSupportedException}
  * @error {@link LoginFlow.RegistrarAccountNotActiveException}
@@ -114,24 +113,15 @@ public class LoginFlow extends Flow {
       throw new BadRegistrarClientIdException(login.getClientId());
     }
 
-    TransportCredentials credentials = sessionMetadata.getTransportCredentials();
     // AuthenticationErrorExceptions will propagate up through here.
-    if (credentials != null) {  // Allow no-credential logins, for load-testing and RDE.
-      try {
-        credentials.validate(registrar);
-      } catch (AuthenticationErrorException e) {
-        sessionMetadata.incrementFailedLoginAttempts();
-        throw e;
-      }
-    }
-
-    final boolean requiresLoginCheck = credentials == null || !credentials.performsLoginCheck();
-    if (requiresLoginCheck && !registrar.testPassword(login.getPassword())) {
+    try {
+      credentials.validate(registrar, login.getPassword());
+    } catch (AuthenticationErrorException e) {
       sessionMetadata.incrementFailedLoginAttempts();
       if (sessionMetadata.getFailedLoginAttempts() > MAX_FAILED_LOGIN_ATTEMPTS_PER_CONNECTION) {
         throw new TooManyFailedLoginsException();
       } else {
-        throw new BadRegistrarPasswordException();
+        throw e;
       }
     }
     if (registrar.getState().equals(Registrar.State.PENDING)) {
@@ -157,13 +147,6 @@ public class LoginFlow extends Flow {
     }
   }
 
-  /** Registrar password is incorrect. */
-  static class BadRegistrarPasswordException extends AuthenticationErrorException {
-    public BadRegistrarPasswordException() {
-      super("Registrar password is incorrect");
-    }
-  }
-
   /** Registrar login failed too many times. */
   static class TooManyFailedLoginsException extends AuthenticationErrorClosingConnectionException {
     public TooManyFailedLoginsException() {

java/google/registry/tools/ValidateLoginCredentialsCommand.java

@@ -104,12 +104,11 @@ final class ValidateLoginCredentialsCommand implements RemoteApiCommand, GtechCo
             LoginFlow.class,
             unmarshal(EppInput.class, inputXmlBytes),
             Trid.create(null),
-            new HttpSessionMetadata(
-                new TlsCredentials(
-                    clientCertificateHash,
-                    Optional.of(clientIpAddress),
-                    "placeholder"),  // behave as if we have SNI on, since we're validating a cert
-                new BasicHttpSession()),
+            new HttpSessionMetadata(new BasicHttpSession()),
+            new TlsCredentials(
+                clientCertificateHash,
+                Optional.of(clientIpAddress),
+                "placeholder"),  // behave as if we have SNI on, since we're validating a cert
             inputXmlBytes,
             null,
             new SystemClock()).run()), UTF_8));

java/google/registry/ui/server/api/CheckApiAction.java

@@ -38,6 +38,7 @@ import dagger.Provides;
 import google.registry.config.RegistryEnvironment;
 import google.registry.flows.EppException;
 import google.registry.flows.FlowRunner;
+import google.registry.flows.PasswordOnlyTransportCredentials;
 import google.registry.flows.SessionMetadata.SessionSource;
 import google.registry.flows.StatelessRequestSessionMetadata;
 import google.registry.flows.domain.DomainCheckFlow;
@@ -119,6 +120,7 @@ public class CheckApiAction implements Runnable {
           unmarshal(EppInput.class, inputXmlBytes),
           Trid.create(getClass().getSimpleName()),
           sessionMetadata,
+          new PasswordOnlyTransportCredentials(),
           inputXmlBytes,
           null,
           clock)