Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 11 additions & 11 deletions docs/SPRING_BOOT_4_ADOPTION_CHECKLIST.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,24 +21,24 @@ This checklist is tailored to the current `wrongsecrets` codebase (Spring Boot `

### 1) Standardize HTTP error responses with `ProblemDetail`

- [ ] Add a global `@RestControllerAdvice` for API endpoints that returns `ProblemDetail`.
- [ ] Keep MVC HTML error handling as-is for Thymeleaf pages; only modernize JSON API errors.
- [ ] Add tests that assert RFC 9457-style payload fields (`type`, `title`, `status`, `detail`, `instance`).
- [x] Add a global `@RestControllerAdvice` for API endpoints that returns `ProblemDetail`.
- [x] Keep MVC HTML error handling as-is for Thymeleaf pages; only modernize JSON API errors.
- [x] Add tests that assert RFC 9457-style payload fields (`type`, `title`, `status`, `detail`, `instance`).

**Why now:** Reduces custom exception payload drift and improves API consistency.

### 2) Replace new `RestTemplate` usage with `RestClient`

- [ ] Stop introducing any new `RestTemplate` usage.
- [ ] Migrate existing bean in `WrongSecretsApplication` from `RestTemplate` to `RestClient.Builder`.
- [ ] Migrate call sites incrementally (start with `SlackNotificationService`).
- [ ] Add timeout and retry policy explicitly for outbound calls.
- [x] Stop introducing any new `RestTemplate` usage.
- [x] Migrate existing bean in `WrongSecretsApplication` from `RestTemplate` to `RestClient.Builder`.
- [x] Migrate call sites incrementally (start with `SlackNotificationService`).
- [x] Add timeout and retry policy explicitly for outbound calls.

**Current state:** `RestTemplate` bean and usage exist and can be migrated safely in phases.

### 3) Add/verify deprecation gate in CI

- [ ] Run compile with deprecation warnings enabled in CI (`-Xlint:deprecation`).
- [x] Run compile with deprecation warnings enabled in CI (`-Xlint:deprecation`).
- [ ] Fail build on newly introduced deprecations (can be soft-fail initially).
- [ ] Track remaining suppressions/deprecations as explicit TODOs.

Expand Down Expand Up @@ -139,8 +139,8 @@ This checklist is tailored to the current `wrongsecrets` codebase (Spring Boot `

## Definition of done for Boot 4 adoption

- [ ] No new `RestTemplate` code introduced.
- [ ] API errors are standardized on `ProblemDetail`.
- [ ] Deprecation warnings are tracked and controlled in CI.
- [x] No new `RestTemplate` code introduced.
- [x] API errors are standardized on `ProblemDetail`.
- [x] Deprecation warnings are tracked and controlled in CI.
- [ ] Observability baseline (metrics, traces, log correlation) is active in non-local profiles.
- [ ] Migration choices and rollout decisions are documented in `docs/`.
3 changes: 3 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -591,6 +591,9 @@
<configuration>
<source>25</source>
<target>25</target>
<compilerArgs>
<arg>-Xlint:deprecation</arg>
</compilerArgs>
</configuration>
</plugin>
<plugin>
Expand Down
55 changes: 55 additions & 0 deletions src/main/java/org/owasp/wrongsecrets/ApiExceptionAdvice.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
package org.owasp.wrongsecrets;

import jakarta.servlet.http.HttpServletRequest;
import java.net.URI;
import org.springframework.http.HttpStatus;
import org.springframework.http.ProblemDetail;
import org.springframework.web.bind.annotation.ExceptionHandler;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.bind.annotation.RestControllerAdvice;
import org.springframework.web.server.ResponseStatusException;

/**
* Global exception handler for REST API endpoints. Returns RFC 9457-style {@link ProblemDetail}
* responses. Scoped to {@link RestController} annotated beans only; Thymeleaf controllers are
* unaffected.
*/
@RestControllerAdvice(annotations = RestController.class)
public class ApiExceptionAdvice {

/**
* Handles {@link ResponseStatusException} thrown from REST controllers and maps it to an RFC
* 9457-compliant {@link ProblemDetail} response.
*
* @param ex the exception to handle
* @param request the current HTTP request
* @return a {@link ProblemDetail} with status, title, detail and instance populated
*/
@ExceptionHandler(ResponseStatusException.class)
public ProblemDetail handleResponseStatus(
ResponseStatusException ex, HttpServletRequest request) {
ProblemDetail pd = ProblemDetail.forStatus(ex.getStatusCode());
pd.setTitle(
ex.getReason() != null ? ex.getReason() : ex.getStatusCode().toString());
pd.setDetail(ex.getMessage());
pd.setInstance(URI.create(request.getRequestURI()));
return pd;
}

/**
* Handles unexpected exceptions thrown from REST controllers and maps them to an RFC 9457-
* compliant {@link ProblemDetail} response with HTTP 500 status.
*
* @param ex the exception to handle
* @param request the current HTTP request
* @return a {@link ProblemDetail} with status 500, title and detail populated
*/
@ExceptionHandler(Exception.class)
public ProblemDetail handleGenericException(Exception ex, HttpServletRequest request) {
ProblemDetail pd = ProblemDetail.forStatus(HttpStatus.INTERNAL_SERVER_ERROR);
pd.setTitle("Internal Server Error");
pd.setDetail(ex.getMessage());
pd.setInstance(URI.create(request.getRequestURI()));
return pd;
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package org.owasp.wrongsecrets;

import java.time.Duration;
import org.owasp.wrongsecrets.challenges.kubernetes.Vaultinjected;
import org.owasp.wrongsecrets.challenges.kubernetes.Vaultpassword;
import org.owasp.wrongsecrets.definitions.ChallengeDefinitionsConfiguration;
Expand All @@ -10,7 +11,8 @@
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Scope;
import org.springframework.context.annotation.ScopedProxyMode;
import org.springframework.web.client.RestTemplate;
import org.springframework.http.client.SimpleClientHttpRequestFactory;
import org.springframework.web.client.RestClient;

@SpringBootApplication
@EnableConfigurationProperties({Vaultpassword.class, Vaultinjected.class})
Expand All @@ -34,7 +36,10 @@ public RuntimeEnvironment runtimeEnvironment(
}

@Bean
public RestTemplate restTemplate() {
return new RestTemplate();
public RestClient restClient(RestClient.Builder builder) {
SimpleClientHttpRequestFactory factory = new SimpleClientHttpRequestFactory();
factory.setConnectTimeout(Duration.ofSeconds(5));
factory.setReadTimeout(Duration.ofSeconds(10));
return builder.requestFactory(factory).build();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,24 +5,22 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.http.HttpEntity;
import org.springframework.http.HttpHeaders;
import org.springframework.http.MediaType;
import org.springframework.stereotype.Service;
import org.springframework.web.client.RestTemplate;
import org.springframework.web.client.RestClient;

/** Service for sending Slack notifications when challenges are completed. */
@Service
public class SlackNotificationService {

private static final Logger logger = LoggerFactory.getLogger(SlackNotificationService.class);

private final RestTemplate restTemplate;
private final RestClient restClient;
private final Optional<Challenge59> challenge59;

public SlackNotificationService(
RestTemplate restTemplate, @Autowired(required = false) Challenge59 challenge59) {
this.restTemplate = restTemplate;
RestClient restClient, @Autowired(required = false) Challenge59 challenge59) {
this.restClient = restClient;
this.challenge59 = Optional.ofNullable(challenge59);
}

Expand All @@ -42,14 +40,16 @@ public void notifyChallengeCompletion(String challengeName, String userName, Str
try {
String message = buildCompletionMessage(challengeName, userName, userAgent);
SlackMessage slackMessage = new SlackMessage(message);
String webhookUrl = challenge59.get().getSlackWebhookUrl();

HttpHeaders headers = new HttpHeaders();
headers.setContentType(MediaType.APPLICATION_JSON);

HttpEntity<SlackMessage> request = new HttpEntity<>(slackMessage, headers);
restClient
.post()
.uri(webhookUrl)
.contentType(MediaType.APPLICATION_JSON)
.body(slackMessage)
.retrieve()
.toEntity(String.class);

String webhookUrl = challenge59.get().getSlackWebhookUrl();
restTemplate.postForEntity(webhookUrl, request, String.class);
logger.info(
"Successfully sent Slack notification for challenge completion: {}", challengeName);

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
package org.owasp.wrongsecrets;

import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.springframework.http.HttpStatus;
import org.springframework.http.MediaType;
import org.springframework.test.web.servlet.MockMvc;
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.server.ResponseStatusException;

/** Tests that {@link ApiExceptionAdvice} returns RFC 9457-style {@code ProblemDetail} payloads. */
class ApiExceptionAdviceTest {

private MockMvc mvc;

@BeforeEach
void setUp() {
mvc =
MockMvcBuilders.standaloneSetup(new TestRestController())
.setControllerAdvice(new ApiExceptionAdvice())
.build();
}

@Test
void shouldReturnProblemDetailWithRfc9457FieldsForResponseStatusException() throws Exception {
mvc.perform(get("/test/not-found").accept(MediaType.APPLICATION_JSON))
.andExpect(status().isNotFound())
.andExpect(jsonPath("$.status").value(404))
.andExpect(jsonPath("$.title").exists())
.andExpect(jsonPath("$.detail").exists())
.andExpect(jsonPath("$.instance").exists());
}

@Test
void shouldReturnProblemDetailWithRfc9457FieldsForGenericException() throws Exception {
mvc.perform(get("/test/error").accept(MediaType.APPLICATION_JSON))
.andExpect(status().isInternalServerError())
.andExpect(jsonPath("$.status").value(500))
.andExpect(jsonPath("$.title").value("Internal Server Error"))
.andExpect(jsonPath("$.detail").exists())
.andExpect(jsonPath("$.instance").exists());
}

@RestController
static class TestRestController {

@GetMapping("/test/not-found")
public String notFound() {
throw new ResponseStatusException(HttpStatus.NOT_FOUND, "Resource not found");
}

@GetMapping("/test/error")
public String error() {
throw new RuntimeException("Unexpected failure");
}
}
}
Loading
Loading