From be75311e92309109147c4b6d742e23a192b253c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa?= <dani-garcia@users.noreply.github.com> Date: Mon, 21 Oct 2024 17:37:04 +0200 Subject: [PATCH] [PM-13420] Support case-insensitive TOTP generation (#1144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## đī¸ Tracking <!-- Paste the link to the Jira or GitHub issue or otherwise describe / point to where this change is coming from. --> https://bitwarden.atlassian.net/browse/PM-13420 ## đ Objective Update TOTP parsing to be case insensitive by lowercasing the input initially. `decode_b32` is already converting the input to uppercase so secret decoding will still work the same. I've also removed the `to_uppercase` from algorithm now that we know the string is lowercase. ## â° Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## đĻŽ Reviewer guidelines <!-- Suggested interactions but feel free to use (or not) as you desire! --> - đ (`:+1:`) or similar for great changes - đ (`:memo:`) or âšī¸ (`:information_source:`) for notes or general info - â (`:question:`) for questions - đ¤ (`:thinking:`) or đ (`:thought_balloon:`) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion - đ¨ (`:art:`) for suggestions / improvements - â (`:x:`) or â ī¸ (`:warning:`) for more significant problems or concerns needing attention - đą (`:seedling:`) or âģī¸ (`:recycle:`) for future improvements or indications of technical debt - â (`:pick:`) for minor or nitpick changes --- crates/bitwarden-vault/src/totp.rs | 44 ++++++++++++++++++++++++++---- 1 file changed, 38 insertions(+), 6 deletions(-) diff --git a/crates/bitwarden-vault/src/totp.rs b/crates/bitwarden-vault/src/totp.rs index 8aee3e694..d6167a1ab 100644 --- a/crates/bitwarden-vault/src/totp.rs +++ b/crates/bitwarden-vault/src/totp.rs @@ -157,17 +157,19 @@ impl FromStr for Totp { /// - OTP Auth URI /// - Steam URI fn from_str(key: &str) -> Result<Self, Self::Err> { + let key = key.to_lowercase(); + let params = if key.starts_with("otpauth://") { - let url = Url::parse(key).map_err(|_| TotpError::InvalidOtpauth)?; + let url = Url::parse(&key).map_err(|_| TotpError::InvalidOtpauth)?; let parts: HashMap<_, _> = url.query_pairs().collect(); Totp { algorithm: parts .get("algorithm") - .and_then(|v| match v.to_uppercase().as_ref() { - "SHA1" => Some(Algorithm::Sha1), - "SHA256" => Some(Algorithm::Sha256), - "SHA512" => Some(Algorithm::Sha512), + .and_then(|v| match v.as_ref() { + "sha1" => Some(Algorithm::Sha1), + "sha256" => Some(Algorithm::Sha256), + "sha512" => Some(Algorithm::Sha512), _ => None, }) .unwrap_or(DEFAULT_ALGORITHM), @@ -200,7 +202,7 @@ impl FromStr for Totp { algorithm: DEFAULT_ALGORITHM, digits: DEFAULT_DIGITS, period: DEFAULT_PERIOD, - secret: decode_b32(key), + secret: decode_b32(&key), } }; @@ -285,6 +287,7 @@ mod tests { ("PIUD1IS!EQYA=", "829846"), // sanitized // Steam ("steam://HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ", "7W6CJ"), + ("StEam://HXDMVJECJJWSRB3HWIZR4IFUGFTMXBOZ", "7W6CJ"), ("steam://ABCD123", "N26DF"), // Various weird lengths ("ddfdf", "932653"), @@ -321,6 +324,20 @@ mod tests { assert_eq!(response.period, 30); } + #[test] + fn test_generate_otpauth_uppercase() { + let key = "OTPauth://totp/test-account?secret=WQIQ25BRKZYCJVYP".to_string(); + let time = Some( + DateTime::parse_from_rfc3339("2023-01-01T00:00:00.000Z") + .unwrap() + .with_timezone(&Utc), + ); + let response = generate_totp(key, time).unwrap(); + + assert_eq!(response.code, "194506".to_string()); + assert_eq!(response.period, 30); + } + #[test] fn test_generate_otpauth_period() { let key = "otpauth://totp/test-account?secret=WQIQ25BRKZYCJVYP&period=60".to_string(); @@ -335,6 +352,21 @@ mod tests { assert_eq!(response.period, 60); } + #[test] + fn test_generate_otpauth_algorithm_sha256() { + let key = + "otpauth://totp/test-account?secret=WQIQ25BRKZYCJVYP&algorithm=SHA256".to_string(); + let time = Some( + DateTime::parse_from_rfc3339("2023-01-01T00:00:00.000Z") + .unwrap() + .with_timezone(&Utc), + ); + let response = generate_totp(key, time).unwrap(); + + assert_eq!(response.code, "842615".to_string()); + assert_eq!(response.period, 30); + } + #[test] fn test_generate_totp_cipher_view() { let view = CipherListView {