fix(totp): accept secrets shorter than 128 bits

feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/login/LoginContent.kt

@@ -61,7 +61,6 @@ import de.davis.keygo.feature.item.create.presentation.component.KeyGoItemForm
 import de.davis.keygo.feature.item.create.presentation.component.OverrideTotpDialog
 import de.davis.keygo.feature.item.create.presentation.component.SelectItemForTotpModificationDialog
 import de.davis.keygo.feature.item.create.presentation.component.TAG_DELIMITERS
-import de.davis.keygo.feature.item.create.presentation.component.TotpParseErrorDialog
 import de.davis.keygo.feature.item.create.presentation.login.model.DialogState
 import de.davis.keygo.feature.item.create.presentation.login.model.LoginBaseState
 import de.davis.keygo.feature.item.create.presentation.login.model.LoginPasskeyInfo
@@ -73,6 +72,7 @@ import de.davis.keygo.feature.item.create.presentation.model.SharedItemState
 import de.davis.keygo.feature.item.create.presentation.model.VaultsState
 import de.davis.keygo.feature.item.create.presentation.password.GeneratePasswordModalBottomSheet
 import de.davis.keygo.feature.totp.presentation.component.QRScanner
+import de.davis.keygo.feature.totp.presentation.component.TotpParseErrorDialog
 import de.davis.keygo.core.item.R as CoreItemR
 import de.davis.keygo.feature.item.core.R as ItemCoreR
 

feature/item/create/src/main/res/values/strings.xml

@@ -11,9 +11,6 @@
     <string name="override_totp_fields">Override TOTP fields?</string>
     <string name="overriding_totp_fields_description">The current item has the following TOTP fields that differ from the fields specified in the totp information. Select fields you want to override.</string>
 
-    <string name="totp_parse_error">TOTP Parse Error</string>
-    <string name="totp_parse_error_description">The TOTP code could not be parsed.</string>
-
     <string name="totp_secret">TOTP Secret</string>
 
     <string name="override">Override</string>
@@ -29,7 +26,6 @@
     <string name="existing_entries_found_description">One or more entries match the issuer or account name from the totp code. Would you like to update an existing entry or create a new one?</string>
 
     <string name="warning">Warning</string>
-    <string name="ok">OK</string>
     <string name="cancel">Cancel</string>
 
     <string name="delete_passkey">Delete Passkey</string>

feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginContent.kt

@@ -92,6 +92,7 @@ import de.davis.keygo.feature.item.view.login.model.ViewLoginUiEvent
 import de.davis.keygo.feature.item.view.onHold
 import de.davis.keygo.feature.totp.domain.model.TotpValue
 import de.davis.keygo.feature.totp.presentation.component.QRScanner
+import de.davis.keygo.feature.totp.presentation.component.TotpParseErrorDialog
 import de.davis.keygo.core.item.R as CoreItemR
 import de.davis.keygo.core.ui.R as CoreUiR
 import de.davis.keygo.feature.item.core.R as ItemCoreR
@@ -460,6 +461,13 @@ fun ViewLoginContent(state: ViewLoginState, onEvent: (ViewLoginUiEvent) -> Unit)
         }
     }
 
+    if (state.totpParseError) {
+        TotpParseErrorDialog(
+            onDismiss = { onEvent(ViewLoginUiEvent.OnTotpParseErrorDismiss) },
+            modifier = Modifier.fillMaxWidth(),
+        )
+    }
+
     if (state.scanning) {
         QRScanner(
             onClose = { onEvent(ViewLoginUiEvent.OnBackClick) },

feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/ViewLoginViewModel.kt

@@ -16,6 +16,7 @@ import de.davis.keygo.core.util.domain.resolver.RegistrableDomainResolver
 import de.davis.keygo.core.util.domain.usecase.SortUseCase
 import de.davis.keygo.core.util.fold
 import de.davis.keygo.core.util.getOrNull
+import de.davis.keygo.core.util.isSuccess
 import de.davis.keygo.core.util.onFailure
 import de.davis.keygo.core.util.onSuccess
 import de.davis.keygo.feature.item.core.domain.model.ItemUpsertError
@@ -76,6 +77,7 @@ internal class ViewLoginViewModel(
 
     private val _modificationDialogState = MutableStateFlow<ModificationDialog?>(null)
     private val _scanning = MutableStateFlow(false)
+    private val _totpParseError = MutableStateFlow(false)
     private val _itemId = MutableStateFlow<ItemId?>(null)
 
     @OptIn(ExperimentalCoroutinesApi::class)
@@ -135,10 +137,12 @@ internal class ViewLoginViewModel(
         _stateWithoutModification,
         _modificationDialogState,
         _scanning,
-    ) { state, modificationDialog, scanning ->
+        _totpParseError,
+    ) { state, modificationDialog, scanning, totpParseError ->
         state.copy(
             modificationDialog = modificationDialog,
             scanning = scanning,
+            totpParseError = totpParseError,
         )
     }.stateIn(
         scope = viewModelScope,
@@ -230,10 +234,14 @@ internal class ViewLoginViewModel(
 
             is ViewLoginUiEvent.OnCodesScanned -> {
                 _scanning.update { false }
-                val uriOrSecret = event.codes.firstNotNullOfOrNull { qrCode ->
-                    // This just validates whether `qrCode` is a valid totp uri - we return qrCode
-                    totpService.getInfoFromUriWithResult(qrCode).getOrNull()?.let { qrCode }
-                } ?: return
+                val uriOrSecret = event.codes.firstOrNull {
+                    totpService.getInfoFromUriWithResult(it).isSuccess()
+                }
+
+                if (uriOrSecret == null) {
+                    _totpParseError.update { true }
+                    return
+                }
 
                 _itemId.value?.let { id ->
                     viewModelScope.launch {
@@ -247,6 +255,10 @@ internal class ViewLoginViewModel(
                 }
             }
 
+            ViewLoginUiEvent.OnTotpParseErrorDismiss -> {
+                _totpParseError.update { false }
+            }
+
             is ViewLoginUiEvent.OnSubmitModification -> {
                 val dialog = _modificationDialogState.value ?: return
                 val newText = fieldUpdate(event.input)

feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/model/ViewLoginState.kt

@@ -21,5 +21,6 @@ data class ViewLoginState(
     val note: String = "",
     val modificationDialog: ModificationDialog? = null,
     val scanning: Boolean = false,
+    val totpParseError: Boolean = false,
     val pinned: Boolean = false,
 )

feature/item/view/src/main/kotlin/de/davis/keygo/feature/item/view/login/model/ViewLoginUiEvent.kt

@@ -16,4 +16,5 @@ sealed interface ViewLoginUiEvent {
 
     data object OnScanCodeRequest : ViewLoginUiEvent
     data class OnCodesScanned(val codes: List<String>) : ViewLoginUiEvent
+    data object OnTotpParseErrorDismiss : ViewLoginUiEvent
 }

feature/item/create/src/main/kotlin/de/davis/keygo/feature/item/create/presentation/component/TotpParseErrorDialog.kt → feature/totp/src/main/kotlin/de/davis/keygo/feature/totp/presentation/component/TotpParseErrorDialog.kt

@@ -1,12 +1,12 @@
-package de.davis.keygo.feature.item.create.presentation.component
+package de.davis.keygo.feature.totp.presentation.component
 
 import androidx.compose.material3.AlertDialog
 import androidx.compose.material3.Text
 import androidx.compose.material3.TextButton
 import androidx.compose.runtime.Composable
 import androidx.compose.ui.Modifier
 import androidx.compose.ui.res.stringResource
-import de.davis.keygo.feature.item.create.R
+import de.davis.keygo.feature.totp.R
 
 @Composable
 fun TotpParseErrorDialog(onDismiss: () -> Unit, modifier: Modifier = Modifier) {

feature/totp/src/main/res/values/strings.xml

@@ -9,4 +9,8 @@
     <string name="permission_title">Camera permission</string>
     <string name="permission_denied_permanently">Camera permission was denied. To scan QR codes, please enable it in your app settings.</string>
     <string name="open_settings">Open Settings</string>
+    <string name="ok">OK</string>
+
+    <string name="totp_parse_error">TOTP Parse Error</string>
+    <string name="totp_parse_error_description">The TOTP code could not be parsed.</string>
 </resources>

rust/rust-code/lib/src/totp.rs

@@ -1,8 +1,12 @@
+use std::ops::RangeInclusive;
 use std::time::SystemTimeError;
 use thiserror::Error;
 pub use totp_rs::Algorithm;
 use totp_rs::{Secret, SecretParseError, TOTP, TotpUrlError};
 
+/// Digit counts RFC 6238 permits.
+const ALLOWED_DIGITS: RangeInclusive<usize> = 6..=8;
+
 #[derive(Debug, Error)]
 pub enum TotpError {
     #[error("url error: {0}")]
@@ -13,17 +17,54 @@ pub enum TotpError {
     Secret(SecretParseError),
 }
 
-pub fn get_totp(
+/// The checks `TOTP::new` runs, minus its 128 bit secret floor. The unchecked
+/// constructors skip all of them, so every caller runs this instead.
+fn validate(totp: &TOTP) -> Result<(), TotpError> {
+    if totp.secret.is_empty() {
+        return Err(TotpError::Url(TotpUrlError::SecretSize(0)));
+    }
+
+    if !ALLOWED_DIGITS.contains(&totp.digits) {
+        return Err(TotpError::Url(TotpUrlError::DigitsNumber(totp.digits)));
+    }
+
+    if let Some(issuer) = totp.issuer.as_deref().filter(|it| it.contains(':')) {
+        return Err(TotpError::Url(TotpUrlError::Issuer(issuer.to_string())));
+    }
+
+    if totp.account_name.contains(':') {
+        return Err(TotpError::Url(TotpUrlError::AccountName(
+            totp.account_name.clone(),
+        )));
+    }
+
+    Ok(())
+}
+
+fn build_totp(
     algorithm: Algorithm,
     digits: usize,
     step: u64,
     secret: String,
-) -> Result<String, TotpError> {
+    issuer: Option<String>,
+    account_name: String,
+) -> Result<TOTP, TotpError> {
     let secret = Secret::Encoded(secret)
         .to_bytes()
         .map_err(TotpError::Secret)?;
-    let totp = TOTP::new(algorithm, digits, 1, step, secret, None, "".to_string())
-        .map_err(TotpError::Url)?;
+    let totp = TOTP::new_unchecked(algorithm, digits, 1, step, secret, issuer, account_name);
+    validate(&totp)?;
+
+    Ok(totp)
+}
+
+pub fn get_totp(
+    algorithm: Algorithm,
+    digits: usize,
+    step: u64,
+    secret: String,
+) -> Result<String, TotpError> {
+    let totp = build_totp(algorithm, digits, step, secret, None, "".to_string())?;
     totp.generate_current().map_err(TotpError::Time)
 }
 
@@ -37,10 +78,11 @@ pub struct TotpInfo {
 }
 
 pub fn get_totp_info_from_uri(uri: String) -> Result<TotpInfo, TotpError> {
-    let totp = TOTP::from_url(uri).map_err(TotpError::Url)?;
-    let secret = Secret::Raw(totp.secret.clone()).to_encoded().to_string();
+    let totp = TOTP::from_url_unchecked(uri).map_err(TotpError::Url)?;
+    validate(&totp)?;
+
     Ok(TotpInfo {
-        secret,
+        secret: Secret::Raw(totp.secret.clone()).to_encoded().to_string(),
         issuer: totp.issuer.clone(),
         account_name: totp.account_name.clone(),
         algorithm: totp.algorithm,
@@ -57,15 +99,82 @@ pub fn get_totp_url(
     issuer: Option<String>,
     account_name: String,
 ) -> Result<String, TotpError> {
-    let secret = Secret::Encoded(secret)
-        .to_bytes()
-        .map_err(TotpError::Secret)?;
-
-    let totp = TOTP::new(algorithm, digits, 1, step, secret, issuer, account_name)
-        .map_err(TotpError::Url)?;
+    let totp = build_totp(algorithm, digits, step, secret, issuer, account_name)?;
     Ok(totp.get_url())
 }
 
 pub(crate) fn is_valid_totp_secret(s: &str) -> bool {
     base32::decode(base32::Alphabet::Rfc4648 { padding: false }, s).is_some()
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    /// 80 bits, the kind of secret services like GitHub hand out.
+    const SHORT_SECRET: &str = "JBSWY3DPEHPK3PXP";
+    /// 160 bits, the length RFC 4226 recommends.
+    const LONG_SECRET: &str = "JBSWY3DPEHPK3PXPJBSWY3DPEHPK3PXP";
+
+    fn uri(secret: &str) -> String {
+        format!("otpauth://totp/GitHub:alice?secret={secret}&issuer=GitHub")
+    }
+
+    fn url_with_issuer(issuer: &str) -> Result<String, TotpError> {
+        get_totp_url(
+            Algorithm::SHA1,
+            6,
+            30,
+            SHORT_SECRET.to_string(),
+            Some(issuer.to_string()),
+            "alice".to_string(),
+        )
+    }
+
+    #[test]
+    fn generates_code_for_secret_below_128_bits() {
+        let code = get_totp(Algorithm::SHA1, 6, 30, SHORT_SECRET.to_string()).unwrap();
+        assert_eq!(6, code.len());
+    }
+
+    #[test]
+    fn parses_uri_regardless_of_secret_length() {
+        let short = get_totp_info_from_uri(uri(SHORT_SECRET)).unwrap();
+        let long = get_totp_info_from_uri(uri(LONG_SECRET)).unwrap();
+
+        assert_eq!(SHORT_SECRET, short.secret);
+        assert_eq!(LONG_SECRET, long.secret);
+    }
+
+    #[test]
+    fn builds_url_for_secret_below_128_bits() {
+        let url = url_with_issuer("GitHub").unwrap();
+
+        assert!(url.contains(SHORT_SECRET), "unexpected url: {url}");
+    }
+
+    #[test]
+    fn rejects_empty_secret() {
+        assert!(get_totp(Algorithm::SHA1, 6, 30, "".to_string()).is_err());
+    }
+
+    #[test]
+    fn rejects_digits_outside_rfc_range() {
+        assert!(get_totp(Algorithm::SHA1, 9, 30, SHORT_SECRET.to_string()).is_err());
+    }
+
+    #[test]
+    fn rejects_colon_in_issuer() {
+        assert!(url_with_issuer("Git:Hub").is_err());
+    }
+
+    #[test]
+    fn rejects_uri_without_secret() {
+        assert!(get_totp_info_from_uri(uri("")).is_err());
+    }
+
+    #[test]
+    fn rejects_secret_that_is_not_base32() {
+        assert!(get_totp(Algorithm::SHA1, 6, 30, "not base32!".to_string()).is_err());
+    }
+}