Skip to content

Commit 6767ba4

Browse files
garypichergarypicher
authored
Merge pull request #2611 from dburdine/formlogout-OL-2
Formlogout ol 2
2 parents 127d9b5 + 07c2c48 commit 6767ba4

File tree

2 files changed

+137
-15
lines changed

2 files changed

+137
-15
lines changed

‎dev/com.ibm.ws.webcontainer.security/src/com/ibm/ws/webcontainer/security/internal/FormLogoutExtensionProcessor.java‎

Lines changed: 29 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ private String removeFirstSlash(String exitPage) {
228228
protected boolean verifyLogoutURL(HttpServletRequest req, String exitPage) {
229229
boolean acceptURL = false;
230230
boolean allowLogoutPageRedirectToAnyHost = webAppSecurityConfig.getAllowLogoutPageRedirectToAnyHost();
231-
if (!exitPage.equals("logon.jsp") && !allowLogoutPageRedirectToAnyHost) {
231+
if (exitPage != null && !exitPage.equals("logon.jsp") && !allowLogoutPageRedirectToAnyHost) {
232232
String logoutURLHost = null;
233233
try {
234234
InetAddress thisHost = InetAddress.getLocalHost();
@@ -243,8 +243,31 @@ protected boolean verifyLogoutURL(HttpServletRequest req, String exitPage) {
243243
if (tc.isDebugEnabled())
244244
Tr.debug(tc, "domain for exitPage url: " + logoutURLHost);
245245
if (!logoutURL.isAbsolute()) {
246-
//always accept relative URLs for redirect.
247-
acceptURL = true;
246+
/*
247+
* If exitPage starts with "//" , it is a NetworkPath. We need to valid its target host.
248+
* While the specification for network paths is preceded by exactly 2 slashes("//"), browsers will
249+
* generally redirect network paths with 2 or more preceding slashes. Thus, we need to try to obtain the hostname
250+
* utilizing only 2 preceding slashes.
251+
*/
252+
if (exitPage.startsWith("//")) {
253+
if (tc.isDebugEnabled())
254+
Tr.debug(tc, "URI " + exitPage + " will be processed as a Network-Path." +
255+
" sendRedirect() defines a Network-Path as starting with // ");
256+
char[] exitPageCharArr = exitPage.toCharArray();
257+
for (int i = 0; i < exitPageCharArr.length; i++) {
258+
if (exitPageCharArr[i] != '/') {
259+
URI uri = new URI(exitPage.substring(i - 2));
260+
//Set hostname for further checks.
261+
logoutURLHost = uri.getHost();
262+
if (tc.isDebugEnabled())
263+
Tr.debug(tc, "SDK indicates " + exitPage + " Network-Path does not contain a valid hostname.");
264+
break;
265+
}
266+
}
267+
} else {
268+
//accept a relative URIs that are not Network-Path's
269+
acceptURL = true;
270+
}
248271
}
249272
} catch (URISyntaxException urise) {
250273
//The URI is invalid and will not be redirected to.
@@ -267,7 +290,9 @@ protected boolean verifyLogoutURL(HttpServletRequest req, String exitPage) {
267290
if (!acceptURL && logoutURLHost != null)
268291
acceptURL = isRequestURLEqualsExitPageHost(req, logoutURLHost);
269292
} else {
270-
acceptURL = true;
293+
if (exitPage != null) {
294+
acceptURL = true;
295+
}
271296
}
272297
if (!acceptURL) {
273298
Tr.error(tc, "SEC_FORM_LOGOUTEXITPAGE_INVALID", new Object[] { req.getRequestURL(), req.getParameter("logoutExitPage") });

‎dev/com.ibm.ws.webcontainer.security/test/com/ibm/ws/webcontainer/security/internal/FormLogoutExtensionProcessorTest.java‎

Lines changed: 108 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -153,28 +153,125 @@ public void testLogoutReqUrlHost_NotEqualsExitPageHost() throws Exception {
153153
mock.assertIsSatisfied();
154154
}
155155

156+
/*
157+
* Test variations of URLs
158+
*/
156159
@Test
157-
public void testIsLogoutPageURLValid() throws Exception {
158-
//List<String> logoutPageRedirectDomainList = new ArrayList<String>();
159-
//logoutPageRedirectDomainList.add("austin.ibm.com");
160-
final String otherHostLogoutURL = "http://localhost:9080 2 3 Location: http://localhost:9080";
160+
public void testIsLogoutPageURLValid_invalidURL() throws Exception {
161+
162+
final String otherHostLogoutURL1 = "http://localhost:9080 2 3 Location: http://localhost:9080";
163+
final String otherHostLogoutURL2 = "mailto:[email protected]";
164+
final String otherHostLogoutURL3 = "news:comp.lang.java";
165+
final String otherHostLogoutURL4 = "urn:isbn:096139210x";
166+
final String otherHostLogoutURL5 = "http://www.ibm.com/j2se/1.3/";
167+
final String otherHostLogoutURL6 = "docs/guide/collections/designfaq.html#28";
168+
final String otherHostLogoutURL7 = "../../../demo/jfc/SwingSet2/src/SwingSet2.java";
169+
final String otherHostLogoutURL8 = "file://www.ibm.com/~/calendar";
170+
final String otherHostLogoutURL9 = null;
171+
final String otherHostLogoutURL10 = ""; //JDK considers "" a valid URI, so we do then as well.
172+
161173
final java.io.PrintWriter pw = mock.mock(java.io.PrintWriter.class);
162174

163175
mock.checking(new Expectations() {
164176
{
165-
one(req).getParameter("logoutExitPage");
166-
will(returnValue(otherHostLogoutURL));
167-
one(req).getRequestURL();
177+
allowing(req).getParameter("logoutExitPage"); //This is a trace record call. Value returned is irrelevant.
178+
will(returnValue(otherHostLogoutURL1));
179+
allowing(req).getRequestURL();
168180
will(returnValue(new StringBuffer("http://localhost:9080/snoop")));
169-
one(resp).getWriter();
181+
allowing(resp).getWriter();
170182
will(returnValue(pw));
171-
one(pw).println(FormLogoutExtensionProcessor.DEFAULT_LOGOUT_MSG);
172-
one(wasc).getLogoutPageRedirectDomainList();
183+
allowing(pw).println(FormLogoutExtensionProcessor.DEFAULT_LOGOUT_MSG);
184+
allowing(wasc).getLogoutPageRedirectDomainList();
185+
}
186+
});
187+
188+
FormLogoutExtensionProcessor processorDouble = new FormLogoutExtensionProcessor(ctx, wasc, authApi);
189+
assertFalse(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1));
190+
assertFalse(processorDouble.verifyLogoutURL(req, otherHostLogoutURL2));
191+
assertFalse(processorDouble.verifyLogoutURL(req, otherHostLogoutURL3));
192+
assertFalse(processorDouble.verifyLogoutURL(req, otherHostLogoutURL4));
193+
assertFalse(processorDouble.verifyLogoutURL(req, otherHostLogoutURL5));
194+
assertTrue(processorDouble.verifyLogoutURL(req, otherHostLogoutURL6));
195+
assertTrue(processorDouble.verifyLogoutURL(req, otherHostLogoutURL7));
196+
assertFalse(processorDouble.verifyLogoutURL(req, otherHostLogoutURL8));
197+
assertFalse(processorDouble.verifyLogoutURL(req, otherHostLogoutURL9));
198+
assertTrue(processorDouble.verifyLogoutURL(req, otherHostLogoutURL10));
199+
}
200+
201+
@Test
202+
public void testIsLogoutPageURLValid_networkPath_listMatch() throws Exception {
203+
final List<String> logoutPageRedirectDomainList = new ArrayList<String>();
204+
logoutPageRedirectDomainList.add("austin.ibm.com");
205+
logoutPageRedirectDomainList.add("myserver.com");
206+
207+
final String otherHostLogoutURL1a = "/////myserver.com";
208+
final String otherHostLogoutURL1b = "////myserver.com";
209+
final String otherHostLogoutURL1c = "///myserver.com";
210+
final String otherHostLogoutURL1d = "//myserver.com";
211+
//Considered relative URLs by WAS. So redirect performed and no hostname list comparision performed.
212+
final String otherHostLogoutURL1e = "/myserver.com";
213+
final String otherHostLogoutURL1f = "myserver.com";
214+
final java.io.PrintWriter pw = mock.mock(java.io.PrintWriter.class);
215+
216+
mock.checking(new Expectations() {
217+
{
218+
allowing(req).getParameter("logoutExitPage");
219+
//will(returnValue(otherHostLogoutURL));
220+
allowing(req).getRequestURL();
221+
will(returnValue(new StringBuffer("http://localhost:9080/snoop")));
222+
allowing(resp).getWriter();
223+
will(returnValue(pw));
224+
allowing(pw).println(FormLogoutExtensionProcessor.DEFAULT_LOGOUT_MSG);
225+
allowing(wasc).getLogoutPageRedirectDomainList();
226+
will(returnValue(logoutPageRedirectDomainList));
227+
}
228+
});
229+
230+
FormLogoutExtensionProcessor processorDouble = new FormLogoutExtensionProcessor(ctx, wasc, authApi);
231+
assertTrue(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1a));
232+
assertTrue(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1b));
233+
assertTrue(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1c));
234+
assertTrue(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1d));
235+
assertTrue(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1e));
236+
assertTrue(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1f));
237+
}
238+
239+
@Test
240+
public void testIsLogoutPageURLValid_networkPath_noListMatch() throws Exception {
241+
final List<String> logoutPageRedirectDomainList = new ArrayList<String>();
242+
logoutPageRedirectDomainList.add("austin.ibm.com");
243+
logoutPageRedirectDomainList.add("yourserver.com");
244+
245+
final String otherHostLogoutURL1a = "/////myserver.com";
246+
final String otherHostLogoutURL1b = "////myserver.com";
247+
final String otherHostLogoutURL1c = "///myserver.com";
248+
final String otherHostLogoutURL1d = "//myserver.com";
249+
//Considered relative URLs by WAS. So redirect performed and no hostname list comparision performed.
250+
final String otherHostLogoutURL1e = "/myserver.com";
251+
final String otherHostLogoutURL1f = "myserver.com";
252+
final java.io.PrintWriter pw = mock.mock(java.io.PrintWriter.class);
253+
254+
mock.checking(new Expectations() {
255+
{
256+
allowing(req).getParameter("logoutExitPage");
257+
//will(returnValue(otherHostLogoutURL));
258+
allowing(req).getRequestURL();
259+
will(returnValue(new StringBuffer("http://localhost:9080/snoop")));
260+
allowing(resp).getWriter();
261+
will(returnValue(pw));
262+
allowing(pw).println(FormLogoutExtensionProcessor.DEFAULT_LOGOUT_MSG);
263+
allowing(wasc).getLogoutPageRedirectDomainList();
264+
will(returnValue(logoutPageRedirectDomainList));
173265
}
174266
});
175267

176268
FormLogoutExtensionProcessor processorDouble = new FormLogoutExtensionProcessor(ctx, wasc, authApi);
177-
assertFalse(processorDouble.verifyLogoutURL(req, otherHostLogoutURL));
269+
assertFalse(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1a));
270+
assertFalse(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1b));
271+
assertFalse(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1c));
272+
assertFalse(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1d));
273+
assertTrue(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1e));
274+
assertTrue(processorDouble.verifyLogoutURL(req, otherHostLogoutURL1f));
178275
}
179276

180277
// Note that we cannot simulate code flow through if (TraceComponent.isAnyTracingEnabled() && tc.isDebugEnabled()) blocks

0 commit comments

Comments
 (0)