Skip to content

Commit

Permalink
Fix issue with changing session ID on login
Browse files Browse the repository at this point in the history
  • Loading branch information
@stuartwdouglas
stuartwdouglas committed Nov 25, 2015
1 parent 65882ba commit 00d988b
Show file tree
Hide file tree
Showing 3 changed files with 54 additions and 53 deletions.
19 changes: 0 additions & 19 deletions core/src/main/java/io/undertow/server/session/SessionAttachmentHandler.java
View file
Expand Up @@ -20,9 +20,6 @@

import io.undertow.Handlers;
import io.undertow.UndertowMessages;
import io.undertow.security.api.NotificationReceiver;
import io.undertow.security.api.SecurityContext;
import io.undertow.security.api.SecurityNotification;
import io.undertow.server.ExchangeCompletionListener;
import io.undertow.server.HttpHandler;
import io.undertow.server.HttpServerExchange;
Expand All @@ -45,18 +42,6 @@ public class SessionAttachmentHandler implements HttpHandler {

private final SessionConfig sessionConfig;

private final NotificationReceiver RECEIVER = new NotificationReceiver() {
@Override
public void handleNotification(SecurityNotification notification) {
if(notification.getEventType() == SecurityNotification.EventType.AUTHENTICATED) {
Session sc = sessionManager.getSession(notification.getExchange(), sessionConfig);
if(sc != null) {
sc.changeSessionId(notification.getExchange(), sessionConfig);
}
}
}
};

public SessionAttachmentHandler(final SessionManager sessionManager, final SessionConfig sessionConfig) {
this.sessionConfig = sessionConfig;
if (sessionManager == null) {
Expand All @@ -78,10 +63,6 @@ public SessionAttachmentHandler(final HttpHandler next, final SessionManager ses
public void handleRequest(final HttpServerExchange exchange) throws Exception {
exchange.putAttachment(SessionManager.ATTACHMENT_KEY, sessionManager);
exchange.putAttachment(SessionConfig.ATTACHMENT_KEY, sessionConfig);
SecurityContext sc = exchange.getSecurityContext();
if(sc != null) {
sc.registerNotificationReceiver(RECEIVER);
}
final UpdateLastAccessTimeListener handler = new UpdateLastAccessTimeListener(sessionConfig, sessionManager);
exchange.addExchangeCompleteListener(handler);
next.handleRequest(exchange);
Expand Down
33 changes: 26 additions & 7 deletions servlet/src/main/java/io/undertow/servlet/api/DeploymentInfo.java
View file
Expand Up @@ -168,6 +168,8 @@ public class DeploymentInfo implements Cloneable {
*/
private int contentTypeCacheSize = 100;

private boolean changeSessionIdOnLogin = true;

private SessionIdGenerator sessionIdGenerator = new SecureRandomSessionIdGenerator();

public void validate() {
Expand Down Expand Up @@ -1017,16 +1019,18 @@ public AuthenticationMechanism getJaspiAuthenticationMechanism() {
return jaspiAuthenticationMechanism;
}

public void setJaspiAuthenticationMechanism(AuthenticationMechanism jaspiAuthenticationMechanism) {
public DeploymentInfo setJaspiAuthenticationMechanism(AuthenticationMechanism jaspiAuthenticationMechanism) {
this.jaspiAuthenticationMechanism = jaspiAuthenticationMechanism;
return this;
}

public SecurityContextFactory getSecurityContextFactory() {
return this.securityContextFactory;
}

public void setSecurityContextFactory(final SecurityContextFactory securityContextFactory) {
public DeploymentInfo setSecurityContextFactory(final SecurityContextFactory securityContextFactory) {
this.securityContextFactory = securityContextFactory;
return this;
}

public String getServerName() {
Expand Down Expand Up @@ -1060,8 +1064,9 @@ public boolean isDisableCachingForSecuredPages() {
return disableCachingForSecuredPages;
}

public void setDisableCachingForSecuredPages(boolean disableCachingForSecuredPages) {
public DeploymentInfo setDisableCachingForSecuredPages(boolean disableCachingForSecuredPages) {
this.disableCachingForSecuredPages = disableCachingForSecuredPages;
return this;
}

public DeploymentInfo addLifecycleInterceptor(final LifecycleInterceptor interceptor) {
Expand Down Expand Up @@ -1102,8 +1107,9 @@ public boolean isEscapeErrorMessage() {
*
* @param escapeErrorMessage If the error message should be escaped
*/
public void setEscapeErrorMessage(boolean escapeErrorMessage) {
public DeploymentInfo setEscapeErrorMessage(boolean escapeErrorMessage) {
this.escapeErrorMessage = escapeErrorMessage;
return this;
}


Expand Down Expand Up @@ -1140,24 +1146,27 @@ public MultipartConfigElement getDefaultMultipartConfig() {
return defaultMultipartConfig;
}

public void setDefaultMultipartConfig(MultipartConfigElement defaultMultipartConfig) {
public DeploymentInfo setDefaultMultipartConfig(MultipartConfigElement defaultMultipartConfig) {
this.defaultMultipartConfig = defaultMultipartConfig;
return this;
}

public int getContentTypeCacheSize() {
return contentTypeCacheSize;
}

public void setContentTypeCacheSize(int contentTypeCacheSize) {
public DeploymentInfo setContentTypeCacheSize(int contentTypeCacheSize) {
this.contentTypeCacheSize = contentTypeCacheSize;
return this;
}

public SessionIdGenerator getSessionIdGenerator() {
return sessionIdGenerator;
}

public void setSessionIdGenerator(SessionIdGenerator sessionIdGenerator) {
public DeploymentInfo setSessionIdGenerator(SessionIdGenerator sessionIdGenerator) {
this.sessionIdGenerator = sessionIdGenerator;
return this;
}


Expand All @@ -1178,6 +1187,15 @@ public DeploymentInfo setSendCustomReasonPhraseOnError(boolean sendCustomReasonP
return this;
}

public boolean isChangeSessionIdOnLogin() {
return changeSessionIdOnLogin;
}

public DeploymentInfo setChangeSessionIdOnLogin(boolean changeSessionIdOnLogin) {
this.changeSessionIdOnLogin = changeSessionIdOnLogin;
return this;
}

@Override
public DeploymentInfo clone() {
final DeploymentInfo info = new DeploymentInfo()
Expand Down Expand Up @@ -1259,6 +1277,7 @@ public DeploymentInfo clone() {
info.contentTypeCacheSize = contentTypeCacheSize;
info.sessionIdGenerator = sessionIdGenerator;
info.sendCustomReasonPhraseOnError = sendCustomReasonPhraseOnError;
info.changeSessionIdOnLogin = changeSessionIdOnLogin;
return info;
}

Expand Down
55 changes: 28 additions & 27 deletions ...rc/main/java/io/undertow/servlet/handlers/security/CachedAuthenticatedSessionHandler.java
View file
Expand Up @@ -47,6 +47,8 @@ public class CachedAuthenticatedSessionHandler implements HttpHandler {

public static final String ATTRIBUTE_NAME = CachedAuthenticatedSessionHandler.class.getName() + ".AuthenticatedSession";

public static final String NO_ID_CHANGE_REQUIRED = CachedAuthenticatedSessionHandler.class.getName() + ".NoIdChangeRequired";

private final NotificationReceiver NOTIFICATION_RECEIVER = new SecurityNotificationReceiver();
private final AuthenticatedSessionManager SESSION_MANAGER = new ServletAuthenticatedSessionManager();

Expand Down Expand Up @@ -83,20 +85,23 @@ public void handleNotification(SecurityNotification notification) {
HttpSessionImpl httpSession = servletContext.getSession(notification.getExchange(), false);
switch (eventType) {
case AUTHENTICATED:
if(httpSession != null && !httpSession.isNew() && !httpSession.isInvalid()) {
ServletRequestContext src = notification.getExchange().getAttachment(ServletRequestContext.ATTACHMENT_KEY);
src.getOriginalRequest().changeSessionId();
if(servletContext.getDeployment().getDeploymentInfo().isChangeSessionIdOnLogin()) {
if (httpSession != null) {
Session session = underlyingSession(httpSession);
if (!httpSession.isNew() &&
!httpSession.isInvalid() &&
session.getAttribute(NO_ID_CHANGE_REQUIRED) == null) {
ServletRequestContext src = notification.getExchange().getAttachment(ServletRequestContext.ATTACHMENT_KEY);
src.getOriginalRequest().changeSessionId();
}
session.setAttribute(NO_ID_CHANGE_REQUIRED, true);
}
}
if (isCacheable(notification)) {
if(httpSession == null) {
httpSession = servletContext.getSession(notification.getExchange(), true);
}
Session session;
if(System.getSecurityManager() == null) {
session = httpSession.getSession();
} else {
session = AccessController.doPrivileged(new HttpSessionImpl.UnwrapSessionAction(httpSession));
}
Session session = underlyingSession(httpSession);

// It is normal for this notification to be received when using a previously cached session - in that
// case the IDM would have been given an opportunity to re-load the Account so updating here ready for
Expand All @@ -107,32 +112,33 @@ public void handleNotification(SecurityNotification notification) {
break;
case LOGGED_OUT:
if (httpSession != null) {
Session session;
if (System.getSecurityManager() == null) {
session = httpSession.getSession();
} else {
session = AccessController.doPrivileged(new HttpSessionImpl.UnwrapSessionAction(httpSession));
}
Session session = underlyingSession(httpSession);
session.removeAttribute(ATTRIBUTE_NAME);
session.removeAttribute(NO_ID_CHANGE_REQUIRED);
}
break;
}
}

}

protected Session underlyingSession(HttpSessionImpl httpSession) {
Session session;
if (System.getSecurityManager() == null) {
session = httpSession.getSession();
} else {
session = AccessController.doPrivileged(new HttpSessionImpl.UnwrapSessionAction(httpSession));
}
return session;
}

private class ServletAuthenticatedSessionManager implements AuthenticatedSessionManager {

@Override
public AuthenticatedSession lookupSession(HttpServerExchange exchange) {
HttpSessionImpl httpSession = servletContext.getSession(exchange, false);
if (httpSession != null) {
Session session;
if (System.getSecurityManager() == null) {
session = httpSession.getSession();
} else {
session = AccessController.doPrivileged(new HttpSessionImpl.UnwrapSessionAction(httpSession));
}
Session session = underlyingSession(httpSession);
return (AuthenticatedSession) session.getAttribute(ATTRIBUTE_NAME);
}
return null;
Expand All @@ -142,12 +148,7 @@ public AuthenticatedSession lookupSession(HttpServerExchange exchange) {
public void clearSession(HttpServerExchange exchange) {
HttpSessionImpl httpSession = servletContext.getSession(exchange, false);
if (httpSession != null) {
Session session;
if (System.getSecurityManager() == null) {
session = httpSession.getSession();
} else {
session = AccessController.doPrivileged(new HttpSessionImpl.UnwrapSessionAction(httpSession));
}
Session session = underlyingSession(httpSession);
session.removeAttribute(ATTRIBUTE_NAME);
}
}
Expand Down

0 comments on commit 00d988b

Please sign in to comment.