Skip to content

Commit 85f405a

Browse files
jungmjungm
committed
TOMEE-4591 - OpenIdAuthenticationMechanism redirectToOriginalResource don't treat case without stored request as an error
1 parent 95e5c29 commit 85f405a

2 files changed

Lines changed: 158 additions & 6 deletions

File tree

‎tomee/tomee-security/src/main/java/org/apache/tomee/security/cdi/OpenIdAuthenticationMechanism.java‎

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -243,11 +243,10 @@ protected AuthenticationStatus performAuthentication(HttpServletRequest request,
243243
}
244244

245245
if (definition.redirectToOriginalResource() && !matchesOriginalRequest) {
246-
if (originalRequest == null) {
247-
throw new IllegalStateException("redirectToOriginalResource=true is configured but no original request has been stored before");
246+
AuthenticationStatus redirectStatus = redirectToStoredOriginalRequest(request, messageContext, originalRequest);
247+
if (redirectStatus != null) {
248+
return redirectStatus;
248249
}
249-
250-
return messageContext.redirect(appendQueryString(originalRequest, request.getQueryString()));
251250
}
252251

253252
// Callback is okay, continue with (4)
@@ -269,8 +268,7 @@ protected AuthenticationStatus performAuthentication(HttpServletRequest request,
269268

270269
// We're finished, restore original request now and clean up
271270
if (definition.redirectToOriginalResource()) {
272-
String originalRequestJson = storageHandler.get(request, response, OpenIdStorageHandler.REQUEST_KEY);
273-
messageContext.withRequest(SavedRequest.fromJson(originalRequestJson).mask(request));
271+
restoreOriginalRequest(request, response, messageContext);
274272
}
275273

276274
storageHandler.delete(request, response, OpenIdStorageHandler.NONCE_KEY);
@@ -284,6 +282,28 @@ protected AuthenticationStatus performAuthentication(HttpServletRequest request,
284282
return null;
285283
}
286284

285+
protected AuthenticationStatus redirectToStoredOriginalRequest(HttpServletRequest request, HttpMessageContext messageContext,
286+
String originalRequest) {
287+
if (originalRequest == null) {
288+
LOGGER.warning("redirectToOriginalResource=true is configured but no original request has been stored before; continuing without redirecting to the original resource");
289+
return null;
290+
}
291+
292+
return messageContext.redirect(appendQueryString(originalRequest, request.getQueryString()));
293+
}
294+
295+
protected void restoreOriginalRequest(HttpServletRequest request, HttpServletResponse response, HttpMessageContext messageContext) {
296+
String originalRequestJson = storageHandler.get(request, response, OpenIdStorageHandler.REQUEST_KEY);
297+
if (originalRequestJson == null) {
298+
return;
299+
}
300+
301+
SavedRequest savedRequest = SavedRequest.fromJson(originalRequestJson);
302+
if (savedRequest != null) {
303+
messageContext.withRequest(savedRequest.mask(request));
304+
}
305+
}
306+
287307
protected AuthenticationStatus handleTokenResponse(TokenResponse tokenResponse, HttpMessageContext httpMessageContext) {
288308
openIdContext.setExpiresIn(tokenResponse.getExpiresIn());
289309
openIdContext.setTokenType(tokenResponse.getTokenType());

‎tomee/tomee-security/src/test/java/org/apache/tomee/security/cdi/OpenIdAuthenticationMechanismUnitTest.java‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more
3+
* contributor license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright ownership.
5+
* The ASF licenses this file to You under the Apache License, Version 2.0
6+
* (the "License"); you may not use this file except in compliance with
7+
* the License. You may obtain a copy of the License at
8+
*
9+
* http://www.apache.org/licenses/LICENSE-2.0
10+
*
11+
* Unless required by applicable law or agreed to in writing, software
12+
* distributed under the License is distributed on an "AS IS" BASIS,
13+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
14+
* See the License for the specific language governing permissions and
15+
* limitations under the License.
16+
*/
17+
package org.apache.tomee.security.cdi;
18+
19+
import com.sun.net.httpserver.HttpServer;
20+
import org.apache.openejb.util.reflection.Reflections;
21+
import org.apache.tomee.security.cdi.openid.TomEEOpenIdContext;
22+
import org.apache.tomee.security.cdi.openid.storage.OpenIdStorageHandler;
23+
import org.junit.Before;
24+
import org.junit.Test;
25+
import org.mockito.Answers;
26+
27+
import jakarta.security.enterprise.AuthenticationStatus;
28+
import jakarta.security.enterprise.authentication.mechanism.http.HttpMessageContext;
29+
import jakarta.security.enterprise.authentication.mechanism.http.OpenIdAuthenticationMechanismDefinition;
30+
import jakarta.security.enterprise.authentication.mechanism.http.openid.OpenIdConstant;
31+
import jakarta.security.enterprise.identitystore.CredentialValidationResult;
32+
import jakarta.security.enterprise.identitystore.IdentityStoreHandler;
33+
import jakarta.servlet.http.HttpServletRequest;
34+
import jakarta.servlet.http.HttpServletResponse;
35+
36+
import java.net.InetSocketAddress;
37+
import java.nio.charset.StandardCharsets;
38+
import java.util.HashMap;
39+
import java.util.Map;
40+
import java.util.Set;
41+
42+
import static org.junit.Assert.assertEquals;
43+
import static org.mockito.ArgumentMatchers.any;
44+
import static org.mockito.Mockito.mock;
45+
import static org.mockito.Mockito.never;
46+
import static org.mockito.Mockito.verify;
47+
import static org.mockito.Mockito.when;
48+
49+
public class OpenIdAuthenticationMechanismUnitTest {
50+
private OpenIdAuthenticationMechanism authenticationMechanism;
51+
private OpenIdAuthenticationMechanismDefinition definition;
52+
private IdentityStoreHandler identityStoreHandler;
53+
private SimpleStorageHandler storageHandler;
54+
55+
@Before
56+
public void setUp() {
57+
authenticationMechanism = new OpenIdAuthenticationMechanism();
58+
definition = mock(OpenIdAuthenticationMechanismDefinition.class, Answers.RETURNS_DEEP_STUBS);
59+
identityStoreHandler = mock(IdentityStoreHandler.class);
60+
storageHandler = new SimpleStorageHandler();
61+
62+
Reflections.set(authenticationMechanism, "definition", definition);
63+
Reflections.set(authenticationMechanism, "identityStoreHandler", identityStoreHandler);
64+
Reflections.set(authenticationMechanism, "openIdContext", new TomEEOpenIdContext());
65+
Reflections.set(authenticationMechanism, "storageHandler", storageHandler);
66+
67+
when(definition.clientId()).thenReturn("tomee-testing");
68+
when(definition.clientSecret()).thenReturn("secret");
69+
when(definition.redirectURI()).thenReturn("https://example.com/redirect");
70+
when(definition.redirectToOriginalResource()).thenReturn(true);
71+
}
72+
73+
@Test
74+
public void callbackWithoutStoredOriginalRequestDoesNotThrowWhenRedirectToOriginalResourceIsEnabled() throws Exception {
75+
HttpServer server = HttpServer.create(new InetSocketAddress(0), 0);
76+
server.createContext("/token", exchange -> {
77+
byte[] body = "{\"token_type\":\"Bearer\",\"access_token\":\"ACCESS\",\"id_token\":\"ID\",\"expires_in\":3600}"
78+
.getBytes(StandardCharsets.UTF_8);
79+
80+
exchange.getResponseHeaders().set("Content-Type", "application/json");
81+
exchange.sendResponseHeaders(200, body.length);
82+
exchange.getResponseBody().write(body);
83+
exchange.close();
84+
});
85+
server.start();
86+
87+
try {
88+
when(definition.providerMetadata().tokenEndpoint())
89+
.thenReturn("http://127.0.0.1:" + server.getAddress().getPort() + "/token");
90+
when(identityStoreHandler.validate(any()))
91+
.thenReturn(new CredentialValidationResult("caller", Set.of("users")));
92+
93+
HttpServletRequest request = mock(HttpServletRequest.class);
94+
HttpServletResponse response = mock(HttpServletResponse.class);
95+
HttpMessageContext messageContext = mock(HttpMessageContext.class, Answers.RETURNS_DEEP_STUBS);
96+
97+
when(request.getParameter(OpenIdConstant.STATE)).thenReturn("STATE");
98+
when(request.getParameter(OpenIdConstant.CODE)).thenReturn("CODE");
99+
when(request.getRequestURL()).thenReturn(new StringBuffer("https://example.com/redirect"));
100+
when(messageContext.notifyContainerAboutLogin(any(CredentialValidationResult.class)))
101+
.thenReturn(AuthenticationStatus.SUCCESS);
102+
103+
storageHandler.set(request, response, OpenIdStorageHandler.STATE_KEY, "STATE");
104+
105+
AuthenticationStatus status = authenticationMechanism.performAuthentication(request, response, messageContext);
106+
107+
assertEquals(AuthenticationStatus.SUCCESS, status);
108+
verify(messageContext, never()).withRequest(any(HttpServletRequest.class));
109+
} finally {
110+
server.stop(0);
111+
}
112+
}
113+
114+
private static class SimpleStorageHandler extends OpenIdStorageHandler {
115+
private final Map<String, String> values = new HashMap<>();
116+
117+
@Override
118+
public String get(HttpServletRequest request, HttpServletResponse response, String key) {
119+
return values.get(key);
120+
}
121+
122+
@Override
123+
public void set(HttpServletRequest request, HttpServletResponse response, String key, String value) {
124+
values.put(key, value);
125+
}
126+
127+
@Override
128+
public void delete(HttpServletRequest request, HttpServletResponse response, String key) {
129+
values.remove(key);
130+
}
131+
}
132+
}

0 commit comments

Comments
 (0)