Merge pull request #1125 from OWASP/nbaars/lazy-load-challenges

refactor: simplify challenges when answer is fixed

Author: commjoen

CONTRIBUTING.md

@@ -267,43 +267,36 @@ First make sure that you have an [Issue](https://github.com/OWASP/wrongsecrets/i
 
 Add the **new challenge** in this folder `wrongsecrets/src/main/java/org/owasp/wrongsecrets/challenges/`.
 These are the things that you have to keep in mind.
--   First and foremost make sure your challenge is coded in **Java**.
--   Here is an example of a possible Challenge 28:
+- First and foremost make sure your challenge is coded in **Java**.
+- Use either `FixedAnswerChallenge` as a class to extend or use the `Challenge` interface to imnplement.
+
+The `FixedAnswerChallenge` can be used for challenges that don't have a dependency on other (sub)systems. Here is an example of a possible Challenge 28:
 
 ```java
     package org.owasp.wrongsecrets.challenges.docker;
     import lombok.extern.slf4j.Slf4j;
     import org.owasp.wrongsecrets.RuntimeEnvironment;
     import org.owasp.wrongsecrets.ScoreCard;
-    import org.owasp.wrongsecrets.challenges.Challenge;
+    import org.owasp.wrongsecrets.challenges.FixedAnswerChallenge;
     import org.owasp.wrongsecrets.challenges.ChallengeTechnology;
     import org.owasp.wrongsecrets.challenges.Spoiler;
     import org.springframework.core.annotation.Order;
     import org.springframework.stereotype.Component;
     import java.util.List;
     /**
-    * Describe what your challenge does
-    */
+     * Describe what your challenge does
+     */
     @Slf4j
     @Component
-    public class Challenge28 implements Challenge {
-    private final String secret;
-    public Challenge28() {
-      secret = "hello world";
-    }
+    public class Challenge28 extends FixedAnswerChallenge {
+    private final String secret = "hello world";
 
-    //return the plain text secret here
-    @Override
-    public Spoiler spoiler() {
-      return new Spoiler(secret);
-    }
-    //here you validate if your answer matches the secret
-    @Override
-    public boolean answerCorrect(String answer) {
-      return secret.equals(answer);
+    public String getAnswer() {
+      return secret;
     }
 }
 ```
+However, if there is a dependency on external components, then you can better implement the interface `Challenge` directly instead of `FixedAnswerChallenge`. For example, see [`Challenge36`](https://github.com/OWASP/wrongsecrets/blob/master/src/main/java/org/owasp/wrongsecrets/challenges/docker/Challenge36.java), where we have to interact with external binaries.
 
 ### Step 3: Adding Test File.
 
@@ -312,27 +305,23 @@ These are the things that you have to keep in mind.
 
  Make sure that this file is also of **Java** type.
 Here is a unit test for reference:
+
 ```java
     package org.owasp.wrongsecrets.challenges.docker;
     import org.assertj.core.api.Assertions;
     import org.junit.jupiter.api.Test;
-    import org.junit.jupiter.api.extension.ExtendWith;
-    import org.mockito.Mock;
-    import org.mockito.Mockito;
-    import org.mockito.junit.jupiter.MockitoExtension;
-    import org.owasp.wrongsecrets.ScoreCard;
-    @ExtendWith(MockitoExtension.class)
+
     class Challenge28Test {
-      @Mock
-      private ScoreCard scoreCard;
+
       @Test
       void rightAnswerShouldSolveChallenge() {
-          var challenge = new Challenge28(scoreCard);
+          var challenge = new Challenge28();
           Assertions.assertThat(challenge.solved("wrong answer")).isFalse();
           Assertions.assertThat(challenge.solved(challenge.spoiler().solution())).isTrue();
       }
     }
 ```
+
 Please note that PRs for new challenges are only accepted when unit tests are added to prove that the challenge works. Normally tests should not immediately leak the actual secret, so leverage the `.spoil()` functionality of your test implementation for this.
 
 ### Step 4: Adding explanations, reasons and hints.

src/main/java/org/owasp/wrongsecrets/challenges/FixedAnswerChallenge.java

@@ -0,0 +1,32 @@
+package org.owasp.wrongsecrets.challenges;
+
+import com.google.common.base.Supplier;
+import com.google.common.base.Suppliers;
+import java.util.Objects;
+
+/**
+ * Use this class when a challenge where the answer is fixed, meaning it will not change and does
+ * not depend on the given answer. For example: a hardcoded key or Spring environment variable.
+ *
+ * <p>Why do we make this distinction? Because in the case of the fixed answer we can cache the
+ * value. It is important to <b>NOT</b> do any reading / calculation in the constructor when using
+ * this interface.
+ *
+ * <p>NOTE: If the challenge depends on a calculation you can implement {@link Challenge}
+ */
+public abstract class FixedAnswerChallenge implements Challenge {
+
+  private Supplier<String> cachedAnswer = Suppliers.memoize(() -> getAnswer());
+
+  @Override
+  public final Spoiler spoiler() {
+    return new Spoiler(cachedAnswer.get());
+  }
+
+  @Override
+  public final boolean answerCorrect(String answer) {
+    return Objects.equals(cachedAnswer.get(), answer);
+  }
+
+  public abstract String getAnswer();
+}

src/main/java/org/owasp/wrongsecrets/challenges/cloud/Challenge10.java

@@ -5,37 +5,31 @@
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import lombok.extern.slf4j.Slf4j;
-import org.owasp.wrongsecrets.challenges.Challenge;
-import org.owasp.wrongsecrets.challenges.Spoiler;
+import org.owasp.wrongsecrets.challenges.FixedAnswerChallenge;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
 
 /** Cloud challenge that leverages the CSI secrets driver of the cloud you are running in. */
 @Component
 @Slf4j
-public class Challenge10 implements Challenge {
+public class Challenge10 extends FixedAnswerChallenge {
 
   private final String awsDefaultValue;
-  private final String challengeAnswer;
+  private final String filePath;
+  private final String fileName;
 
   public Challenge10(
       @Value("${secretmountpath}") String filePath,
       @Value("${default_aws_value_challenge_10}") String awsDefaultValue,
       @Value("${FILENAME_CHALLENGE10}") String fileName) {
     this.awsDefaultValue = awsDefaultValue;
-    this.challengeAnswer = getCloudChallenge9and10Value(filePath, fileName);
+    this.filePath = filePath;
+    this.fileName = fileName;
   }
 
-  /** {@inheritDoc} */
   @Override
-  public Spoiler spoiler() {
-    return new Spoiler(challengeAnswer);
-  }
-
-  /** {@inheritDoc} */
-  @Override
-  public boolean answerCorrect(String answer) {
-    return challengeAnswer.equals(answer);
+  public String getAnswer() {
+    return getCloudChallenge9and10Value(filePath, fileName);
   }
 
   @SuppressFBWarnings(

src/main/java/org/owasp/wrongsecrets/challenges/cloud/Challenge9.java

@@ -5,18 +5,18 @@
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import lombok.extern.slf4j.Slf4j;
-import org.owasp.wrongsecrets.challenges.Challenge;
-import org.owasp.wrongsecrets.challenges.Spoiler;
+import org.owasp.wrongsecrets.challenges.FixedAnswerChallenge;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
 
 /** Cloud challenge which focuses on Terraform and secrets. */
 @Component
 @Slf4j
-public class Challenge9 implements Challenge {
+public class Challenge9 extends FixedAnswerChallenge {
 
   private final String awsDefaultValue;
-  private final String challengeAnswer;
+  private final String filePath;
+  private final String fileName;
 
   /**
    * Cloud challenge which focuses on Terraform and secrets.
@@ -31,19 +31,8 @@ public Challenge9(
       @Value("${default_aws_value_challenge_9}") String awsDefaultValue,
       @Value("${FILENAME_CHALLENGE9}") String fileName) {
     this.awsDefaultValue = awsDefaultValue;
-    this.challengeAnswer = getCloudChallenge9and10Value(filePath, fileName);
-  }
-
-  /** {@inheritDoc} */
-  @Override
-  public Spoiler spoiler() {
-    return new Spoiler(challengeAnswer);
-  }
-
-  /** {@inheritDoc} */
-  @Override
-  public boolean answerCorrect(String answer) {
-    return challengeAnswer.equals(answer);
+    this.filePath = filePath;
+    this.fileName = fileName;
   }
 
   @SuppressFBWarnings(
@@ -60,4 +49,9 @@ private String getCloudChallenge9and10Value(String filePath, String fileName) {
       return awsDefaultValue;
     }
   }
+
+  @Override
+  public String getAnswer() {
+    return getCloudChallenge9and10Value(filePath, fileName);
+  }
 }

src/main/java/org/owasp/wrongsecrets/challenges/cloud/challenge11/Challenge11Aws.java

@@ -1,15 +1,13 @@
 package org.owasp.wrongsecrets.challenges.cloud.challenge11;
 
 import com.google.common.base.Strings;
-import com.google.common.base.Supplier;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
 import java.nio.file.Files;
 import java.nio.file.Paths;
 import lombok.extern.slf4j.Slf4j;
-import org.owasp.wrongsecrets.challenges.Challenge;
-import org.owasp.wrongsecrets.challenges.Spoiler;
+import org.owasp.wrongsecrets.challenges.FixedAnswerChallenge;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
 import software.amazon.awssdk.regions.Region;
@@ -25,13 +23,12 @@
 /** Cloud challenge which uses IAM privilelge escalation (differentiating per cloud). */
 @Component
 @Slf4j
-public class Challenge11Aws implements Challenge {
+public class Challenge11Aws extends FixedAnswerChallenge {
 
   private final String awsRoleArn;
   private final String awsRegion;
   private final String tokenFileLocation;
   private final String awsDefaultValue;
-  private final Supplier<String> challengeAnswer;
   private final String ctfValue;
   private final boolean ctfEnabled;
 
@@ -48,32 +45,24 @@ public Challenge11Aws(
     this.awsDefaultValue = awsDefaultValue;
     this.ctfValue = ctfValue;
     this.ctfEnabled = ctfEnabled;
-    this.challengeAnswer = getChallenge11Value();
   }
 
-  /** {@inheritDoc} */
   @Override
-  public Spoiler spoiler() {
-    return new Spoiler(challengeAnswer.get());
+  public String getAnswer() {
+    return getChallenge11Value();
   }
 
-  /** {@inheritDoc} */
-  @Override
-  public boolean answerCorrect(String answer) {
-    return challengeAnswer.get().equals(answer);
-  }
-
-  private Supplier<String> getChallenge11Value() {
+  private String getChallenge11Value() {
     if (!ctfEnabled) {
-      return () -> getAWSChallenge11Value();
+      return getAWSChallenge11Value();
     } else if (!Strings.isNullOrEmpty(ctfValue)
         && !Strings.isNullOrEmpty(awsDefaultValue)
         && !ctfValue.equals(awsDefaultValue)) {
-      return () -> ctfValue;
+      return ctfValue;
     }
 
     log.info("CTF enabled, skipping challenge11");
-    return () -> "please_use_supported_cloud_env";
+    return "please_use_supported_cloud_env";
   }
 
   @SuppressFBWarnings(

src/main/java/org/owasp/wrongsecrets/challenges/cloud/challenge11/Challenge11Azure.java

@@ -1,22 +1,17 @@
 package org.owasp.wrongsecrets.challenges.cloud.challenge11;
 
 import com.google.common.base.Strings;
-import com.google.common.base.Supplier;
-import com.google.common.base.Suppliers;
 import lombok.extern.slf4j.Slf4j;
-import org.owasp.wrongsecrets.RuntimeEnvironment;
-import org.owasp.wrongsecrets.challenges.Challenge;
-import org.owasp.wrongsecrets.challenges.Spoiler;
+import org.owasp.wrongsecrets.challenges.FixedAnswerChallenge;
 import org.springframework.beans.factory.annotation.Value;
 import org.springframework.stereotype.Component;
 
 /** Cloud challenge which uses IAM privilelge escalation (differentiating per cloud). */
 @Component
 @Slf4j
-public class Challenge11Azure implements Challenge {
+public class Challenge11Azure extends FixedAnswerChallenge {
 
   private final String azureDefaultValue;
-  private final Supplier<String> challengeAnswer;
   private final String azureVaultUri;
   private final String azureWrongSecret3;
   private final String ctfValue;
@@ -28,43 +23,35 @@ public Challenge11Azure(
           String azureVaultUri,
       @Value("${wrongsecret-3}") String azureWrongSecret3, // Exclusively auto-wired for Azure
       @Value("${default_aws_value_challenge_11}") String ctfValue,
-      @Value("${ctf_enabled}") boolean ctfEnabled,
-      RuntimeEnvironment runtimeEnvironment) {
+      @Value("${ctf_enabled}") boolean ctfEnabled) {
+
     this.azureDefaultValue = azureDefaultValue;
     this.azureVaultUri = azureVaultUri;
     this.azureWrongSecret3 = azureWrongSecret3;
     this.ctfValue = ctfValue;
     this.ctfEnabled = ctfEnabled;
-    this.challengeAnswer = Suppliers.memoize(getChallenge11Value(runtimeEnvironment));
-  }
-
-  /** {@inheritDoc} */
-  @Override
-  public Spoiler spoiler() {
-    return new Spoiler(challengeAnswer.get());
-  }
-
-  /** {@inheritDoc} */
-  @Override
-  public boolean answerCorrect(String answer) {
-    return challengeAnswer.get().equals(answer);
   }
 
-  private Supplier<String> getChallenge11Value(RuntimeEnvironment runtimeEnvironment) {
+  private String getChallenge11Value() {
     if (!ctfEnabled) {
-      return () -> getAzureChallenge11Value();
+      return getAzureChallenge11Value();
     } else if (!Strings.isNullOrEmpty(ctfValue)
         && !Strings.isNullOrEmpty(azureDefaultValue)
         && !ctfValue.equals(azureDefaultValue)) {
-      return () -> ctfValue;
+      return ctfValue;
     }
 
     log.info("CTF enabled, skipping challenge11");
-    return () -> "please_use_supported_cloud_env";
+    return "please_use_supported_cloud_env";
   }
 
   private String getAzureChallenge11Value() {
     log.info(String.format("Using Azure Key Vault URI: %s", azureVaultUri));
     return azureWrongSecret3;
   }
+
+  @Override
+  public String getAnswer() {
+    return getChallenge11Value();
+  }
 }