From 30e4f63de575653f83e2c2f4b1c905f04f32bb84 Mon Sep 17 00:00:00 2001 From: Ben Dean-Kawamura Date: Thu, 5 Oct 2023 12:25:19 -0400 Subject: [PATCH] More fixes from review - Define constants for magic values - Go back to the old network retry logic - Removed the require_scoped_key param. Scoped keys are always required for the `oldsync` scope and never for other scopes. - Added back the `StateV2.device_capabilities` field. This tracks something different than `server_device_info`. Instead of tracking what the server thinks about our device, this tracks the capabilities that we know we have. It's a subtle difference, but `device_capabilities` is what we want to use for `reregister_capabilities`. - Added optional metrics parameter for BeginOAuthFlow and BeginPairingFlow --- .../fxaclient/FxaActionProcessor.kt | 35 ++++--- .../appservices/fxaclient/FxaClient.kt | 3 +- .../appservices/fxaclient/StateTypes.kt | 5 + .../fxaclient/FxaActionProcessorTest.kt | 99 +++++++------------ components/fxa-client/src/error.rs | 14 +-- components/fxa-client/src/fxa_client.udl | 8 +- components/fxa-client/src/internal/device.rs | 17 ++-- components/fxa-client/src/internal/mod.rs | 7 +- components/fxa-client/src/internal/oauth.rs | 23 ++--- components/fxa-client/src/internal/profile.rs | 2 +- .../fxa-client/src/internal/state_manager.rs | 21 +++- .../src/internal/state_persistence.rs | 6 +- components/fxa-client/src/token.rs | 11 +-- examples/cli-support/src/fxa_creds.rs | 4 +- 14 files changed, 129 insertions(+), 126 deletions(-) diff --git a/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/FxaActionProcessor.kt b/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/FxaActionProcessor.kt index d9d7e59363..0a6f667edb 100644 --- a/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/FxaActionProcessor.kt +++ b/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/FxaActionProcessor.kt @@ -14,6 +14,18 @@ import kotlin.time.Duration import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.seconds +// How many items should our channel hold? This should be big enough that it will never be filled +// under normal circumstances, but small enough that it will get filled if the `processChannel()` +// task fails (and therefore we will see exceptions when we call `trySend` rather than failing +// silently). +private const val CHANNEL_SIZE = 100 + +// How many times should we retry in the face of network errors? +private const val NETWORK_RETRY_MAX = 3 + +// How often should we try to recover from auth issues? +private val AUTH_RECOVERY_PERIOD = 60.seconds + /** * Processes actions sent to [FxaClient.queueAction] and sends events to the [FxaEventHandler] * @@ -32,7 +44,7 @@ internal class FxaActionProcessor( ) { // Set a high bound on the channel so that if our the processChannel() task dies, or is never // started, we should eventually see error reports. - private val channel = Channel(100) + private val channel = Channel(CHANNEL_SIZE) internal val retryLogic = RetryLogic() internal var currentState = initialState @@ -41,8 +53,6 @@ internal class FxaActionProcessor( // Queue a new action for processing fun queue(action: FxaAction) { - // trySend allows this function to be non-suspend and will never fail since the channel size - // is UNLIMITED channel.trySend(action) } @@ -66,6 +76,7 @@ internal class FxaActionProcessor( @Suppress("ComplexMethod") internal suspend fun processAction(action: FxaAction) { + retryLogic.newActionStarted() val isValid = when (action) { // Auth flow actions are valid if you're disconnected and also if you're already // authenticating. If a consumer accidentally starts multiple flows we should not @@ -180,13 +191,13 @@ internal class FxaActionProcessor( private suspend fun handleBeginOAuthFlow(action: FxaAction.BeginOAuthFlow) { handleBeginEitherOAuthFlow(action.result) { - inner.beginOauthFlow(action.scopes.toList(), action.entrypoint, MetricsParams(mapOf())) + inner.beginOauthFlow(action.scopes.toList(), action.entrypoint, action.metrics ?: MetricsParams(mapOf())) } } private suspend fun handleBeginPairingFlow(action: FxaAction.BeginPairingFlow) { handleBeginEitherOAuthFlow(action.result) { - inner.beginPairingFlow(action.pairingUrl, action.scopes.toList(), action.entrypoint, MetricsParams(mapOf())) + inner.beginPairingFlow(action.pairingUrl, action.scopes.toList(), action.entrypoint, action.metrics ?: MetricsParams(mapOf())) } } @@ -327,22 +338,25 @@ internal class FxaActionProcessor( } internal class RetryLogic { - private var lastNetworkRetry: Long = 0 + private var networkRetryCount = 0 private var lastAuthCheck: Long = 0 fun shouldRetryAfterNetworkError(): Boolean { - val elasped = (System.currentTimeMillis() - lastNetworkRetry).milliseconds - if (elasped > 30.seconds) { - lastNetworkRetry = System.currentTimeMillis() + if (networkRetryCount < NETWORK_RETRY_MAX) { + networkRetryCount += 1 return true } else { return false } } + fun newActionStarted() { + networkRetryCount = 0 + } + fun shouldRecheckAuthStatus(): Boolean { val elasped = (System.currentTimeMillis() - lastAuthCheck).milliseconds - if (elasped > 60.seconds) { + if (elasped > AUTH_RECOVERY_PERIOD) { lastAuthCheck = System.currentTimeMillis() return true } else { @@ -352,7 +366,6 @@ internal class RetryLogic { // For testing fun fastForward(amount: Duration) { - lastNetworkRetry -= amount.inWholeMilliseconds lastAuthCheck -= amount.inWholeMilliseconds } } diff --git a/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/FxaClient.kt b/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/FxaClient.kt index fb0ff4792e..d9adb0a2be 100644 --- a/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/FxaClient.kt +++ b/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/FxaClient.kt @@ -223,9 +223,8 @@ class FxaClient private constructor( suspend fun getAccessToken( scope: String, ttl: Long? = null, - requireScopedKey: Boolean = false, ): AccessTokenInfo = wrapMethodCallAndPersist("getAccessToken") { - inner.getAccessToken(scope, ttl, requireScopedKey) + inner.getAccessToken(scope, ttl) } /** diff --git a/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/StateTypes.kt b/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/StateTypes.kt index 222b41817c..41bb780766 100644 --- a/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/StateTypes.kt +++ b/components/fxa-client/android/src/main/java/mozilla/appservices/fxaclient/StateTypes.kt @@ -39,11 +39,13 @@ sealed class FxaAction { * @param scopes OAuth scopes to request * @param entrypoint OAuth entrypoint * @param result If present, will be completed with the OAuth URL to navigate users too + * @param metrics If present, extra metrics trackingquery parameters in the resulting URL */ data class BeginOAuthFlow( val scopes: Array, val entrypoint: String, val result: CompletableDeferred? = null, + val metrics: MetricsParams? = null, ) : FxaAction() /** @@ -58,12 +60,14 @@ sealed class FxaAction { * @param scopes OAuth scopes to request * @param entrypoint OAuth entrypoint * @param result If present, will be completed with the OAuth URL to navigate users too + * @param metrics If present, extra metrics trackingquery parameters in the resulting URL */ data class BeginPairingFlow( val pairingUrl: String, val scopes: Array, val entrypoint: String, val result: CompletableDeferred? = null, + val metrics: MetricsParams? = null, ) : FxaAction() /** @@ -247,6 +251,7 @@ enum class FxaAuthEventKind { AUTH_CHECK_STARTED, AUTH_CHECK_FAILED, AUTH_CHECK_SUCCESS, + // This is sent back when the consumer sends the `LogoutFromAuthIssues` action LOGOUT_FROM_AUTH_ISSUES, } diff --git a/components/fxa-client/android/src/test/java/mozilla/appservices/fxaclient/FxaActionProcessorTest.kt b/components/fxa-client/android/src/test/java/mozilla/appservices/fxaclient/FxaActionProcessorTest.kt index 5afc6b0935..b26c7543f6 100644 --- a/components/fxa-client/android/src/test/java/mozilla/appservices/fxaclient/FxaActionProcessorTest.kt +++ b/components/fxa-client/android/src/test/java/mozilla/appservices/fxaclient/FxaActionProcessorTest.kt @@ -747,13 +747,15 @@ class FxaActionProcessorTest { class FxaRetryTest { @Test - fun `FxaActionProcessor retries after network errors`() = runTest { + fun `FxaActionProcessor retries 3 times after network errors`() = runTest { val mocks = Mocks.create(FxaAuthState.CONNECTED) - every { mocks.firefoxAccount.setDeviceName(any()) } throws networkException andThen testLocalDevice + every { mocks.firefoxAccount.setDeviceName(any()) } throwsMany listOf(networkException, networkException, networkException) andThen testLocalDevice mocks.verifyAction(setDeviceNameAction) { _, inner, eventHandler, _ -> coVerifySequence { - // This throws FxaException.Network, we should retry + // These throws FxaException.Network, we should retry + inner.setDeviceName("My Phone") + inner.setDeviceName("My Phone") inner.setDeviceName("My Phone") // This time it work inner.setDeviceName("My Phone") @@ -763,35 +765,18 @@ class FxaRetryTest { } FxaAuthState.CONNECTED } - } - @Test - fun `FxaActionProcessor fails after 2 network errors`() = runTest { - val mocks = Mocks.create(FxaAuthState.CONNECTED) - every { mocks.firefoxAccount.setDeviceName(any()) } throws networkException + // Each action gets a fresh retry count. Test out another action that fails 3 times then + // succeeds. + every { mocks.firefoxAccount.setDeviceName(any()) } throwsMany listOf(networkException, networkException, networkException) andThen testLocalDevice mocks.verifyAction(setDeviceNameAction) { _, inner, eventHandler, _ -> coVerifySequence { - // This throws FxaException.Network, we should retry + // These throws FxaException.Network, we should retry inner.setDeviceName("My Phone") - // This throws again, so the operation fails inner.setDeviceName("My Phone") - eventHandler.onFxaEvent(FxaEvent.DeviceOperationFailed(FxaDeviceOperation.SET_DEVICE_NAME)) - } - FxaAuthState.CONNECTED - } - } - - @Test - fun `FxaActionProcessor fails after multiple network errors in a short time`() = runTest { - val mocks = Mocks.create(FxaAuthState.CONNECTED) - every { mocks.firefoxAccount.setDeviceName(any()) } throws networkException andThen testLocalDevice - - mocks.verifyAction(setDeviceNameAction) { _, inner, eventHandler, _ -> - coVerifySequence { - // This fails with FxaException.Network, we should retry inner.setDeviceName("My Phone") - // This time it works + // This time it work inner.setDeviceName("My Phone") eventHandler.onFxaEvent( FxaEvent.DeviceOperationComplete(FxaDeviceOperation.SET_DEVICE_NAME, testLocalDevice), @@ -799,54 +784,22 @@ class FxaRetryTest { } FxaAuthState.CONNECTED } - - mocks.actionProcessor.retryLogic.fastForward(29.seconds) - every { - mocks.firefoxAccount.setDeviceName(any()) - } throws networkException andThen testLocalDevice - - mocks.verifyAction(setDeviceNameAction) { _, inner, eventHandler, _ -> - coVerifySequence { - // This throws again and the timeout period is still active, we should fail - inner.setDeviceName("My Phone") - eventHandler.onFxaEvent( - FxaEvent.DeviceOperationFailed(FxaDeviceOperation.SET_DEVICE_NAME), - ) - } - FxaAuthState.CONNECTED - } } @Test - fun `FxaActionProcessor retrys network errors again after a timeout period`() = runTest { + fun `FxaActionProcessor fails after 4 network errors in a row`() = runTest { val mocks = Mocks.create(FxaAuthState.CONNECTED) - every { mocks.firefoxAccount.setDeviceName(any()) } throws networkException andThen testLocalDevice + every { mocks.firefoxAccount.setDeviceName(any()) } throws networkException mocks.verifyAction(setDeviceNameAction) { _, inner, eventHandler, _ -> coVerifySequence { - // This fails with FxaException.Network, we should retry + // These throws FxaException.Network and we should retry inner.setDeviceName("My Phone") - // This time it works inner.setDeviceName("My Phone") - eventHandler.onFxaEvent( - FxaEvent.DeviceOperationComplete(FxaDeviceOperation.SET_DEVICE_NAME, testLocalDevice), - ) - } - FxaAuthState.CONNECTED - } - - mocks.actionProcessor.retryLogic.fastForward(31.seconds) - every { mocks.firefoxAccount.setDeviceName(any()) } throws networkException andThen testLocalDevice - - mocks.verifyAction(setDeviceNameAction) { _, inner, eventHandler, _ -> - coVerifySequence { - // Timeout period over, we should retry this time inner.setDeviceName("My Phone") - // This time it works + // On the 4th error, we give up inner.setDeviceName("My Phone") - eventHandler.onFxaEvent( - FxaEvent.DeviceOperationComplete(FxaDeviceOperation.SET_DEVICE_NAME, testLocalDevice), - ) + eventHandler.onFxaEvent(FxaEvent.DeviceOperationFailed(FxaDeviceOperation.SET_DEVICE_NAME)) } FxaAuthState.CONNECTED } @@ -1158,3 +1111,25 @@ class FxaRetryTest { } } } + +class MetricsParamsTest { + @Test + fun `FxaActionProcessor handles BeginOAuthFlow metrics`() = runTest { + val mocks = Mocks.create(FxaAuthState.DISCONNECTED) + val testMetrics = MetricsParams( + parameters = mapOf("foo" to "bar"), + ) + mocks.actionProcessor.processAction(beginOAuthFlowAction.copy(metrics = testMetrics)) + coVerify { mocks.firefoxAccount.beginOauthFlow(any(), any(), testMetrics) } + } + + @Test + fun `FxaActionProcessor handles BeginPairingFlow metrics`() = runTest { + val mocks = Mocks.create(FxaAuthState.DISCONNECTED) + val testMetrics = MetricsParams( + parameters = mapOf("foo" to "bar"), + ) + mocks.actionProcessor.processAction(beginPairingFlowAction.copy(metrics = testMetrics)) + coVerify { mocks.firefoxAccount.beginPairingFlow(any(), any(), any(), testMetrics) } + } +} diff --git a/components/fxa-client/src/error.rs b/components/fxa-client/src/error.rs index 3d9f5d6df2..e3c3d6ceff 100644 --- a/components/fxa-client/src/error.rs +++ b/components/fxa-client/src/error.rs @@ -39,8 +39,8 @@ pub enum FxaError { #[error("the requested authentication flow was not active")] WrongAuthFlow, /// A required scoped key was missing in the server response - #[error("A required scoped key was missing")] - ScopedKeyMissing, + #[error("The sync scoped key was missing")] + SyncScopedKeyMissing, /// Thrown if there is a panic in the underlying Rust code. /// /// **Note:** This error is currently only thrown in the Kotlin language bindings. @@ -106,8 +106,8 @@ pub enum Error { #[error("Remote key and local key mismatch")] MismatchedKeys, - #[error("A required scoped key was missing")] - ScopedKeyMissing, + #[error("The sync scoped key was missing")] + SyncScopedKeyMissing, #[error("Client: {0} is not allowed to request scope: {1}")] ScopeNotAllowed(String, String), @@ -199,8 +199,10 @@ impl GetErrorHandling for Error { Error::RequestError(_) => { ErrorHandling::convert(crate::FxaError::Network).log_warning() } - Error::ScopedKeyMissing => ErrorHandling::convert(crate::FxaError::ScopedKeyMissing) - .report_error("fxa-client-scoped-key-missing"), + Error::SyncScopedKeyMissing => { + ErrorHandling::convert(crate::FxaError::SyncScopedKeyMissing) + .report_error("fxa-client-scoped-key-missing") + } _ => ErrorHandling::convert(crate::FxaError::Other) .report_error("fxa-client-other-error"), } diff --git a/components/fxa-client/src/fxa_client.udl b/components/fxa-client/src/fxa_client.udl index 5d17d23796..a62805e655 100644 --- a/components/fxa-client/src/fxa_client.udl +++ b/components/fxa-client/src/fxa_client.udl @@ -75,8 +75,8 @@ enum FxaError { // **Note:** This error is currently only thrown in the Swift language bindings. "WrongAuthFlow", - // A required scoped key was missing in the server response - "ScopedKeyMissing", + // The sync scoped key was missing in the server response + "SyncScopedKeyMissing", // Thrown if there is a panic in the underlying Rust code. // @@ -583,8 +583,6 @@ interface FirefoxAccount { // - This must be one of the scopes requested during the signin flow. // - Only a single scope is supported; for multiple scopes request multiple tokens. // - `ttl` - optionally, the time for which the token should be valid, in seconds. - // - `require_scoped_key` - optionally, throw [FxaError.ScopedKeyMissing] if the scoped_key - // is not present in the server response. // // # Notes // @@ -593,7 +591,7 @@ interface FirefoxAccount { // before requesting a fresh token. // [Throws=FxaError] - AccessTokenInfo get_access_token([ByRef] string scope, optional i64? ttl = null, optional boolean require_scoped_key = false); + AccessTokenInfo get_access_token([ByRef] string scope, optional i64? ttl = null); // Get the session token for the user's account, if one is available. // diff --git a/components/fxa-client/src/internal/device.rs b/components/fxa-client/src/internal/device.rs index e6a30e85fd..4167bb82d3 100644 --- a/components/fxa-client/src/internal/device.rs +++ b/components/fxa-client/src/internal/device.rs @@ -97,6 +97,8 @@ impl FirefoxAccount { device_type: DeviceType, capabilities: &[DeviceCapability], ) -> Result { + self.state + .set_device_capabilities(capabilities.iter().cloned()); let commands = self.register_capabilities(capabilities)?; let update = DeviceUpdateRequestBuilder::new() .display_name(name) @@ -117,6 +119,8 @@ impl FirefoxAccount { &mut self, capabilities: &[DeviceCapability], ) -> Result { + self.state + .set_device_capabilities(capabilities.iter().cloned()); // Don't re-register if we already have exactly those capabilities. if let Some(local_device) = self.state.server_local_device_info() { if capabilities == local_device.capabilities { @@ -132,13 +136,12 @@ impl FirefoxAccount { /// Re-register the device capabilities, this should only be used internally. pub(crate) fn reregister_current_capabilities(&mut self) -> Result<()> { - if let Some(local_device) = self.state.server_local_device_info().cloned() { - let commands = self.register_capabilities(&local_device.capabilities)?; - let update = DeviceUpdateRequestBuilder::new() - .available_commands(&commands) - .build(); - self.update_device(update)?; - } + let capabilities: Vec<_> = self.state.device_capabilities().iter().cloned().collect(); + let commands = self.register_capabilities(&capabilities)?; + let update = DeviceUpdateRequestBuilder::new() + .available_commands(&commands) + .build(); + self.update_device(update)?; Ok(()) } diff --git a/components/fxa-client/src/internal/mod.rs b/components/fxa-client/src/internal/mod.rs index 917c7e81d1..234502cfc1 100644 --- a/components/fxa-client/src/internal/mod.rs +++ b/components/fxa-client/src/internal/mod.rs @@ -13,7 +13,10 @@ use self::{ }; use crate::{Error, FxaConfig, FxaRustAuthState, Result}; use serde_derive::*; -use std::{collections::HashMap, sync::Arc}; +use std::{ + collections::{HashMap, HashSet}, + sync::Arc, +}; use url::Url; #[cfg(feature = "integration_test")] @@ -73,6 +76,7 @@ impl FirefoxAccount { scoped_keys: HashMap::new(), last_handled_command: None, commands_data: HashMap::new(), + device_capabilities: HashSet::new(), server_local_device_info: None, session_token: None, current_device_id: None, @@ -261,7 +265,6 @@ mod tests { use crate::internal::device::*; use crate::internal::http_client::FxAClientMock; use crate::internal::oauth::*; - use std::collections::HashSet; #[test] fn test_fxa_is_send() { diff --git a/components/fxa-client/src/internal/oauth.rs b/components/fxa-client/src/internal/oauth.rs index b9c62775c8..3fdc66a804 100644 --- a/components/fxa-client/src/internal/oauth.rs +++ b/components/fxa-client/src/internal/oauth.rs @@ -42,21 +42,16 @@ impl FirefoxAccount { /// in the server response. /// /// **💾 This method may alter the persisted account state.** - pub fn get_access_token( - &mut self, - scope: &str, - ttl: Option, - require_scoped_key: bool, - ) -> Result { + pub fn get_access_token(&mut self, scope: &str, ttl: Option) -> Result { if scope.contains(' ') { return Err(Error::MultipleScopesRequested); } if let Some(oauth_info) = self.state.get_cached_access_token(scope) { if oauth_info.expires_at > util::now_secs() + OAUTH_MIN_TIME_LEFT { - if require_scoped_key { - oauth_info.check_scoped_key_present()? + // If the cached key is missing the required sync scoped key, try to fetch it again + if oauth_info.check_missing_sync_scoped_key().is_ok() { + return Ok(oauth_info.clone()); } - return Ok(oauth_info.clone()); } } let resp = match self.state.refresh_token() { @@ -93,9 +88,7 @@ impl FirefoxAccount { }; self.state .add_cached_access_token(scope, token_info.clone()); - if require_scoped_key { - token_info.check_scoped_key_present()? - } + token_info.check_missing_sync_scoped_key()?; Ok(token_info) } @@ -564,9 +557,9 @@ pub struct AccessTokenInfo { } impl AccessTokenInfo { - pub fn check_scoped_key_present(&self) -> Result<()> { - if self.key.is_none() { - Err(Error::ScopedKeyMissing) + pub fn check_missing_sync_scoped_key(&self) -> Result<()> { + if self.scope == scopes::OLD_SYNC && self.key.is_none() { + Err(Error::SyncScopedKeyMissing) } else { Ok(()) } diff --git a/components/fxa-client/src/internal/profile.rs b/components/fxa-client/src/internal/profile.rs index d719103b72..c7e58d8945 100644 --- a/components/fxa-client/src/internal/profile.rs +++ b/components/fxa-client/src/internal/profile.rs @@ -45,7 +45,7 @@ impl FirefoxAccount { } etag = Some(cached_profile.etag.clone()); } - let profile_access_token = self.get_access_token(scopes::PROFILE, None, false)?.token; + let profile_access_token = self.get_access_token(scopes::PROFILE, None)?.token; match self .client .get_profile(self.state.config(), &profile_access_token, etag)? diff --git a/components/fxa-client/src/internal/state_manager.rs b/components/fxa-client/src/internal/state_manager.rs index d6c91e87aa..2fa283a569 100644 --- a/components/fxa-client/src/internal/state_manager.rs +++ b/components/fxa-client/src/internal/state_manager.rs @@ -2,7 +2,7 @@ * 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 std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use crate::{ internal::{ @@ -11,7 +11,7 @@ use crate::{ state_persistence::state_to_json, CachedResponse, Config, OAuthFlow, PersistedState, }, - FxaRustAuthState, LocalDevice, Result, ScopedKey, + DeviceCapability, FxaRustAuthState, LocalDevice, Result, ScopedKey, }; /// Stores and manages the current state of the FxA client @@ -49,6 +49,21 @@ impl StateManager { self.persisted_state.session_token.as_deref() } + /// Get our device capabilities + /// + /// This is the last set of capabilities passed to `initialize_device` or `ensure_capabilities` + pub fn device_capabilities(&self) -> &HashSet { + &self.persisted_state.device_capabilities + } + + /// Set our device capabilities + pub fn set_device_capabilities( + &mut self, + capabilities_set: impl IntoIterator, + ) { + self.persisted_state.device_capabilities = HashSet::from_iter(capabilities_set); + } + /// Get the last known LocalDevice info sent back from the server pub fn server_local_device_info(&self) -> Option<&LocalDevice> { self.persisted_state.server_local_device_info.as_ref() @@ -180,6 +195,7 @@ impl StateManager { self.persisted_state.last_handled_command = None; self.persisted_state.commands_data = HashMap::new(); self.persisted_state.access_token_cache = HashMap::new(); + self.persisted_state.device_capabilities = HashSet::new(); self.persisted_state.server_local_device_info = None; self.persisted_state.session_token = None; self.persisted_state.logged_out_from_auth_issues = false; @@ -194,6 +210,7 @@ impl StateManager { self.persisted_state.last_handled_command = None; self.persisted_state.commands_data = HashMap::new(); self.persisted_state.access_token_cache = HashMap::new(); + self.persisted_state.device_capabilities = HashSet::new(); self.persisted_state.server_local_device_info = None; self.persisted_state.session_token = None; self.persisted_state.logged_out_from_auth_issues = true; diff --git a/components/fxa-client/src/internal/state_persistence.rs b/components/fxa-client/src/internal/state_persistence.rs index 6b0c5cbffe..3f7e1643c3 100644 --- a/components/fxa-client/src/internal/state_persistence.rs +++ b/components/fxa-client/src/internal/state_persistence.rs @@ -30,7 +30,7 @@ //! The code that was deleted demonstrates how we can implement the migration use serde_derive::*; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use super::{ config::Config, @@ -38,7 +38,7 @@ use super::{ profile::Profile, CachedResponse, Result, }; -use crate::{LocalDevice, ScopedKey}; +use crate::{DeviceCapability, LocalDevice, ScopedKey}; // These are the public API for working with the persisted state. @@ -100,6 +100,8 @@ pub(crate) struct StateV2 { #[serde(default)] pub(crate) commands_data: HashMap, #[serde(default)] + pub(crate) device_capabilities: HashSet, + #[serde(default)] pub(crate) access_token_cache: HashMap, pub(crate) session_token: Option, // Hex-formatted string. pub(crate) last_seen_profile: Option>, diff --git a/components/fxa-client/src/token.rs b/components/fxa-client/src/token.rs index f46bed285a..ca9e45b43c 100644 --- a/components/fxa-client/src/token.rs +++ b/components/fxa-client/src/token.rs @@ -38,8 +38,6 @@ impl FirefoxAccount { /// - This must be one of the scopes requested during the signin flow. /// - Only a single scope is supported; for multiple scopes request multiple tokens. /// - `ttl` - optionally, the time for which the token should be valid, in seconds. - /// - `require_scoped_key` - optionally, throw [FxaError.ScopedKeyMissing] if the scoped_key - /// is not present in the server response. /// /// # Notes /// @@ -47,17 +45,12 @@ impl FirefoxAccount { /// token, it should call [`clear_access_token_cache`](FirefoxAccount::clear_access_token_cache) /// before requesting a fresh token. #[handle_error(Error)] - pub fn get_access_token( - &self, - scope: &str, - ttl: Option, - requre_scoped_key: bool, - ) -> ApiResult { + pub fn get_access_token(&self, scope: &str, ttl: Option) -> ApiResult { // Signedness converstion for Kotlin compatibility :-/ let ttl = ttl.map(|ttl| u64::try_from(ttl).unwrap_or_default()); self.internal .lock() - .get_access_token(scope, ttl, requre_scoped_key)? + .get_access_token(scope, ttl)? .try_into() } diff --git a/examples/cli-support/src/fxa_creds.rs b/examples/cli-support/src/fxa_creds.rs index 4de6b13974..4def44a933 100644 --- a/examples/cli-support/src/fxa_creds.rs +++ b/examples/cli-support/src/fxa_creds.rs @@ -85,7 +85,7 @@ pub fn get_account_and_token( // TODO: we should probably set a persist callback on acct? let mut acct = load_or_create_fxa_creds(cred_file, config.clone())?; // `scope` could be a param, but I can't see it changing. - match acct.get_access_token(SYNC_SCOPE, None, false) { + match acct.get_access_token(SYNC_SCOPE, None) { Ok(t) => Ok((acct, t)), Err(e) => { match e { @@ -93,7 +93,7 @@ pub fn get_account_and_token( FxaError::Authentication => { println!("Saw an auth error using stored credentials - recreating them..."); acct = create_fxa_creds(cred_file, config)?; - let token = acct.get_access_token(SYNC_SCOPE, None, false)?; + let token = acct.get_access_token(SYNC_SCOPE, None)?; Ok((acct, token)) } _ => Err(e.into()),