deprecate engine-jwt-enabled, add engine-jwt-disabled. This is to pre…

…vent unintentional "flipping" of this boolean for configs using the flag without a boolean value (#3913) Signed-off-by: garyschulte <garyschulte@gmail.com>
hyperledger · May 27, 2022 · 4230f7c · 4230f7c
1 parent 8472797
commit 4230f7c
Show file tree

Hide file tree

Showing 6 changed files with 46 additions and 5 deletions.
diff --git a/...ance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java b/...ance-tests/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/BesuNode.java
@@ -234,6 +234,12 @@ public boolean isEngineRpcEnabled() {
     return engineRpcConfiguration.isPresent() && engineRpcConfiguration.get().isEnabled();
   }
 
+  public boolean isEngineAuthDisabled() {
+    return engineRpcConfiguration
+        .map(engineConf -> !engineConf.isAuthenticationEnabled())
+        .orElse(false);
+  }
+
   private boolean isWebSocketsRpcEnabled() {
     return webSocketConfiguration().isEnabled();
   }

diff --git a/...l/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java b/...l/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java
@@ -193,6 +193,10 @@ public void startNode(final BesuNode node) {
     if (node.isEngineRpcEnabled()) {
       params.add("--engine-rpc-port");
       params.add(node.jsonEngineListenPort().get().toString());
+
+      if (node.isEngineAuthDisabled()) {
+        params.add("--engine-jwt-disabled");
+      }
     }
 
     if (node.wsRpcEnabled()) {

diff --git a/...yperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java b/...yperledger/besu/tests/acceptance/dsl/node/configuration/BesuNodeConfigurationBuilder.java
@@ -147,6 +147,7 @@ public BesuNodeConfigurationBuilder engineRpcEnabled(final boolean enabled) {
     this.engineRpcConfiguration.setEnabled(enabled);
     this.engineRpcConfiguration.setPort(0);
     this.engineRpcConfiguration.setHostsAllowlist(singletonList("*"));
+    this.engineRpcConfiguration.setAuthenticationEnabled(false);
 
     return this;
   }

diff --git a/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java b/besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java
@@ -588,8 +588,15 @@ static class EngineRPCOptionGroup {
 
     @Option(
         names = {"--engine-jwt-enabled"},
-        description = "Require authentication for Engine APIs (default: ${DEFAULT-VALUE})")
-    private final Boolean isEngineAuthEnabled = false;
+        description = "deprecated option, engine jwt auth is enabled by default",
+        hidden = true)
+    @SuppressWarnings({"FieldCanBeFinal", "UnusedVariable"})
+    private final Boolean deprecatedIsEngineAuthEnabled = true;
+
+    @Option(
+        names = {"--engine-jwt-disabled"},
+        description = "Disable authentication for Engine APIs (default: ${DEFAULT-VALUE})")
+    private final Boolean isEngineAuthDisabled = false;
 
     @Option(
         names = {"--engine-host-allowlist"},
@@ -2118,7 +2125,7 @@ private JsonRpcConfiguration createEngineJsonRpcConfiguration(
               + "Merge support is implicitly enabled by the presence of terminalTotalDifficulty in the genesis config.");
     }
     engineConfig.setEnabled(isMergeEnabled());
-    if (engineRPCOptionGroup.isEngineAuthEnabled) {
+    if (!engineRPCOptionGroup.isEngineAuthDisabled) {
       engineConfig.setAuthenticationEnabled(true);
       engineConfig.setAuthenticationAlgorithm(JwtAlgorithm.HS256);
       if (Objects.nonNull(engineRPCOptionGroup.engineJwtKeyFile)

diff --git a/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java b/besu/src/test/java/org/hyperledger/besu/cli/BesuCommandTest.java
@@ -1992,14 +1992,37 @@ public void rpcApiNoAuthMethodsIgnoresDuplicatesAndMustBeUsed() {
 
   @Test
   public void engineApiAuthOptions() {
+    // TODO: once we have mainnet TTD, we can remove the TTD override parameter here
+    // https://github.com/hyperledger/besu/issues/3874
     parseCommand(
-        "--rpc-http-enabled", "--engine-jwt-enabled", "--engine-jwt-secret", "/tmp/fakeKey.hex");
+        "--override-genesis-config",
+        "terminalTotalDifficulty=1337",
+        "--rpc-http-enabled",
+        "--engine-jwt-secret",
+        "/tmp/fakeKey.hex");
     verify(mockRunnerBuilder).engineJsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture());
     assertThat(jsonRpcConfigArgumentCaptor.getValue().isAuthenticationEnabled()).isTrue();
     assertThat(commandOutput.toString(UTF_8)).isEmpty();
     assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
   }
 
+  @Test
+  public void engineApiDisableAuthOptions() {
+    // TODO: once we have mainnet TTD, we can remove the TTD override parameter here
+    // https://github.com/hyperledger/besu/issues/3874
+    parseCommand(
+        "--override-genesis-config",
+        "terminalTotalDifficulty=1337",
+        "--rpc-http-enabled",
+        "--engine-jwt-disabled",
+        "--engine-jwt-secret",
+        "/tmp/fakeKey.hex");
+    verify(mockRunnerBuilder).engineJsonRpcConfiguration(jsonRpcConfigArgumentCaptor.capture());
+    assertThat(jsonRpcConfigArgumentCaptor.getValue().isAuthenticationEnabled()).isFalse();
+    assertThat(commandOutput.toString(UTF_8)).isEmpty();
+    assertThat(commandErrorOutput.toString(UTF_8)).isEmpty();
+  }
+
   @Test
   public void rpcHttpNoAuthApiMethodsCannotBeInvalid() {
     parseCommand("--rpc-http-enabled", "--rpc-http-api-method-no-auth", "invalid");

diff --git a/besu/src/test/resources/everything_config.toml b/besu/src/test/resources/everything_config.toml
@@ -44,7 +44,7 @@ random-peer-priority-enabled=false
 host-whitelist=["all"]
 host-allowlist=["all"]
 engine-host-allowlist=["all"]
-engine-jwt-enabled=false
+engine-jwt-disabled=true
 engine-jwt-secret="/tmp/jwt.hex"
 required-blocks=["8675309=123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"]
 discovery-dns-url="enrtree://AM5FCQLWIZX2QFPNJAP7VUERCCRNGRHWZG3YYHIUV7BVDQ5FDPRT2@nodes.example.org"