From 33a2ccada25906a6d55a873b00f22dbef24b29ad Mon Sep 17 00:00:00 2001 From: Vlad Filippov Date: Mon, 20 Jan 2020 11:02:22 -0500 Subject: [PATCH] Add offline use case for 'migrateFromSessionToken' Fixes #2396 --- CHANGES_UNRELEASED.md | 13 +++++ Cargo.lock | 1 + .../appservices/fxaclient/FirefoxAccount.kt | 51 ++++++++++++++++--- .../appservices/fxaclient/rust/LibFxAFFI.kt | 2 +- components/fxa-client/examples/migration.rs | 11 ++-- components/fxa-client/ffi/Cargo.toml | 1 + components/fxa-client/ffi/src/lib.rs | 11 ++-- components/fxa-client/src/error.rs | 3 ++ components/fxa-client/src/lib.rs | 11 ++++ components/fxa-client/src/migrator.rs | 49 +++++++++++++++--- .../fxa-client/src/state_persistence.rs | 1 + 11 files changed, 129 insertions(+), 25 deletions(-) diff --git a/CHANGES_UNRELEASED.md b/CHANGES_UNRELEASED.md index b089083300..5d22f9e0a2 100644 --- a/CHANGES_UNRELEASED.md +++ b/CHANGES_UNRELEASED.md @@ -15,3 +15,16 @@ Applications that submit telemetry via Glean must request a data review for these metrics before integrating the places component. See the component README.md for more details. ([#2431](https://github.com/mozilla/application-services/pull/2431)) + +## FxA Client + +### Breaking Changes + +* `migrateFromSessionToken` now returns a metrics JSON object if the migration succeeded. + ([#2492](https://github.com/mozilla/application-services/pull/2492)) + +### Features + +* `migrateFromSessionToken` now handles offline use cases. It caches the data the consumers originally provide. + If there's no network connectivity then the migration could be retried using the new `retryMigration` method. + ([#2492](https://github.com/mozilla/application-services/pull/2492)) diff --git a/Cargo.lock b/Cargo.lock index 8d6ec42003..fb3bc5a491 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -762,6 +762,7 @@ dependencies = [ "lazy_static 1.4.0 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", "prost 0.6.1 (registry+https://github.com/rust-lang/crates.io-index)", + "serde_json 1.0.44 (registry+https://github.com/rust-lang/crates.io-index)", "url 2.1.1 (registry+https://github.com/rust-lang/crates.io-index)", "viaduct 0.1.0", ] diff --git a/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/FirefoxAccount.kt b/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/FirefoxAccount.kt index 2c99d1e6eb..482e876eb1 100644 --- a/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/FirefoxAccount.kt +++ b/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/FirefoxAccount.kt @@ -317,14 +317,16 @@ class FirefoxAccount(handle: FxaHandle, persistCallback: PersistCallback?) : Aut * @param sessionToken 64 character string of hex-encoded bytes * @param kSync 128 character string of hex-encoded bytes * @param kXCS 32 character string of hex-encoded bytes + * @return JSONObject JSON object with the result of the migration * This performs network requests, and should not be used on the main thread. */ - fun migrateFromSessionToken(sessionToken: String, kSync: String, kXCS: String) { - rustCallWithLock { e -> - LibFxAFFI.INSTANCE.fxa_migrate_from_session_token(this.handle.get(), sessionToken, kSync, kXCS, - false, e) + fun migrateFromSessionToken(sessionToken: String, kSync: String, kXCS: String): JSONObject { + val json = rustCallForString(this) { e -> + LibFxAFFI.INSTANCE.fxa_migrate_from_session_token(this.handle.get(), sessionToken, kSync, kXCS, false, e) } + this.tryPersistState() + return JSONObject(json) } /** @@ -334,14 +336,35 @@ class FirefoxAccount(handle: FxaHandle, persistCallback: PersistCallback?) : Aut * @param sessionToken 64 character string of hex-encoded bytes * @param kSync 128 character string of hex-encoded bytes * @param kXCS 32 character string of hex-encoded bytes + * @return JSONObject JSON object with the result of the migration * This performs network requests, and should not be used on the main thread. */ - fun copyFromSessionToken(sessionToken: String, kSync: String, kXCS: String) { - rustCallWithLock { e -> - LibFxAFFI.INSTANCE.fxa_migrate_from_session_token(this.handle.get(), sessionToken, kSync, kXCS, - true, e) + fun copyFromSessionToken(sessionToken: String, kSync: String, kXCS: String): JSONObject { + val json = rustCallForString(this) { e -> + LibFxAFFI.INSTANCE.fxa_migrate_from_session_token(this.handle.get(), sessionToken, kSync, kXCS, true, e) + } + + this.tryPersistState() + return JSONObject(json) + } + + /** + * Retry migration from a logged-in Firefox Account. + * + * Modifies the FirefoxAccount state. + * @param sessionToken 64 character string of hex-encoded bytes + * @param kSync 128 character string of hex-encoded bytes + * @param kXCS 32 character string of hex-encoded bytes + * @return JSONObject JSON object with the result of the migration + * This performs network requests, and should not be used on the main thread. + */ + fun retryMigrateFromSessionToken(): JSONObject { + val json = rustCallForString(this) { e -> + LibFxAFFI.INSTANCE.fxa_retry_migrate_from_session_token(this.handle.get(), e) } + this.tryPersistState() + return JSONObject(json) } /** @@ -546,6 +569,18 @@ class FirefoxAccount(handle: FxaHandle, persistCallback: PersistCallback?) : Aut } } +@Suppress("TooGenericExceptionThrown") +internal inline fun rustCallForString(syncOn: Any, callback: (RustError.ByReference) -> Pointer?): String { + val cstring = rustCall(syncOn, callback) + ?: throw RuntimeException("Bug: Don't use this function when you can return" + + " null on success.") + try { + return cstring.getString(0, "utf8") + } finally { + LibPlacesFFI.INSTANCE.places_destroy_string(cstring) + } +} + // In practice we usually need to be synchronized to call this safely, so it doesn't // synchronize itself private inline fun nullableRustCall(callback: (RustError.ByReference) -> U?): U? { diff --git a/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/rust/LibFxAFFI.kt b/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/rust/LibFxAFFI.kt index e335571325..61f7230145 100644 --- a/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/rust/LibFxAFFI.kt +++ b/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/rust/LibFxAFFI.kt @@ -92,7 +92,7 @@ internal interface LibFxAFFI : Library { kXCS: String, copySessionToken: Boolean, e: RustError.ByReference - ) + ): Pointer? fun fxa_str_free(string: Pointer) fun fxa_bytebuffer_free(buffer: RustBuffer.ByValue) diff --git a/components/fxa-client/examples/migration.rs b/components/fxa-client/examples/migration.rs index 17f1264d6f..f2b01af37a 100644 --- a/components/fxa-client/examples/migration.rs +++ b/components/fxa-client/examples/migration.rs @@ -3,9 +3,9 @@ use fxa_client::FirefoxAccount; static CLIENT_ID: &str = "3c49430b43dfba77"; //static CONTENT_SERVER: &str = "https://latest.dev.lcip.org"; -static CONTENT_SERVER: &str = "http://127.0.0.1:3030"; +static CONTENT_SERVER: &str = "https://accounts.firefox.com"; //static REDIRECT_URI: &str = "https://latest.dev.lcip.org/oauth/success/3c49430b43dfba77"; -static REDIRECT_URI: &str = "http://127.0.0.1:3030/oauth/success/3c49430b43dfba77"; +static REDIRECT_URI: &str = "https://accounts.firefox.com/oauth/success/3c49430b43dfba77"; fn main() { let mut fxa = FirefoxAccount::new(CONTENT_SERVER, CLIENT_ID, REDIRECT_URI); @@ -15,8 +15,11 @@ fn main() { let k_sync: String = prompt_string("k_sync").unwrap(); println!("Enter kXCS (hex-string):"); let k_xcs: String = prompt_string("k_xcs").unwrap(); - fxa.migrate_from_session_token(&session_token, &k_sync, &k_xcs, true) + let migration_result = fxa + .migrate_from_session_token(&session_token, &k_sync, &k_xcs, true) .unwrap(); - println!("WOW! You've been migrated."); + + println!("WOW! You've been migrated in {:?}.", migration_result); + println!("JSON: {}", fxa.to_json().unwrap()); } diff --git a/components/fxa-client/ffi/Cargo.toml b/components/fxa-client/ffi/Cargo.toml index 73b6ea0cea..3c5e9d197e 100644 --- a/components/fxa-client/ffi/Cargo.toml +++ b/components/fxa-client/ffi/Cargo.toml @@ -12,6 +12,7 @@ crate-type = ["lib"] [dependencies] ffi-support = { path = "../../support/ffi" } log = "0.4.8" +serde_json = "1.0.44" lazy_static = "1.4.0" url = "2.1.1" prost = "0.6.1" diff --git a/components/fxa-client/ffi/src/lib.rs b/components/fxa-client/ffi/src/lib.rs index 900f79e995..c334c8af92 100644 --- a/components/fxa-client/ffi/src/lib.rs +++ b/components/fxa-client/ffi/src/lib.rs @@ -239,14 +239,17 @@ pub extern "C" fn fxa_migrate_from_session_token( k_xcs: FfiStr<'_>, copy_session_token: bool, error: &mut ExternError, -) { +) -> *mut c_char { log::debug!("fxa_migrate_from_session_token"); - ACCOUNTS.call_with_result_mut(error, handle, |fxa| { + ACCOUNTS.call_with_result_mut(error, handle, |fxa| -> fxa_client::Result { let session_token = session_token.as_str(); let k_sync = k_sync.as_str(); let k_xcs = k_xcs.as_str(); - fxa.migrate_from_session_token(session_token, k_sync, k_xcs, copy_session_token) - }); + let migration_metrics = + fxa.migrate_from_session_token(session_token, k_sync, k_xcs, copy_session_token)?; + let result = serde_json::to_string(&migration_metrics)?; + Ok(result) + }) } /// Try to get an access token. diff --git a/components/fxa-client/src/error.rs b/components/fxa-client/src/error.rs index b377569e8b..8803b61bd6 100644 --- a/components/fxa-client/src/error.rs +++ b/components/fxa-client/src/error.rs @@ -32,6 +32,9 @@ pub enum ErrorKind { #[fail(display = "No stored session token")] NoSessionToken, + #[fail(display = "No stored migration data")] + NoMigrationData, + #[fail(display = "No stored current device id")] NoCurrentDeviceId, diff --git a/components/fxa-client/src/lib.rs b/components/fxa-client/src/lib.rs index 55bb69a59a..50bcf3889e 100644 --- a/components/fxa-client/src/lib.rs +++ b/components/fxa-client/src/lib.rs @@ -57,6 +57,14 @@ pub struct FirefoxAccount { flow_store: HashMap, } +#[derive(Clone, Serialize, Deserialize)] +pub struct MigrationData { + k_xcs: String, + k_sync: String, + copy_session_token: bool, + session_token: String, +} + // If this structure is modified, please: // 1. Check if a migration needs to be done, as // these fields are persisted as a JSON string @@ -79,6 +87,7 @@ pub(crate) struct StateV2 { access_token_cache: HashMap, session_token: Option, // Hex-formatted string. last_seen_profile: Option>, + in_flight_migration: Option, } impl StateV2 { @@ -97,6 +106,7 @@ impl StateV2 { access_token_cache: HashMap::new(), device_capabilities: HashSet::new(), session_token: None, + in_flight_migration: None, } } } @@ -125,6 +135,7 @@ impl FirefoxAccount { current_device_id: None, last_seen_profile: None, access_token_cache: HashMap::new(), + in_flight_migration: None, }) } diff --git a/components/fxa-client/src/migrator.rs b/components/fxa-client/src/migrator.rs index ddfc7f43d2..0d9b61ad7d 100644 --- a/components/fxa-client/src/migrator.rs +++ b/components/fxa-client/src/migrator.rs @@ -2,7 +2,14 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -use crate::{error::*, scoped_keys::ScopedKey, scopes, FirefoxAccount}; +use crate::{error::*, scoped_keys::ScopedKey, scopes, FirefoxAccount, MigrationData}; +use serde_derive::*; +use std::time::Instant; + +#[derive(Serialize, Deserialize, PartialEq, Debug, Clone, Default)] +pub struct FxAMigrationResult { + pub total_duration: u128, +} impl FirefoxAccount { /// Migrate from a logged-in with a sessionToken Firefox Account. @@ -24,20 +31,39 @@ impl FirefoxAccount { k_sync: &str, k_xcs: &str, copy_session_token: bool, - ) -> Result<()> { + ) -> Result { // if there is already a session token on account, we error out. if self.state.session_token.is_some() { return Err(ErrorKind::IllegalState("Session Token is already set.").into()); } - let migration_session_token = if copy_session_token { + self.state.in_flight_migration = Some(MigrationData { + k_sync: k_sync.to_string(), + k_xcs: k_xcs.to_string(), + copy_session_token, + session_token: session_token.to_string(), + }); + + self.try_migration() + } + + pub fn try_migration(&mut self) -> Result { + let import_start = Instant::now(); + let migration_data = match self.state.in_flight_migration { + Some(ref data) => data.clone(), + None => { + return Err(ErrorKind::NoMigrationData.into()); + } + }; + + let migration_session_token = if migration_data.copy_session_token { let duplicate_session = self .client - .duplicate_session(&self.state.config, &session_token)?; + .duplicate_session(&self.state.config, &migration_data.session_token)?; duplicate_session.session_token } else { - session_token.to_string() + migration_data.session_token.to_string() }; // Trade our session token for a refresh token. @@ -49,9 +75,9 @@ impl FirefoxAccount { self.handle_oauth_response(oauth_response, None)?; // Synthesize a scoped key from our kSync. - let k_sync = hex::decode(k_sync)?; + let k_sync = hex::decode(&migration_data.k_sync)?; let k_sync = base64::encode_config(&k_sync, base64::URL_SAFE_NO_PAD); - let k_xcs = hex::decode(k_xcs)?; + let k_xcs = hex::decode(&migration_data.k_xcs)?; let k_xcs = base64::encode_config(&k_xcs, base64::URL_SAFE_NO_PAD); let scoped_key_data = self.client.scoped_key_data( &self.state.config, @@ -72,6 +98,13 @@ impl FirefoxAccount { self.state .scoped_keys .insert(scopes::OLD_SYNC.to_string(), k_sync_scoped_key); - Ok(()) + + self.state.in_flight_migration = None; + + let metrics = FxAMigrationResult { + total_duration: import_start.elapsed().as_millis(), + }; + + Ok(metrics) } } diff --git a/components/fxa-client/src/state_persistence.rs b/components/fxa-client/src/state_persistence.rs index 9fe979a250..0afc545404 100644 --- a/components/fxa-client/src/state_persistence.rs +++ b/components/fxa-client/src/state_persistence.rs @@ -90,6 +90,7 @@ impl From for Result { session_token: None, current_device_id: None, last_seen_profile: None, + in_flight_migration: None, access_token_cache: HashMap::new(), }) }