From fa9e7999daffd9a848c0abc9e0f30468d40b716d Mon Sep 17 00:00:00 2001 From: Rob Winch Date: Tue, 18 Nov 2014 13:08:00 -0600 Subject: [PATCH] SEC-2569: SavedRequestAwareWrapper no longer overrides getCookies() Previously SavedRequestAwareWrapper overrode the getCookies() method. This meant that the cookies from the original request were used instead of the new request. In general, this does not make sense since cookies are automatically submitted in every request by a client. Additionally, this caused problems with using a locale cookie that was specified after the secured page was requested. Now SavedRequestAwareWrapper uses the new incoming request for determining the cookies. --- .../web/savedrequest/RequestCacheAdapter.java | 108 ------------------ .../SavedRequestAwareWrapper.java | 7 -- .../SavedRequestAwareWrapperTests.java | 9 +- 3 files changed, 6 insertions(+), 118 deletions(-) delete mode 100644 web/src/main/java/org/springframework/security/web/savedrequest/RequestCacheAdapter.java diff --git a/web/src/main/java/org/springframework/security/web/savedrequest/RequestCacheAdapter.java b/web/src/main/java/org/springframework/security/web/savedrequest/RequestCacheAdapter.java deleted file mode 100644 index 85df5c1f91..0000000000 --- a/web/src/main/java/org/springframework/security/web/savedrequest/RequestCacheAdapter.java +++ /dev/null @@ -1,108 +0,0 @@ -/* - * Copyright 2002-2014 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.springframework.security.web.savedrequest; - -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.Locale; -import java.util.Map; - -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - -import org.springframework.util.Assert; - -public class RequestCacheAdapter implements RequestCache { - - private final RequestCache delegate; - - public RequestCacheAdapter() { - this(new HttpSessionRequestCache()); - } - - public RequestCacheAdapter(RequestCache delegate) { - Assert.notNull(delegate, "delegate cannot be null"); - this.delegate = delegate; - } - - public void saveRequest(HttpServletRequest request, - HttpServletResponse response) { - delegate.saveRequest(request, response); - } - - public SavedRequest getRequest(HttpServletRequest request, - HttpServletResponse response) { - SavedRequest result = delegate.getRequest(request, response); - Cookie[] cookies = request.getCookies(); - return new SavedRequestAdapter(result, cookies == null ? null : Arrays.asList(cookies)); - } - - public HttpServletRequest getMatchingRequest(HttpServletRequest request, - HttpServletResponse response) { - return delegate.getMatchingRequest(request, response); - } - - public void removeRequest(HttpServletRequest request, - HttpServletResponse response) { - delegate.removeRequest(request, response); - } - - private static class SavedRequestAdapter implements SavedRequest { - private SavedRequest delegate; - private List cookies; - - public SavedRequestAdapter(SavedRequest delegate, List cookies) { - this.delegate = delegate; - this.cookies = cookies; - } - - public String getRedirectUrl() { - return delegate.getRedirectUrl(); - } - - public List getCookies() { - return cookies; - } - - public String getMethod() { - return delegate.getMethod(); - } - - public List getHeaderValues(String name) { - return delegate.getHeaderValues(name); - } - - public Collection getHeaderNames() { - return delegate.getHeaderNames(); - } - - public List getLocales() { - return delegate.getLocales(); - } - - public String[] getParameterValues(String name) { - return delegate.getParameterValues(name); - } - - public Map getParameterMap() { - return delegate.getParameterMap(); - } - - private static final long serialVersionUID = 1184951442151447331L; - } -} diff --git a/web/src/main/java/org/springframework/security/web/savedrequest/SavedRequestAwareWrapper.java b/web/src/main/java/org/springframework/security/web/savedrequest/SavedRequestAwareWrapper.java index 32826a6ae5..4664c0df8e 100644 --- a/web/src/main/java/org/springframework/security/web/savedrequest/SavedRequestAwareWrapper.java +++ b/web/src/main/java/org/springframework/security/web/savedrequest/SavedRequestAwareWrapper.java @@ -85,13 +85,6 @@ class SavedRequestAwareWrapper extends HttpServletRequestWrapper { //~ Methods ======================================================================================================== - @Override - public Cookie[] getCookies() { - List cookies = savedRequest.getCookies(); - - return cookies.toArray(new Cookie[cookies.size()]); - } - @Override public long getDateHeader(String name) { String value = getHeader(name); diff --git a/web/src/test/java/org/springframework/security/web/savedrequest/SavedRequestAwareWrapperTests.java b/web/src/test/java/org/springframework/security/web/savedrequest/SavedRequestAwareWrapperTests.java index 9b12e67665..c8197abec4 100644 --- a/web/src/test/java/org/springframework/security/web/savedrequest/SavedRequestAwareWrapperTests.java +++ b/web/src/test/java/org/springframework/security/web/savedrequest/SavedRequestAwareWrapperTests.java @@ -23,13 +23,16 @@ public class SavedRequestAwareWrapperTests { return new SavedRequestAwareWrapper(saved, requestToWrap); } + // SEC-2569 @Test - public void savedRequestCookiesAreReturnedIfSavedRequestIsSet() throws Exception { + public void savedRequestCookiesAreIgnored() throws Exception { + MockHttpServletRequest newRequest = new MockHttpServletRequest(); + newRequest.setCookies(new Cookie[] {new Cookie("cookie", "fromnew")}); MockHttpServletRequest savedRequest = new MockHttpServletRequest(); savedRequest.setCookies(new Cookie[] {new Cookie("cookie", "fromsaved")}); - SavedRequestAwareWrapper wrapper = createWrapper(savedRequest, new MockHttpServletRequest()); + SavedRequestAwareWrapper wrapper = createWrapper(savedRequest, newRequest); assertEquals(1, wrapper.getCookies().length); - assertEquals("fromsaved", wrapper.getCookies()[0].getValue()); + assertEquals("fromnew", wrapper.getCookies()[0].getValue()); } @Test