Skip to content

Commit 2a64c0f

Browse files
author
Nanne Baars
committed
refactor: simplify challenges when answer is fixed
When the answer is always the same we can cache it and the challenge only need to implement one method. This removes the necessity to implement methods `spoiler()` and `answerCorrect()` in every subclass.
1 parent 6cecd8d commit 2a64c0f

32 files changed

Lines changed: 207 additions & 462 deletions

CONTRIBUTING.md

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -275,64 +275,52 @@ These are the things that you have to keep in mind.
275275
import lombok.extern.slf4j.Slf4j;
276276
import org.owasp.wrongsecrets.RuntimeEnvironment;
277277
import org.owasp.wrongsecrets.ScoreCard;
278-
import org.owasp.wrongsecrets.challenges.Challenge;
278+
import org.owasp.wrongsecrets.challenges.FixedAnswerChallenge;
279279
import org.owasp.wrongsecrets.challenges.ChallengeTechnology;
280280
import org.owasp.wrongsecrets.challenges.Spoiler;
281281
import org.springframework.core.annotation.Order;
282282
import org.springframework.stereotype.Component;
283283
import java.util.List;
284284
/**
285-
* Describe what your challenge does
286-
*/
285+
* Describe what your challenge does
286+
*/
287287
@Slf4j
288288
@Component
289-
public class Challenge28 implements Challenge {
290-
private final String secret;
291-
public Challenge28() {
292-
secret = "hello world";
293-
}
289+
public class Challenge28 extends FixedAnswerChallenge {
290+
private final String secret = "hello world";
294291
295-
//return the plain text secret here
296-
@Override
297-
public Spoiler spoiler() {
298-
return new Spoiler(secret);
299-
}
300-
//here you validate if your answer matches the secret
301-
@Override
302-
public boolean answerCorrect(String answer) {
303-
return secret.equals(answer);
304-
}
292+
public String getAnswer() {
293+
return secret;
294+
}
305295
}
306296
```
307297
298+
If solving the challenge depends on the answer you can implement the interface `Challenge` directly instead of `FixedAnswerChallenge`. For example, see `Challenge36`.
299+
308300
### Step 3: Adding Test File.
309301
310302
Add the **new TestFile** in this folder `wrongsecrets/src/test/java/org/owasp/wrongsecrets/challenges/`. TestFile is required to do **unit testing.**
311303
These are the things that you have to keep in mind.
312304
313305
Make sure that this file is also of **Java** type.
314306
Here is a unit test for reference:
307+
315308
```java
316309
package org.owasp.wrongsecrets.challenges.docker;
317310
import org.assertj.core.api.Assertions;
318311
import org.junit.jupiter.api.Test;
319-
import org.junit.jupiter.api.extension.ExtendWith;
320-
import org.mockito.Mock;
321-
import org.mockito.Mockito;
322-
import org.mockito.junit.jupiter.MockitoExtension;
323-
import org.owasp.wrongsecrets.ScoreCard;
324-
@ExtendWith(MockitoExtension.class)
312+
325313
class Challenge28Test {
326-
@Mock
327-
private ScoreCard scoreCard;
314+
328315
@Test
329316
void rightAnswerShouldSolveChallenge() {
330-
var challenge = new Challenge28(scoreCard);
317+
var challenge = new Challenge28();
331318
Assertions.assertThat(challenge.solved("wrong answer")).isFalse();
332319
Assertions.assertThat(challenge.solved(challenge.spoiler().solution())).isTrue();
333320
}
334321
}
335322
```
323+
336324
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.
337325
338326
### Step 4: Adding explanations, reasons and hints.
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
package org.owasp.wrongsecrets.challenges;
2+
3+
import com.google.common.base.Supplier;
4+
import com.google.common.base.Suppliers;
5+
import java.util.Objects;
6+
7+
/**
8+
* Use this class when a challenge where the answer is fixed, meaning it will not change and does
9+
* not depend on the given answer. For example: a hardcoded key or Spring environment variable.
10+
*
11+
* <p>Why do we make this distinction? Because in the case of the fixed answer we can cache the
12+
* value. It is important to <b>NOT</b> do any reading / calculation in the constructor when using
13+
* this interface.
14+
*
15+
* <p>NOTE: If the challenge depends on a calculation you can implement {@link Challenge}
16+
*/
17+
public abstract class FixedAnswerChallenge implements Challenge {
18+
19+
private Supplier<String> cachedAnswer = Suppliers.memoize(() -> getAnswer());
20+
21+
@Override
22+
public final Spoiler spoiler() {
23+
return new Spoiler(cachedAnswer.get());
24+
}
25+
26+
@Override
27+
public final boolean answerCorrect(String answer) {
28+
return Objects.equals(cachedAnswer.get(), answer);
29+
}
30+
31+
public abstract String getAnswer();
32+
}

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

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,37 +5,31 @@
55
import java.nio.file.Files;
66
import java.nio.file.Paths;
77
import lombok.extern.slf4j.Slf4j;
8-
import org.owasp.wrongsecrets.challenges.Challenge;
9-
import org.owasp.wrongsecrets.challenges.Spoiler;
8+
import org.owasp.wrongsecrets.challenges.FixedAnswerChallenge;
109
import org.springframework.beans.factory.annotation.Value;
1110
import org.springframework.stereotype.Component;
1211

1312
/** Cloud challenge that leverages the CSI secrets driver of the cloud you are running in. */
1413
@Component
1514
@Slf4j
16-
public class Challenge10 implements Challenge {
15+
public class Challenge10 extends FixedAnswerChallenge {
1716

1817
private final String awsDefaultValue;
19-
private final String challengeAnswer;
18+
private final String filePath;
19+
private final String fileName;
2020

2121
public Challenge10(
2222
@Value("${secretmountpath}") String filePath,
2323
@Value("${default_aws_value_challenge_10}") String awsDefaultValue,
2424
@Value("${FILENAME_CHALLENGE10}") String fileName) {
2525
this.awsDefaultValue = awsDefaultValue;
26-
this.challengeAnswer = getCloudChallenge9and10Value(filePath, fileName);
26+
this.filePath = filePath;
27+
this.fileName = fileName;
2728
}
2829

29-
/** {@inheritDoc} */
3030
@Override
31-
public Spoiler spoiler() {
32-
return new Spoiler(challengeAnswer);
33-
}
34-
35-
/** {@inheritDoc} */
36-
@Override
37-
public boolean answerCorrect(String answer) {
38-
return challengeAnswer.equals(answer);
31+
public String getAnswer() {
32+
return getCloudChallenge9and10Value(filePath, fileName);
3933
}
4034

4135
@SuppressFBWarnings(

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

Lines changed: 11 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,18 @@
55
import java.nio.file.Files;
66
import java.nio.file.Paths;
77
import lombok.extern.slf4j.Slf4j;
8-
import org.owasp.wrongsecrets.challenges.Challenge;
9-
import org.owasp.wrongsecrets.challenges.Spoiler;
8+
import org.owasp.wrongsecrets.challenges.FixedAnswerChallenge;
109
import org.springframework.beans.factory.annotation.Value;
1110
import org.springframework.stereotype.Component;
1211

1312
/** Cloud challenge which focuses on Terraform and secrets. */
1413
@Component
1514
@Slf4j
16-
public class Challenge9 implements Challenge {
15+
public class Challenge9 extends FixedAnswerChallenge {
1716

1817
private final String awsDefaultValue;
19-
private final String challengeAnswer;
18+
private final String filePath;
19+
private final String fileName;
2020

2121
/**
2222
* Cloud challenge which focuses on Terraform and secrets.
@@ -31,19 +31,8 @@ public Challenge9(
3131
@Value("${default_aws_value_challenge_9}") String awsDefaultValue,
3232
@Value("${FILENAME_CHALLENGE9}") String fileName) {
3333
this.awsDefaultValue = awsDefaultValue;
34-
this.challengeAnswer = getCloudChallenge9and10Value(filePath, fileName);
35-
}
36-
37-
/** {@inheritDoc} */
38-
@Override
39-
public Spoiler spoiler() {
40-
return new Spoiler(challengeAnswer);
41-
}
42-
43-
/** {@inheritDoc} */
44-
@Override
45-
public boolean answerCorrect(String answer) {
46-
return challengeAnswer.equals(answer);
34+
this.filePath = filePath;
35+
this.fileName = fileName;
4736
}
4837

4938
@SuppressFBWarnings(
@@ -60,4 +49,9 @@ private String getCloudChallenge9and10Value(String filePath, String fileName) {
6049
return awsDefaultValue;
6150
}
6251
}
52+
53+
@Override
54+
public String getAnswer() {
55+
return getCloudChallenge9and10Value(filePath, fileName);
56+
}
6357
}

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

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,13 @@
11
package org.owasp.wrongsecrets.challenges.cloud.challenge11;
22

33
import com.google.common.base.Strings;
4-
import com.google.common.base.Supplier;
54
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
65
import java.io.IOException;
76
import java.nio.charset.StandardCharsets;
87
import java.nio.file.Files;
98
import java.nio.file.Paths;
109
import lombok.extern.slf4j.Slf4j;
11-
import org.owasp.wrongsecrets.challenges.Challenge;
12-
import org.owasp.wrongsecrets.challenges.Spoiler;
10+
import org.owasp.wrongsecrets.challenges.FixedAnswerChallenge;
1311
import org.springframework.beans.factory.annotation.Value;
1412
import org.springframework.stereotype.Component;
1513
import software.amazon.awssdk.regions.Region;
@@ -25,13 +23,12 @@
2523
/** Cloud challenge which uses IAM privilelge escalation (differentiating per cloud). */
2624
@Component
2725
@Slf4j
28-
public class Challenge11Aws implements Challenge {
26+
public class Challenge11Aws extends FixedAnswerChallenge {
2927

3028
private final String awsRoleArn;
3129
private final String awsRegion;
3230
private final String tokenFileLocation;
3331
private final String awsDefaultValue;
34-
private final Supplier<String> challengeAnswer;
3532
private final String ctfValue;
3633
private final boolean ctfEnabled;
3734

@@ -48,32 +45,24 @@ public Challenge11Aws(
4845
this.awsDefaultValue = awsDefaultValue;
4946
this.ctfValue = ctfValue;
5047
this.ctfEnabled = ctfEnabled;
51-
this.challengeAnswer = getChallenge11Value();
5248
}
5349

54-
/** {@inheritDoc} */
5550
@Override
56-
public Spoiler spoiler() {
57-
return new Spoiler(challengeAnswer.get());
51+
public String getAnswer() {
52+
return getChallenge11Value();
5853
}
5954

60-
/** {@inheritDoc} */
61-
@Override
62-
public boolean answerCorrect(String answer) {
63-
return challengeAnswer.get().equals(answer);
64-
}
65-
66-
private Supplier<String> getChallenge11Value() {
55+
private String getChallenge11Value() {
6756
if (!ctfEnabled) {
68-
return () -> getAWSChallenge11Value();
57+
return getAWSChallenge11Value();
6958
} else if (!Strings.isNullOrEmpty(ctfValue)
7059
&& !Strings.isNullOrEmpty(awsDefaultValue)
7160
&& !ctfValue.equals(awsDefaultValue)) {
72-
return () -> ctfValue;
61+
return ctfValue;
7362
}
7463

7564
log.info("CTF enabled, skipping challenge11");
76-
return () -> "please_use_supported_cloud_env";
65+
return "please_use_supported_cloud_env";
7766
}
7867

7968
@SuppressFBWarnings(
Lines changed: 13 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,17 @@
11
package org.owasp.wrongsecrets.challenges.cloud.challenge11;
22

33
import com.google.common.base.Strings;
4-
import com.google.common.base.Supplier;
5-
import com.google.common.base.Suppliers;
64
import lombok.extern.slf4j.Slf4j;
7-
import org.owasp.wrongsecrets.RuntimeEnvironment;
8-
import org.owasp.wrongsecrets.challenges.Challenge;
9-
import org.owasp.wrongsecrets.challenges.Spoiler;
5+
import org.owasp.wrongsecrets.challenges.FixedAnswerChallenge;
106
import org.springframework.beans.factory.annotation.Value;
117
import org.springframework.stereotype.Component;
128

139
/** Cloud challenge which uses IAM privilelge escalation (differentiating per cloud). */
1410
@Component
1511
@Slf4j
16-
public class Challenge11Azure implements Challenge {
12+
public class Challenge11Azure extends FixedAnswerChallenge {
1713

1814
private final String azureDefaultValue;
19-
private final Supplier<String> challengeAnswer;
2015
private final String azureVaultUri;
2116
private final String azureWrongSecret3;
2217
private final String ctfValue;
@@ -28,43 +23,35 @@ public Challenge11Azure(
2823
String azureVaultUri,
2924
@Value("${wrongsecret-3}") String azureWrongSecret3, // Exclusively auto-wired for Azure
3025
@Value("${default_aws_value_challenge_11}") String ctfValue,
31-
@Value("${ctf_enabled}") boolean ctfEnabled,
32-
RuntimeEnvironment runtimeEnvironment) {
26+
@Value("${ctf_enabled}") boolean ctfEnabled) {
27+
3328
this.azureDefaultValue = azureDefaultValue;
3429
this.azureVaultUri = azureVaultUri;
3530
this.azureWrongSecret3 = azureWrongSecret3;
3631
this.ctfValue = ctfValue;
3732
this.ctfEnabled = ctfEnabled;
38-
this.challengeAnswer = Suppliers.memoize(getChallenge11Value(runtimeEnvironment));
39-
}
40-
41-
/** {@inheritDoc} */
42-
@Override
43-
public Spoiler spoiler() {
44-
return new Spoiler(challengeAnswer.get());
45-
}
46-
47-
/** {@inheritDoc} */
48-
@Override
49-
public boolean answerCorrect(String answer) {
50-
return challengeAnswer.get().equals(answer);
5133
}
5234

53-
private Supplier<String> getChallenge11Value(RuntimeEnvironment runtimeEnvironment) {
35+
private String getChallenge11Value() {
5436
if (!ctfEnabled) {
55-
return () -> getAzureChallenge11Value();
37+
return getAzureChallenge11Value();
5638
} else if (!Strings.isNullOrEmpty(ctfValue)
5739
&& !Strings.isNullOrEmpty(azureDefaultValue)
5840
&& !ctfValue.equals(azureDefaultValue)) {
59-
return () -> ctfValue;
41+
return ctfValue;
6042
}
6143

6244
log.info("CTF enabled, skipping challenge11");
63-
return () -> "please_use_supported_cloud_env";
45+
return "please_use_supported_cloud_env";
6446
}
6547

6648
private String getAzureChallenge11Value() {
6749
log.info(String.format("Using Azure Key Vault URI: %s", azureVaultUri));
6850
return azureWrongSecret3;
6951
}
52+
53+
@Override
54+
public String getAnswer() {
55+
return getChallenge11Value();
56+
}
7057
}

0 commit comments

Comments
 (0)