remove: refactor to make it more reliable (#257)

fixes #256

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) {
+			let mut revocation: Option<String> = None;
-				Ok(success) => {
+			loop {
-					if success {
+				match account.remove_authenticator(revocation.as_ref()) {
-						println!("Removed authenticator from {}", account.account_name);
+					Ok(_) => {
+						info!("Removed authenticator from {}", account.account_name);
 						successful.push(account.account_name.clone());
-					} else {
+						break;
-						println!(
+					}
-							"Failed to remove authenticator from {}",
+					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(
+						eprint!("Enter the revocation code for {}: ", account.account_name);
-							"Would you like to remove it from the manifest anyway?",
+						let code = tui::prompt();
-							"yN",
+						revocation = Some(code);
-						) == 'y'
-						{
-							successful.push(account.account_name.clone());
-						}
-					}
 					}
 					Err(err) => {
 						error!(
 							"Unexpected error when removing authenticator from {}: {}",
 							account.account_name, err
 						);
+						break;
+					}
 				}
 			}
 		}

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<String>) -> anyhow::Result<bool> {
+	pub fn remove_authenticator(
-		ensure!(
+		&self,
-			matches!(revocation_code, Some(_)) || !self.revocation_code.expose_secret().is_empty(),
+		revocation_code: Option<&String>,
-			"Revocation code not provided."
+	) -> 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))
+		// returns EResult::TwoFactorCodeMismatch if the revocation code is incorrect
-		} else {
+		if resp.result != EResult::OK && resp.result != EResult::TwoFactorCodeMismatch {
-			Ok(true)
+			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<EResult> for RemoveAuthenticatorError {
+	fn from(e: EResult) -> Self {
+		Self::UnknownEResult(e)
+	}
 }

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<ApiResponse<CTwoFactor_RemoveAuthenticator_Response>> {
+	) -> Result<ApiResponse<CTwoFactor_RemoveAuthenticator_Response>, TransportError> {
 		let req = ApiRequest::new(SERVICE_NAME, "RemoveAuthenticator", 1, req)
 			.with_access_token(access_token);
 		let resp = self