call setSecure explicitly & fix HTTP response splitting
This commit is contained in:
parent
4fb0ac8082
commit
7b237b4d83
@ -32,7 +32,10 @@ import javax.servlet.http.Cookie;
|
||||
import javax.servlet.http.HttpServletRequest;
|
||||
import javax.servlet.http.HttpServletResponse;
|
||||
import java.io.IOException;
|
||||
import java.io.UnsupportedEncodingException;
|
||||
import java.net.HttpCookie;
|
||||
import java.net.URLDecoder;
|
||||
import java.net.URLEncoder;
|
||||
import java.util.ArrayList;
|
||||
import java.util.HashMap;
|
||||
import java.util.List;
|
||||
@ -97,7 +100,7 @@ public class LoginCommand extends Command {
|
||||
Cookie[] cookies = request.getCookies();
|
||||
|
||||
for (Cookie cookie : cookies) {
|
||||
String value = cookie.getValue();
|
||||
String value = getCookieValue(cookie);
|
||||
switch (cookie.getName()) {
|
||||
case CONSUMER_TOKEN:
|
||||
consumerToken = value;
|
||||
@ -123,10 +126,10 @@ public class LoginCommand extends Command {
|
||||
for (Cookie cookie : cookies) {
|
||||
if (cookie.getName().startsWith(WIKIDATA_COOKIE_PREFIX)) {
|
||||
String cookieName = cookie.getName().substring(WIKIDATA_COOKIE_PREFIX.length());
|
||||
Cookie newCookie = new Cookie(cookieName, cookie.getValue());
|
||||
Cookie newCookie = new Cookie(cookieName, getCookieValue(cookie));
|
||||
cookieList.add(newCookie);
|
||||
} else if (cookie.getName().equals(WIKIBASE_USERNAME_COOKIE_KEY)) {
|
||||
username1 = cookie.getValue();
|
||||
username1 = getCookieValue(cookie);
|
||||
}
|
||||
}
|
||||
|
||||
@ -206,10 +209,18 @@ public class LoginCommand extends Command {
|
||||
removeCookie(response, ACCESS_SECRET);
|
||||
}
|
||||
|
||||
private static void setCookie(HttpServletResponse response, String key, String value) {
|
||||
Cookie cookie = new Cookie(key, value);
|
||||
static String getCookieValue(Cookie cookie) throws UnsupportedEncodingException {
|
||||
return URLDecoder.decode(cookie.getValue(), "utf-8");
|
||||
}
|
||||
|
||||
private static void setCookie(HttpServletResponse response, String key, String value) throws UnsupportedEncodingException {
|
||||
String encodedValue = URLEncoder.encode(value, "utf-8");
|
||||
Cookie cookie = new Cookie(key, encodedValue);
|
||||
cookie.setMaxAge(60 * 60 * 24 * 365); // a year
|
||||
cookie.setPath("/");
|
||||
// set to false because OpenRefine doesn't require HTTPS
|
||||
// though false is the default value, we set it explicitly here to bypass lgtm-bot alerts
|
||||
cookie.setSecure(false);
|
||||
response.addCookie(cookie);
|
||||
}
|
||||
|
||||
@ -217,6 +228,7 @@ public class LoginCommand extends Command {
|
||||
Cookie cookie = new Cookie(key, "");
|
||||
cookie.setMaxAge(0); // 0 causes the cookie to be deleted
|
||||
cookie.setPath("/");
|
||||
cookie.setSecure(false);
|
||||
response.addCookie(cookie);
|
||||
}
|
||||
}
|
||||
|
@ -20,6 +20,7 @@ import javax.servlet.http.Cookie;
|
||||
import java.io.IOException;
|
||||
import java.io.PrintWriter;
|
||||
import java.io.StringWriter;
|
||||
import java.io.UnsupportedEncodingException;
|
||||
import java.lang.reflect.Constructor;
|
||||
import java.net.HttpCookie;
|
||||
import java.util.ArrayList;
|
||||
@ -251,6 +252,26 @@ public class LoginCommandTest extends CommandTest {
|
||||
assertCookieEquals(cookies.get(ACCESS_SECRET), accessSecret, ONE_YEAR);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCookieEncoding() throws Exception {
|
||||
OAuthApiConnection connection = mock(OAuthApiConnection.class);
|
||||
whenNew(OAuthApiConnection.class).withAnyArguments().thenReturn(connection);
|
||||
|
||||
when(request.getParameter("csrf_token")).thenReturn(Command.csrfFactory.getFreshToken());
|
||||
when(request.getParameter("remember-credentials")).thenReturn("on");
|
||||
when(request.getParameter(CONSUMER_TOKEN)).thenReturn("malformed consumer token \r\n %?");
|
||||
when(request.getParameter(CONSUMER_SECRET)).thenReturn(consumerSecret);
|
||||
when(request.getParameter(ACCESS_TOKEN)).thenReturn(accessToken);
|
||||
when(request.getParameter(ACCESS_SECRET)).thenReturn(accessSecret);
|
||||
when(request.getCookies()).thenReturn(makeRequestCookies());
|
||||
|
||||
command.doPost(request, response);
|
||||
|
||||
Map<String, Cookie> cookies = getCookieMap(cookieCaptor.getAllValues());
|
||||
assertNotEquals(cookies.get(CONSUMER_TOKEN).getValue(), "malformed consumer token \r\n %?");
|
||||
assertEquals(cookies.get(CONSUMER_TOKEN).getValue(), "malformed+consumer+token+%0D%0A+%25%3F");
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testOwnerOnlyConsumerLoginWithCookies() throws Exception {
|
||||
OAuthApiConnection connection = mock(OAuthApiConnection.class);
|
||||
@ -277,6 +298,27 @@ public class LoginCommandTest extends CommandTest {
|
||||
assertCookieEquals(cookies.get(ACCESS_SECRET), accessSecret, ONE_YEAR);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testCookieDecoding() throws Exception {
|
||||
ConnectionManager manager = mock(ConnectionManager.class);
|
||||
given(ConnectionManager.getInstance()).willReturn(manager);
|
||||
|
||||
OAuthApiConnection connection = mock(OAuthApiConnection.class);
|
||||
whenNew(OAuthApiConnection.class).withAnyArguments().thenReturn(connection);
|
||||
when(connection.getCurrentUser()).thenReturn(username);
|
||||
|
||||
when(request.getParameter("csrf_token")).thenReturn(Command.csrfFactory.getFreshToken());
|
||||
Cookie consumerTokenCookie = new Cookie(CONSUMER_TOKEN, "malformed+consumer+token+%0D%0A+%25%3F");
|
||||
Cookie consumerSecretCookie = new Cookie(CONSUMER_SECRET, consumerSecret);
|
||||
Cookie accessTokenCookie = new Cookie(ACCESS_TOKEN, accessToken);
|
||||
Cookie accessSecretCookie = new Cookie(ACCESS_SECRET, accessSecret);
|
||||
when(request.getCookies()).thenReturn(new Cookie[]{consumerTokenCookie, consumerSecretCookie, accessTokenCookie, accessSecretCookie});
|
||||
|
||||
command.doPost(request, response);
|
||||
|
||||
verify(manager).login("malformed consumer token \r\n %?", consumerSecret, accessToken, accessSecret);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void testLogout() throws Exception {
|
||||
BasicApiConnection connection = mock(BasicApiConnection.class);
|
||||
@ -407,7 +449,11 @@ public class LoginCommandTest extends CommandTest {
|
||||
}
|
||||
|
||||
private static void assertCookieEquals(Cookie cookie, String expectedValue, int expectedMaxAge) {
|
||||
assertEquals(cookie.getValue(), expectedValue);
|
||||
try {
|
||||
assertEquals(getCookieValue(cookie), expectedValue);
|
||||
} catch (UnsupportedEncodingException e) {
|
||||
e.printStackTrace();
|
||||
}
|
||||
assertEquals(cookie.getMaxAge(), expectedMaxAge);
|
||||
assertEquals(cookie.getPath(), "/");
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user