From f887a71bedf0feb9a216c4804be8eba8ee26262c Mon Sep 17 00:00:00 2001 From: costin Date: Mon, 30 Aug 2010 20:25:41 +0000 Subject: [PATCH] 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. --- .../android/c2dm/server/C2DMessaging.java | 2 +- .../chrometophone/server/DeviceInfo.java | 108 +++++++++++++++++ .../chrometophone/server/RegisterServlet.java | 75 +++++++++--- .../chrometophone/server/SendServlet.java | 109 +++++++++++------- .../server/UnregisterServlet.java | 32 +++-- 5 files changed, 248 insertions(+), 78 deletions(-) diff --git a/appengine/c2dm/com/google/android/c2dm/server/C2DMessaging.java b/appengine/c2dm/com/google/android/c2dm/server/C2DMessaging.java index 186f7dd..7bfd6ca 100644 --- a/appengine/c2dm/com/google/android/c2dm/server/C2DMessaging.java +++ b/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 + " " + diff --git a/appengine/src/com/google/android/chrometophone/server/DeviceInfo.java b/appengine/src/com/google/android/chrometophone/server/DeviceInfo.java index fe0e5b6..6a2fa58 100644 --- a/appengine/src/com/google/android/chrometophone/server/DeviceInfo.java +++ b/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 getDeviceInfoForUser(PersistenceManager pm, String user) { + Query query = pm.newQuery(DeviceInfo.class); + query.setFilter("key >= '" + + user + "' && key < '" + user + "$'"); + List qresult = (List) query.execute(); + // Copy to array - we need to close the query + List result = new ArrayList(); + for (DeviceInfo di: qresult) { + result.add(di); + } + query.closeAll(); + return result; + } + } diff --git a/appengine/src/com/google/android/chrometophone/server/RegisterServlet.java b/appengine/src/com/google/android/chrometophone/server/RegisterServlet.java index 1406e02..6c27913 100644 --- a/appengine/src/com/google/android/chrometophone/server/RegisterServlet.java +++ b/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 - //resp.setStatus(400); - //resp.getWriter().println(ERROR_STATUS + " (Missing X-Same-Domain header)"); - //return; + log.warning("Blocked XSRF"); + resp.setStatus(400); + resp.getWriter().println(ERROR_STATUS + " (Missing X-Same-Domain header)"); + 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 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); + 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(); } diff --git a/appengine/src/com/google/android/chrometophone/server/SendServlet.java b/appengine/src/com/google/android/chrometophone/server/SendServlet.java index 0f7cf0f..b2250e5 100644 --- a/appengine/src/com/google/android/chrometophone/server/SendServlet.java +++ b/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,12 +40,8 @@ public class SendServlet extends HttpServlet { private static final String DEVICE_NOT_REGISTERED_STATUS = "DEVICE_NOT_REGISTERED"; private static final String ERROR_STATUS = "ERROR"; - @Deprecated - @Override - public void doGet(HttpServletRequest req, HttpServletResponse resp) throws IOException { - doPost(req, resp); - } - + // GET not supported + @Override public void doPost(HttpServletRequest req, HttpServletResponse resp) throws IOException { resp.setContentType("text/plain"); @@ -79,63 +76,91 @@ public class SendServlet extends HttpServlet { resp.getWriter().println(ERROR_STATUS + " (Must specify url and title parameters)"); 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 { - // Get device info - DeviceInfo deviceInfo = null; - // Shared PMF + String userAccount, String deviceId, + String deviceName, HttpServletResponse resp) throws IOException { + + // ok = we sent to at least one phone. + boolean ok = false; + + // Send push message to phone + C2DMessaging push = C2DMessaging.get(getServletContext()); + boolean res = false; + + String collapseKey = "" + url.hashCode(); + PersistenceManager pm = - C2DMessaging.getPMF(getServletContext()).getPersistenceManager(); + C2DMessaging.getPMF(getServletContext()).getPersistenceManager(); + List registrations = null; 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; - } + registrations = DeviceInfo.getDeviceInfoForUser(C2DMessaging.getPMF(getServletContext()) + .getPersistenceManager(), userAccount); } 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 { - res = push.sendNoRetry(deviceInfo.getDeviceRegistrationID(), - collapseKey, - "url", url, - "title", title, - "sel", sel); + if (registrations.size() == 0) { + log.warning("Device not registered"); + resp.getWriter().println(DEVICE_NOT_REGISTERED_STATUS); + return false; } - if (res) { - log.info("Link sent to phone! collapse_key:" + collapseKey); + + 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; + 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; + return false; } } } diff --git a/appengine/src/com/google/android/chrometophone/server/UnregisterServlet.java b/appengine/src/com/google/android/chrometophone/server/UnregisterServlet.java index 770f5c3..ff34913 100644 --- a/appengine/src/com/google/android/chrometophone/server/UnregisterServlet.java +++ b/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.getWriter().println(ERROR_STATUS + " (Missing X-Same-Domain header)"); - //return; + resp.setStatus(400); + resp.getWriter().println(ERROR_STATUS + " (Missing X-Same-Domain header)"); + return; } String deviceRegistrationID = req.getParameter("devregid"); @@ -66,17 +57,24 @@ public class UnregisterServlet extends HttpServlet { resp.getWriter().println(ERROR_STATUS + " (Must specify devregid)"); return; } - + // Authorize & store device info 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); - pm.deletePersistent(device); + List registrations = DeviceInfo.getDeviceInfoForUser(pm, user.getEmail()); + 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);