Skip to content

Commit 3c8e9ed

Browse files
jansingerjansinger
committed
Refactoring, moved session state management classes to web/sessionstate package
Added tests for session state management Removed NYI from post logout uri registrations in client
2 parents adfb767 + f1395b5 commit 3c8e9ed

File tree

15 files changed

+776
-39
lines changed

15 files changed

+776
-39
lines changed

‎openid-connect-server-webapp/pom.xml‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,21 @@
147147
<groupId>com.zaxxer</groupId>
148148
<artifactId>HikariCP</artifactId>
149149
</dependency>
150+
151+
<!-- https://mvnrepository.com/artifact/javax.servlet/javax.servlet-api -->
152+
<dependency>
153+
<groupId>javax.servlet</groupId>
154+
<artifactId>javax.servlet-api</artifactId>
155+
<version>3.1.0</version>
156+
<scope>test</scope>
157+
</dependency>
158+
159+
<dependency>
160+
<groupId>org.springframework.security</groupId>
161+
<artifactId>spring-security-test</artifactId>
162+
<scope>test</scope>
163+
</dependency>
164+
150165
</dependencies>
151166
<description>Deployable package of the OpenID Connect server</description>
152167
</project>

‎openid-connect-server-webapp/src/main/test/java/org/mitre/openid/connect/web/sessionstate/SessionStateManagementControllerTest.java‎

Lines changed: 149 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,149 @@
1+
package org.mitre.openid.connect.web.sessionstate;
2+
3+
import org.junit.Before;
4+
import org.junit.Test;
5+
import org.junit.runner.RunWith;
6+
import org.mitre.openid.connect.config.ConfigurationPropertiesBean;
7+
import org.mitre.openid.connect.view.JsonEntityView;
8+
import org.springframework.beans.factory.annotation.Autowired;
9+
import org.springframework.http.HttpStatus;
10+
import org.springframework.mock.web.MockHttpSession;
11+
import org.springframework.test.context.ContextConfiguration;
12+
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
13+
import org.springframework.test.context.web.WebAppConfiguration;
14+
import org.springframework.test.web.servlet.MockMvc;
15+
import org.springframework.test.web.servlet.MvcResult;
16+
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
17+
import org.springframework.web.context.WebApplicationContext;
18+
19+
import javax.servlet.http.Cookie;
20+
import javax.servlet.http.HttpSession;
21+
22+
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestBuilders.formLogin;
23+
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestBuilders.logout;
24+
import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity;
25+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
26+
import static org.springframework.test.web.servlet.result.MockMvcResultHandlers.print;
27+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content;
28+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.cookie;
29+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.redirectedUrl;
30+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
31+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.view;
32+
33+
@WebAppConfiguration
34+
@ContextConfiguration("file:src/main/webapp/WEB-INF/application-context.xml")
35+
@RunWith(SpringJUnit4ClassRunner.class)
36+
public class SessionStateManagementControllerTest {
37+
38+
@Autowired
39+
private WebApplicationContext wac;
40+
41+
@Autowired
42+
private ConfigurationPropertiesBean config;
43+
44+
private MockMvc mockMvc;
45+
46+
@Before
47+
public void setUp() {
48+
this.mockMvc = MockMvcBuilders
49+
.webAppContextSetup(this.wac)
50+
.apply(springSecurity())
51+
.build();
52+
}
53+
54+
@Test
55+
public void showCheckSession() throws Exception {
56+
mockMvc.perform(get("/" + SessionStateManagementController.FRAME_URL))
57+
.andDo(print())
58+
.andExpect(status().isOk())
59+
.andExpect(view().name(SessionStateManagementController.URL))
60+
;
61+
}
62+
63+
@Test
64+
public void doCheckSession() throws Exception {
65+
mockMvc.perform(get("/" + SessionStateManagementController.VALIDATION_URL))
66+
.andDo(print())
67+
.andExpect(status().isOk())
68+
.andExpect(view().name(JsonEntityView.VIEWNAME))
69+
.andExpect(content().string("\"unchanged\""));
70+
71+
// Login to get a session
72+
MvcResult mvcResult = mockMvc.perform(formLogin("/login").user("user").password("password"))
73+
.andDo(print())
74+
.andExpect(status().is(HttpStatus.FOUND.value()))
75+
.andExpect(redirectedUrl("/"))
76+
.andExpect(cookie().exists(config.getSessionStateCookieName()))
77+
.andReturn();
78+
79+
HttpSession session = mvcResult.getRequest().getSession();
80+
Cookie opssCookie = mvcResult.getResponse().getCookie(config.getSessionStateCookieName());
81+
82+
// Should respond unchanged
83+
mockMvc.perform(get("/" + SessionStateManagementController.VALIDATION_URL)
84+
.session((MockHttpSession)session).cookie(opssCookie))
85+
.andDo(print())
86+
.andExpect(status().isOk())
87+
.andExpect(view().name(JsonEntityView.VIEWNAME))
88+
.andExpect(content().string("\"unchanged\""));
89+
90+
// No Session -> respond with "changed"
91+
mockMvc.perform(get("/" + SessionStateManagementController.VALIDATION_URL)
92+
.cookie(opssCookie))
93+
.andDo(print())
94+
.andExpect(status().isOk())
95+
.andExpect(view().name(JsonEntityView.VIEWNAME))
96+
.andExpect(content().string("\"changed\""));
97+
98+
// No Cookie -> respond with changed
99+
mockMvc.perform(get("/" + SessionStateManagementController.VALIDATION_URL)
100+
.session((MockHttpSession)session))
101+
.andDo(print())
102+
.andExpect(status().isOk())
103+
.andExpect(view().name(JsonEntityView.VIEWNAME))
104+
.andExpect(content().string("\"changed\""));
105+
106+
String tmpVal = opssCookie.getValue();
107+
// Changed cookie value -> changed
108+
opssCookie.setValue("_invalid_");
109+
mockMvc.perform(get("/" + SessionStateManagementController.VALIDATION_URL)
110+
.session((MockHttpSession)session).cookie(opssCookie))
111+
.andDo(print())
112+
.andExpect(status().isOk())
113+
.andExpect(view().name(JsonEntityView.VIEWNAME))
114+
.andExpect(content().string("\"changed\""));
115+
116+
// restore cookie value
117+
opssCookie.setValue(tmpVal);
118+
119+
// Should respond unchanged, just to be sure...
120+
mockMvc.perform(get("/" + SessionStateManagementController.VALIDATION_URL)
121+
.session((MockHttpSession)session).cookie(opssCookie))
122+
.andDo(print())
123+
.andExpect(status().isOk())
124+
.andExpect(view().name(JsonEntityView.VIEWNAME))
125+
.andExpect(content().string("\"unchanged\""));
126+
127+
// Logut to finish the session
128+
HttpSession session1 = mockMvc.perform(logout())
129+
.andDo(print())
130+
.andExpect(status().is(HttpStatus.FOUND.value()))
131+
.andExpect(redirectedUrl("/"))
132+
// expect the session state cookie to be deleted
133+
.andExpect(cookie().exists(config.getSessionStateCookieName()))
134+
.andExpect(cookie().maxAge(config.getSessionStateCookieName(), 0))
135+
.andExpect(cookie().value(config.getSessionStateCookieName(), ""))
136+
.andReturn()
137+
.getRequest()
138+
.getSession();
139+
140+
mockMvc.perform(get("/" + SessionStateManagementController.VALIDATION_URL)
141+
.session((MockHttpSession)session1).cookie(opssCookie))
142+
.andDo(print())
143+
.andExpect(status().isOk())
144+
.andExpect(view().name(JsonEntityView.VIEWNAME))
145+
.andExpect(content().string("\"changed\""));
146+
147+
}
148+
149+
}

‎openid-connect-server-webapp/src/main/webapp/WEB-INF/application-context.xml‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@
8383
<mvc:interceptor>
8484
<mvc:mapping path="/authorize"/>
8585
<!-- Inject the session state into the response -->
86-
<bean id="sessionStateAuthorizationInterceptor" class="org.mitre.openid.connect.web.SessionStateAuthorizationInterceptor" />
86+
<bean id="sessionStateAuthorizationInterceptor" class="org.mitre.openid.connect.web.sessionstate.SessionStateAuthorizationInterceptor" />
8787
</mvc:interceptor>
8888
</mvc:interceptors>
8989

@@ -215,11 +215,11 @@
215215
<security:csrf disabled="true"/>
216216
</security:http>
217217

218-
<security:http pattern="/#{T(org.mitre.openid.connect.web.SessionStateManagementController).URL}**"
218+
<security:http pattern="/#{T(org.mitre.openid.connect.web.sessionstate.SessionStateManagementController).URL}**"
219219
create-session="always"
220220
use-expressions="true"
221221
entry-point-ref="http403EntryPoint" >
222-
<security:intercept-url pattern="/#{T(org.mitre.openid.connect.web.SessionStateManagementController).URL}**" access="permitAll"/>
222+
<security:intercept-url pattern="/#{T(org.mitre.openid.connect.web.sessionstate.SessionStateManagementController).URL}**" access="permitAll"/>
223223
<security:custom-filter ref="corsFilter" after="SECURITY_CONTEXT_FILTER" />
224224
<security:csrf disabled="true"/>
225225
<security:headers disabled="false">

‎openid-connect-server-webapp/src/main/webapp/WEB-INF/views/sessionState.jsp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
<%@ page contentType="text/html;charset=UTF-8" %>
2-
<%@ page import="org.mitre.openid.connect.web.SessionStateManagementController" %>
2+
<%@ page import="org.mitre.openid.connect.web.sessionstate.SessionStateManagementController" %>
33
<!DOCTYPE html>
44
<html>
55
<head>

‎openid-connect-server-webapp/src/main/webapp/resources/template/dynreg.html‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ <h1 data-i18n="client.client-form.edit"></h1>
565565
</div>
566566

567567
<div class="control-group" id="postLogoutRedirectUris">
568-
<label class="control-label"><span class="label label-default nyi"><i class="icon-road icon-white"></i> NYI </span> <span data-i18n="client.client-form.post-logout">Post-Logout Redirect</span></label>
568+
<label class="control-label"><span data-i18n="client.client-form.post-logout">Post-Logout Redirect</span></label>
569569
<div class="controls">
570570
</div>
571571
</div>

‎openid-connect-server/pom.xml‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,13 @@
7272
<artifactId>org.eclipse.persistence.jpa</artifactId>
7373
<scope>test</scope>
7474
</dependency>
75+
<!-- https://mvnrepository.com/artifact/javax.servlet/javax.servlet-api -->
76+
<dependency>
77+
<groupId>javax.servlet</groupId>
78+
<artifactId>javax.servlet-api</artifactId>
79+
<version>3.1.0</version>
80+
<scope>test</scope>
81+
</dependency>
7582

7683
<dependency>
7784
<groupId>org.apache.commons</groupId>

‎openid-connect-server/src/main/java/org/mitre/discovery/web/DiscoveryEndpoint.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@
3838
import org.mitre.openid.connect.web.DynamicClientRegistrationEndpoint;
3939
import org.mitre.openid.connect.web.EndSessionEndpoint;
4040
import org.mitre.openid.connect.web.JWKSetPublishingEndpoint;
41-
import org.mitre.openid.connect.web.SessionStateManagementController;
41+
import org.mitre.openid.connect.web.sessionstate.SessionStateManagementController;
4242
import org.mitre.openid.connect.web.UserInfoEndpoint;
4343
import org.slf4j.Logger;
4444
import org.slf4j.LoggerFactory;
@@ -386,6 +386,7 @@ OPTIONAL. JSON array containing a list of the JWS signing algorithms (alg values
386386

387387
m.put("device_authorization_endpoint", baseUrl + DeviceEndpoint.URL);
388388

389+
// Session management endpoints as defined in the specification
389390
if (config.isSessionStateEnabled()) {
390391
m.put("check_session_iframe", baseUrl + SessionStateManagementController.FRAME_URL);
391392
m.put("end_session_endpoint", baseUrl + EndSessionEndpoint.URL);

‎openid-connect-server/src/main/java/org/mitre/openid/connect/filter/SessionStateManagementFilter.java‎

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
package org.mitre.openid.connect.filter;
22

3-
import org.mitre.openid.connect.session.SessionStateManagementService;
3+
import org.mitre.openid.connect.web.sessionstate.SessionStateManagementService;
44
import org.springframework.beans.factory.annotation.Autowired;
55
import org.springframework.stereotype.Component;
66
import org.springframework.web.filter.OncePerRequestFilter;
@@ -11,22 +11,42 @@
1111
import javax.servlet.http.HttpServletResponse;
1212
import java.io.IOException;
1313

14+
/**
15+
* Filter to check session state on each request.
16+
*
17+
* Checks if the session state has changed (e.g. the session has timed out) and
18+
* writes a new session state cookie of needed.
19+
*
20+
* @author jsinger
21+
*/
22+
1423
@Component("sessionStateManagementFilter")
1524
public class SessionStateManagementFilter extends OncePerRequestFilter {
1625

26+
// the session state management service to be used
1727
private final SessionStateManagementService sessionStateManagementService;
1828

29+
/**
30+
* Constructor for the filter
31+
*
32+
* @param sessionStateManagementService the session state management service to be used
33+
*/
1934
@Autowired
2035
public SessionStateManagementFilter(SessionStateManagementService sessionStateManagementService) {
2136
this.sessionStateManagementService = sessionStateManagementService;
2237
}
2338

39+
/**
40+
* This {@code doFilterInternal} implementation checks if the session state is changed
41+
* and writes a new session state cookie if needed.
42+
*
43+
*/
2444
@Override
2545
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response, FilterChain filterChain) throws ServletException, IOException {
2646

2747
if (sessionStateManagementService.isSessionStateChanged(request)) {
28-
// Session state changed, write a new session state cookie
29-
sessionStateManagementService.processSessionStateCookie(request, response, request.getSession(false));
48+
// Session state changed, write a new session state cookie, try to restore the session value
49+
sessionStateManagementService.writeSessionStateCookie(request, response, request.getSession(false), false);
3050
}
3151
filterChain.doFilter(request, response);
3252
}

‎openid-connect-server/src/main/java/org/mitre/openid/connect/web/AuthenticationTimeStamper.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@
2929
import javax.servlet.http.HttpSession;
3030

3131
import org.mitre.openid.connect.filter.AuthorizationRequestFilter;
32-
import org.mitre.openid.connect.session.SessionStateManagementService;
32+
import org.mitre.openid.connect.web.sessionstate.SessionStateManagementService;
3333
import org.slf4j.Logger;
3434
import org.slf4j.LoggerFactory;
3535
import org.springframework.beans.factory.annotation.Autowired;
@@ -81,7 +81,7 @@ public void onAuthenticationSuccess(HttpServletRequest request, HttpServletRespo
8181
}
8282

8383
// Set additional session state cookie for tracking session changes with session management extensions
84-
sessionStateManagementService.processSessionStateCookie(request, response, session);
84+
sessionStateManagementService.writeSessionStateCookie(request, response, session);
8585

8686
logger.info("Successful Authentication of " + authentication.getName() + " at " + authTimestamp.toString());
8787

‎openid-connect-server/src/main/java/org/mitre/openid/connect/web/SessionStateAuthorizationInterceptor.java‎ renamed to ‎openid-connect-server/src/main/java/org/mitre/openid/connect/web/sessionstate/SessionStateAuthorizationInterceptor.java‎

Lines changed: 43 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
1-
package org.mitre.openid.connect.web;
1+
package org.mitre.openid.connect.web.sessionstate;
22

3-
import org.mitre.openid.connect.session.SessionStateManagementService;
3+
import com.google.common.base.Strings;
44
import org.springframework.beans.factory.annotation.Autowired;
55
import org.springframework.security.oauth2.common.util.OAuth2Utils;
66
import org.springframework.util.MultiValueMap;
@@ -12,31 +12,66 @@
1212
import javax.servlet.http.HttpServletRequest;
1313
import javax.servlet.http.HttpServletResponse;
1414

15+
/**
16+
* Interceptor to append the session state parameter to an
17+
* successful authorization response.
18+
*
19+
* Should be applied to the authorization url only.
20+
*
21+
* @author jsinger
22+
*/
1523
public class SessionStateAuthorizationInterceptor extends HandlerInterceptorAdapter {
1624

25+
// The session state management service to use
1726
private final SessionStateManagementService sessionStateManagementService;
1827

28+
/**
29+
* Constructor for the session state management interceptor
30+
*
31+
* @param sessionStateManagementService The session state management service to use
32+
*/
1933
@Autowired
2034
public SessionStateAuthorizationInterceptor(SessionStateManagementService sessionStateManagementService) {
2135
this.sessionStateManagementService = sessionStateManagementService;
2236
}
2337

38+
/**
39+
* Add session state parameter to successful authorization response.
40+
*
41+
* Checks if the authorization request has generated a successful
42+
* response containing the "code" parameter in the redirect url and
43+
* adds the session state parameter accordingly.
44+
*
45+
* @param request The http request
46+
* @param response The http response
47+
* @param handler The servlet handler
48+
* @param modelAndView The returned ModelAndView
49+
* @throws Exception if any exception occurs
50+
*/
2451
@Override
2552
public void postHandle(HttpServletRequest request, HttpServletResponse response, Object handler, ModelAndView modelAndView) throws Exception {
26-
// Only on redirects...
53+
// Apply only on redirects
2754
if (sessionStateManagementService.isEnabled() && modelAndView != null && (modelAndView.getView() instanceof RedirectView)) {
28-
// Parse the returned url
55+
// Parse the redirect url
2956
RedirectView view = (RedirectView) modelAndView.getView();
3057
UriComponentsBuilder uriComponentsBuilder = UriComponentsBuilder.fromUriString(view.getUrl());
3158
MultiValueMap<String, String> parameters = uriComponentsBuilder.build().getQueryParams();
32-
// Only if a code or token is returned
33-
if (parameters.containsKey("code") || parameters.containsKey("access_token")) {
59+
String fragment = Strings.nullToEmpty(uriComponentsBuilder.build().getFragment());
60+
// Only if a code or an access token is returned
61+
if (parameters.containsKey("code") ||fragment.contains("access_token")) {
3462
// get the current session state value
3563
String sessionState = sessionStateManagementService.getSessionState(request);
3664
if (sessionState != null) {
37-
// and append it to the query parameters
65+
// build the hash
3866
String stateParam = sessionStateManagementService.buildSessionStateParam(sessionState, request.getParameter(OAuth2Utils.CLIENT_ID), view.getUrl());
39-
uriComponentsBuilder.queryParam(SessionStateManagementService.SESSION_STATE_PARAM, stateParam);
67+
if (parameters.containsKey("code")) {
68+
// and append to the query string or ...
69+
uriComponentsBuilder.queryParam(SessionStateManagementService.SESSION_STATE_PARAM, stateParam);
70+
} else {
71+
// the fragment.
72+
uriComponentsBuilder.fragment(fragment + "&" + SessionStateManagementService.SESSION_STATE_PARAM + "=" + stateParam);
73+
}
74+
// set the new url as redirect url
4075
view.setUrl(uriComponentsBuilder.build().toUriString());
4176
}
4277
}

0 commit comments

Comments
 (0)