From b3c6639146d922eda3046082aa78043e767deca2 Mon Sep 17 00:00:00 2001 From: Carson McManus Date: Fri, 30 Jun 2023 10:53:05 -0400 Subject: [PATCH] remove: refactor to make it more reliable (#257) fixes #256 --- src/commands/remove.rs | 65 ++++++++++++++++++---------- steamguard/src/lib.rs | 55 ++++++++++++++++++----- steamguard/src/steamapi/twofactor.rs | 4 +- 3 files changed, 87 insertions(+), 37 deletions(-) diff --git a/src/commands/remove.rs b/src/commands/remove.rs index bcc743f..81cf107 100644 --- a/src/commands/remove.rs +++ b/src/commands/remove.rs @@ -1,6 +1,7 @@ use std::sync::{Arc, Mutex}; use log::*; +use steamguard::{transport::TransportError, RemoveAuthenticatorError}; use crate::{errors::UserError, tui, AccountManager}; @@ -37,35 +38,51 @@ impl AccountCommand for RemoveCommand { let mut successful = vec![]; for a in accounts { let mut account = a.lock().unwrap(); - if !account.is_logged_in() { - info!("Account does not have tokens, logging in"); - crate::do_login(&mut account)?; - } - match account.remove_authenticator(None) { - Ok(success) => { - if success { - println!("Removed authenticator from {}", account.account_name); + let mut revocation: Option = None; + loop { + match account.remove_authenticator(revocation.as_ref()) { + Ok(_) => { + info!("Removed authenticator from {}", account.account_name); successful.push(account.account_name.clone()); - } else { - println!( - "Failed to remove authenticator from {}", + break; + } + Err(RemoveAuthenticatorError::TransportError(TransportError::Unauthorized)) => { + error!("Account {} is not logged in", account.account_name); + crate::do_login(&mut account)?; + continue; + } + Err(RemoveAuthenticatorError::IncorrectRevocationCode { + attempts_remaining, + }) => { + error!( + "Revocation code was incorrect for {} ({} attempts remaining)", + account.account_name, attempts_remaining + ); + if attempts_remaining == 0 { + error!("No attempts remaining, aborting!"); + break; + } + eprint!("Enter the revocation code for {}: ", account.account_name); + let code = tui::prompt(); + revocation = Some(code); + } + Err(RemoveAuthenticatorError::MissingRevocationCode) => { + error!( + "Account {} does not have a revocation code", account.account_name ); - if tui::prompt_char( - "Would you like to remove it from the manifest anyway?", - "yN", - ) == 'y' - { - successful.push(account.account_name.clone()); - } + eprint!("Enter the revocation code for {}: ", account.account_name); + let code = tui::prompt(); + revocation = Some(code); + } + Err(err) => { + error!( + "Unexpected error when removing authenticator from {}: {}", + account.account_name, err + ); + break; } - } - Err(err) => { - error!( - "Unexpected error when removing authenticator from {}: {}", - account.account_name, err - ); } } } diff --git a/steamguard/src/lib.rs b/steamguard/src/lib.rs index 0e8594e..4c8c5c4 100644 --- a/steamguard/src/lib.rs +++ b/steamguard/src/lib.rs @@ -10,6 +10,7 @@ pub use secrecy::{ExposeSecret, SecretString}; use serde::{Deserialize, Serialize}; use std::io::Read; use token::Tokens; +use transport::TransportError; pub use userlogin::{DeviceDetails, LoginError, UserLogin}; #[macro_use] @@ -98,24 +99,56 @@ impl SteamGuardAccount { /// Removes the mobile authenticator from the steam account. If this operation succeeds, this object can no longer be considered valid. /// Returns whether or not the operation was successful. - pub fn remove_authenticator(&self, revocation_code: Option) -> anyhow::Result { - ensure!( - matches!(revocation_code, Some(_)) || !self.revocation_code.expose_secret().is_empty(), - "Revocation code not provided." - ); + pub fn remove_authenticator( + &self, + revocation_code: Option<&String>, + ) -> Result<(), RemoveAuthenticatorError> { + if !matches!(revocation_code, Some(_)) && self.revocation_code.expose_secret().is_empty() { + return Err(RemoveAuthenticatorError::MissingRevocationCode); + } let Some(tokens) = &self.tokens else { - return Err(anyhow!("Tokens not set, login required")); + return Err(RemoveAuthenticatorError::TransportError(TransportError::Unauthorized)); }; let mut client = TwoFactorClient::new(WebApiTransport::new()); let mut req = CTwoFactor_RemoveAuthenticator_Request::new(); req.set_revocation_code( - revocation_code.unwrap_or(self.revocation_code.expose_secret().to_owned()), + revocation_code + .unwrap_or(self.revocation_code.expose_secret()) + .to_owned(), ); let resp = client.remove_authenticator(req, tokens.access_token())?; - if resp.result != EResult::OK { - Err(anyhow!("Failed to remove authenticator: {:?}", resp.result)) - } else { - Ok(true) + + // returns EResult::TwoFactorCodeMismatch if the revocation code is incorrect + if resp.result != EResult::OK && resp.result != EResult::TwoFactorCodeMismatch { + return Err(resp.result.into()); } + let resp = resp.into_response_data(); + if !resp.success() { + return Err(RemoveAuthenticatorError::IncorrectRevocationCode { + attempts_remaining: resp.revocation_attempts_remaining(), + }); + } + + Ok(()) + } +} + +#[derive(Debug, thiserror::Error)] +pub enum RemoveAuthenticatorError { + #[error("Missing revocation code")] + MissingRevocationCode, + #[error("Incorrect revocation code, {attempts_remaining} attempts remaining")] + IncorrectRevocationCode { attempts_remaining: u32 }, + #[error("Transport error: {0}")] + TransportError(#[from] TransportError), + #[error("Steam returned an enexpected result: {0:?}")] + UnknownEResult(EResult), + #[error("Unexpected error: {0}")] + Unknown(#[from] anyhow::Error), +} + +impl From for RemoveAuthenticatorError { + fn from(e: EResult) -> Self { + Self::UnknownEResult(e) } } diff --git a/steamguard/src/steamapi/twofactor.rs b/steamguard/src/steamapi/twofactor.rs index 369e389..86f5774 100644 --- a/steamguard/src/steamapi/twofactor.rs +++ b/steamguard/src/steamapi/twofactor.rs @@ -1,5 +1,5 @@ use crate::token::Jwt; -use crate::transport::Transport; +use crate::transport::{Transport, TransportError}; use super::{ApiRequest, ApiResponse, BuildableRequest}; @@ -59,7 +59,7 @@ where &mut self, req: CTwoFactor_RemoveAuthenticator_Request, access_token: &Jwt, - ) -> anyhow::Result> { + ) -> Result, TransportError> { let req = ApiRequest::new(SERVICE_NAME, "RemoveAuthenticator", 1, req) .with_access_token(access_token); let resp = self