Skip to content

Commit 630fa6a

Browse files
MarkADomMarkADom
authored
Merge pull request #11 from MarkADom/fix/hardening
fix: hardening - remove exposed error messages, externalize timeout, …
2 parents 7ba3989 + bda9e01 commit 630fa6a

3 files changed

Lines changed: 214 additions & 2 deletions

File tree

‎src/main/java/com/synchlabs/geolocateapi/config/AppConfig.java‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import org.apache.hc.client5.http.impl.classic.HttpClientBuilder;
55
import org.apache.hc.client5.http.impl.classic.CloseableHttpClient;
66
import org.apache.hc.core5.util.Timeout;
7+
import org.springframework.beans.factory.annotation.Value;
78
import org.springframework.context.annotation.Bean;
89
import org.springframework.context.annotation.Configuration;
910
import org.springframework.http.client.HttpComponentsClientHttpRequestFactory;
@@ -22,12 +23,15 @@
2223
@Configuration
2324
public class AppConfig {
2425

26+
@Value("${external.timeout-ms:3000}")
27+
private int timeoutMs;
28+
2529
@Bean
2630
public RestTemplate restTemplate() {
2731

2832
RequestConfig requestConfig = RequestConfig.custom()
29-
.setConnectionRequestTimeout(Timeout.ofMilliseconds(3000))
30-
.setResponseTimeout(Timeout.ofMilliseconds(3000))
33+
.setConnectionRequestTimeout(Timeout.ofMilliseconds(timeoutMs))
34+
.setResponseTimeout(Timeout.ofMilliseconds(timeoutMs))
3135
.build();
3236

3337
CloseableHttpClient httpClient = HttpClientBuilder.create()

‎src/test/java/com/synchlabs/geolocateapi/application/exception/GlobalExceptionHandlerTest.java‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,100 @@
1+
package com.synchlabs.geolocateapi.application.exception;
2+
3+
import com.synchlabs.geolocateapi.application.port.in.GeoQueryUseCase;
4+
import com.synchlabs.geolocateapi.presentation.controller.GeoController;
5+
import com.synchlabs.geolocateapi.presentation.ratelimit.RateLimitService;
6+
import com.synchlabs.geolocateapi.presentation.ratelimit.RateLimitTier;
7+
import io.github.bucket4j.Bandwidth;
8+
import io.github.bucket4j.Bucket;
9+
import org.junit.jupiter.api.BeforeEach;
10+
import org.junit.jupiter.api.Test;
11+
import org.springframework.beans.factory.annotation.Autowired;
12+
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
13+
import org.springframework.boot.test.mock.mockito.MockBean;
14+
import org.springframework.test.web.servlet.MockMvc;
15+
16+
import java.time.Duration;
17+
18+
import static org.hamcrest.Matchers.containsString;
19+
import static org.hamcrest.Matchers.not;
20+
import static org.mockito.ArgumentMatchers.any;
21+
import static org.mockito.ArgumentMatchers.eq;
22+
import static org.mockito.Mockito.when;
23+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
24+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
25+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
26+
27+
/**
28+
* Tests for {@link GlobalExceptionHandler}.
29+
*
30+
* Verifies that:
31+
* - Unexpected exceptions never expose internal details in the response body
32+
* - 500 responses always return a safe, generic message
33+
* - 502 responses return the provider error message
34+
*
35+
* Type: Controller slice test (MockMvc).
36+
*/
37+
@WebMvcTest(controllers = GeoController.class)
38+
class GlobalExceptionHandlerTest {
39+
40+
@Autowired
41+
private MockMvc mockMvc;
42+
43+
@MockBean
44+
private GeoQueryUseCase service;
45+
46+
@MockBean
47+
private RateLimitService rateLimitService;
48+
49+
@BeforeEach
50+
void setUp() {
51+
when(rateLimitService.resolveClientId(any())).thenReturn("test-client");
52+
when(rateLimitService.resolveBucket(eq("test-client"), any(RateLimitTier.class)))
53+
.thenAnswer(inv -> fullBucket(((RateLimitTier) inv.getArgument(1)).getCapacity()));
54+
}
55+
56+
@Test
57+
void shouldReturn500ForUnexpectedException() throws Exception {
58+
when(service.findByIp("1.1.1.1")).thenThrow(new RuntimeException("secret internal detail"));
59+
60+
mockMvc.perform(get("/api/v1/geo/ip/1.1.1.1"))
61+
.andExpect(status().isInternalServerError());
62+
}
63+
64+
@Test
65+
void shouldNotExposeInternalMessageIn500Response() throws Exception {
66+
when(service.findByIp("1.1.1.1")).thenThrow(new RuntimeException("secret internal detail"));
67+
68+
mockMvc.perform(get("/api/v1/geo/ip/1.1.1.1"))
69+
.andExpect(status().isInternalServerError())
70+
.andExpect(jsonPath("$.message", not(containsString("secret internal detail"))));
71+
}
72+
73+
@Test
74+
void shouldReturnSafeMessageIn500Response() throws Exception {
75+
when(service.findByIp("1.1.1.1")).thenThrow(new RuntimeException("secret internal detail"));
76+
77+
mockMvc.perform(get("/api/v1/geo/ip/1.1.1.1"))
78+
.andExpect(status().isInternalServerError())
79+
.andExpect(jsonPath("$.message").value("An unexpected error occurred. Please try again later."));
80+
}
81+
82+
@Test
83+
void shouldReturn502ForExternalServiceException() throws Exception {
84+
when(service.findByIp("8.8.8.8")).thenThrow(new ExternalServiceException("All providers failed for IP: 8.8.8.8"));
85+
86+
mockMvc.perform(get("/api/v1/geo/ip/8.8.8.8"))
87+
.andExpect(status().isBadGateway())
88+
.andExpect(jsonPath("$.error").value("Bad Gateway"))
89+
.andExpect(jsonPath("$.message").value("All providers failed for IP: 8.8.8.8"));
90+
}
91+
92+
private Bucket fullBucket(long capacity) {
93+
return Bucket.builder()
94+
.addLimit(Bandwidth.builder()
95+
.capacity(capacity)
96+
.refillGreedy(capacity, Duration.ofMinutes(1))
97+
.build())
98+
.build();
99+
}
100+
}

‎src/test/java/com/synchlabs/geolocateapi/presentation/controller/GeoControllerValidationTest.java‎

Lines changed: 108 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,108 @@
1+
package com.synchlabs.geolocateapi.presentation.controller;
2+
3+
import com.synchlabs.geolocateapi.application.port.in.GeoQueryUseCase;
4+
import com.synchlabs.geolocateapi.presentation.ratelimit.RateLimitService;
5+
import com.synchlabs.geolocateapi.presentation.ratelimit.RateLimitTier;
6+
import io.github.bucket4j.Bandwidth;
7+
import io.github.bucket4j.Bucket;
8+
import org.junit.jupiter.api.BeforeEach;
9+
import org.junit.jupiter.api.Test;
10+
import org.springframework.beans.factory.annotation.Autowired;
11+
import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest;
12+
import org.springframework.boot.test.mock.mockito.MockBean;
13+
import org.springframework.test.web.servlet.MockMvc;
14+
15+
import java.time.Duration;
16+
17+
import static org.mockito.ArgumentMatchers.any;
18+
import static org.mockito.ArgumentMatchers.eq;
19+
import static org.mockito.Mockito.when;
20+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
21+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
22+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
23+
24+
/**
25+
* Input validation tests for {@link GeoController}.
26+
*
27+
* Verifies that constraint violations on path variables and query parameters
28+
* are rejected with HTTP 400 before reaching the application layer.
29+
*
30+
* Type: Controller slice test (MockMvc).
31+
*/
32+
@WebMvcTest(controllers = GeoController.class)
33+
class GeoControllerValidationTest {
34+
35+
@Autowired
36+
private MockMvc mockMvc;
37+
38+
@MockBean
39+
private GeoQueryUseCase service;
40+
41+
@MockBean
42+
private RateLimitService rateLimitService;
43+
44+
@BeforeEach
45+
void setUp() {
46+
when(rateLimitService.resolveClientId(any())).thenReturn("test-client");
47+
when(rateLimitService.resolveBucket(eq("test-client"), any(RateLimitTier.class)))
48+
.thenAnswer(inv -> fullBucket(((RateLimitTier) inv.getArgument(1)).getCapacity()));
49+
}
50+
51+
// ========= COORDINATES VALIDATION =========
52+
53+
@Test
54+
void shouldReturn400WhenLatitudeExceedsMaximum() throws Exception {
55+
mockMvc.perform(get("/api/v1/geo/coordinates?lat=999&lon=0"))
56+
.andExpect(status().isBadRequest())
57+
.andExpect(jsonPath("$.message").exists());
58+
}
59+
60+
@Test
61+
void shouldReturn400WhenLatitudeBelowMinimum() throws Exception {
62+
mockMvc.perform(get("/api/v1/geo/coordinates?lat=-999&lon=0"))
63+
.andExpect(status().isBadRequest())
64+
.andExpect(jsonPath("$.message").exists());
65+
}
66+
67+
@Test
68+
void shouldReturn400WhenLongitudeExceedsMaximum() throws Exception {
69+
mockMvc.perform(get("/api/v1/geo/coordinates?lat=0&lon=999"))
70+
.andExpect(status().isBadRequest())
71+
.andExpect(jsonPath("$.message").exists());
72+
}
73+
74+
@Test
75+
void shouldReturn400WhenLongitudeBelowMinimum() throws Exception {
76+
mockMvc.perform(get("/api/v1/geo/coordinates?lat=0&lon=-999"))
77+
.andExpect(status().isBadRequest())
78+
.andExpect(jsonPath("$.message").exists());
79+
}
80+
81+
// ========= CITY VALIDATION =========
82+
83+
@Test
84+
void shouldReturn400WhenCityIsBlank() throws Exception {
85+
// use template form so MockMvc encodes the space correctly to %20
86+
mockMvc.perform(get("/api/v1/geo/city/{city}", " "))
87+
.andExpect(status().isBadRequest())
88+
.andExpect(jsonPath("$.message").exists());
89+
}
90+
91+
@Test
92+
void shouldReturn400WhenCityExceedsMaxLength() throws Exception {
93+
var longCity = "A".repeat(101);
94+
95+
mockMvc.perform(get("/api/v1/geo/city/" + longCity))
96+
.andExpect(status().isBadRequest())
97+
.andExpect(jsonPath("$.message").exists());
98+
}
99+
100+
private Bucket fullBucket(long capacity) {
101+
return Bucket.builder()
102+
.addLimit(Bandwidth.builder()
103+
.capacity(capacity)
104+
.refillGreedy(capacity, Duration.ofMinutes(1))
105+
.build())
106+
.build();
107+
}
108+
}

0 commit comments

Comments
 (0)