James Moger
2015-09-18 03223516b32758fe9d5602ffb4ce10a8a308f0e9
Merge pull request #908 from mrjoel/mrjoel-authrequestnotsession

prevent session fixation for external authentication
4 files modified
113 ■■■■■ changed files
src/main/java/com/gitblit/Constants.java 4 ●●● patch | view | raw | blame | history
src/main/java/com/gitblit/manager/AuthenticationManager.java 35 ●●●● patch | view | raw | blame | history
src/main/java/com/gitblit/wicket/pages/RootPage.java 6 ●●●● patch | view | raw | blame | history
src/main/java/com/gitblit/wicket/pages/SessionPage.java 68 ●●●●● patch | view | raw | blame | history
src/main/java/com/gitblit/Constants.java
@@ -137,7 +137,9 @@
    public static final String DEVELOP = "develop";
    public static final String AUTHENTICATION_TYPE = "authentication-type";
    public static final String ATTRIB_AUTHTYPE = NAME + ":authentication-type";
    public static final String ATTRIB_AUTHUSER = NAME + ":authenticated-user";
    public static String getVersion() {
        String v = Constants.class.getPackage().getImplementationVersion();
src/main/java/com/gitblit/manager/AuthenticationManager.java
@@ -194,6 +194,14 @@
     */
    @Override
    public UserModel authenticate(HttpServletRequest httpRequest, boolean requiresCertificate) {
        // Check if this request has already been authenticated, and trust that instead of re-processing
        String reqAuthUser = (String) httpRequest.getAttribute(Constants.ATTRIB_AUTHUSER);
        if (!StringUtils.isEmpty(reqAuthUser)) {
            logger.warn("Called servlet authenticate when request is already authenticated.");
            return userManager.getUserModel(reqAuthUser);
        }
        // try to authenticate by servlet container principal
        if (!requiresCertificate) {
            Principal principal = httpRequest.getUserPrincipal();
@@ -204,7 +212,7 @@
                    UserModel user = userManager.getUserModel(username);
                    if (user != null) {
                        // existing user
                        flagSession(httpRequest, AuthenticationType.CONTAINER);
                        flagRequest(httpRequest, AuthenticationType.CONTAINER, user.username);
                        logger.debug(MessageFormat.format("{0} authenticated by servlet container principal from {1}",
                                user.username, httpRequest.getRemoteAddr()));
                        return validateAuthentication(user, AuthenticationType.CONTAINER);
@@ -239,7 +247,7 @@
                        }
                        
                        userManager.updateUserModel(user);
                        flagSession(httpRequest, AuthenticationType.CONTAINER);
                        flagRequest(httpRequest, AuthenticationType.CONTAINER, user.username);
                        logger.debug(MessageFormat.format("{0} authenticated and created by servlet container principal from {1}",
                                user.username, httpRequest.getRemoteAddr()));
                        return validateAuthentication(user, AuthenticationType.CONTAINER);
@@ -260,7 +268,7 @@
            UserModel user = userManager.getUserModel(model.username);
            X509Metadata metadata = HttpUtils.getCertificateMetadata(httpRequest);
            if (user != null) {
                flagSession(httpRequest, AuthenticationType.CERTIFICATE);
                flagRequest(httpRequest, AuthenticationType.CERTIFICATE, user.username);
                logger.debug(MessageFormat.format("{0} authenticated by client certificate {1} from {2}",
                        user.username, metadata.serialNumber, httpRequest.getRemoteAddr()));
                return validateAuthentication(user, AuthenticationType.CERTIFICATE);
@@ -282,7 +290,7 @@
        if (!StringUtils.isEmpty(cookie)) {
            user = userManager.getUserModel(cookie.toCharArray());
            if (user != null) {
                flagSession(httpRequest, AuthenticationType.COOKIE);
                flagRequest(httpRequest, AuthenticationType.COOKIE, user.username);
                logger.debug(MessageFormat.format("{0} authenticated by cookie from {1}",
                    user.username, httpRequest.getRemoteAddr()));
                return validateAuthentication(user, AuthenticationType.COOKIE);
@@ -304,7 +312,7 @@
                char[] password = values[1].toCharArray();
                user = authenticate(username, password);
                if (user != null) {
                    flagSession(httpRequest, AuthenticationType.CREDENTIALS);
                    flagRequest(httpRequest, AuthenticationType.CREDENTIALS, user.username);
                    logger.debug(MessageFormat.format("{0} authenticated by BASIC request header from {1}",
                            user.username, httpRequest.getRemoteAddr()));
                    return validateAuthentication(user, AuthenticationType.CREDENTIALS);
@@ -423,8 +431,9 @@
        return user;
    }
    protected void flagSession(HttpServletRequest httpRequest, AuthenticationType authenticationType) {
        httpRequest.getSession().setAttribute(Constants.AUTHENTICATION_TYPE, authenticationType);
    protected void flagRequest(HttpServletRequest httpRequest, AuthenticationType authenticationType, String authedUsername) {
        httpRequest.setAttribute(Constants.ATTRIB_AUTHUSER,  authedUsername);
        httpRequest.setAttribute(Constants.ATTRIB_AUTHTYPE,  authenticationType);
    }
    /**
@@ -545,9 +554,15 @@
    @Override
    public void setCookie(HttpServletRequest request, HttpServletResponse response, UserModel user) {
        if (settings.getBoolean(Keys.web.allowCookieAuthentication, true)) {
            HttpSession session = request.getSession();
            AuthenticationType authenticationType = (AuthenticationType) session.getAttribute(Constants.AUTHENTICATION_TYPE);
            boolean standardLogin = authenticationType.isStandard();
            boolean standardLogin = true;
            if (null != request) {
                // Pull the auth type from the request, it is set there if container managed
                AuthenticationType authenticationType = (AuthenticationType) request.getAttribute(Constants.ATTRIB_AUTHTYPE);
                if (null != authenticationType)
                    standardLogin = authenticationType.isStandard();
            }
            if (standardLogin) {
                Cookie userCookie;
src/main/java/com/gitblit/wicket/pages/RootPage.java
@@ -279,7 +279,7 @@
            request = ((WebRequest) getRequest()).getHttpServletRequest();
            response = ((WebResponse) getResponse()).getHttpServletResponse();
            request.getSession().setAttribute(Constants.AUTHENTICATION_TYPE, AuthenticationType.CREDENTIALS);
            request.getSession().setAttribute(Constants.ATTRIB_AUTHTYPE, AuthenticationType.CREDENTIALS);
            // Set Cookie
            app().authentication().setCookie(request, response, user);
@@ -608,8 +608,8 @@
            UserModel user = session.getUser();
            boolean editCredentials = app().authentication().supportsCredentialChanges(user);
            HttpServletRequest request = ((WebRequest) getRequest()).getHttpServletRequest();
            AuthenticationType authenticationType = (AuthenticationType) request.getSession().getAttribute(Constants.AUTHENTICATION_TYPE);
            boolean standardLogin = authenticationType.isStandard();
            AuthenticationType authenticationType = (AuthenticationType) request.getAttribute(Constants.ATTRIB_AUTHTYPE);
            boolean standardLogin = (null != authenticationType) ? authenticationType.isStandard() : true;
            if (app().settings().getBoolean(Keys.web.allowGravatar, true)) {
                add(new AvatarImage("username", user, "navbarGravatar", 20, false));
src/main/java/com/gitblit/wicket/pages/SessionPage.java
@@ -56,34 +56,50 @@
        HttpServletRequest request = ((WebRequest) getRequest()).getHttpServletRequest();
        HttpServletResponse response = ((WebResponse) getResponse()).getHttpServletResponse();
        if (session.isLoggedIn() && !session.isSessionInvalidated()) {
            // already have a session, refresh usermodel to pick up
            // any changes to permissions or roles (issue-186)
            UserModel user = app().users().getUserModel(session.getUser().username);
        // If using container/external servlet authentication, use request attribute
        String authedUser = (String) request.getAttribute(Constants.ATTRIB_AUTHUSER);
            if (user == null || user.disabled) {
                // user was deleted/disabled during session
                app().authentication().logout(request, response, user);
                session.setUser(null);
                session.invalidateNow();
                return;
        // Default to trusting session authentication if not set in request by external processing
        if (StringUtils.isEmpty(authedUser) && session.isLoggedIn()) {
            authedUser = session.getUsername();
        }
        if (!StringUtils.isEmpty(authedUser)) {
            // Avoid session fixation for non-session authentication
            // If the authenticated user is different from the session user, discard
            // the old session entirely, without trusting any session values
            if (!authedUser.equals(session.getUsername())) {
                session.replaceSession();
            }
            // validate cookie during session (issue-361)
            if (user != null && app().settings().getBoolean(Keys.web.allowCookieAuthentication, true)) {
                String requestCookie = app().authentication().getCookie(request);
                if (!StringUtils.isEmpty(requestCookie) && !StringUtils.isEmpty(user.cookie)) {
                    if (!requestCookie.equals(user.cookie)) {
                        // cookie was changed during our session
                        app().authentication().logout(request, response, user);
                        session.setUser(null);
                        session.invalidateNow();
                        return;
            if (!session.isSessionInvalidated()) {
                // Refresh usermodel to pick up any changes to permissions or roles (issue-186)
                UserModel user = app().users().getUserModel(authedUser);
                if (user == null || user.disabled) {
                    // user was deleted/disabled during session
                    app().authentication().logout(request, response, user);
                    session.setUser(null);
                    session.invalidateNow();
                    return;
                }
                // validate cookie during session (issue-361)
                if (app().settings().getBoolean(Keys.web.allowCookieAuthentication, true)) {
                    String requestCookie = app().authentication().getCookie(request);
                    if (!StringUtils.isEmpty(requestCookie) && !StringUtils.isEmpty(user.cookie)) {
                        if (!requestCookie.equals(user.cookie)) {
                            // cookie was changed during our session
                            app().authentication().logout(request, response, user);
                            session.setUser(null);
                            session.invalidateNow();
                            return;
                        }
                    }
                }
                session.setUser(user);
                return;
            }
            session.setUser(user);
            return;
        }
        // try to authenticate by servlet request
@@ -91,9 +107,7 @@
        // Login the user
        if (user != null) {
            // preserve the authentication type across session replacement
            AuthenticationType authenticationType = (AuthenticationType) request.getSession()
                    .getAttribute(Constants.AUTHENTICATION_TYPE);
            AuthenticationType authenticationType = (AuthenticationType) request.getAttribute(Constants.ATTRIB_AUTHTYPE);
            // issue 62: fix session fixation vulnerability
            // but only if authentication was done in the container.
@@ -101,10 +115,8 @@
            // don't like
            if (AuthenticationType.CONTAINER != authenticationType) {
                session.replaceSession();
            }
            }
            session.setUser(user);
            request.getSession().setAttribute(Constants.AUTHENTICATION_TYPE, authenticationType);
            // Set Cookie
            app().authentication().setCookie(request, response, user);