Skip to content

Commit 2ecfbf4

Browse files
maximthomasmaximthomas
authored
[#877] Return Bad Request error if CORS failed (#882)
1 parent fc328a5 commit 2ecfbf4

File tree

2 files changed

+50
-21
lines changed

2 files changed

+50
-21
lines changed

‎openam-rest/src/main/java/org/forgerock/openam/cors/CORSService.java‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,20 @@
1212
* information: "Portions copyright [year] [name of copyright owner]".
1313
*
1414
* Copyright 2014 ForgeRock AS.
15+
* Portions copyright 2024-2025 3A Systems LLC.
1516
*/
1617
package org.forgerock.openam.cors;
1718

1819
import com.sun.identity.shared.debug.Debug;
1920
import org.apache.commons.collections4.CollectionUtils;
21+
import org.forgerock.json.JsonValue;
22+
import org.forgerock.json.resource.ResourceException;
2023
import org.forgerock.openam.cors.utils.CSVHelper;
2124
import org.forgerock.util.Reject;
2225

2326
import javax.servlet.http.HttpServletRequest;
2427
import javax.servlet.http.HttpServletResponse;
28+
import java.io.IOException;
2529
import java.util.ArrayList;
2630
import java.util.Arrays;
2731
import java.util.Collections;
@@ -126,8 +130,10 @@ public CORSService(final boolean enabled, List<String> acceptedOrigins, List<Str
126130
* @param req CORS HTTP request
127131
* @param res HTTP response
128132
* @return true if the caller is to continue processing the request
133+
*
134+
* @throws IOException if an I/O error occurs while handling the response
129135
*/
130-
public boolean handleRequest(final HttpServletRequest req, final HttpServletResponse res) {
136+
public boolean handleRequest(final HttpServletRequest req, final HttpServletResponse res) throws IOException {
131137
if(!this.enabled) {
132138
return true;
133139
}
@@ -136,6 +142,7 @@ public boolean handleRequest(final HttpServletRequest req, final HttpServletResp
136142
}
137143

138144
if (!isValidCORSRequest(req)) {
145+
handleFailedCORS(res);
139146
return false;
140147
}
141148

@@ -148,6 +155,22 @@ public boolean handleRequest(final HttpServletRequest req, final HttpServletResp
148155

149156
}
150157

158+
/**
159+
* Handles a failed CORS (Cross-Origin Resource Sharing) request by generating a standardized
160+
* JSON error response and setting the appropriate HTTP status code.
161+
*
162+
* @param res the {@link HttpServletResponse} to which the error response will be written
163+
* @throws IOException if an I/O error occurs while writing the response
164+
*/
165+
private void handleFailedCORS(HttpServletResponse res) throws IOException {
166+
ResourceException resourceException = ResourceException.getException(HttpServletResponse.SC_BAD_REQUEST, "CORS error occurred");
167+
JsonValue jsonValue = resourceException.toJsonValue();
168+
res.setStatus(resourceException.getCode());
169+
res.setContentType("application/json");
170+
res.setCharacterEncoding("UTF-8");
171+
res.getWriter().write(jsonValue.toString());
172+
}
173+
151174
/**
152175
* Handles the preflight flow.
153176
*

‎openam-rest/src/test/java/org/forgerock/openam/cors/CORSServiceTest.java‎

Lines changed: 26 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,12 @@
1212
* information: "Portions copyright [year] [name of copyright owner]".
1313
*
1414
* Copyright 2014 ForgeRock AS.
15-
* Portions Copyrighted 2024 3A Systems LLC.
15+
* Portions Copyrighted 2024-2025 3A Systems LLC.
1616
*/
1717
package org.forgerock.openam.cors;
1818

19+
import java.io.IOException;
20+
import java.io.PrintWriter;
1921
import java.util.ArrayList;
2022
import javax.servlet.http.HttpServletRequest;
2123
import javax.servlet.http.HttpServletResponse;
@@ -27,6 +29,8 @@
2729
import static org.mockito.Mockito.times;
2830
import static org.mockito.Mockito.verify;
2931
import static org.mockito.Mockito.verifyZeroInteractions;
32+
import static org.mockito.Mockito.when;
33+
3034
import org.testng.annotations.BeforeMethod;
3135
import org.testng.annotations.Test;
3236

@@ -37,7 +41,7 @@ public class CORSServiceTest {
3741
private HttpServletResponse mockResponse;
3842

3943
@BeforeMethod
40-
public void setUp() {
44+
public void setUp() throws IOException {
4145
ArrayList<String> origins = new ArrayList<>();
4246
origins.add("www.google.com");
4347
ArrayList<String> methods = new ArrayList<>();
@@ -54,6 +58,8 @@ public void setUp() {
5458

5559
mockRequest = mock(HttpServletRequest.class);
5660
mockResponse = mock(HttpServletResponse.class);
61+
62+
when(mockResponse.getWriter()).thenReturn(mock(PrintWriter.class));
5763
}
5864

5965
@Test(expectedExceptions = IllegalArgumentException.class)
@@ -108,7 +114,7 @@ public void EmptyMethodsThrowIllegalArgument() {
108114
}
109115

110116
@Test
111-
public void shouldNotTouchResponseAsOriginNull() {
117+
public void shouldNotTouchResponseAsOriginNull() throws IOException {
112118
//given
113119
given(mockRequest.getHeader(CORSConstants.ORIGIN)).willReturn(null);
114120

@@ -120,44 +126,44 @@ public void shouldNotTouchResponseAsOriginNull() {
120126
}
121127

122128
@Test
123-
public void shouldNotTouchResponseAsOriginEmpty() {
129+
public void shouldReturnBadRequestAsOriginEmpty() throws IOException {
124130
//given
125131
given(mockRequest.getHeader(CORSConstants.ORIGIN)).willReturn("");
126132

127133
//when
128134
testService.handleRequest(mockRequest, mockResponse);
129135

130136
//then
131-
verifyZeroInteractions(mockResponse);
137+
verify(mockResponse, times(1)).setStatus(eq(HttpServletResponse.SC_BAD_REQUEST));
132138
}
133139

134140
@Test
135-
public void shouldNotTouchResponseAsOriginInvalid() {
141+
public void shouldReturnBadRequestAsOriginInvalid() throws IOException {
136142
//given
137143
given(mockRequest.getHeader(CORSConstants.ORIGIN)).willReturn("www.yahoo.com");
138144

139145
//when
140146
testService.handleRequest(mockRequest, mockResponse);
141147

142148
//then
143-
verifyZeroInteractions(mockResponse);
149+
verify(mockResponse, times(1)).setStatus(eq(HttpServletResponse.SC_BAD_REQUEST));
144150
}
145151

146152
@Test
147-
public void shouldNotTouchResponseAsOriginCaseInvalid() {
153+
public void shouldReturnBadRequestAsOriginCaseInvalid() throws IOException {
148154
//given
149155
given(mockRequest.getHeader(CORSConstants.ORIGIN)).willReturn("www.GOOGLE.com");
150156

151157
//when
152158
testService.handleRequest(mockRequest, mockResponse);
153159

154160
//then
155-
verifyZeroInteractions(mockResponse);
161+
verify(mockResponse, times(1)).setStatus(eq(HttpServletResponse.SC_BAD_REQUEST));
156162
}
157163

158164

159165
@Test
160-
public void shouldNotTouchResponseAsMethodInvalid() {
166+
public void shouldReturnBadRequestAsMethodInvalid() throws IOException {
161167
//given
162168
given(mockRequest.getHeader(CORSConstants.ORIGIN)).willReturn("www.google.com");
163169
given(mockRequest.getMethod()).willReturn("PUT");
@@ -166,11 +172,11 @@ public void shouldNotTouchResponseAsMethodInvalid() {
166172
testService.handleRequest(mockRequest, mockResponse);
167173

168174
//then
169-
verifyZeroInteractions(mockResponse);
175+
verify(mockResponse, times(1)).setStatus(eq(HttpServletResponse.SC_BAD_REQUEST));
170176
}
171177

172178
@Test
173-
public void shouldFollowNormalFlowApplyOriginCredsAndExpose() {
179+
public void shouldFollowNormalFlowApplyOriginCredsAndExpose() throws IOException {
174180
//given
175181
given(mockRequest.getHeader(CORSConstants.ORIGIN)).willReturn("www.google.com");
176182
given(mockRequest.getMethod()).willReturn("POST");
@@ -186,7 +192,7 @@ public void shouldFollowNormalFlowApplyOriginCredsAndExpose() {
186192
}
187193

188194
@Test
189-
public void shouldFollowNormalFlowJustApplyOrigin() {
195+
public void shouldFollowNormalFlowJustApplyOrigin() throws IOException {
190196
//given
191197
ArrayList<String> origins = new ArrayList<String>();
192198
origins.add("*");
@@ -206,7 +212,7 @@ public void shouldFollowNormalFlowJustApplyOrigin() {
206212
}
207213

208214
@Test
209-
public void shouldFollowPreflightFlow() {
215+
public void shouldFollowPreflightFlow() throws IOException {
210216
given(mockRequest.getHeader(CORSConstants.ORIGIN)).willReturn("www.google.com");
211217
given(mockRequest.getHeader(CORSConstants.AC_REQUEST_METHOD)).willReturn("POST");
212218
given(mockRequest.getMethod()).willReturn("OPTIONS");
@@ -224,7 +230,7 @@ public void shouldFollowPreflightFlow() {
224230
}
225231

226232
@Test
227-
public void shouldDoNothingIfPreflightAndNotOptions() {
233+
public void shouldDoNothingIfPreflightAndNotOptions() throws IOException {
228234
given(mockRequest.getHeader(CORSConstants.AC_REQUEST_METHOD)).willReturn("POST");
229235
given(mockRequest.getMethod()).willReturn("GET");
230236

@@ -236,7 +242,7 @@ public void shouldDoNothingIfPreflightAndNotOptions() {
236242
}
237243

238244
@Test
239-
public void shouldDoNothingIfPreflightAndNullRequestMethod() {
245+
public void shouldDoNothingIfPreflightAndNullRequestMethod() throws IOException {
240246
given(mockRequest.getHeader(CORSConstants.AC_REQUEST_METHOD)).willReturn(null);
241247
given(mockRequest.getMethod()).willReturn("GET");
242248

@@ -248,7 +254,7 @@ public void shouldDoNothingIfPreflightAndNullRequestMethod() {
248254
}
249255

250256
@Test
251-
public void shouldDoNothingIfPreflightAndEmptyRequestMethod() {
257+
public void shouldDoNothingIfPreflightAndEmptyRequestMethod() throws IOException {
252258
given(mockRequest.getHeader(CORSConstants.AC_REQUEST_METHOD)).willReturn("");
253259
given(mockRequest.getMethod()).willReturn("GET");
254260

@@ -260,7 +266,7 @@ public void shouldDoNothingIfPreflightAndEmptyRequestMethod() {
260266
}
261267

262268
@Test
263-
public void testInvalidHostnameFailsValidation() {
269+
public void testInvalidHostnameFailsValidation() throws IOException {
264270

265271
ArrayList<String> origins = new ArrayList<String>();
266272
origins.add("www.google.com");
@@ -282,12 +288,12 @@ public void testInvalidHostnameFailsValidation() {
282288
testService.handleRequest(mockRequest, mockResponse);
283289

284290
//then
285-
verifyZeroInteractions(mockResponse);
291+
verify(mockResponse, times(1)).setStatus(eq(HttpServletResponse.SC_BAD_REQUEST));
286292
}
287293

288294

289295
@Test
290-
public void testHandleNormalIncludesExposedHeadersInResponse() {
296+
public void testHandleNormalIncludesExposedHeadersInResponse() throws IOException {
291297

292298
ArrayList<String> origins = new ArrayList<String>();
293299
origins.add("www.google.com");

0 commit comments

Comments
 (0)