Update OTP Auth Model to closer match spec

AuthenticatorShared/Core/Vault/Services/TOTP/OTPAuthModel.swift

@@ -1,52 +1,128 @@
 import Foundation
 
-/// Model representing the components extracted from an OTP Auth URI.
+// MARK: - OTPAuthModel
+
+/// Model representing the components extracted from an OTP Auth URI
+/// as defined by https://github.com/google/google-authenticator/wiki/Key-Uri-Format
 ///
-/// This model includes the Base32-encoded key, the period, the number of digits, and the hashing algorithm.
 struct OTPAuthModel: Equatable {
-    /// The Base32-encoded key used for generating the OTP.
-    let keyB32: String
-
-    /// The time period in seconds for which the OTP is valid.
-    let period: Int
+    // MARK: Properties
 
-    /// The number of digits in the OTP.
-    let digits: Int
+    /// The username or email associated with this account.
+    /// The google spec suggests this is required, but is optional in the google app.
+    let accountName: String?
 
     /// The hashing algorithm used for generating the OTP.
     let algorithm: TOTPCryptoHashAlgorithm
 
-    /// Initializes a new instance of `OTPAuthModel`.
+    /// The number of digits in the OTP. Default is 6.
+    let digits: Int
+
+    /// The provider or service the account is associated with.
+    let issuer: String?
+
+    /// The time period in seconds for which the OTP is valid. Default is 30.
+    let period: Int
+
+    /// An arbitrary key value encoded in Base 32, used to generate the OTP.
+    let secret: String
+
+    /// A standardized (with all parameters) OTP Auth URI representing this model.
+    var otpAuthUri: String {
+        let label: String
+        let issuerParameter: String
+        switch (issuer, accountName) {
+        case let (.some(issuer), .some(accountName)):
+            label = "\(issuer):\(accountName)"
+            issuerParameter = "&issuer=\(issuer)"
+        case let (.some(issuer), .none):
+            label = ""
+            issuerParameter = "&issuer=\(issuer)"
+        case let (.none, .some(accountName)):
+            label = "\(accountName)"
+            issuerParameter = ""
+        case (.none, .none):
+            label = ""
+            issuerParameter = ""
+        }
+
+        // swiftlint:disable:next line_length
+        return "otpauth://totp/\(label)?secret=\(secret)\(issuerParameter)&algorithm=\(algorithm.rawValue)&digits=\(digits)&period=\(period)"
+    }
+
+    // MARK: Initialization
+
+    /// Initializes a new instance of `OTPAuthModel` by components
     ///
     /// - Parameters:
-    ///   - keyB32: The Base32-encoded key.
-    ///   - period: The time period in seconds for which the OTP is valid.
-    ///   - digits: The number of digits in the OTP.
-    ///   - algorithm: The hashing algorithm used for generating the OTP.
-    init(keyB32: String, period: Int, digits: Int, algorithm: TOTPCryptoHashAlgorithm) {
-        self.keyB32 = keyB32
-        self.period = period
-        self.digits = digits
+    ///   - accountName: The username associated with the account
+    ///   - algorithm: The hashing algorithm to use
+    ///   - digits: The number of digits in the code
+    ///   - issuer: The provider or service of the account
+    ///   - period: The length of time in seconds an OTP is valid
+    ///   - secret: The key value, encoded in Base 32
+    init(
+        accountName: String?,
+        algorithm: TOTPCryptoHashAlgorithm,
+        digits: Int,
+        issuer: String?,
+        period: Int,
+        secret: String
+    ) {
+        self.accountName = accountName
         self.algorithm = algorithm
+        self.digits = digits
+        self.issuer = issuer
+        self.period = period
+        self.secret = secret
     }
 
     /// Parses an OTP Auth URI into its components.
     ///
-    /// - Parameter otpAuthKey: A string representing the OTP Auth URI.
-    ///
-    init?(otpAuthKey: String) {
-        guard let urlComponents = URLComponents(string: otpAuthKey.lowercased()),
+    /// - Parameters:
+    ///   - otpAuthUri: The OTP Auth URI as a string
+    init?(otpAuthUri: String) {
+        guard let urlComponents = URLComponents(string: otpAuthUri),
               urlComponents.scheme == "otpauth",
+              urlComponents.host == "totp",
               let queryItems = urlComponents.queryItems,
               let secret = queryItems.first(where: { $0.name == "secret" })?.value,
-              secret.uppercased().isBase32 else {
+              secret.uppercased().isBase32
+        else {
             return nil
         }
 
-        let period = queryItems.first { $0.name == "period" }?.value.flatMap(Int.init) ?? 30
-        let digits = queryItems.first { $0.name == "digits" }?.value.flatMap(Int.init) ?? 6
         let algorithm = TOTPCryptoHashAlgorithm(from: queryItems.first { $0.name == "algorithm" }?.value)
+        let digits = queryItems.first { $0.name == "digits" }?.value.flatMap(Int.init) ?? 6
+        let issuer = queryItems.first { $0.name == "issuer" }?.value
+        let period = queryItems.first { $0.name == "period" }?.value.flatMap(Int.init) ?? 30
+
+        let labelComponents = urlComponents.path.dropFirst().split(separator: ":")
+
+        let accountName: String?
+        let labelIssuer: String?
+
+        switch labelComponents.count {
+        case 0:
+            accountName = nil
+            labelIssuer = nil
+        case 1:
+            accountName = String(labelComponents[0])
+            labelIssuer = nil
+        case 2:
+            accountName = String(labelComponents[1])
+            labelIssuer = String(labelComponents[0])
+        default:
+            return nil
+        }
 
-        self.init(keyB32: secret, period: period, digits: digits, algorithm: algorithm)
+        self.init(
+            accountName: accountName,
+            algorithm: algorithm,
+            digits: digits,
+            issuer: issuer ?? labelIssuer,
+            period: period,
+            secret: secret
+        )
     }
 }

AuthenticatorShared/Core/Vault/Services/TOTP/OTPAuthModelTests.swift

@@ -7,44 +7,158 @@ import XCTest
 class OTPAuthModelTests: AuthenticatorTestCase {
     // MARK: Tests
 
-    /// Tests that a malformed string does not create a model.
-    func test_init_otpAuthKey_failure_base32() {
-        let subject = OTPAuthModel(otpAuthKey: .base32Key)
+    // MARK: Init Success
+
+    /// `init` parses an account if there is no issuer
+    func test_init_accountNoIssuer() {
+        let key = "otpauth://totp/[email protected]?secret=JBSWY3DPEHPK3PXP"
+        guard let subject = OTPAuthModel(otpAuthUri: key)
+        else { XCTFail("Unable to parse auth model!"); return }
+        XCTAssertEqual(subject.accountName, "[email protected]")
+    }
+
+    /// `init` parses all parameters when available
+    func test_init_allParams() {
+        // swiftlint:disable:next line_length
+        let key = "otpauth://totp/Example:[email protected]?secret=JBSWY3DPEHPK3PXP&issuer=Example&algorithm=SHA256&digits=8&period=60"
+        guard let subject = OTPAuthModel(otpAuthUri: key)
+        else { XCTFail("Unable to parse auth model!"); return }
+        XCTAssertEqual(subject.accountName, "[email protected]")
+        XCTAssertEqual(subject.algorithm, .sha256)
+        XCTAssertEqual(subject.digits, 8)
+        XCTAssertEqual(subject.issuer, "Example")
+        XCTAssertEqual(subject.period, 60)
+        XCTAssertEqual(subject.secret, "JBSWY3DPEHPK3PXP")
+    }
+
+    /// `init`parses the issuer from the label if it's not a parameter
+    func test_init_issuerFromLabel() {
+        let key = "otpauth://totp/Bitwarden:[email protected]?secret=JBSWY3DPEHPK3PXP"
+        guard let subject = OTPAuthModel(otpAuthUri: key)
+        else { XCTFail("Unable to parse auth model!"); return }
+        XCTAssertEqual(subject.issuer, "Bitwarden")
+    }
+
+    /// `init` choses the issuer parameter if it differs from the label
+    func test_init_issuerParameter() {
+        let key = "otpauth://totp/8bit:[email protected]?secret=JBSWY3DPEHPK3PXP&issuer=Bitwarden"
+        guard let subject = OTPAuthModel(otpAuthUri: key)
+        else { XCTFail("Unable to parse auth model!"); return }
+        XCTAssertEqual(subject.issuer, "Bitwarden")
+    }
+
+    /// `init` parses the minimal necessary parameters and picks defaults
+    func test_init_minimumParams() {
+        let key = "otpauth://totp/?secret=JBSWY3DPEHPK3PXP"
+        guard let subject = OTPAuthModel(otpAuthUri: key)
+        else { XCTFail("Unable to parse auth model!"); return }
+        XCTAssertNil(subject.accountName)
+        XCTAssertEqual(subject.algorithm, .sha1)
+        XCTAssertEqual(subject.digits, 6)
+        XCTAssertNil(subject.issuer)
+        XCTAssertEqual(subject.period, 30)
+        XCTAssertEqual(subject.secret, "JBSWY3DPEHPK3PXP")
+    }
+
+    /// `init` handles a percent-encoded colon for issuer and account
+    func test_init_percentEncoding() {
+        let key = "otpauth://totp/Bitwarden%[email protected]?secret=JBSWY3DPEHPK3PXP"
+        guard let subject = OTPAuthModel(otpAuthUri: key)
+        else { XCTFail("Unable to parse auth model!"); return }
+        XCTAssertEqual(subject.accountName, "[email protected]")
+        XCTAssertEqual(subject.issuer, "Bitwarden")
+    }
+
+    // MARK: Init Failure
+
+    /// `init` returns nil if the label contains more than one colon
+    /// since account and issuer cannot have a colon in them
+    func test_init_failure_invalidLabel() {
+        let key = "otpauth://totp/Bitwarden:Engineering:[email protected]?secret=JBSWY3DPEHPK3PXP"
+        let subject = OTPAuthModel(otpAuthUri: key)
         XCTAssertNil(subject)
     }
 
-    /// Tests that a malformed string does not create a model.
-    func test_init_otpAuthKey_failure_incompletePrefix() {
-        let subject = OTPAuthModel(otpAuthKey: "totp/Example:[email protected]?secret=JBSWY3DPEHPK3PXP")
+    /// `init` returns nil if the secret is not valid base 32
+    func test_init_failure_invalidSecret() {
+        let key = "otpauth://totp/[email protected]?secret=invalid-secret"
+        let subject = OTPAuthModel(otpAuthUri: key)
         XCTAssertNil(subject)
     }
 
-    /// Tests that a malformed string does not create a model.
-    func test_init_otpAuthKey_failure_noSecret() {
-        let subject = OTPAuthModel(
-            otpAuthKey: "otpauth://totp/Example:[email protected]?issuer=Example&algorithm=SHA256&digits=6&period=30"
-        )
+    /// `init` returns nil if the scheme is missing or incorrect
+    func test_init_failure_noScheme() {
+        let subject = OTPAuthModel(otpAuthUri: "http://example?secret=JBSWY3DPEHPK3PXP")
         XCTAssertNil(subject)
     }
 
-    /// Tests that a malformed string does not create a model.
-    func test_init_otpAuthKey_failure_steam() {
-        let subject = OTPAuthModel(otpAuthKey: .steamUriKey)
+    /// `init` returns nil if the secret is missing
+    func test_init_failure_noSecret() {
+        let key = "otpauth://totp/Bitwarden:[email protected]?issuer=Bitwarden"
+        let subject = OTPAuthModel(otpAuthUri: key)
         XCTAssertNil(subject)
     }
 
-    /// Tests that a fully formatted OTP Auth string creates the model.
-    func test_init_otpAuthKey_success_full() {
-        let subject = OTPAuthModel(otpAuthKey: .otpAuthUriKeyComplete)
-        XCTAssertNotNil(subject)
+    /// `init` returns nil if the type is not "totp"
+    func test_init_failure_notTotp() {
+        let subject = OTPAuthModel(otpAuthUri: "otpauth://hotp/example?secret=JBSWY3DPEHPK3PXP")
+        XCTAssertNil(subject)
+    }
+
+    // MARK: OTP Auth URI
+
+    /// `otpAuthUri` handles having both an account and an issuer
+    func test_otpAuthUri_BothIssuerAndAccount() {
+        let subject = OTPAuthModel(
+            accountName: "[email protected]",
+            algorithm: .sha1,
+            digits: 6,
+            issuer: "Bitwarden",
+            period: 30,
+            secret: "JBSWY3DPEHPK3PXP"
+        )
+        // swiftlint:disable:next line_length
+        XCTAssertEqual(subject.otpAuthUri, "otpauth://totp/Bitwarden:[email protected]?secret=JBSWY3DPEHPK3PXP&issuer=Bitwarden&algorithm=SHA1&digits=6&period=30")
+    }
+
+    /// `otpAuthUri` handles having neither an issuer nor an account name
+    func test_otpAuthUri_noIssuerOrAccount() {
+        let subject = OTPAuthModel(
+            accountName: nil,
+            algorithm: .sha1,
+            digits: 6,
+            issuer: nil,
+            period: 30,
+            secret: "JBSWY3DPEHPK3PXP"
+        )
+        XCTAssertEqual(subject.otpAuthUri, "otpauth://totp/?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30")
+    }
+
+    /// `otpAuthUri` handles having an issuer but no account
+    func test_otpAuthUri_noAccount() {
+        let subject = OTPAuthModel(
+            accountName: nil,
+            algorithm: .sha1,
+            digits: 6,
+            issuer: "Bitwarden",
+            period: 30,
+            secret: "JBSWY3DPEHPK3PXP"
+        )
+        // swiftlint:disable:next line_length
+        XCTAssertEqual(subject.otpAuthUri, "otpauth://totp/?secret=JBSWY3DPEHPK3PXP&issuer=Bitwarden&algorithm=SHA1&digits=6&period=30")
     }
 
-    /// Tests that a partially formatted OTP Auth string creates the model.
-    func test_init_otpAuthKey_success_partial() {
-        let subject = OTPAuthModel(otpAuthKey: .otpAuthUriKeyPartial)
-        XCTAssertNotNil(subject)
-        XCTAssertEqual(subject?.digits, 6)
-        XCTAssertEqual(subject?.period, 30)
-        XCTAssertEqual(subject?.algorithm, .sha1)
+    /// `otpAuthUri` handles having an account but no issuer
+    func test_otpAuthUri_noIssuer() {
+        let subject = OTPAuthModel(
+            accountName: "[email protected]",
+            algorithm: .sha1,
+            digits: 6,
+            issuer: nil,
+            period: 30,
+            secret: "JBSWY3DPEHPK3PXP"
+        )
+        // swiftlint:disable:next line_length
+        XCTAssertEqual(subject.otpAuthUri, "otpauth://totp/[email protected]?secret=JBSWY3DPEHPK3PXP&algorithm=SHA1&digits=6&period=30")
     }
 }

AuthenticatorShared/Core/Vault/Services/TOTP/TOTPCodeConfigTests.swift

@@ -40,7 +40,7 @@ final class TOTPCodeConfigTests: AuthenticatorTestCase {
         XCTAssertNotNil(subject)
         XCTAssertEqual(
             subject?.base32Key,
-            .base32Key.lowercased()
+            .base32Key
         )
     }
 
@@ -52,7 +52,7 @@ final class TOTPCodeConfigTests: AuthenticatorTestCase {
         XCTAssertNotNil(subject)
         XCTAssertEqual(
             subject?.base32Key,
-            .base32Key.lowercased()
+            .base32Key
         )
     }
 
@@ -64,7 +64,7 @@ final class TOTPCodeConfigTests: AuthenticatorTestCase {
         XCTAssertNotNil(subject)
         XCTAssertEqual(
             subject?.base32Key,
-            .base32Key.lowercased()
+            .base32Key
         )
     }
 

AuthenticatorShared/Core/Vault/Services/TOTP/TOTPKey.swift

@@ -44,7 +44,7 @@ enum TOTPKey: Equatable {
              let .steamUri(key):
             return key
         case let .otpAuthUri(model):
-            return model.keyB32
+            return model.secret
         }
     }
 
@@ -73,7 +73,7 @@ enum TOTPKey: Equatable {
         if key.uppercased().isBase32 {
             self = .base32(key: key)
         } else if key.hasOTPAuthPrefix,
-                  let otpAuthModel = OTPAuthModel(otpAuthKey: key) {
+                  let otpAuthModel = OTPAuthModel(otpAuthUri: key) {
             self = .otpAuthUri(otpAuthModel)
         } else if let keyIndexOffset = key.steamURIKeyIndexOffset {
             let steamKey = String(key.suffix(from: keyIndexOffset))