From 9c2a8e93e0dd812a412f343e20aecc0e0c763730 Mon Sep 17 00:00:00 2001 From: Grigory Kruglov Date: Wed, 8 Mar 2017 18:14:43 -0800 Subject: [PATCH] Bug 1329793 - Re-subscribe for a push channel periodically r=eoger,nalexander On startup and at the beginning of a sync we check how long it has been since we've subscribed to a channel for fxa service. If it's been over 21 days, request re-subscription. MozReview-Commit-ID: GzvPecZ9hTy --HG-- extra : rebase_source : d0292acddbdd231502808469d4e5502a4ac93779 --- .../org/mozilla/gecko/push/PushManager.java | 1 - .../org/mozilla/gecko/push/PushService.java | 55 +++++++++++++++ mobile/android/components/FxAccountsPush.js | 3 + .../gecko/fxa/FxAccountDeviceRegistrator.java | 67 +++++++++++++++---- .../fxa/authenticator/AndroidFxAccount.java | 30 ++++++++- .../gecko/fxa/sync/FxAccountSyncAdapter.java | 12 +++- .../SharedPreferencesClientsDataDelegate.java | 1 + 7 files changed, 151 insertions(+), 18 deletions(-) diff --git a/mobile/android/base/java/org/mozilla/gecko/push/PushManager.java b/mobile/android/base/java/org/mozilla/gecko/push/PushManager.java index 42ef60b61904..282472a5c725 100644 --- a/mobile/android/base/java/org/mozilla/gecko/push/PushManager.java +++ b/mobile/android/base/java/org/mozilla/gecko/push/PushManager.java @@ -17,7 +17,6 @@ import org.mozilla.gecko.util.ThreadUtils; import java.io.IOException; import java.util.Collections; -import java.util.HashMap; import java.util.Iterator; import java.util.Map; diff --git a/mobile/android/base/java/org/mozilla/gecko/push/PushService.java b/mobile/android/base/java/org/mozilla/gecko/push/PushService.java index 77bacb7760e6..3b8ea2e522f5 100644 --- a/mobile/android/base/java/org/mozilla/gecko/push/PushService.java +++ b/mobile/android/base/java/org/mozilla/gecko/push/PushService.java @@ -5,6 +5,8 @@ package org.mozilla.gecko.push; +import android.accounts.Account; +import android.accounts.AccountManager; import android.content.Context; import android.content.Intent; import android.os.Bundle; @@ -22,7 +24,11 @@ import org.mozilla.gecko.Telemetry; import org.mozilla.gecko.TelemetryContract; import org.mozilla.gecko.annotation.ReflectionTarget; import org.mozilla.gecko.db.BrowserDB; +import org.mozilla.gecko.fxa.FxAccountConstants; +import org.mozilla.gecko.fxa.FxAccountDeviceRegistrator; import org.mozilla.gecko.fxa.FxAccountPushHandler; +import org.mozilla.gecko.fxa.authenticator.AndroidFxAccount; +import org.mozilla.gecko.fxa.login.State; import org.mozilla.gecko.gcm.GcmTokenClient; import org.mozilla.gecko.push.autopush.AutopushClientException; import org.mozilla.gecko.util.BundleEventListener; @@ -102,7 +108,14 @@ public class PushService implements BundleEventListener { private boolean isReadyFxAccountsPush = false; private final List pendingPushMessages; + // NB, on context use in AccountManager and AndroidFxAccount: + // We are not going to register any listeners, or surface any UI out of AccountManager. + // It should be fine to use a potentially short-lived context then, as opposed to a long-lived + // application context, contrary to what AndroidFxAccount docs ask for. + private final Context context; + public PushService(Context context) { + this.context = context; pushManager = new PushManager(new PushState(context, "GeckoPushState.json"), new GcmTokenClient(context), new PushManager.PushClientFactory() { @Override public PushClient getPushClient(String autopushEndpoint, boolean debug) { @@ -119,6 +132,48 @@ public class PushService implements BundleEventListener { try { pushManager.startup(System.currentTimeMillis()); + + // Determine if we need to renew our FxA Push Subscription. Unused subscriptions expire + // once a month, and so we do a simple check on startup to determine if it's time to get + // a new one. Note that this is sub-optimal, as we might have a perfectly valid (but old) + // subscription which we'll nevertheless unsubscribe in lieu of a new one. Improvements + // to this will be addressed as part of a larger Bug 1345651. + + // From the Android permission docs: + // Prior to API 23, GET_ACCOUNTS permission was necessary to get access to information + // about any account. Beginning with API 23, if an app shares the signature of the + // authenticator that manages an account, it does not need "GET_ACCOUNTS" permission to + // read information about that account. + // We list GET_ACCOUNTS in our manifest for pre-23 devices. + final AccountManager accountManager = AccountManager.get(context); + final Account[] fxAccounts = accountManager.getAccountsByType(FxAccountConstants.ACCOUNT_TYPE); + + // Nothing to renew if there isn't an account. + if (fxAccounts.length == 0) { + return; + } + + // Defensively obtain account state. We are in a startup situation: try to not crash. + final AndroidFxAccount fxAccount = new AndroidFxAccount(context, fxAccounts[0]); + final State fxAccountState; + try { + fxAccountState = fxAccount.getState(); + } catch (IllegalStateException e) { + Log.e(LOG_TAG, "Failed to obtain FxA account state while renewing registration", e); + return; + } + + // This decision will be re-addressed as part of Bug 1346061. + if (!State.StateLabel.Married.equals(fxAccountState.getStateLabel())) { + Log.i(LOG_TAG, "FxA account not in Married state, not proceeding with registration renewal"); + return; + } + + // We'll obtain a new subscription as part of device registration. + if (FxAccountDeviceRegistrator.needToRenewRegistration(fxAccount.getDeviceRegistrationTimestamp())) { + Log.i(LOG_TAG, "FxA device needs registration renewal"); + FxAccountDeviceRegistrator.renewRegistration(context); + } } catch (Exception e) { Log.e(LOG_TAG, "Got exception during startup; ignoring.", e); return; diff --git a/mobile/android/components/FxAccountsPush.js b/mobile/android/components/FxAccountsPush.js index 5336e40cfab5..28b1bc48b404 100644 --- a/mobile/android/components/FxAccountsPush.js +++ b/mobile/android/components/FxAccountsPush.js @@ -38,6 +38,9 @@ FxAccountsPush.prototype = { this._subscribe(); } else if (data === "android-fxa-unsubscribe") { this._unsubscribe(); + } else if (data === "android-fxa-resubscribe") { + // If unsubscription fails, we still want to try to subscribe. + this._unsubscribe().then(this._subscribe, this._subscribe); } break; case "FxAccountsPush:ReceivedPushMessageToDecode": diff --git a/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/FxAccountDeviceRegistrator.java b/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/FxAccountDeviceRegistrator.java index 00018e552cf7..0496102b8276 100644 --- a/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/FxAccountDeviceRegistrator.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/FxAccountDeviceRegistrator.java @@ -40,6 +40,13 @@ import java.util.concurrent.Executors; public class FxAccountDeviceRegistrator implements BundleEventListener { private static final String LOG_TAG = "FxADeviceRegistrator"; + // The autopush endpoint expires stale channel subscriptions every 30 days (at a set time during + // the month, although we don't depend on this). To avoid the FxA service channel silently + // expiring from underneath us, we unsubscribe and resubscribe every 21 days. + // Note that this simple schedule means that we might unsubscribe perfectly valid (but old) + // subscriptions. This will be improved as part of Bug 1345651. + private static final long TIME_BETWEEN_CHANNEL_REGISTRATION_IN_MILLIS = 21 * 24 * 60 * 60 * 1000L; + // The current version of the device registration, we use this to re-register // devices after we update what we send on device registration. public static final Integer DEVICE_REGISTRATION_VERSION = 2; @@ -60,6 +67,14 @@ public class FxAccountDeviceRegistrator implements BundleEventListener { return instance; } + public static boolean needToRenewRegistration(final long timestamp) { + // NB: we're comparing wall clock to wall clock, at different points in time. + // It's possible that wall clocks have changed, and our comparison will be meaningless. + // However, this happens in the context of a sync, and we won't be able to sync anyways if our + // wall clock deviates too much from time on the server. + return (System.currentTimeMillis() - timestamp) > TIME_BETWEEN_CHANNEL_REGISTRATION_IN_MILLIS; + } + public static void register(Context context) { Context appContext = context.getApplicationContext(); try { @@ -69,21 +84,42 @@ public class FxAccountDeviceRegistrator implements BundleEventListener { } } + public static void renewRegistration(Context context) { + Context appContext = context.getApplicationContext(); + try { + getInstance(appContext).beginRegistrationRenewal(appContext); + } catch (Exception e) { + Log.e(LOG_TAG, "Could not start FxA device re-registration", e); + } + } + private void beginRegistration(Context context) { // Fire up gecko and send event // We create the Intent ourselves instead of using GeckoService.getIntentToCreateServices // because we can't import these modules (circular dependency between browser and services) - final Intent geckoIntent = new Intent(); - geckoIntent.setAction("create-services"); - geckoIntent.setClassName(context, "org.mozilla.gecko.GeckoService"); - geckoIntent.putExtra("category", "android-push-service"); - geckoIntent.putExtra("data", "android-fxa-subscribe"); - final AndroidFxAccount fxAccount = AndroidFxAccount.fromContext(context); - geckoIntent.putExtra("org.mozilla.gecko.intent.PROFILE_NAME", fxAccount.getProfile()); + final Intent geckoIntent = buildCreatePushServiceIntent(context, "android-fxa-subscribe"); context.startService(geckoIntent); // -> handleMessage() } + private void beginRegistrationRenewal(Context context) { + // Same as registration, but unsubscribe first to get a fresh subscription. + final Intent geckoIntent = buildCreatePushServiceIntent(context, "android-fxa-resubscribe"); + context.startService(geckoIntent); + // -> handleMessage() + } + + private Intent buildCreatePushServiceIntent(final Context context, final String data) { + final Intent intent = new Intent(); + intent.setAction("create-services"); + intent.setClassName(context, "org.mozilla.gecko.GeckoService"); + intent.putExtra("category", "android-push-service"); + intent.putExtra("data", data); + final AndroidFxAccount fxAccount = AndroidFxAccount.fromContext(context); + intent.putExtra("org.mozilla.gecko.intent.PROFILE_NAME", fxAccount.getProfile()); + return intent; + } + @Override public void handleMessage(String event, GeckoBundle message, EventCallback callback) { if ("FxAccountsPush:Subscribe:Response".equals(event)) { @@ -135,11 +171,15 @@ public class FxAccountDeviceRegistrator implements BundleEventListener { @Override public void handleError(Exception e) { Log.e(LOG_TAG, "Error while updating a device registration: ", e); + fxAccount.setDeviceRegistrationTimestamp(0L); } @Override public void handleFailure(FxAccountClientRemoteException error) { Log.e(LOG_TAG, "Error while updating a device registration: ", error); + + fxAccount.setDeviceRegistrationTimestamp(0L); + if (error.httpStatusCode == 400) { if (error.apiErrorNumber == FxAccountRemoteError.UNKNOWN_DEVICE) { recoverFromUnknownDevice(fxAccount); @@ -152,7 +192,7 @@ public class FxAccountDeviceRegistrator implements BundleEventListener { && error.apiErrorNumber == FxAccountRemoteError.INVALID_AUTHENTICATION_TOKEN) { handleTokenError(error, fxAccountClient, fxAccount); } else { - logErrorAndResetDeviceRegistrationVersion(error, fxAccount); + logErrorAndResetDeviceRegistrationVersionAndTimestamp(error, fxAccount); } } @@ -160,15 +200,16 @@ public class FxAccountDeviceRegistrator implements BundleEventListener { public void handleSuccess(FxAccountDevice result) { Log.i(LOG_TAG, "Device registration complete"); Logger.pii(LOG_TAG, "Registered device ID: " + result.id); - fxAccount.setFxAUserData(result.id, DEVICE_REGISTRATION_VERSION); + fxAccount.setFxAUserData(result.id, DEVICE_REGISTRATION_VERSION, System.currentTimeMillis()); } }); } - private static void logErrorAndResetDeviceRegistrationVersion( + private static void logErrorAndResetDeviceRegistrationVersionAndTimestamp( final FxAccountClientRemoteException error, final AndroidFxAccount fxAccount) { Log.e(LOG_TAG, "Device registration failed", error); fxAccount.resetDeviceRegistrationVersion(); + fxAccount.setDeviceRegistrationTimestamp(0L); } @Nullable @@ -187,7 +228,7 @@ public class FxAccountDeviceRegistrator implements BundleEventListener { final FxAccountClient fxAccountClient, final AndroidFxAccount fxAccount) { Log.i(LOG_TAG, "Recovering from invalid token error: ", error); - logErrorAndResetDeviceRegistrationVersion(error, fxAccount); + logErrorAndResetDeviceRegistrationVersionAndTimestamp(error, fxAccount); fxAccountClient.accountStatus(fxAccount.getState().uid, new RequestDelegate() { @Override @@ -233,7 +274,7 @@ public class FxAccountDeviceRegistrator implements BundleEventListener { fxAccountClient.deviceList(sessionToken, new RequestDelegate() { private void onError() { Log.e(LOG_TAG, "failed to recover from device-session conflict"); - logErrorAndResetDeviceRegistrationVersion(error, fxAccount); + logErrorAndResetDeviceRegistrationVersionAndTimestamp(error, fxAccount); } @Override @@ -250,7 +291,7 @@ public class FxAccountDeviceRegistrator implements BundleEventListener { public void handleSuccess(FxAccountDevice[] devices) { for (FxAccountDevice device : devices) { if (device.isCurrentDevice) { - fxAccount.setFxAUserData(device.id, 0); // Reset device registration version + fxAccount.setFxAUserData(device.id, 0, 0L); // Reset device registration version/timestamp if (!allowRecursion) { Log.d(LOG_TAG, "Failure to register a device on the second try"); break; diff --git a/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/authenticator/AndroidFxAccount.java b/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/authenticator/AndroidFxAccount.java index d7ce7c47f77d..d1a614ee6ca2 100644 --- a/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/authenticator/AndroidFxAccount.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/authenticator/AndroidFxAccount.java @@ -80,6 +80,7 @@ public class AndroidFxAccount { public static final String ACCOUNT_KEY_DEVICE_ID = "deviceId"; public static final String ACCOUNT_KEY_DEVICE_REGISTRATION_VERSION = "deviceRegistrationVersion"; + private static final String ACCOUNT_KEY_DEVICE_REGISTRATION_TIMESTAMP = "deviceRegistrationTimestamp"; // Account authentication token type for fetching account profile. public static final String PROFILE_OAUTH_TOKEN_TYPE = "oauth::profile"; @@ -402,6 +403,7 @@ public class AndroidFxAccount { } o.put("fxaDeviceId", getDeviceId()); o.put("fxaDeviceRegistrationVersion", getDeviceRegistrationVersion()); + o.put("fxaDeviceRegistrationTimestamp", getDeviceRegistrationTimestamp()); return o; } @@ -829,6 +831,23 @@ public class AndroidFxAccount { } } + public synchronized long getDeviceRegistrationTimestamp() { + final String timestampStr = accountManager.getUserData(account, ACCOUNT_KEY_DEVICE_REGISTRATION_TIMESTAMP); + + if (TextUtils.isEmpty(timestampStr)) { + return 0L; + } + + // Long.valueOf might throw; while it's not expected that this might happen, let's not risk + // crashing here as this method will be called on startup. + try { + return Long.valueOf(timestampStr); + } catch (NumberFormatException e) { + Logger.warn(LOG_TAG, "Couldn't parse deviceRegistrationTimestamp; defaulting to 0L.", e); + return 0L; + } + } + public synchronized void setDeviceId(String id) { accountManager.setUserData(account, ACCOUNT_KEY_DEVICE_ID, id); } @@ -838,14 +857,21 @@ public class AndroidFxAccount { Integer.toString(deviceRegistrationVersion)); } + public synchronized void setDeviceRegistrationTimestamp(long timestamp) { + accountManager.setUserData(account, ACCOUNT_KEY_DEVICE_REGISTRATION_TIMESTAMP, + Long.toString(timestamp)); + } + public synchronized void resetDeviceRegistrationVersion() { setDeviceRegistrationVersion(0); } - public synchronized void setFxAUserData(String id, int deviceRegistrationVersion) { + public synchronized void setFxAUserData(String id, int deviceRegistrationVersion, long timestamp) { accountManager.setUserData(account, ACCOUNT_KEY_DEVICE_ID, id); accountManager.setUserData(account, ACCOUNT_KEY_DEVICE_REGISTRATION_VERSION, - Integer.toString(deviceRegistrationVersion)); + Integer.toString(deviceRegistrationVersion)); + accountManager.setUserData(account, ACCOUNT_KEY_DEVICE_REGISTRATION_TIMESTAMP, + Long.toString(timestamp)); } @SuppressLint("ParcelCreator") // The CREATOR field is defined in the super class. diff --git a/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/sync/FxAccountSyncAdapter.java b/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/sync/FxAccountSyncAdapter.java index 6df68c6a82b0..df31f19cd6fc 100644 --- a/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/sync/FxAccountSyncAdapter.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/fxa/sync/FxAccountSyncAdapter.java @@ -567,10 +567,18 @@ public class FxAccountSyncAdapter extends AbstractThreadedSyncAdapter { assertion, tokenServerEndpointURI, tokenBackoffHandler, sharedPrefs, syncKeyBundle, clientState, sessionCallback, extras, fxAccount, syncDeadline); - // Register the device if necessary (asynchronous, in another thread) + // Register the device if necessary (asynchronous, in another thread). + // As part of device registration, we obtain a PushSubscription, register our push endpoint + // with FxA, and update account data with fxaDeviceId, which is part of our synced + // clients record. if (fxAccount.getDeviceRegistrationVersion() != FxAccountDeviceRegistrator.DEVICE_REGISTRATION_VERSION - || TextUtils.isEmpty(fxAccount.getDeviceId())) { + || TextUtils.isEmpty(fxAccount.getDeviceId())) { FxAccountDeviceRegistrator.register(context); + // We might need to re-register periodically to ensure our FxA push subscription is valid. + // This involves unsubscribing, subscribing and updating remote FxA device record with + // new push subscription information. + } else if (FxAccountDeviceRegistrator.needToRenewRegistration(fxAccount.getDeviceRegistrationTimestamp())) { + FxAccountDeviceRegistrator.renewRegistration(context); } // Force fetch the profile avatar information. (asynchronous, in another thread) diff --git a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/SharedPreferencesClientsDataDelegate.java b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/SharedPreferencesClientsDataDelegate.java index 4c1584d5a181..4d921f3dc72c 100644 --- a/mobile/android/services/src/main/java/org/mozilla/gecko/sync/SharedPreferencesClientsDataDelegate.java +++ b/mobile/android/services/src/main/java/org/mozilla/gecko/sync/SharedPreferencesClientsDataDelegate.java @@ -63,6 +63,7 @@ public class SharedPreferencesClientsDataDelegate implements ClientsDataDelegate if (account != null) { final AndroidFxAccount fxAccount = new AndroidFxAccount(context, account); fxAccount.resetDeviceRegistrationVersion(); + fxAccount.setDeviceRegistrationTimestamp(0L); } }