Skip to content

Commit

Permalink
[PM-13420] Support case-insensitive TOTP generation (#1144)
Browse files Browse the repository at this point in the history
## 🎟️ 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
  • Loading branch information
dani-garcia authored Oct 21, 2024
1 parent 39358ac commit be75311
Showing 1 changed file with 38 additions and 6 deletions.
44 changes: 38 additions & 6 deletions crates/bitwarden-vault/src/totp.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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),
}
};

Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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();
Expand All @@ -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 {
Expand Down

0 comments on commit be75311

Please sign in to comment.