Skip to content

Commit 135904c

Browse files
CarsonCookbalhar-jakub
CarsonCook
and
balhar-jakub
authored
feat: Make cookie samesite configurable (#1545)
* Add configuration property for cookie samesite Signed-off-by: Carson Cook <carson.cook@ibm.com> * Use tomcat samesite enum Signed-off-by: Carson Cook <carson.cook@ibm.com> * Implement samesite in cookie Signed-off-by: Carson Cook <carson.cook@ibm.com> * Add samesite test to integration tests Signed-off-by: Carson Cook <carson.cook@ibm.com> * Add clarifying comment Signed-off-by: Carson Cook <carson.cook@ibm.com> * Change default to strict Signed-off-by: Carson Cook <carson.cook@ibm.com> * Use max age properly Signed-off-by: Carson Cook <carson.cook@ibm.com> * Move set-cookie behaviour to common util Signed-off-by: Carson Cook <carson.cook@ibm.com> * Add samesite for auth token in uri Signed-off-by: Carson Cook <carson.cook@ibm.com> Co-authored-by: Jakub Balhar <jakub@balhar.net>
1 parent 40777a0 commit 135904c

File tree

8 files changed

+169
-37
lines changed

8 files changed

+169
-37
lines changed

‎apiml-security-common/src/main/java/org/zowe/apiml/security/common/config/AuthConfigurationProperties.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
package org.zowe.apiml.security.common.config;
1111

1212
import lombok.Data;
13+
import org.apache.tomcat.util.http.SameSiteCookies;
1314
import org.springframework.boot.context.properties.ConfigurationProperties;
1415
import org.springframework.security.authentication.AuthenticationServiceException;
1516
import org.springframework.stereotype.Component;
@@ -76,7 +77,8 @@ public static class CookieProperties {
7677
private boolean cookieSecure = true;
7778
private String cookiePath = "/";
7879
private String cookieComment = "API Mediation Layer security token";
79-
private Integer cookieMaxAge = -1;
80+
private Integer cookieMaxAge = null;
81+
private SameSiteCookies cookieSameSite = SameSiteCookies.STRICT;
8082
}
8183

8284
@Data

‎apiml-security-common/src/main/java/org/zowe/apiml/security/common/login/SuccessfulLoginHandler.java

Lines changed: 18 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,18 @@
99
*/
1010
package org.zowe.apiml.security.common.login;
1111

12-
import org.zowe.apiml.security.common.config.AuthConfigurationProperties;
13-
import org.zowe.apiml.security.common.token.TokenAuthentication;
1412
import lombok.RequiredArgsConstructor;
1513
import org.springframework.http.HttpStatus;
1614
import org.springframework.security.core.Authentication;
1715
import org.springframework.security.web.authentication.AuthenticationSuccessHandler;
1816
import org.springframework.stereotype.Component;
17+
import org.zowe.apiml.security.common.config.AuthConfigurationProperties;
18+
import org.zowe.apiml.security.common.token.TokenAuthentication;
19+
import org.zowe.apiml.util.CookieUtil;
1920

20-
import javax.servlet.http.Cookie;
2121
import javax.servlet.http.HttpServletRequest;
2222
import javax.servlet.http.HttpServletResponse;
2323

24-
2524
/**
2625
* Handles the successful login
2726
*/
@@ -54,13 +53,21 @@ public void onAuthenticationSuccess(HttpServletRequest request, HttpServletRespo
5453
* @param response send back this response
5554
*/
5655
private void setCookie(String token, HttpServletResponse response) {
57-
Cookie tokenCookie = new Cookie(authConfigurationProperties.getCookieProperties().getCookieName(), token);
58-
tokenCookie.setComment(authConfigurationProperties.getCookieProperties().getCookieComment());
59-
tokenCookie.setPath(authConfigurationProperties.getCookieProperties().getCookiePath());
60-
tokenCookie.setHttpOnly(true);
61-
tokenCookie.setMaxAge(authConfigurationProperties.getCookieProperties().getCookieMaxAge());
62-
tokenCookie.setSecure(authConfigurationProperties.getCookieProperties().isCookieSecure());
56+
// SameSite attribute is not supported in Cookie used in HttpServletResponse.addCookie,
57+
// so specify Set-Cookie header directly
58+
59+
AuthConfigurationProperties.CookieProperties cp = authConfigurationProperties.getCookieProperties();
60+
String cookieHeader = CookieUtil.setCookieHeader(
61+
cp.getCookieName(),
62+
token,
63+
cp.getCookieComment(),
64+
cp.getCookiePath(),
65+
cp.getCookieSameSite().getValue(),
66+
cp.getCookieMaxAge(),
67+
true,
68+
cp.isCookieSecure()
69+
);
6370

64-
response.addCookie(tokenCookie);
71+
response.addHeader("Set-Cookie", cookieHeader);
6572
}
6673
}

‎apiml-security-common/src/test/java/org/zowe/apiml/security/common/config/AuthConfigurationPropertiesTest.java

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,10 @@ void shouldReturnWhenZosmfIsConfigured() {
3030
authConfigurationProperties.getZosmf().setServiceId("ZOSMF_SID");
3131
assertEquals("ZOSMF_SID", authConfigurationProperties.validatedZosmfServiceId());
3232
}
33+
34+
@Test
35+
void whenGetDefaultCookieSameSite_thenReturnStrict() {
36+
String sameSite = authConfigurationProperties.getCookieProperties().getCookieSameSite().getValue();
37+
assertEquals("Strict", sameSite);
38+
}
3339
}

‎apiml-security-common/src/test/java/org/zowe/apiml/security/common/login/SuccessfulLoginHandlerTest.java

Lines changed: 40 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,22 @@
99
*/
1010
package org.zowe.apiml.security.common.login;
1111

12-
import org.zowe.apiml.security.common.config.AuthConfigurationProperties;
13-
import org.zowe.apiml.security.common.token.TokenAuthentication;
1412
import org.junit.jupiter.api.BeforeEach;
13+
import org.junit.jupiter.api.Nested;
1514
import org.junit.jupiter.api.Test;
1615
import org.springframework.http.HttpStatus;
1716
import org.springframework.mock.web.MockHttpServletRequest;
1817
import org.springframework.mock.web.MockHttpServletResponse;
18+
import org.zowe.apiml.security.common.config.AuthConfigurationProperties;
19+
import org.zowe.apiml.security.common.token.TokenAuthentication;
20+
21+
import javax.servlet.http.Cookie;
1922

20-
import static org.junit.jupiter.api.Assertions.assertEquals;
21-
import static org.junit.jupiter.api.Assertions.assertNotNull;
23+
import static org.junit.jupiter.api.Assertions.*;
2224

2325
class SuccessfulLoginHandlerTest {
26+
private final TokenAuthentication dummyAuth = new TokenAuthentication("TEST_TOKEN_STRING");
27+
2428
private AuthConfigurationProperties authConfigurationProperties;
2529
private MockHttpServletRequest httpServletRequest;
2630
private MockHttpServletResponse httpServletResponse;
@@ -35,15 +39,38 @@ void setup() {
3539
successfulLoginHandler = new SuccessfulLoginHandler(authConfigurationProperties);
3640
}
3741

38-
@Test
39-
void testOnAuthenticationSuccess() {
40-
successfulLoginHandler.onAuthenticationSuccess(
41-
httpServletRequest,
42-
httpServletResponse,
43-
new TokenAuthentication("TEST_TOKEN_STRING")
44-
);
42+
@Nested
43+
class WhenAuthenticationIsSuccessful {
44+
@Test
45+
void givenAuthentication_thenReturn204AndCookie() {
46+
executeLoginHandler();
47+
48+
assertEquals(HttpStatus.NO_CONTENT.value(), httpServletResponse.getStatus());
49+
50+
Cookie cookie = httpServletResponse.getCookie(authConfigurationProperties.getCookieProperties().getCookieName());
51+
assertNotNull(cookie);
52+
53+
AuthConfigurationProperties.CookieProperties cp = authConfigurationProperties.getCookieProperties();
54+
assertEquals(cp.getCookieName(), cookie.getName());
55+
assertEquals(dummyAuth.getCredentials(), cookie.getValue());
56+
assertEquals(cp.getCookiePath(), cookie.getPath());
57+
assertEquals(-1, cookie.getMaxAge());
58+
assertTrue(cookie.isHttpOnly());
59+
assertTrue(cookie.getSecure());
60+
}
61+
62+
@Test
63+
void givenCookieSecureNotSet_thenReturnCookieWithoutSecure() {
64+
authConfigurationProperties.getCookieProperties().setCookieSecure(false);
65+
executeLoginHandler();
66+
67+
Cookie cookie = httpServletResponse.getCookie(authConfigurationProperties.getCookieProperties().getCookieName());
68+
assertNotNull(cookie);
69+
assertFalse(cookie.getSecure());
70+
}
71+
}
4572

46-
assertEquals(HttpStatus.NO_CONTENT.value(), httpServletResponse.getStatus());
47-
assertNotNull(httpServletResponse.getCookie(authConfigurationProperties.getCookieProperties().getCookieName()));
73+
private void executeLoginHandler() {
74+
successfulLoginHandler.onAuthenticationSuccess(httpServletRequest, httpServletResponse, dummyAuth);
4875
}
4976
}

‎common-service-core/src/main/java/org/zowe/apiml/util/CookieUtil.java

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,12 +18,38 @@
1818
@UtilityClass
1919
public final class CookieUtil {
2020

21+
public static String setCookieHeader(String name, String value, String comment, String path, String sameSite,
22+
Integer maxAge, boolean isHttpOnly, boolean isSecure) {
23+
String cookieHeader = String.format(
24+
"%s=%s; Comment=%s; Path=%s; SameSite=%s;",
25+
name,
26+
value,
27+
comment,
28+
path,
29+
sameSite
30+
);
31+
32+
if (maxAge != null) {
33+
cookieHeader += " Max-Age=" + maxAge + ";";
34+
}
35+
36+
if (isHttpOnly) {
37+
cookieHeader += " HttpOnly;";
38+
}
39+
40+
if (isSecure) {
41+
cookieHeader += " Secure;";
42+
}
43+
44+
return cookieHeader;
45+
}
46+
2147
/**
2248
* It replace or add cookie into header string value (see header with name "Cookie").
2349
*
2450
* @param cookieHeader original header string value
25-
* @param name name of cookie to add or replace
26-
* @param value value of cookie to add or replace
51+
* @param name name of cookie to add or replace
52+
* @param value value of cookie to add or replace
2753
* @return new header string, which contains a new cookie
2854
*/
2955
public static String setCookie(String cookieHeader, String name, String value) {
@@ -51,7 +77,7 @@ public static String setCookie(String cookieHeader, String name, String value) {
5177
* cookie, it return original header value string.
5278
*
5379
* @param cookieHeader original header string value
54-
* @param name name of cookie to remove
80+
* @param name name of cookie to remove
5581
* @return new header string, without a specified cookie
5682
*/
5783
public static String removeCookie(String cookieHeader, String name) {

‎common-service-core/src/test/java/org/zowe/apiml/util/CookieUtilTest.java

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,12 @@
99
*/
1010
package org.zowe.apiml.util;
1111

12+
import org.junit.jupiter.api.Nested;
1213
import org.junit.jupiter.api.Test;
1314

15+
import static org.hamcrest.CoreMatchers.not;
16+
import static org.hamcrest.MatcherAssert.assertThat;
17+
import static org.hamcrest.core.StringContains.containsString;
1418
import static org.junit.jupiter.api.Assertions.assertEquals;
1519
import static org.junit.jupiter.api.Assertions.assertSame;
1620

@@ -35,4 +39,52 @@ void removeCookie() {
3539
assertEquals("", CookieUtil.removeCookie("a=1;a=2;a=3", "a"));
3640
}
3741

42+
@Nested
43+
class WhenGetSetCookieHeader {
44+
private static final String NAME = "name";
45+
private static final String VALUE = "value";
46+
private static final String COMMENT = "comment";
47+
private static final String PATH = "/";
48+
private static final String SAME_SITE = "samesite";
49+
50+
@Test
51+
void givenAllAttributesSet_thenReturnSetCookieWithAttributes() {
52+
String setCookieHeader = CookieUtil.setCookieHeader(
53+
NAME, VALUE, COMMENT, PATH, SAME_SITE, 1, true, true
54+
);
55+
56+
assertEquals(setCookieHeader, String.format("%s=%s; Comment=%s; Path=%s; SameSite=%s; Max-Age=%d; HttpOnly; Secure;",
57+
NAME,
58+
VALUE,
59+
COMMENT,
60+
PATH,
61+
SAME_SITE,
62+
1
63+
));
64+
}
65+
66+
@Test
67+
void givenNullMaxAge_thenReturnSetCookieWithNoMaxAge() {
68+
String setCookieHeader = CookieUtil.setCookieHeader(
69+
NAME, VALUE, COMMENT, PATH, SAME_SITE, null, true, true
70+
);
71+
assertThat(setCookieHeader, not(containsString("Max-Age")));
72+
}
73+
74+
@Test
75+
void givenNotHttpOnly_thenReturnSetCookieWithoutHttpOnly() {
76+
String setCookieHeader = CookieUtil.setCookieHeader(
77+
NAME, VALUE, COMMENT, PATH, SAME_SITE, 1, false, true
78+
);
79+
assertThat(setCookieHeader, not(containsString("HttpOnly")));
80+
}
81+
82+
@Test
83+
void givenNotSecure_thenReturnSetCookieWithoutSecure() {
84+
String setCookieHeader = CookieUtil.setCookieHeader(
85+
NAME, VALUE, COMMENT, PATH, SAME_SITE, 1, true, false
86+
);
87+
assertThat(setCookieHeader, not(containsString("Secure")));
88+
}
89+
}
3890
}

‎gateway-service/src/main/java/org/zowe/apiml/gateway/filters/post/ConvertAuthTokenInUriToCookieFilter.java

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@
99
*/
1010
package org.zowe.apiml.gateway.filters.post;
1111

12-
import org.zowe.apiml.security.common.config.AuthConfigurationProperties;
1312
import com.netflix.zuul.ZuulFilter;
1413
import com.netflix.zuul.context.RequestContext;
14+
import org.zowe.apiml.security.common.config.AuthConfigurationProperties;
15+
import org.zowe.apiml.util.CookieUtil;
1516

16-
import javax.servlet.http.Cookie;
1717
import javax.servlet.http.HttpServletResponse;
1818

1919
import static org.springframework.cloud.netflix.zuul.filters.support.FilterConstants.POST_TYPE;
@@ -47,13 +47,20 @@ public Object run() {
4747
AuthConfigurationProperties.CookieProperties cp = authConfigurationProperties.getCookieProperties();
4848
if ((context.getRequestQueryParams() != null) && context.getRequestQueryParams().containsKey(cp.getCookieName())) {
4949
HttpServletResponse servletResponse = context.getResponse();
50-
Cookie cookie = new Cookie(cp.getCookieName(), context.getRequestQueryParams().get(cp.getCookieName()).get(0));
51-
cookie.setPath(cp.getCookiePath());
52-
cookie.setSecure(true);
53-
cookie.setHttpOnly(true);
54-
cookie.setMaxAge(cp.getCookieMaxAge());
55-
cookie.setComment(cp.getCookieComment());
56-
servletResponse.addCookie(cookie);
50+
51+
// SameSite attribute is not supported in Cookie used in HttpServletResponse.addCookie,
52+
// so specify Set-Cookie header directly
53+
String cookieHeader = CookieUtil.setCookieHeader(
54+
cp.getCookieName(),
55+
context.getRequestQueryParams().get(cp.getCookieName()).get(0),
56+
cp.getCookieComment(),
57+
cp.getCookiePath(),
58+
cp.getCookieSameSite().getValue(),
59+
cp.getCookieMaxAge(),
60+
true,
61+
true
62+
);
63+
servletResponse.addHeader("Set-Cookie", cookieHeader);
5764

5865
String url = context.getRequest().getRequestURL().toString();
5966
String newUrl;

‎integration-tests/src/test/java/org/zowe/apiml/integration/authentication/providers/LoginTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import static io.restassured.RestAssured.given;
3737
import static io.restassured.http.ContentType.JSON;
3838
import static org.apache.http.HttpStatus.*;
39+
import static org.hamcrest.CoreMatchers.containsString;
3940
import static org.hamcrest.CoreMatchers.equalTo;
4041
import static org.hamcrest.Matchers.isEmptyString;
4142
import static org.hamcrest.core.Is.is;
@@ -99,6 +100,8 @@ void givenValidCredentialsInBody(URI loginUrl) {
99100
.post(loginUrl)
100101
.then()
101102
.statusCode(is(SC_NO_CONTENT))
103+
// RestAssured version in use doesn't have SameSite attribute in cookie so validate using the Set-Cookie header
104+
.header("Set-Cookie", containsString("SameSite=Strict"))
102105
.cookie(COOKIE_NAME, not(isEmptyString()))
103106
.extract().detailedCookie(COOKIE_NAME);
104107

@@ -115,6 +118,8 @@ void givenValidCredentialsInHeader(URI loginUrl) {
115118
.post(loginUrl)
116119
.then()
117120
.statusCode(is(SC_NO_CONTENT))
121+
// RestAssured version in use doesn't have SameSite attribute in cookie so validate using the Set-Cookie header
122+
.header("Set-Cookie", containsString("SameSite=Strict"))
118123
.cookie(COOKIE_NAME, not(isEmptyString()))
119124
.extract().cookie(COOKIE_NAME);
120125

0 commit comments

Comments
 (0)