Add support for multiple devices. Chrome extension will send to all until it is modified to specify a device by ID or name.

This change also removes the legacy code that had security problems, i.e. accepting GET and requests without the XSRF header.

If a registration is reported as invalid ( i.e. application uninstalled ) - we clean up our database.

appengine/c2dm/com/google/android/c2dm/server/C2DMessaging.java

@@ -205,7 +205,7 @@ public class C2DMessaging {
             log.warning("Got error response from Google datamessaging endpoint: " + err);
             // No retry.
             // TODO(costin): show a nicer error to the user.
-            throw new IOException("Server error: " + err);
+            throw new IOException(err);
         } else {
             // 500 or unparseable response - server error, needs to retry
             log.warning("Invalid response from google " + responseLine + " " +

appengine/src/com/google/android/chrometophone/server/DeviceInfo.java

@@ -16,6 +16,12 @@
 
 package com.google.android.chrometophone.server;
 
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+import javax.jdo.PersistenceManager;
+import javax.jdo.Query;
 import javax.jdo.annotations.IdentityType;
 import javax.jdo.annotations.PersistenceCapable;
 import javax.jdo.annotations.Persistent;
@@ -23,8 +29,24 @@ import javax.jdo.annotations.PrimaryKey;
 
 import com.google.appengine.api.datastore.Key;
 
+/**
+ * Registration info.
+ * 
+ * An account may be associated with multiple phones,
+ * and a phone may be associated with multiple accounts.
+ * 
+ * registrations lists different phones registered to that account.
+ */
 @PersistenceCapable(identityType = IdentityType.APPLICATION)
 public class DeviceInfo {
+    /**
+     * User-email # device-id
+     * 
+     * Device-id can be specified by device, default is hash of abs(registration
+     * id).
+     * 
+     * user@example.com#1234
+     */
     @PrimaryKey
     @Persistent
     private Key key;
@@ -32,12 +54,47 @@ public class DeviceInfo {
     @Persistent
     private String deviceRegistrationID;
 
+    /** 
+     * Each device should provide a stable ID. It can be the 
+     * hash of the first registration, the phone ID, etc.
+     * Using the name seems error-prone, users may use the default 
+     * which may be the same in identical phones, they may change name, etc. 
+     */
+    @Persistent
+    private String id;
+        
+    /**
+     * Current supported types:
+     *   (default) - ac2dm, regular froyo+ devices using C2DM protocol
+     *   
+     * New types may be defined - for example for sending to chrome.   
+     */
+    @Persistent
+    private String type;
+        
+    /** 
+     * Friendly name for the device. May be edited by the user.
+     */
+    @Persistent
+    private String name;
+        
+    /**
+     * For statistics - and to provide hints to the user.
+     */
+    @Persistent
+    private Date registrationTimestamp;
+    
     @Persistent
     private Boolean debug;
 
     public DeviceInfo(Key key, String deviceRegistrationID) {
         this.key = key;
         this.deviceRegistrationID = deviceRegistrationID;
+        this.setRegistrationTimestamp(new Date()); // now
+    }
+
+    public DeviceInfo(Key key) {
+        this.key = key;
     }
 
     public boolean getDebug() {
@@ -63,4 +120,55 @@ public class DeviceInfo {
     public void setDeviceRegistrationID(String deviceRegistrationID) {
         this.deviceRegistrationID = deviceRegistrationID;
     }
+
+    
+    public void setId(String id) {
+        this.id = id;
+    }
+
+    public String getId() {
+        return id;
+    }
+
+    public void setType(String type) {
+        this.type = type;
+    }
+
+    public String getType() {
+        return type;
+    }
+
+    public void setName(String name) {
+        this.name = name;
+    }
+
+    public String getName() {
+        return name;
+    }
+
+    public void setRegistrationTimestamp(Date registrationTimestamp) {
+        this.registrationTimestamp = registrationTimestamp;
+    }
+
+    public Date getRegistrationTimestamp() {
+        return registrationTimestamp;
+    }
+    
+    /**
+     * Helper function - will query all registrations for a user.
+     */
+    public static List<DeviceInfo> getDeviceInfoForUser(PersistenceManager pm, String user) {
+        Query query = pm.newQuery(DeviceInfo.class);
+        query.setFilter("key >= '" +
+                user + "' && key < '" + user + "$'");
+        List<DeviceInfo> qresult = (List<DeviceInfo>) query.execute();
+        // Copy to array - we need to close the query
+        List<DeviceInfo> result = new ArrayList<DeviceInfo>();
+        for (DeviceInfo di: qresult) {
+            result.add(di);
+        }
+        query.closeAll();
+        return result;
+    }
+    
 }

appengine/src/com/google/android/chrometophone/server/RegisterServlet.java

@@ -17,6 +17,8 @@
 package com.google.android.chrometophone.server;
 
 import java.io.IOException;
+import java.util.List;
+import java.util.logging.Level;
 import java.util.logging.Logger;
 
 import javax.jdo.PersistenceManager;
@@ -40,6 +42,8 @@ public class RegisterServlet extends HttpServlet {
     private static final String OK_STATUS = "OK";
     private static final String ERROR_STATUS = "ERROR";
 
+    private static int MAX_DEVICES = 3;
+    
     /**
      * Get the user using the UserService.
      *
@@ -74,25 +78,16 @@ public class RegisterServlet extends HttpServlet {
         return user;
     }
 
-    /**
-     * @deprecated will be removed in next rel.
-     */
-    @Deprecated
-    @Override
-    public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
-        doPost(req, resp);
-    }
-
     @Override
     public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException {
         resp.setContentType("text/plain");
 
         // Basic XSRF protection
         if (req.getHeader("X-Same-Domain") == null) {
-            // TODO: Enable at consumer launch
+            log.warning("Blocked XSRF");
-            //resp.setStatus(400);
+            resp.setStatus(400);
-            //resp.getWriter().println(ERROR_STATUS + " (Missing X-Same-Domain header)");
+            resp.getWriter().println(ERROR_STATUS + " (Missing X-Same-Domain header)");
-            //return;
+            return;
         }
 
         String deviceRegistrationID = req.getParameter("devregid");
@@ -102,20 +97,64 @@ public class RegisterServlet extends HttpServlet {
             return;
         }
 
+        String deviceName = req.getParameter("deviceName");
+        if (deviceName == null) {
+            deviceName = "Phone";
+        }
+        
+        String deviceId = req.getParameter("deviceId");
+        if (deviceId == null) {
+            deviceId = Long.toHexString(Math.abs(deviceRegistrationID.hashCode()));
+        }
+
+        String deviceType = req.getParameter("deviceType");
+        if (deviceType == null) {
+            deviceType = "ac2dm"; 
+        }
+        
         User user = checkUser(req, resp, true);
         if (user != null) {
-            Key key = KeyFactory.createKey(DeviceInfo.class.getSimpleName(), user.getEmail());
-            DeviceInfo device = new DeviceInfo(key, deviceRegistrationID);
             // Context-shared PMF.
             PersistenceManager pm =
-                    C2DMessaging.getPMF(getServletContext()).getPersistenceManager();
+                C2DMessaging.getPMF(getServletContext()).getPersistenceManager();
+
             try {
+                List<DeviceInfo> registrations = DeviceInfo.getDeviceInfoForUser(pm,
+                        user.getEmail());
+
+                if (registrations.size() > MAX_DEVICES) {
+                    // we could return an error - but user can't handle it yet.
+                    // we can't let it grow out of bounds.
+                    // TODO: we should also define a 'ping' message and expire/remove
+                    // unused registrations
+                    DeviceInfo oldest = registrations.get(0);
+                    long oldestTime = oldest.getRegistrationTimestamp().getTime();
+                    for (int i = 1; i < registrations.size(); i++) {
+                        if (registrations.get(i).getRegistrationTimestamp().getTime() <
+                                oldestTime) {
+                            oldest = registrations.get(i);
+                            oldestTime = oldest.getRegistrationTimestamp().getTime();
+                        }
+                    }
+                    pm.deletePersistent(oldest);
+                }
+                
+                // TODO: dup ? update
+                String id = Long.toHexString(Math.abs(deviceRegistrationID.hashCode()));
+
+                Key key = KeyFactory.createKey(DeviceInfo.class.getSimpleName(), 
+                        user.getEmail() + "#" + id);
+                
+                DeviceInfo device = new DeviceInfo(key, deviceRegistrationID);
+                device.setId(deviceId);
+                device.setName(deviceName);
+                
                 pm.makePersistent(device);
                 resp.getWriter().println(OK_STATUS);            
             } catch (Exception e) {
                 resp.setStatus(500);
                 resp.getWriter().println(ERROR_STATUS + " (Error registering device)");
-                log.warning("Error registering device.");
+                log.log(Level.WARNING, "Error registering device.", e);
             } finally {
                 pm.close();
             }

appengine/src/com/google/android/chrometophone/server/SendServlet.java

@@ -17,6 +17,7 @@
 package com.google.android.chrometophone.server;
 
 import java.io.IOException;
+import java.util.List;
 import java.util.logging.Logger;
 
 import javax.jdo.JDOObjectNotFoundException;
@@ -39,11 +40,7 @@ public class SendServlet extends HttpServlet {
     private static final String DEVICE_NOT_REGISTERED_STATUS = "DEVICE_NOT_REGISTERED";
     private static final String ERROR_STATUS = "ERROR";
 
-    @Deprecated
+    // GET not supported
-    @Override
-    public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
-        doPost(req, resp);
-    }
     
     @Override
     public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException {
@@ -80,59 +77,87 @@ public class SendServlet extends HttpServlet {
             return;
         }
         
+        String deviceId = req.getParameter("deviceId");
+        String deviceName = req.getParameter("deviceName");
+
         User user = RegisterServlet.checkUser(req, resp, false);
         if (user != null) {
-            doSendToPhone(url, title, sel, user.getEmail(), resp);
+            doSendToPhone(url, title, sel, user.getEmail(), deviceId, deviceName, 
+                    resp);
         } else {
             resp.getWriter().println(LOGIN_REQUIRED_STATUS);
         }
     }
 
     private boolean doSendToPhone(String url, String title, String sel,
-            String userAccount, HttpServletResponse resp) throws IOException {
+            String userAccount, String deviceId, 
-        // Get device info
+            String deviceName, HttpServletResponse resp) throws IOException {
-        DeviceInfo deviceInfo = null;
+
-        // Shared PMF
+        // ok = we sent to at least one phone.
-        PersistenceManager pm =
+        boolean ok = false;
-                C2DMessaging.getPMF(getServletContext()).getPersistenceManager();
-        try {
-            Key key = KeyFactory.createKey(DeviceInfo.class.getSimpleName(), userAccount);
-            try {
-                deviceInfo = pm.getObjectById(DeviceInfo.class, key);
-            } catch (JDOObjectNotFoundException e) {
-                log.warning("Device not registered");
-                resp.getWriter().println(DEVICE_NOT_REGISTERED_STATUS);
-                return false;
-            }
-        } finally {
-            pm.close();
-        }
         
         // Send push message to phone
         C2DMessaging push = C2DMessaging.get(getServletContext());
         boolean res = false;
-        String collapseKey = "" + url.hashCode();
-        if (deviceInfo.getDebug()) {
-            res = push.sendNoRetry(deviceInfo.getDeviceRegistrationID(),
-                    collapseKey,
-                    "url", url,
-                    "title", title,
-                    "sel", sel,
-                    "debug", "1");
         
-        } else {
+        String collapseKey = "" + url.hashCode();
-            res = push.sendNoRetry(deviceInfo.getDeviceRegistrationID(),
+ 
-                    collapseKey,
+        PersistenceManager pm =
-                    "url", url,
+            C2DMessaging.getPMF(getServletContext()).getPersistenceManager();
-                    "title", title,
+        List<DeviceInfo> registrations = null; 
-                    "sel", sel);
+        try {
+            registrations = DeviceInfo.getDeviceInfoForUser(C2DMessaging.getPMF(getServletContext())
+                    .getPersistenceManager(), userAccount);
+        } finally {
+            pm.close();
         }
-        if (res) {
+
-            log.info("Link sent to phone! collapse_key:" + collapseKey);
+
+        if (registrations.size() == 0) {
+            log.warning("Device not registered");
+            resp.getWriter().println(DEVICE_NOT_REGISTERED_STATUS);
+            return false;
+        }
+
+        for (DeviceInfo deviceInfo: registrations) {
+            if (deviceId != null && !deviceId.equals(deviceInfo.getId())) {
+                continue; // user-specified device
+            }
+            if (deviceName != null && !deviceName.equals(deviceInfo.getName())) {
+                continue; // user-specified device
+            }
+
+            // if name or value are null - they'll be skipped
+            try {
+                res = push.sendNoRetry(deviceInfo.getDeviceRegistrationID(),
+                        collapseKey, 
+                        "url", url, 
+                        "title", title,
+                        "sel", sel,
+                        "debug", deviceInfo.getDebug() ? "1" : null);
+
+                if (res) {
+                    log.info("Link sent to phone! collapse_key:" + collapseKey);
+                    ok = true;
+                } else {
+                    log.warning("Error: Unable to send link to phone.");
+                }
+            } catch (IOException ex) {
+                if ("NotRegistered".equals(ex.getMessage()) ||
+                        "InvalidRegistration".equals(ex.getMessage())) {
+                    // remove registrations, it no longer works
+                    pm.deletePersistent(deviceInfo);
+                    throw ex;
+                } else {
+                    throw ex;
+                }
+            }
+        }
+        
+        if (ok) {
             resp.getWriter().println(OK_STATUS);
             return true;            
         } else {
-            log.warning("Error: Unable to send link to phone.");
             resp.setStatus(500);
             resp.getWriter().println(ERROR_STATUS + " (Unable to send link)");
             return false;            

appengine/src/com/google/android/chrometophone/server/UnregisterServlet.java

@@ -17,6 +17,7 @@
 package com.google.android.chrometophone.server;
 
 import java.io.IOException;
+import java.util.List;
 import java.util.logging.Logger;
 
 import javax.jdo.JDOObjectNotFoundException;
@@ -39,25 +40,15 @@ public class UnregisterServlet extends HttpServlet {
     private static final String OK_STATUS = "OK";
     private static final String ERROR_STATUS = "ERROR";
 
-    /**
-     * @deprecated Will be removed in next rel cycle.
-     */
-    @Deprecated
-    @Override
-    public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException {
-        doPost(req, resp);
-    }
-
     @Override
     public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException {
         resp.setContentType("text/plain");
 
         // Basic XSRF protection
         if (req.getHeader("X-Same-Domain") == null) {
-            // TODO: Enable at consumer launch
+            resp.setStatus(400);
-            //resp.setStatus(400);
+            resp.getWriter().println(ERROR_STATUS + " (Missing X-Same-Domain header)");
-            //resp.getWriter().println(ERROR_STATUS + " (Missing X-Same-Domain header)");
+            return;
-            //return;
         }
 
         String deviceRegistrationID = req.getParameter("devregid");
@@ -71,12 +62,19 @@ public class UnregisterServlet extends HttpServlet {
         UserService userService = UserServiceFactory.getUserService();
         User user = userService.getCurrentUser();
         if (user != null) {
-            Key key = KeyFactory.createKey(DeviceInfo.class.getSimpleName(), user.getEmail());
             PersistenceManager pm =
                     C2DMessaging.getPMF(getServletContext()).getPersistenceManager();
             try {
-                DeviceInfo device = pm.getObjectById(DeviceInfo.class, key);
+                List<DeviceInfo> registrations = DeviceInfo.getDeviceInfoForUser(pm, user.getEmail());
-                pm.deletePersistent(device);
+                for (int i = 0; i < registrations.size(); i++) {
+                    DeviceInfo deviceInfo = registrations.get(i);
+                    if (deviceInfo.getDeviceRegistrationID().equals(deviceRegistrationID)) {
+                        pm.deletePersistent(deviceInfo);
+                        registrations.remove(i);
+                        break;
+                    }
+                }
+                
                 resp.getWriter().println(OK_STATUS);
             } catch (JDOObjectNotFoundException e) {
                 resp.setStatus(400);