Refactor StringGenerator bindings

Make every dependency request explicit on what encoding is used. Also get rid of InjectRule in XjcToDomainResourceConverterTest.

Random number generator providers are separated to secure and insecure ones. The insecure ones must be explicitly requested (usually for use cases where security is not of concern, for better speed).

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

java/google/registry/config/RegistryConfig.java

@@ -28,11 +28,15 @@ import com.google.common.net.HostAndPort;
 import dagger.Module;
 import dagger.Provides;
 import google.registry.config.RegistryConfigSettings.AppEngine.ToolsServiceUrl;
+import google.registry.util.RandomStringGenerator;
+import google.registry.util.StringGenerator;
 import google.registry.util.TaskQueueUtils;
 import java.lang.annotation.Documented;
 import java.lang.annotation.Retention;
 import java.net.URI;
 import java.net.URL;
+import java.security.NoSuchAlgorithmException;
+import java.security.ProviderException;
 import java.security.SecureRandom;
 import java.util.Optional;
 import java.util.Random;
@@ -1261,14 +1265,43 @@ public final class RegistryConfig {
     }
 
     /**
-     * Returns a singleton random number generator.
+     * Returns a singleton insecure random number generator that is fast.
      *
-     * @see google.registry.util.UrlFetchUtils
+     * <p>This binding is intentionally qualified so that any requester must explicitly acknowledge
+     * that using an insecure random number generator is fine for its use case.
      */
     @Singleton
     @Provides
-    public static Random provideRandom() {
+    @Config("insecureRandom")
-      return new SecureRandom();
+    public static Random provideInsecureRandom() {
+      return new Random();
+    };
+
+    /** Returns a singleton secure random number generator this is slow. */
+    @Singleton
+    @Provides
+    public static SecureRandom provideSecureRandom() {
+      try {
+        return SecureRandom.getInstance("NativePRNG");
+      } catch (NoSuchAlgorithmException e) {
+        throw new ProviderException(e);
+      }
+    }
+
+    /** Returns a singleton random string generator using Base58 encoding. */
+    @Singleton
+    @Provides
+    @Config("base58StringGenerator")
+    public static StringGenerator provideBase58StringGenerator(SecureRandom secureRandom) {
+      return new RandomStringGenerator(StringGenerator.Alphabets.BASE_58, secureRandom);
+    }
+
+    /** Returns a singleton random string generator using Base58 encoding. */
+    @Singleton
+    @Provides
+    @Config("base64StringGenerator")
+    public static StringGenerator provideBase64StringGenerator(SecureRandom secureRandom) {
+      return new RandomStringGenerator(StringGenerator.Alphabets.BASE_64, secureRandom);
     }
   }
 

java/google/registry/rde/imports/RdeDomainImportAction.java

@@ -54,6 +54,7 @@ import google.registry.request.Action;
 import google.registry.request.Parameter;
 import google.registry.request.Response;
 import google.registry.request.auth.Auth;
+import google.registry.util.StringGenerator;
 import google.registry.util.SystemClock;
 import google.registry.xjc.JaxbFragment;
 import google.registry.xjc.rdedomain.XjcRdeDomain;
@@ -83,6 +84,7 @@ public class RdeDomainImportAction implements Runnable {
   protected final String importBucketName;
   protected final String importFileName;
   protected final Optional<Integer> mapShards;
+  protected final StringGenerator stringGenerator;
 
   @Inject
   public RdeDomainImportAction(
@@ -90,12 +92,14 @@ public class RdeDomainImportAction implements Runnable {
       Response response,
       @Config("rdeImportBucket") String importBucketName,
       @Parameter(PATH) String importFileName,
-      @Parameter(PARAM_MAP_SHARDS) Optional<Integer> mapShards) {
+      @Parameter(PARAM_MAP_SHARDS) Optional<Integer> mapShards,
+      @Config("base64StringGenerator") StringGenerator stringGenerator) {
     this.mrRunner = mrRunner;
     this.response = response;
     this.importBucketName = importBucketName;
     this.importFileName = importFileName;
     this.mapShards = mapShards;
+    this.stringGenerator = stringGenerator;
   }
 
   @Override
@@ -122,7 +126,7 @@ public class RdeDomainImportAction implements Runnable {
    * Creates a new {@link RdeDomainImportMapper}
    */
   private RdeDomainImportMapper createMapper() {
-    return new RdeDomainImportMapper(importBucketName);
+    return new RdeDomainImportMapper(importBucketName, stringGenerator);
   }
 
   /** Mapper to import domains from an escrow file. */
@@ -132,11 +136,13 @@ public class RdeDomainImportAction implements Runnable {
     private static final long serialVersionUID = -7645091075256589374L;
 
     private final String importBucketName;
+    private final StringGenerator stringGenerator;
     private transient RdeImportUtils importUtils;
     private transient DnsQueue dnsQueue;
 
-    public RdeDomainImportMapper(String importBucketName) {
+    public RdeDomainImportMapper(String importBucketName, StringGenerator stringGenerator) {
       this.importBucketName = importBucketName;
+      this.stringGenerator = stringGenerator;
     }
 
     private RdeImportUtils getImportUtils() {
@@ -196,7 +202,7 @@ public class RdeDomainImportAction implements Runnable {
           createAutoRenewPollMessageForDomainImport(xjcDomain, historyEntry);
       DomainResource domain =
           XjcToDomainResourceConverter.convertDomain(
-              xjcDomain, autorenewBillingEvent, autorenewPollMessage);
+              xjcDomain, autorenewBillingEvent, autorenewPollMessage, stringGenerator);
       getDnsQueue().addDomainRefreshTask(domain.getFullyQualifiedDomainName());
       // Keep a list of "extra objects" that need to be saved along with the domain
       // and add to it if necessary.

java/google/registry/rde/imports/XjcToDomainResourceConverter.java

@@ -42,8 +42,6 @@ import google.registry.model.registry.Registries;
 import google.registry.model.registry.Registry;
 import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferStatus;
-import google.registry.util.NonFinalForTesting;
-import google.registry.util.RandomStringGenerator;
 import google.registry.util.StringGenerator;
 import google.registry.util.XmlToEnumMapper;
 import google.registry.xjc.domain.XjcDomainContactType;
@@ -54,27 +52,12 @@ import google.registry.xjc.rdedomain.XjcRdeDomainElement;
 import google.registry.xjc.rdedomain.XjcRdeDomainTransferDataType;
 import google.registry.xjc.rgp.XjcRgpStatusType;
 import google.registry.xjc.secdns.XjcSecdnsDsDataType;
-import java.security.NoSuchAlgorithmException;
-import java.security.ProviderException;
-import java.security.SecureRandom;
 import java.util.function.Function;
 import org.joda.time.DateTime;
 
 /** Utility class that converts an {@link XjcRdeDomainElement} into a {@link DomainResource}. */
 final class XjcToDomainResourceConverter extends XjcToEppResourceConverter {
 
-  @NonFinalForTesting
-  static StringGenerator stringGenerator =
-      new RandomStringGenerator(StringGenerator.Alphabets.BASE_64, getRandom());
-
-  static SecureRandom getRandom() {
-    try {
-      return SecureRandom.getInstance("NativePRNG");
-    } catch (NoSuchAlgorithmException e) {
-      throw new ProviderException(e);
-    }
-  }
-
   private static final XmlToEnumMapper<TransferStatus> TRANSFER_STATUS_MAPPER =
       XmlToEnumMapper.create(TransferStatus.values());
 
@@ -152,7 +135,8 @@ final class XjcToDomainResourceConverter extends XjcToEppResourceConverter {
   static DomainResource convertDomain(
       XjcRdeDomain domain,
       BillingEvent.Recurring autoRenewBillingEvent,
-      PollMessage.Autorenew autoRenewPollMessage) {
+      PollMessage.Autorenew autoRenewPollMessage,
+      StringGenerator stringGenerator) {
     GracePeriodConverter gracePeriodConverter =
         new GracePeriodConverter(domain, Key.create(autoRenewBillingEvent));
     DomainResource.Builder builder =

java/google/registry/tmch/NordnUploadAction.java

@@ -89,7 +89,7 @@ public final class NordnUploadAction implements Runnable {
 
   @Inject Clock clock;
   @Inject Retrier retrier;
-  @Inject Random random;
+  @Inject @Config("insecureRandom") Random random;
   @Inject LordnRequestInitializer lordnRequestInitializer;
   @Inject URLFetchService fetchService;
   @Inject @Config("tmchMarksdbUrl") String tmchMarksdbUrl;

java/google/registry/tools/CreateAnchorTenantCommand.java

@@ -25,6 +25,7 @@ import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
 import com.google.common.net.InternetDomainName;
 import com.google.template.soy.data.SoyMapData;
+import google.registry.config.RegistryConfig.Config;
 import google.registry.tools.soy.CreateAnchorTenantSoyInfo;
 import google.registry.util.StringGenerator;
 import javax.inject.Inject;
@@ -72,6 +73,7 @@ final class CreateAnchorTenantCommand extends MutatingEppToolCommand {
   private boolean fee;
 
   @Inject
+  @Config("base64StringGenerator")
   StringGenerator passwordGenerator;
 
   @Override

java/google/registry/tools/CreateContactCommand.java

@@ -20,6 +20,7 @@ import static com.google.common.base.Strings.isNullOrEmpty;
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
 import com.google.template.soy.data.SoyMapData;
+import google.registry.config.RegistryConfig.Config;
 import google.registry.tools.params.PhoneNumberParameter;
 import google.registry.tools.soy.ContactCreateSoyInfo;
 import google.registry.util.StringGenerator;
@@ -103,6 +104,7 @@ final class CreateContactCommand extends MutatingEppToolCommand {
   private String password;
 
   @Inject
+  @Config("base64StringGenerator")
   StringGenerator passwordGenerator;
 
   private static final int PASSWORD_LENGTH = 16;

java/google/registry/tools/CreateDomainCommand.java

@@ -23,6 +23,7 @@ import static org.joda.time.DateTimeZone.UTC;
 import com.beust.jcommander.Parameter;
 import com.beust.jcommander.Parameters;
 import com.google.template.soy.data.SoyMapData;
+import google.registry.config.RegistryConfig.Config;
 import google.registry.model.pricing.PremiumPricingEngine.DomainPrices;
 import google.registry.tools.soy.DomainCreateSoyInfo;
 import google.registry.util.StringGenerator;
@@ -46,6 +47,7 @@ final class CreateDomainCommand extends CreateOrUpdateDomainCommand
   private boolean forcePremiums;
 
   @Inject
+  @Config("base64StringGenerator")
   StringGenerator passwordGenerator;
 
   private static final int PASSWORD_LENGTH = 16;

java/google/registry/tools/GenerateAllocationTokensCommand.java

@@ -31,6 +31,7 @@ import com.google.common.base.Splitter;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.io.Files;
 import com.googlecode.objectify.Key;
+import google.registry.config.RegistryConfig.Config;
 import google.registry.model.domain.token.AllocationToken;
 import google.registry.util.NonFinalForTesting;
 import google.registry.util.Retrier;
@@ -40,7 +41,6 @@ import java.io.IOException;
 import java.util.Collection;
 import java.util.Deque;
 import javax.inject.Inject;
-import javax.inject.Named;
 
 /** Command to generate and persist {@link AllocationToken}s. */
 @Parameters(
@@ -80,7 +80,10 @@ class GenerateAllocationTokensCommand implements CommandWithRemoteApi {
       description = "Do not actually persist the tokens; defaults to false")
   boolean dryRun;
 
-  @Inject @Named("base58StringGenerator") StringGenerator stringGenerator;
+  @Inject
+  @Config("base58StringGenerator")
+  StringGenerator stringGenerator;
+
   @Inject Retrier retrier;
 
   private static final int BATCH_SIZE = 20;

java/google/registry/tools/RegistryToolComponent.java

@@ -64,7 +64,6 @@ import javax.inject.Singleton;
       KeyringModule.class,
       KmsModule.class,
       RdeModule.class,
-      RegistryToolModule.class,
       SystemClockModule.class,
       SystemSleeperModule.class,
       URLFetchServiceModule.class,

java/google/registry/tools/RegistryToolModule.java

@@ -1,67 +0,0 @@
-// Copyright 2017 The Nomulus 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.tools;
-
-import dagger.Binds;
-import dagger.Module;
-import dagger.Provides;
-import google.registry.util.RandomStringGenerator;
-import google.registry.util.StringGenerator;
-import google.registry.util.StringGenerator.Alphabets;
-import java.security.NoSuchAlgorithmException;
-import java.security.ProviderException;
-import java.security.SecureRandom;
-import javax.inject.Named;
-
-/** Dagger module for Registry Tool. */
-@Module
-abstract class RegistryToolModule {
-
-  @Provides
-  static RegistryToolEnvironment provideRegistryToolEnvironment() {
-    return RegistryToolEnvironment.get();
-  }
-
-  @Binds
-  abstract StringGenerator provideStringGenerator(RandomStringGenerator stringGenerator);
-
-  @Provides
-  static SecureRandom provideSecureRandom() {
-    try {
-      return SecureRandom.getInstance("NativePRNG");
-    } catch (NoSuchAlgorithmException e) {
-      throw new ProviderException(e);
-    }
-  }
-
-  @Provides
-  @Named("alphabetBase64")
-  static String provideAlphabetBase64() {
-    return Alphabets.BASE_64;
-  }
-
-  @Provides
-  @Named("alphabetBase58")
-  static String provideAlphabetBase58() {
-    return Alphabets.BASE_58;
-  }
-
-  @Provides
-  @Named("base58StringGenerator")
-  static StringGenerator provideBase58StringGenerator(
-      @Named("alphabetBase58") String alphabet, SecureRandom random) {
-    return new RandomStringGenerator(alphabet, random);
-  }
-}

java/google/registry/tools/SetupOteCommand.java

@@ -27,6 +27,7 @@ import com.google.common.base.Throwables;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableSortedMap;
 import com.google.re2j.Pattern;
+import google.registry.config.RegistryConfig.Config;
 import google.registry.config.RegistryEnvironment;
 import google.registry.model.common.GaeUserIdConverter;
 import google.registry.model.registrar.Registrar;
@@ -139,7 +140,9 @@ final class SetupOteCommand extends ConfirmingCommand implements CommandWithRemo
   )
   private boolean eapOnly = false;
 
-  @Inject StringGenerator passwordGenerator;
+  @Inject
+  @Config("base64StringGenerator")
+  StringGenerator passwordGenerator;
 
   /**
    * Long registrar names are truncated and then have an incrementing digit appended at the end so

java/google/registry/util/RandomStringGenerator.java

@@ -17,16 +17,13 @@ package google.registry.util;
 import static com.google.common.base.Preconditions.checkArgument;
 
 import java.security.SecureRandom;
-import javax.inject.Inject;
-import javax.inject.Named;
 
 /** Random string generator. */
 public class RandomStringGenerator extends StringGenerator {
 
   private final SecureRandom random;
 
-  @Inject
+  public RandomStringGenerator(String alphabet, SecureRandom random) {
-  public RandomStringGenerator(@Named("alphabetBase64") String alphabet, SecureRandom random) {
     super(alphabet);
     this.random = random;
   }

java/google/registry/util/StringGenerator.java

@@ -18,10 +18,11 @@ import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Strings.isNullOrEmpty;
 
 import com.google.common.collect.ImmutableList;
+import java.io.Serializable;
 import java.util.Collection;
 
 /** String generator. */
-public abstract class StringGenerator {
+public abstract class StringGenerator implements Serializable {
 
   public static final int DEFAULT_PASSWORD_LENGTH = 16;
 

javatests/google/registry/rde/imports/RdeDomainImportActionTest.java

@@ -50,9 +50,12 @@ import google.registry.model.transfer.TransferStatus;
 import google.registry.request.Response;
 import google.registry.testing.FakeResponse;
 import google.registry.testing.mapreduce.MapreduceTestCase;
+import google.registry.util.RandomStringGenerator;
+import google.registry.util.StringGenerator;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.security.SecureRandom;
 import java.util.List;
 import java.util.Optional;
 import javax.annotation.Nullable;
@@ -91,12 +94,14 @@ public class RdeDomainImportActionTest extends MapreduceTestCase<RdeDomainImport
     persistActiveContact("sh8013");
     response = new FakeResponse();
     mrRunner = makeDefaultRunner();
-    action = new RdeDomainImportAction(
+    action =
+        new RdeDomainImportAction(
             mrRunner,
             response,
             IMPORT_BUCKET_NAME,
             IMPORT_FILE_NAME,
-        Optional.of(3));
+            Optional.of(3),
+            new RandomStringGenerator(StringGenerator.Alphabets.BASE_64, new SecureRandom()));
   }
 
   @Test

javatests/google/registry/rde/imports/XjcToDomainResourceConverterTest.java

@@ -51,7 +51,6 @@ import google.registry.model.transfer.TransferData;
 import google.registry.model.transfer.TransferStatus;
 import google.registry.testing.AppEngineRule;
 import google.registry.testing.DeterministicStringGenerator;
-import google.registry.testing.InjectRule;
 import google.registry.util.StringGenerator;
 import google.registry.xjc.rdedomain.XjcRdeDomain;
 import google.registry.xjc.rdedomain.XjcRdeDomainElement;
@@ -90,17 +89,16 @@ public class XjcToDomainResourceConverterTest {
       "google.registry.xjc.smd"));
 
   @Rule public final AppEngineRule appEngine = AppEngineRule.builder().withDatastore().build();
-  @Rule public final InjectRule inject = new InjectRule();
 
   private Unmarshaller unmarshaller;
-  private DeterministicStringGenerator stringGenerator;
+
+  private final DeterministicStringGenerator stringGenerator =
+      new DeterministicStringGenerator(StringGenerator.Alphabets.BASE_64);
 
   @Before
   public void before() throws Exception {
     createTld("example");
     unmarshaller = JAXBContext.newInstance(JAXB_CONTEXT_PACKAGES).createUnmarshaller();
-    stringGenerator = new DeterministicStringGenerator(StringGenerator.Alphabets.BASE_64);
-    inject.setStaticField(XjcToDomainResourceConverter.class, "stringGenerator", stringGenerator);
   }
 
   @Test
@@ -415,7 +413,7 @@ public class XjcToDomainResourceConverterTest {
     // without that there's no way to actually test the capping of the projected registration here.
   }
 
-  private static DomainResource convertDomainInTransaction(final XjcRdeDomain xjcDomain) {
+  private DomainResource convertDomainInTransaction(final XjcRdeDomain xjcDomain) {
     return ofy()
         .transact(
             () -> {
@@ -426,7 +424,7 @@ public class XjcToDomainResourceConverterTest {
                   createAutoRenewPollMessageForDomainImport(xjcDomain, historyEntry);
               ofy().save().entities(historyEntry, autorenewBillingEvent, autorenewPollMessage);
               return XjcToDomainResourceConverter.convertDomain(
-                  xjcDomain, autorenewBillingEvent, autorenewPollMessage);
+                  xjcDomain, autorenewBillingEvent, autorenewPollMessage, stringGenerator);
             });
   }