James Moger
2014-01-28 7ab32b65fcb20ca68d7afc357befb3a34de662bf
issue-361: Reset user cookie after administrative password change

Cookies were not reset on administrative password change of a user
account. This allowed accounts with changed passwords to continue
authenticating. Cookies are now reset on password changes, they are
validated on each page request, AND they will now expire 7 days after
generation.
8 files modified
110 ■■■■ changed files
releases.moxie 7 ●●●● patch | view | raw | blame | history
src/main/java/com/gitblit/ConfigUserService.java 3 ●●●●● patch | view | raw | blame | history
src/main/java/com/gitblit/client/EditUserDialog.java 3 ●●●●● patch | view | raw | blame | history
src/main/java/com/gitblit/manager/AuthenticationManager.java 61 ●●●●● patch | view | raw | blame | history
src/main/java/com/gitblit/manager/GitblitManager.java 5 ●●●●● patch | view | raw | blame | history
src/main/java/com/gitblit/manager/IAuthenticationManager.java 8 ●●●●● patch | view | raw | blame | history
src/main/java/com/gitblit/wicket/pages/EditUserPage.java 3 ●●●●● patch | view | raw | blame | history
src/main/java/com/gitblit/wicket/pages/SessionPage.java 20 ●●●●● patch | view | raw | blame | history
releases.moxie
@@ -8,7 +8,12 @@
    note: "The default access restriction has been elevated from NONE to PUSH and anonymous push access has been disabled."
    html: ~
    text: ~
    security: ~
    security:
    - ''issue-361: Cookies were not reset on administrative password change of a user account.
        This allowed accounts with changed passwords to continue authenticating.
        Cookies are now reset on password changes, they are validated on each page request,
        AND they will now expire 7 days after generation.
        ''
    fixes:
    - Fixed incorrect tagger attribution in the dashboard (issue-276)
    - Fixed support for implied SSH urls in web.otherUrls (issue-311)
src/main/java/com/gitblit/ConfigUserService.java
@@ -272,6 +272,9 @@
            }
            read();
            originalUser = users.remove(username.toLowerCase());
            if (originalUser != null) {
                cookies.remove(originalUser.cookie);
            }
            users.put(model.username.toLowerCase(), model);
            // null check on "final" teams because JSON-sourced UserModel
            // can have a null teams object
src/main/java/com/gitblit/client/EditUserDialog.java
@@ -325,6 +325,9 @@
                return false;
            }
            // change the cookie
            user.cookie = StringUtils.getSHA1(user.username + password);
            String type = settings.get(Keys.realm.passwordStorage).getString("md5");
            if (type.equalsIgnoreCase("md5")) {
                // store MD5 digest of password
src/main/java/com/gitblit/manager/AuthenticationManager.java
@@ -22,6 +22,7 @@
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import javax.servlet.http.Cookie;
import javax.servlet.http.HttpServletRequest;
@@ -235,13 +236,18 @@
            return null;
        }
        UserModel user = null;
        // try to authenticate by cookie
        UserModel user = authenticate(httpRequest.getCookies());
        if (user != null) {
            flagWicketSession(AuthenticationType.COOKIE);
            logger.debug(MessageFormat.format("{0} authenticated by cookie from {1}",
        String cookie = getCookie(httpRequest);
        if (!StringUtils.isEmpty(cookie)) {
            user = userManager.getUserModel(cookie.toCharArray());
            if (user != null) {
                flagWicketSession(AuthenticationType.COOKIE);
                logger.debug(MessageFormat.format("{0} authenticated by cookie from {1}",
                    user.username, httpRequest.getRemoteAddr()));
            return user;
                return user;
            }
        }
        // try to authenticate by BASIC
@@ -266,26 +272,6 @@
                } else {
                    logger.warn(MessageFormat.format("Failed login attempt for {0}, invalid credentials from {1}",
                            username, httpRequest.getRemoteAddr()));
                }
            }
        }
        return null;
    }
    /**
     * Authenticate a user based on their cookie.
     *
     * @param cookies
     * @return a user object or null
     */
    protected UserModel authenticate(Cookie[] cookies) {
        if (settings.getBoolean(Keys.web.allowCookieAuthentication, true)) {
            if (cookies != null && cookies.length > 0) {
                for (Cookie cookie : cookies) {
                    if (cookie.getName().equals(Constants.NAME)) {
                        String value = cookie.getValue();
                        return userManager.getUserModel(value.toCharArray());
                    }
                }
            }
        }
@@ -365,6 +351,28 @@
    }
    /**
     * Returns the Gitlbit cookie in the request.
     *
     * @param request
     * @return the Gitblit cookie for the request or null if not found
     */
    @Override
    public String getCookie(HttpServletRequest request) {
        if (settings.getBoolean(Keys.web.allowCookieAuthentication, true)) {
            Cookie[] cookies = request.getCookies();
            if (cookies != null && cookies.length > 0) {
                for (Cookie cookie : cookies) {
                    if (cookie.getName().equals(Constants.NAME)) {
                        String value = cookie.getValue();
                        return value;
                    }
                }
            }
        }
        return null;
    }
    /**
     * Sets a cookie for the specified user.
     *
     * @param response
@@ -390,7 +398,8 @@
                    } else {
                        // create real cookie
                        userCookie = new Cookie(Constants.NAME, cookie);
                        userCookie.setMaxAge(Integer.MAX_VALUE);
                        // expire the cookie in 7 days
                        userCookie.setMaxAge((int) TimeUnit.DAYS.toSeconds(7));
                    }
                }
                userCookie.setPath("/");
src/main/java/com/gitblit/manager/GitblitManager.java
@@ -634,6 +634,11 @@
    }
    @Override
    public String getCookie(HttpServletRequest request) {
        return authenticationManager.getCookie(request);
    }
    @Override
    public void setCookie(HttpServletResponse response, UserModel user) {
        authenticationManager.setCookie(response, user);
    }
src/main/java/com/gitblit/manager/IAuthenticationManager.java
@@ -56,6 +56,14 @@
    UserModel authenticate(String username, char[] password);
    /**
     * Returns the Gitlbit cookie in the request.
     *
     * @param request
     * @return the Gitblit cookie for the request or null if not found
     */
    String getCookie(HttpServletRequest request);
    /**
     * Sets a cookie for the specified user.
     *
     * @param response
src/main/java/com/gitblit/wicket/pages/EditUserPage.java
@@ -154,6 +154,9 @@
                            return;
                        }
                        // change the cookie
                        userModel.cookie = StringUtils.getSHA1(userModel.username + password);
                        // Optionally store the password MD5 digest.
                        String type = app().settings().getString(Keys.realm.passwordStorage, "md5");
                        if (type.equalsIgnoreCase("md5")) {
src/main/java/com/gitblit/wicket/pages/SessionPage.java
@@ -16,6 +16,7 @@
package com.gitblit.wicket.pages;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import org.apache.wicket.PageParameters;
import org.apache.wicket.markup.html.WebPage;
@@ -24,6 +25,7 @@
import com.gitblit.Keys;
import com.gitblit.models.UserModel;
import com.gitblit.utils.StringUtils;
import com.gitblit.wicket.GitBlitWebApp;
import com.gitblit.wicket.GitBlitWebSession;
@@ -53,6 +55,24 @@
            // 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);
            // validate cookie during session (issue-361)
            if (app().settings().getBoolean(Keys.web.allowCookieAuthentication, true)) {
                HttpServletRequest request = ((WebRequest) getRequestCycle().getRequest())
                        .getHttpServletRequest();
                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
                        HttpServletResponse response = ((WebResponse) getRequestCycle().getResponse())
                                .getHttpServletResponse();
                        app().authentication().logout(response, user);
                        session.setUser(null);
                        session.invalidateNow();
                        return;
                    }
                }
            }
            session.setUser(user);
            return;
        }