Skip to content
Merged
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
54 changes: 54 additions & 0 deletions src/main/java/org/owasp/wrongsecrets/ApiExceptionAdvice.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
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