[Yanel-dev] AutoLogin improved (important bug fix!)

basZero baszero at gmail.com
Fri Aug 5 15:20:07 CEST 2011


Dear Michael,

I have made the AutoLogin more stable in the use case where the browser send
multiple cookies of the same name.

This can happen, if the application issues different cookies for different
contexts, e.g.
Cookie 1: path = "/"
Cookie 2: path = "/archive/
Cookie 3: path = "/archive/2011/

In case the user opens the page to archive/2011 directly, the browser sends
3 cookies with the request.
The current logic only checks the first cookie in the request, and if the
autologin does not work, the user is logged out again.

I have now extended the AutoLogin class so that it loops over ALL
Autologin-Cookies until it finds a valid one.

The patch is attached.

IMPORTANT side note: This is also a critical update to an application that
issues only one cookie path, e.g. all for "/", but with different values.
If the user clicks LOGOUT, the token/cookie gets deleted on the server side,
but the browser does not delete the cookie (e.g. Google Chrome never deletes
cookies, even if you tell it to do so).
So as soon as AutoLogin is enabled again by the user, it might not work
correctly, because it processes only one of the cookies in the request,
which might be the old one...

Cheers
Balz
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.wyona.org/pipermail/yanel-development/attachments/20110805/314cca4a/attachment.html>
-------------- next part --------------
Index: src/webapp/src/java/org/wyona/yanel/servlet/security/impl/AutoLogin.java
===================================================================
--- src/webapp/src/java/org/wyona/yanel/servlet/security/impl/AutoLogin.java	(revision 59749)
+++ src/webapp/src/java/org/wyona/yanel/servlet/security/impl/AutoLogin.java	(working copy)
@@ -1,8 +1,10 @@
 package org.wyona.yanel.servlet.security.impl;
 
 import java.text.SimpleDateFormat;
+import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Date;
+import java.util.List;
 import java.util.UUID;
 
 import javax.servlet.http.Cookie;
@@ -40,22 +42,27 @@
     // However: this expiry date is only verified and maybe replaced if the user starts a new session.
     // Means: if the session timeout is 4h and you configure here 30min, the cookie token won't be replaced within the session.
     private static final int TOKEN_EXPIRY_UNIT = Calendar.MINUTE;
-    private static final int TOKEN_EXPIRY_AMOUNT = 30;
+    private static final int TOKEN_EXPIRY_AMOUNT = 480;
 
     /**
      * Disable Auto Login feature (deletes cookie)
      */
     public static void disableAutoLogin(HttpServletRequest request, HttpServletResponse response, Repository repo) {
-        Cookie currentCookie = getCookie(request);
-        if (currentCookie != null) {
-            deleteToken(repo, getYarepPath(getUsername(currentCookie), getToken(currentCookie)));
-
-            log.warn("DEBUG: Remove auto login cookie...");
-            Cookie newCookie = new Cookie(COOKIE_NAME, null);
-            newCookie.setMaxAge(0); // INFO: A zero value tells the browser to delete the cookie immediately.
-            response.addCookie(newCookie);
+        Cookie[] currentCookies = getCookies(request);
+        if (currentCookies.length > 0) {
+            for (Cookie cookie: currentCookies) {
+                deleteToken(repo, getYarepPath(getUsername(cookie), getToken(cookie)));
+                log.warn("Remove auto login cookie for user '"+getUsername(cookie)+"', path = '"+cookie.getPath()+
+                        "' and domain = '"+cookie.getDomain()+"' and cookie token '"+getToken(cookie)+"'");
+                // for each cookie we found in the request (there might be more than one!) we delete it
+                Cookie newCookie = new Cookie(COOKIE_NAME, null);
+                newCookie.setMaxAge(0); // INFO: A zero value tells the browser to delete the cookie immediately.
+                newCookie.setPath(cookie.getPath());
+                newCookie.setDomain(cookie.getDomain());
+                response.addCookie(newCookie);
+            }
         } else {
-            log.warn("No auto login cookie to delete!");
+            log.warn("No auto login cookie to delete in Yarep!");
         }
     }
  
@@ -77,7 +84,12 @@
     }
     
     public static String getUsername(HttpServletRequest request) {
-        return getUsername(getCookie(request));
+        String result = null;
+        Cookie[] cookies = getCookies(request);
+        if (cookies.length > 0) {
+            result = getUsername(cookies[0]);
+        }
+        return result;
     }
 
     /**
@@ -87,8 +99,8 @@
      */
     public static boolean existsAutoLoginCookie(HttpServletRequest request) {
         try {
-            Cookie cookie = getCookie(request);
-            if (cookie != null) {
+            Cookie[] cookies = getCookies(request);
+            if (cookies.length > 0) {
                 return true;
             } else {
                 //log.debug("No auto login cookie exists yet.");
@@ -108,60 +120,57 @@
      * @return true means that this user can be logged in automatically.
      */
     public static boolean tryAutoLogin(HttpServletRequest request, HttpServletResponse response, Realm realm) {
+        boolean result = false;
         try {
-            Cookie cookie = getCookie(request);
-            if (cookie != null) {
-                log.debug("Checking Autologin Cookie");
-                String username = getUsername(cookie);
-                String token = getToken(cookie);
-                if (username != null && token != null) {
-                    String yarepPath = getYarepPath(username, token);
-                    log.debug("Checking node "+yarepPath);
-                    Node node = null;
-                    try {
-                        node = realm.getRepository().getNode(yarepPath);
-                    } catch (Exception e) {
-                        // TODO ...
-                        //log.warn("Node '" + yarepPath + "' does not exist. We did not login the user automatically although auto login cookie seems to exist!");
-                        return false;
-                    }
+            Cookie[] cookies = getCookies(request);
+            if (cookies.length > 0) {
+                for (Cookie cookie: cookies) {
+                    log.debug("Checking Autologin Cookie");
+                    String username = getUsername(cookie);
+                    String token = getToken(cookie);
+                    if (username != null && token != null) {
+                        String yarepPath = getYarepPath(username, token);
+                        log.debug("Checking node "+yarepPath);
+                        Node node = null;
+                        try {
+                            node = realm.getRepository().getNode(yarepPath);
+                        } catch (Exception e) {
+                        }
 
-                    if (node != null) {
-                        Document doc = XMLHelper.readDocument(node.getInputStream());
-                        Element el = (Element)doc.getElementsByTagNameNS(NAMESPACE, XML_ATTR_TOKEN).item(0);
-                        String savedUsername = el.getAttribute(XML_ATTR_USERNAME);
-                        String savedToken = el.getAttribute("value");
-                        String expiryString = el.getAttribute(XML_ATTR_EXPIRES);
-                        log.debug("Retrieved username '"+username+"' and token '"+savedToken+"' from saved Token");
-                        if (username.equals(savedUsername) && token.equals(savedToken)) {
-                            log.debug("retrieved cookie matches for user '"+username+"'");
-                            if (hasTokenExpired(expiryString)) {
-                                Cookie newCookie = setNewCookie(username, request, response);
-                                saveToken(newCookie, realm.getRepository());
-                                deleteToken(realm.getRepository(), yarepPath);
-                                log.debug("Token was expired and has been renewed now.");
+                        if (node != null) {
+                            Document doc = XMLHelper.readDocument(node.getInputStream());
+                            Element el = (Element)doc.getElementsByTagNameNS(NAMESPACE, XML_ATTR_TOKEN).item(0);
+                            String savedUsername = el.getAttribute(XML_ATTR_USERNAME);
+                            String savedToken = el.getAttribute("value");
+                            String expiryString = el.getAttribute(XML_ATTR_EXPIRES);
+                            log.debug("Retrieved username '"+username+"' and token '"+savedToken+"' from saved Token");
+                            if (username.equals(savedUsername) && token.equals(savedToken)) {
+                                log.debug("retrieved cookie matches for user '"+username+"'");
+                                if (hasTokenExpired(expiryString)) {
+                                    Cookie newCookie = setNewCookie(username, request, response);
+                                    saveToken(newCookie, realm.getRepository());
+                                    deleteToken(realm.getRepository(), yarepPath);
+                                    log.debug("Token was expired and has been renewed now.");
+                                }
+                                result = true;
+                                break; // we do not process the remaining autologin cookies in the request
+                            } else {
+                                log.warn("Username/Token did no match (" + username + ", " + token + "), hence about auto login");
                             }
-                            return true;
                         } else {
-                            log.warn("Username/Token did no match (" + username + ", " + token + "), hence about auto login");
-                            return false;
+                            log.warn("No persistent yarep node with path '"+yarepPath+"' containing username/token!");
                         }
                     } else {
-                        log.warn("No persistent yarep node containing username/token!");
-                        return false;
+                        log.warn("No username/token inside auto login cookie!");
                     }
-                } else {
-                    log.warn("No username/token inside auto login cookie!");
-                    return false;
                 }
             } else {
-                //log.debug("No auto login cookie exists yet.");
-                return false;
+                log.debug("No auto login cookie exists yet.");
             }
         } catch (Exception e) {
             log.error("Can not find out whether to perform auto login or not! Exception message: " + e.getMessage(), e);
-            return false;
         }
+        return result;
     }
 
     /**
@@ -174,25 +183,26 @@
             String token = UUID.randomUUID().toString();
             Cookie cookie = new Cookie(COOKIE_NAME,token+SEP+username);
             cookie.setMaxAge(Integer.MAX_VALUE);
-            cookie.setPath(request.getContextPath());
+            cookie.setPath("/");
             response.addCookie(cookie);
             result = cookie;
+            log.debug("Setting new AUTOLOGIN COOKIE. Context Path would have been: "+request.getContextPath());
         }
         return result;
     }
 
     /**
-     * Get specific auto login cookie
+     * Returns all AUTOLOGIN cookies that are available in the request
+     * @return never null, but can be empty
      */
-    private static Cookie getCookie(HttpServletRequest request) {
-        Cookie result = null;
+    private static Cookie[] getCookies(HttpServletRequest request) {
+        List<Cookie> result = new ArrayList<Cookie>();
         try {
             Cookie[] cookies = request.getCookies();
             if (cookies != null) {
                 for (Cookie c : request.getCookies()) {
                     if (c.getName().equals(COOKIE_NAME)) {
-                        result = c;
-                        break;
+                        result.add(c);
                     }
                 }
             }
@@ -202,7 +212,7 @@
             log.error(e,e);
         }
         
-        return result;
+        return result.toArray(new Cookie[0]);
     }
  
     /**


More information about the Yanel-development mailing list