From 164bb68c519c42627603ae0a5eacafacb4d0c436 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Wed, 20 Sep 2017 17:06:55 -0400 Subject: [PATCH 01/82] Update to recommended build settings with Xcode 9 --- OneTimePassword.xcodeproj/project.pbxproj | 10 +++++++++- .../xcschemes/OneTimePassword (iOS).xcscheme | 4 +++- .../xcschemes/OneTimePassword (watchOS).xcscheme | 4 +++- 3 files changed, 15 insertions(+), 3 deletions(-) diff --git a/OneTimePassword.xcodeproj/project.pbxproj b/OneTimePassword.xcodeproj/project.pbxproj index 85c5e6f0..4cf4c78f 100644 --- a/OneTimePassword.xcodeproj/project.pbxproj +++ b/OneTimePassword.xcodeproj/project.pbxproj @@ -521,7 +521,7 @@ attributes = { LastSwiftMigration = 0700; LastSwiftUpdateCheck = 0700; - LastUpgradeCheck = 0800; + LastUpgradeCheck = 0900; ORGANIZATIONNAME = "Matt Rubin"; TargetAttributes = { 5B39F4931DBD06BA00CD2DAB = { @@ -730,6 +730,10 @@ isa = XCBuildConfiguration; baseConfigurationReference = C996EC2C1A74D5830076B105 /* Debug.xcconfig */; buildSettings = { + CLANG_WARN_BLOCK_CAPTURE_AUTORELEASING = YES; + CLANG_WARN_COMMA = YES; + CLANG_WARN_RANGE_LOOP_ANALYSIS = YES; + CLANG_WARN_STRICT_PROTOTYPES = YES; IPHONEOS_DEPLOYMENT_TARGET = 8.0; SWIFT_VERSION = 3.0; WATCHOS_DEPLOYMENT_TARGET = 2.0; @@ -740,6 +744,10 @@ isa = XCBuildConfiguration; baseConfigurationReference = C996EC2E1A74D5830076B105 /* Release.xcconfig */; buildSettings = { + CLANG_WARN_BLOCK_CAPTURE_AUTORELEASING = YES; + CLANG_WARN_COMMA = YES; + CLANG_WARN_RANGE_LOOP_ANALYSIS = YES; + CLANG_WARN_STRICT_PROTOTYPES = YES; IPHONEOS_DEPLOYMENT_TARGET = 8.0; SWIFT_VERSION = 3.0; WATCHOS_DEPLOYMENT_TARGET = 2.0; diff --git a/OneTimePassword.xcodeproj/xcshareddata/xcschemes/OneTimePassword (iOS).xcscheme b/OneTimePassword.xcodeproj/xcshareddata/xcschemes/OneTimePassword (iOS).xcscheme index 6b03da75..97fa36ad 100644 --- a/OneTimePassword.xcodeproj/xcshareddata/xcschemes/OneTimePassword (iOS).xcscheme +++ b/OneTimePassword.xcodeproj/xcshareddata/xcschemes/OneTimePassword (iOS).xcscheme @@ -1,6 +1,6 @@ @@ -66,6 +67,7 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" + language = "" launchStyle = "0" useCustomWorkingDirectory = "NO" ignoresPersistentStateOnLaunch = "NO" diff --git a/OneTimePassword.xcodeproj/xcshareddata/xcschemes/OneTimePassword (watchOS).xcscheme b/OneTimePassword.xcodeproj/xcshareddata/xcschemes/OneTimePassword (watchOS).xcscheme index c5f6ec4d..af2297cd 100644 --- a/OneTimePassword.xcodeproj/xcshareddata/xcschemes/OneTimePassword (watchOS).xcscheme +++ b/OneTimePassword.xcodeproj/xcshareddata/xcschemes/OneTimePassword (watchOS).xcscheme @@ -1,6 +1,6 @@ @@ -46,6 +47,7 @@ buildConfiguration = "Debug" selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB" + language = "" launchStyle = "0" useCustomWorkingDirectory = "NO" ignoresPersistentStateOnLaunch = "NO" From 3955b1638a989ecf37e23e0f32883e87db963956 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Wed, 20 Sep 2017 17:10:20 -0400 Subject: [PATCH 02/82] Update to Swift 4 --- OneTimePassword.xcodeproj/project.pbxproj | 11 ++++++----- OneTimePasswordLegacyTests/OTPToken.swift | 18 ++++++++++-------- Sources/Token+URL.swift | 4 ++-- Sources/Token.swift | 4 ++-- Tests/TokenSerializationTests.swift | 2 +- 5 files changed, 21 insertions(+), 18 deletions(-) diff --git a/OneTimePassword.xcodeproj/project.pbxproj b/OneTimePassword.xcodeproj/project.pbxproj index 4cf4c78f..5d0e8520 100644 --- a/OneTimePassword.xcodeproj/project.pbxproj +++ b/OneTimePassword.xcodeproj/project.pbxproj @@ -530,22 +530,23 @@ }; C97C82371946E51D00FD9F4C = { CreatedOnToolsVersion = 6.0; - LastSwiftMigration = 0800; + LastSwiftMigration = 0900; ProvisioningStyle = Manual; }; C97C82421946E51D00FD9F4C = { CreatedOnToolsVersion = 6.0; - LastSwiftMigration = 0800; + LastSwiftMigration = 0900; ProvisioningStyle = Manual; TestTargetID = FD6C3C0B1E0200F800EC4528; }; C9A486B2196F352E00524F51 = { CreatedOnToolsVersion = 6.0; - LastSwiftMigration = 0800; + LastSwiftMigration = 0900; ProvisioningStyle = Manual; }; FD6C3C0B1E0200F800EC4528 = { CreatedOnToolsVersion = 8.2; + LastSwiftMigration = 0900; ProvisioningStyle = Automatic; }; FDD3B8661DD6E59E00F87980 = { @@ -735,7 +736,7 @@ CLANG_WARN_RANGE_LOOP_ANALYSIS = YES; CLANG_WARN_STRICT_PROTOTYPES = YES; IPHONEOS_DEPLOYMENT_TARGET = 8.0; - SWIFT_VERSION = 3.0; + SWIFT_VERSION = 4.0; WATCHOS_DEPLOYMENT_TARGET = 2.0; }; name = Debug; @@ -749,7 +750,7 @@ CLANG_WARN_RANGE_LOOP_ANALYSIS = YES; CLANG_WARN_STRICT_PROTOTYPES = YES; IPHONEOS_DEPLOYMENT_TARGET = 8.0; - SWIFT_VERSION = 3.0; + SWIFT_VERSION = 4.0; WATCHOS_DEPLOYMENT_TARGET = 2.0; }; name = Release; diff --git a/OneTimePasswordLegacyTests/OTPToken.swift b/OneTimePasswordLegacyTests/OTPToken.swift index 9c4945de..0affdb97 100644 --- a/OneTimePasswordLegacyTests/OTPToken.swift +++ b/OneTimePasswordLegacyTests/OTPToken.swift @@ -32,14 +32,14 @@ import OneTimePassword public final class OTPToken: NSObject { required public override init() {} - public var name: String = OTPToken.defaultName - public var issuer: String = OTPToken.defaultIssuer - public var type: OTPTokenType = .timer - public var secret: Data = Data() - public var algorithm: OTPAlgorithm = OTPToken.defaultAlgorithm - public var digits: UInt = OTPToken.defaultDigits - public var period: TimeInterval = OTPToken.defaultPeriod - public var counter: UInt64 = OTPToken.defaultInitialCounter + @objc public var name: String = OTPToken.defaultName + @objc public var issuer: String = OTPToken.defaultIssuer + @objc public var type: OTPTokenType = .timer + @objc public var secret: Data = Data() + @objc public var algorithm: OTPAlgorithm = OTPToken.defaultAlgorithm + @objc public var digits: UInt = OTPToken.defaultDigits + @objc public var period: TimeInterval = OTPToken.defaultPeriod + @objc public var counter: UInt64 = OTPToken.defaultInitialCounter private static let defaultName: String = "" private static let defaultIssuer: String = "" @@ -71,6 +71,7 @@ public final class OTPToken: NSObject { update(with: token) } + @objc public func validate() -> Bool { return (tokenForOTPToken(self) != nil) } @@ -90,6 +91,7 @@ public extension OTPToken { return self.init(token: token) } + @objc func url() -> URL? { guard let token = tokenForOTPToken(self) else { return nil diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 3404803e..1de14be0 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -178,7 +178,7 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? let path = url.path if path.characters.count > 1 { // Skip the leading "/" - name = path.substring(from: path.characters.index(after: path.startIndex)) + name = String(path[path.index(after: path.startIndex)...]) } var issuer = Token.defaultIssuer @@ -195,7 +195,7 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? if !issuer.isEmpty { let prefix = issuer + ":" if name.hasPrefix(prefix), let prefixRange = name.range(of: prefix) { - name = name.substring(from: prefixRange.upperBound) + name = String(name[prefixRange.upperBound...]) name = name.trimmingCharacters(in: CharacterSet.whitespaces) } } diff --git a/Sources/Token.swift b/Sources/Token.swift index d1ca7f12..8c3fc71b 100644 --- a/Sources/Token.swift +++ b/Sources/Token.swift @@ -53,10 +53,10 @@ public struct Token: Equatable { // MARK: Defaults /// The default token name, an empty string. - internal static let defaultName: String = "" + public static let defaultName: String = "" /// The default token issuer, an empty string. - internal static let defaultIssuer: String = "" + public static let defaultIssuer: String = "" // MARK: Password Generation diff --git a/Tests/TokenSerializationTests.swift b/Tests/TokenSerializationTests.swift index 36581498..7703400d 100644 --- a/Tests/TokenSerializationTests.swift +++ b/Tests/TokenSerializationTests.swift @@ -93,7 +93,7 @@ class TokenSerializationTests: XCTestCase { XCTAssertEqual(url.host!, expectedHost, "The url host should be \"\(expectedHost)\"") // Test name let path = url.path - XCTAssertEqual(path.substring(from: path.index(after: path.startIndex)), name, + XCTAssertEqual(String(path[path.index(after: path.startIndex)...]), name, "The url path should be \"\(name)\"") let urlComponents = URLComponents(url: url, resolvingAgainstBaseURL: false) From 9d8785a8c1d8617f8f5e899b57fd566f6dd58cd5 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 21 Sep 2017 12:27:00 -0400 Subject: [PATCH 03/82] Simplify string slicing --- Sources/Token+URL.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 1de14be0..18b6d224 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -178,7 +178,7 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? let path = url.path if path.characters.count > 1 { // Skip the leading "/" - name = String(path[path.index(after: path.startIndex)...]) + name = String(path.dropFirst()) } var issuer = Token.defaultIssuer From 9ec83008444baeed4842135c642acab013369162 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 21 Sep 2017 12:29:32 -0400 Subject: [PATCH 04/82] Simplify string comparison in serialization tests --- Tests/TokenSerializationTests.swift | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/Tests/TokenSerializationTests.swift b/Tests/TokenSerializationTests.swift index 7703400d..72c7cf94 100644 --- a/Tests/TokenSerializationTests.swift +++ b/Tests/TokenSerializationTests.swift @@ -92,9 +92,7 @@ class TokenSerializationTests: XCTestCase { } XCTAssertEqual(url.host!, expectedHost, "The url host should be \"\(expectedHost)\"") // Test name - let path = url.path - XCTAssertEqual(String(path[path.index(after: path.startIndex)...]), name, - "The url path should be \"\(name)\"") + XCTAssertEqual(url.path, "/" + name, "The url path should be \"/\(name)\"") let urlComponents = URLComponents(url: url, resolvingAgainstBaseURL: false) let items = urlComponents?.queryItems From f814e1940181d200127a359c3c08494314ccaef4 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 21 Sep 2017 12:48:01 -0400 Subject: [PATCH 05/82] Use an intermediate Substring to avoid an unnecessary String buffer copy --- Sources/Token+URL.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 18b6d224..0c43eb8b 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -195,8 +195,8 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? if !issuer.isEmpty { let prefix = issuer + ":" if name.hasPrefix(prefix), let prefixRange = name.range(of: prefix) { - name = String(name[prefixRange.upperBound...]) - name = name.trimmingCharacters(in: CharacterSet.whitespaces) + let substringAfterSeparator = name[prefixRange.upperBound...] + name = substringAfterSeparator.trimmingCharacters(in: CharacterSet.whitespaces) } } From 4ee24b8bde7ae4242132e0f6849ac1eb1740e457 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Sep 2017 11:23:59 -0400 Subject: [PATCH 06/82] Refactor name-from-path string manipulation --- Sources/Token+URL.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 0c43eb8b..ac177f6d 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -175,10 +175,11 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? } var name = Token.defaultName - let path = url.path - if path.characters.count > 1 { - // Skip the leading "/" - name = String(path.dropFirst()) + + // Skip the leading "/" + let potentialNameFromPath = url.path.dropFirst() + if !potentialNameFromPath.isEmpty { + name = String(potentialNameFromPath) } var issuer = Token.defaultIssuer From a36aa7a664790d8385409e89da12e14ad6a09c03 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Sep 2017 11:27:09 -0400 Subject: [PATCH 07/82] Refactor token name parsing to simplify handling of empty strings --- Sources/Token+URL.swift | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index ac177f6d..99dab351 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -174,13 +174,8 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? return nil } - var name = Token.defaultName - // Skip the leading "/" - let potentialNameFromPath = url.path.dropFirst() - if !potentialNameFromPath.isEmpty { - name = String(potentialNameFromPath) - } + var name = String(url.path.dropFirst()) var issuer = Token.defaultIssuer if let issuerString = queryDictionary[kQueryIssuerKey] { From 1bf2cd6e422aa226bd70e803c614c49614332085 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Sep 2017 11:28:18 -0400 Subject: [PATCH 08/82] Replace fileprivate with private where possible --- OneTimePasswordLegacyTests/OTPToken.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OneTimePasswordLegacyTests/OTPToken.swift b/OneTimePasswordLegacyTests/OTPToken.swift index 0affdb97..c4df1104 100644 --- a/OneTimePasswordLegacyTests/OTPToken.swift +++ b/OneTimePasswordLegacyTests/OTPToken.swift @@ -66,7 +66,7 @@ public final class OTPToken: NSObject { } } - fileprivate convenience init(token: Token) { + private convenience init(token: Token) { self.init() update(with: token) } From e2fc3c822b5214e3aa1753944c11d03f233d8b9a Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Sep 2017 11:32:20 -0400 Subject: [PATCH 09/82] Remove the defaultName constant Swift 4 required this constant be public in order to be used as a default argument in a public method. Instead of exposing a new static property on Token, just use the empty string literal in the method definition. --- Sources/Token.swift | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Sources/Token.swift b/Sources/Token.swift index 8c3fc71b..bb619d0c 100644 --- a/Sources/Token.swift +++ b/Sources/Token.swift @@ -44,7 +44,7 @@ public struct Token: Equatable { /// - parameter generator: The password generator. /// /// - returns: A new token with the given parameters. - public init(name: String = defaultName, issuer: String = defaultIssuer, generator: Generator) { + public init(name: String = "", issuer: String = defaultIssuer, generator: Generator) { self.name = name self.issuer = issuer self.generator = generator @@ -52,9 +52,6 @@ public struct Token: Equatable { // MARK: Defaults - /// The default token name, an empty string. - public static let defaultName: String = "" - /// The default token issuer, an empty string. public static let defaultIssuer: String = "" From d51fe321df7a88895033968aad8937831ce6df61 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Sep 2017 11:42:30 -0400 Subject: [PATCH 10/82] Remove the defaultIssuer constant Swift 4 required this constant be public in order to be used as a default argument in a public method. Instead of exposing a new static property on Token, just use the empty string literal instead. --- Sources/Token+URL.swift | 2 +- Sources/Token.swift | 7 +------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 99dab351..2acb3aa5 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -177,7 +177,7 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? // Skip the leading "/" var name = String(url.path.dropFirst()) - var issuer = Token.defaultIssuer + var issuer = "" if let issuerString = queryDictionary[kQueryIssuerKey] { issuer = issuerString } else { diff --git a/Sources/Token.swift b/Sources/Token.swift index bb619d0c..55a4bbb7 100644 --- a/Sources/Token.swift +++ b/Sources/Token.swift @@ -44,17 +44,12 @@ public struct Token: Equatable { /// - parameter generator: The password generator. /// /// - returns: A new token with the given parameters. - public init(name: String = "", issuer: String = defaultIssuer, generator: Generator) { + public init(name: String = "", issuer: String = "", generator: Generator) { self.name = name self.issuer = issuer self.generator = generator } - // MARK: Defaults - - /// The default token issuer, an empty string. - public static let defaultIssuer: String = "" - // MARK: Password Generation /// Calculates the current password based on the token's generator. The password generated will From 92b9fa448cfb11f2411b62f7c93185fabac020a9 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Sep 2017 11:46:55 -0400 Subject: [PATCH 11/82] Refactor the parsing of an issuer string from a prefixed name string --- Sources/Token+URL.swift | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 2acb3aa5..72b8c38a 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -180,12 +180,9 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? var issuer = "" if let issuerString = queryDictionary[kQueryIssuerKey] { issuer = issuerString - } else { + } else if let separatorRange = name.range(of: ":") { // If there is no issuer string, try to extract one from the name - let components = name.components(separatedBy: ":") - if components.count > 1 { - issuer = components[0] - } + issuer = String(name[.. Date: Fri, 22 Sep 2017 11:49:03 -0400 Subject: [PATCH 12/82] Refactor issuer string parsing to not use a mutable variable --- Sources/Token+URL.swift | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 72b8c38a..0892a221 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -177,13 +177,17 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? // Skip the leading "/" var name = String(url.path.dropFirst()) - var issuer = "" + let issuer: String if let issuerString = queryDictionary[kQueryIssuerKey] { issuer = issuerString } else if let separatorRange = name.range(of: ":") { // If there is no issuer string, try to extract one from the name issuer = String(name[.. Date: Sat, 23 Sep 2017 19:37:39 -0400 Subject: [PATCH 13/82] Set the .swift-version to 4.0 --- .swift-version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.swift-version b/.swift-version index 9f55b2cc..5186d070 100644 --- a/.swift-version +++ b/.swift-version @@ -1 +1 @@ -3.0 +4.0 From 943ec93967ce6239e66f70a25ef6e325cc1c792e Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Sat, 23 Sep 2017 19:38:05 -0400 Subject: [PATCH 14/82] [Travis] Use Xcode 9 for CI builds --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 18e47890..e04fe08e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -5,7 +5,7 @@ language: objective-c xcode_workspace: OneTimePassword.xcworkspace xcode_scheme: OneTimePassword (iOS) -osx_image: xcode8.2 +osx_image: xcode9 env: - RUNTIME="iOS 8.1" DEVICE="iPad 2" From c4cf6f113821eea77dc8c56ba411f26e636f8d00 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Sat, 23 Sep 2017 19:42:33 -0400 Subject: [PATCH 15/82] [Travis] Configure CI builds to cover new devices and OS versions --- .travis.yml | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/.travis.yml b/.travis.yml index e04fe08e..95179aa1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,24 +9,22 @@ osx_image: xcode9 env: - RUNTIME="iOS 8.1" DEVICE="iPad 2" - - RUNTIME="iOS 8.2" DEVICE="iPhone 4s" - - RUNTIME="iOS 8.3" DEVICE="iPhone 5" - RUNTIME="iOS 8.4" DEVICE="iPhone 5s" - RUNTIME="iOS 9.0" DEVICE="iPhone 6" - - RUNTIME="iOS 9.1" DEVICE="iPhone 6 Plus" - - RUNTIME="iOS 9.2" DEVICE="iPhone 6s" - - RUNTIME="iOS 9.3" DEVICE="iPhone 6s Plus" + - RUNTIME="iOS 9.3" DEVICE="iPhone 7" - RUNTIME="iOS 10.0" DEVICE="iPhone SE" - - RUNTIME="iOS 10.1" DEVICE="iPhone 7" - - RUNTIME="iOS 10.2" DEVICE="iPhone 7 Plus" + - RUNTIME="iOS 10.3" DEVICE="iPhone 8 Plus" + - RUNTIME="iOS 11.0" DEVICE="iPhone X" # Include builds for watchOS matrix: include: - xcode_scheme: OneTimePassword (watchOS) - env: BUILD_ONLY="YES" RUNTIME="watchOS 3.1" DEVICE="Apple Watch Series 2 - 42mm" + env: BUILD_ONLY="YES" RUNTIME="watchOS 4.0" DEVICE="Apple Watch Series 3 - 38mm" - xcode_scheme: OneTimePassword (watchOS) - env: BUILD_ONLY="YES" RUNTIME="watchOS 2.0" DEVICE="Apple Watch - 38mm" + env: BUILD_ONLY="YES" RUNTIME="watchOS 3.2" DEVICE="Apple Watch Series 2 - 42mm" + - xcode_scheme: OneTimePassword (watchOS) + env: BUILD_ONLY="YES" RUNTIME="watchOS 2.2" DEVICE="Apple Watch - 38mm" # Check out nested dependencies before_install: git submodule update --init --recursive From 11b1acc9b6e4ede1e4bcfc9fb897e52a0a9c9de9 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Sat, 23 Sep 2017 20:03:51 -0400 Subject: [PATCH 16/82] [Travis] Adjust iOS and device pairings for CI builds --- .travis.yml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 95179aa1..347f0a3b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -9,11 +9,11 @@ osx_image: xcode9 env: - RUNTIME="iOS 8.1" DEVICE="iPad 2" - - RUNTIME="iOS 8.4" DEVICE="iPhone 5s" - - RUNTIME="iOS 9.0" DEVICE="iPhone 6" - - RUNTIME="iOS 9.3" DEVICE="iPhone 7" + - RUNTIME="iOS 8.4" DEVICE="iPhone 4s" + - RUNTIME="iOS 9.0" DEVICE="iPhone 5s" + - RUNTIME="iOS 9.3" DEVICE="iPhone 6s" - RUNTIME="iOS 10.0" DEVICE="iPhone SE" - - RUNTIME="iOS 10.3" DEVICE="iPhone 8 Plus" + - RUNTIME="iOS 10.3" DEVICE="iPhone 7 Plus" - RUNTIME="iOS 11.0" DEVICE="iPhone X" # Include builds for watchOS From beb95f4ec8b2140e740f1a445a077fd48c42442c Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 21:17:11 -0400 Subject: [PATCH 17/82] Remove invalid SwiftLint rule identifiers --- .swiftlint.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index fb3eebbb..cd57f54f 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -11,7 +11,6 @@ opt_in_rules: - explicit_init - file_header - first_where -# - missing_docs - nimble_operator - operator_usage_whitespace - overridden_super_call @@ -26,7 +25,6 @@ disabled_rules: - cyclomatic_complexity - function_body_length - syntactic_sugar - - valid_docs trailing_comma: mandatory_comma: true From e98e863e6dee5cafba3796a064a18fa2ea13e4ab Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 21:18:37 -0400 Subject: [PATCH 18/82] Temporarily disable SwiftLint's xctfail_message rule --- .swiftlint.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.swiftlint.yml b/.swiftlint.yml index cd57f54f..fd2677ae 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -25,6 +25,7 @@ disabled_rules: - cyclomatic_complexity - function_body_length - syntactic_sugar + - xctfail_message trailing_comma: mandatory_comma: true From 987e12edcb1e1c21ffa47daf9f64d7b5cbe95a3e Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 21:51:16 -0400 Subject: [PATCH 19/82] Enable several new opt-in SwiftLint rules --- .swiftlint.yml | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/.swiftlint.yml b/.swiftlint.yml index fd2677ae..607acfbd 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -9,15 +9,23 @@ opt_in_rules: - conditional_returns_on_newline - empty_count - explicit_init + - fatal_error_message - file_header - first_where + - implicit_return + - implicitly_unwrapped_optional + - joined_default_parameter + - let_var_whitespace - nimble_operator - operator_usage_whitespace - overridden_super_call + - pattern_matching_keywords - private_outlet - prohibited_super_call - redundant_nil_coalescing + - single_test_class - switch_case_on_newline + - unneeded_parentheses_in_closure_argument - vertical_whitespace disabled_rules: - colon From 1df3e40d2f22a3107482d10cb92ee8115fbb9f2d Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 22:32:18 -0400 Subject: [PATCH 20/82] Configure SwiftLint to ignore line length on function declarations --- .swiftlint.yml | 2 ++ Sources/Token+URL.swift | 6 ++---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index 607acfbd..a7223ccc 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -16,6 +16,7 @@ opt_in_rules: - implicitly_unwrapped_optional - joined_default_parameter - let_var_whitespace + - multiline_parameters - nimble_operator - operator_usage_whitespace - overridden_super_call @@ -40,6 +41,7 @@ trailing_comma: line_length: warning: 120 + ignores_function_declarations: true file_header: required_pattern: | diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 0892a221..ebab6da0 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -98,8 +98,7 @@ private func algorithmFromString(_ string: String) -> Generator.Algorithm? { } } -private func urlForToken(name: String, issuer: String, factor: Generator.Factor, algorithm: Generator.Algorithm, - digits: Int) throws -> URL { +private func urlForToken(name: String, issuer: String, factor: Generator.Factor, algorithm: Generator.Algorithm, digits: Int) throws -> URL { var urlComponents = URLComponents() urlComponents.scheme = kOTPAuthScheme urlComponents.path = "/" + name @@ -200,8 +199,7 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? return Token(name: name, issuer: issuer, generator: generator) } -private func parse(_ item: P?, with parser: ((P) -> T?), defaultTo defaultValue: T? = nil, - overrideWith overrideValue: T? = nil) -> T? { +private func parse(_ item: P?, with parser: ((P) -> T?), defaultTo defaultValue: T? = nil, overrideWith overrideValue: T? = nil) -> T? { if let value = overrideValue { return value } From a872b54634adf3899e6d32b4c177568382618c8e Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 22:37:31 -0400 Subject: [PATCH 21/82] Include error messages in all calls to XCTFail() --- .swiftlint.yml | 1 - Tests/EquatableTests.swift | 2 +- Tests/TokenSerializationTests.swift | 2 +- Tests/TokenTests.swift | 18 +++++++++--------- 4 files changed, 11 insertions(+), 12 deletions(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index a7223ccc..1a20006c 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -34,7 +34,6 @@ disabled_rules: - cyclomatic_complexity - function_body_length - syntactic_sugar - - xctfail_message trailing_comma: mandatory_comma: true diff --git a/Tests/EquatableTests.swift b/Tests/EquatableTests.swift index 989656ec..c822115f 100644 --- a/Tests/EquatableTests.swift +++ b/Tests/EquatableTests.swift @@ -68,7 +68,7 @@ class EquatableTests: XCTestCase { func testTokenEquality() { guard let generator = Generator(factor: .counter(0), secret: Data(), algorithm: .sha1, digits: 6), let other_generator = Generator(factor: .counter(1), secret: Data(), algorithm: .sha512, digits: 8) else { - XCTFail() + XCTFail("Failed to construct Generator.") return } diff --git a/Tests/TokenSerializationTests.swift b/Tests/TokenSerializationTests.swift index 72c7cf94..b95a6ffb 100644 --- a/Tests/TokenSerializationTests.swift +++ b/Tests/TokenSerializationTests.swift @@ -64,7 +64,7 @@ class TokenSerializationTests: XCTestCase { algorithm: algorithm, digits: digitNumber ) else { - XCTFail() + XCTFail("Failed to construct Generator.") continue } diff --git a/Tests/TokenTests.swift b/Tests/TokenTests.swift index b80bd600..a1ba2495 100644 --- a/Tests/TokenTests.swift +++ b/Tests/TokenTests.swift @@ -40,7 +40,7 @@ class TokenTests: XCTestCase { algorithm: .sha1, digits: 6 ) else { - XCTFail() + XCTFail("Failed to construct Generator.") return } @@ -63,7 +63,7 @@ class TokenTests: XCTestCase { algorithm: .sha512, digits: 8 ) else { - XCTFail() + XCTFail("Failed to construct Generator.") return } @@ -90,7 +90,7 @@ class TokenTests: XCTestCase { algorithm: .sha1, digits: 6 ) else { - XCTFail() + XCTFail("Failed to construct Generator.") return } let n = "Test Name" @@ -116,7 +116,7 @@ class TokenTests: XCTestCase { algorithm: .sha1, digits: 6 ) else { - XCTFail() + XCTFail("Failed to construct Generator.") return } let timerToken = Token(generator: timerGenerator) @@ -128,7 +128,7 @@ class TokenTests: XCTestCase { let oldPassword = try timerToken.generator.password(at: Date(timeIntervalSince1970: 0)) XCTAssertNotEqual(timerToken.currentPassword, oldPassword) } catch { - XCTFail() + XCTFail("Failed to generate password with error: \(error)") return } @@ -138,7 +138,7 @@ class TokenTests: XCTestCase { algorithm: .sha1, digits: 6 ) else { - XCTFail() + XCTFail("Failed to construct Generator.") return } let counterToken = Token(generator: counterGenerator) @@ -150,7 +150,7 @@ class TokenTests: XCTestCase { let oldPassword = try counterToken.generator.password(at: Date(timeIntervalSince1970: 0)) XCTAssertEqual(counterToken.currentPassword, oldPassword) } catch { - XCTFail() + XCTFail("Failed to generate password with error: \(error)") return } } @@ -162,7 +162,7 @@ class TokenTests: XCTestCase { algorithm: .sha1, digits: 6 ) else { - XCTFail() + XCTFail("Failed to construct Generator.") return } let timerToken = Token(generator: timerGenerator) @@ -177,7 +177,7 @@ class TokenTests: XCTestCase { algorithm: .sha1, digits: 6 ) else { - XCTFail() + XCTFail("Failed to construct Generator.") return } let counterToken = Token(generator: counterGenerator) From a93db54f5c059f096cd54101ab888e1335446ac2 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 22:49:28 -0400 Subject: [PATCH 22/82] Split the parse function into two versions: defaultTo and overrideWith --- Sources/Token+URL.swift | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index ebab6da0..fff84d19 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -199,11 +199,21 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? return Token(name: name, issuer: issuer, generator: generator) } -private func parse(_ item: P?, with parser: ((P) -> T?), defaultTo defaultValue: T? = nil, overrideWith overrideValue: T? = nil) -> T? { +private func parse(_ item: P?, with parser: ((P) -> T?), overrideWith overrideValue: T?) -> T? { if let value = overrideValue { return value } + if let concrete = item { + guard let value = parser(concrete) else { + return nil + } + return value + } + return nil +} + +private func parse(_ item: P?, with parser: ((P) -> T?), defaultTo defaultValue: T?) -> T? { if let concrete = item { guard let value = parser(concrete) else { return nil From 5f6683f41e8dc13fce196f44ef4712d8fc629799 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 23:08:22 -0400 Subject: [PATCH 23/82] Factor out parseCounterValue and parseTimerPeriod helper functions --- Sources/Token+URL.swift | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index fff84d19..8a777ead 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -139,23 +139,13 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? let factorParser: (String) -> Generator.Factor? = { string in if string == kFactorCounterKey { if let counter: UInt64 = parse(queryDictionary[kQueryCounterKey], - with: { - guard let counterValue = UInt64($0, radix: 10) else { - return nil - } - return counterValue - }, + with: parseCounterValue, defaultTo: defaultCounter) { return .counter(counter) } } else if string == kFactorTimerKey { if let period: TimeInterval = parse(queryDictionary[kQueryPeriodKey], - with: { - guard let int = Int($0) else { - return nil - } - return TimeInterval(int) - }, + with: parseTimerPeriod, defaultTo: defaultPeriod) { return .timer(period: period) } @@ -199,6 +189,20 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? return Token(name: name, issuer: issuer, generator: generator) } +private func parseCounterValue(_ rawValue: String) -> UInt64? { + guard let counterValue = UInt64(rawValue, radix: 10) else { + return nil + } + return counterValue +} + +private func parseTimerPeriod(_ rawValue: String) -> TimeInterval? { + guard let int = Int(rawValue) else { + return nil + } + return TimeInterval(int) +} + private func parse(_ item: P?, with parser: ((P) -> T?), overrideWith overrideValue: T?) -> T? { if let value = overrideValue { return value From 35737c57b6d245c2830d6d210e0ea11dbe274cc7 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 23:09:08 -0400 Subject: [PATCH 24/82] Factor out secret and digit parsers, convert parsers to throw on failure --- Sources/Token+URL.swift | 51 ++++++++++++++++++++++++++++++----------- 1 file changed, 37 insertions(+), 14 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 8a777ead..98c4b09b 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -54,6 +54,15 @@ internal enum SerializationError: Swift.Error { case urlGenerationFailure } +internal enum DeserializationError: Swift.Error { + case factor + case counterValue + case timerPeriod + case secret + case algorithm + case digits +} + private let defaultAlgorithm: Generator.Algorithm = .sha1 private let defaultDigits: Int = 6 private let defaultCounter: UInt64 = 0 @@ -85,7 +94,7 @@ private func stringForAlgorithm(_ algorithm: Generator.Algorithm) -> String { } } -private func algorithmFromString(_ string: String) -> Generator.Algorithm? { +private func algorithmFromString(_ string: String) throws -> Generator.Algorithm { switch string { case kAlgorithmSHA1: return .sha1 @@ -94,7 +103,7 @@ private func algorithmFromString(_ string: String) -> Generator.Algorithm? { case kAlgorithmSHA512: return .sha512 default: - return nil + throw DeserializationError.algorithm } } @@ -136,7 +145,7 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? queryDictionary[item.name] = item.value } - let factorParser: (String) -> Generator.Factor? = { string in + let factorParser: (String) throws -> Generator.Factor = { string in if string == kFactorCounterKey { if let counter: UInt64 = parse(queryDictionary[kQueryCounterKey], with: parseCounterValue, @@ -150,15 +159,15 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? return .timer(period: period) } } - return nil + throw DeserializationError.factor } guard let factor = parse(url.host, with: factorParser, defaultTo: nil), - let secret = parse(queryDictionary[kQuerySecretKey], with: { MF_Base32Codec.data(fromBase32String: $0) }, + let secret = parse(queryDictionary[kQuerySecretKey], with: parseSecret, overrideWith: externalSecret), let algorithm = parse(queryDictionary[kQueryAlgorithmKey], with: algorithmFromString, defaultTo: defaultAlgorithm), - let digits = parse(queryDictionary[kQueryDigitsKey], with: { Int($0) }, defaultTo: defaultDigits), + let digits = parse(queryDictionary[kQueryDigitsKey], with: parseDigits, defaultTo: defaultDigits), let generator = Generator(factor: factor, secret: secret, algorithm: algorithm, digits: digits) else { return nil } @@ -189,27 +198,41 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? return Token(name: name, issuer: issuer, generator: generator) } -private func parseCounterValue(_ rawValue: String) -> UInt64? { +private func parseCounterValue(_ rawValue: String) throws -> UInt64 { guard let counterValue = UInt64(rawValue, radix: 10) else { - return nil + throw DeserializationError.counterValue } return counterValue } -private func parseTimerPeriod(_ rawValue: String) -> TimeInterval? { +private func parseTimerPeriod(_ rawValue: String) throws -> TimeInterval { guard let int = Int(rawValue) else { - return nil + throw DeserializationError.timerPeriod } return TimeInterval(int) } -private func parse(_ item: P?, with parser: ((P) -> T?), overrideWith overrideValue: T?) -> T? { +private func parseSecret(_ rawValue: String) throws -> Data { + guard let data = MF_Base32Codec.data(fromBase32String: rawValue) else { + throw DeserializationError.secret + } + return data +} + +private func parseDigits(_ rawValue: String) throws -> Int { + guard let intValue = Int(rawValue) else { + throw DeserializationError.digits + } + return intValue +} + +private func parse(_ item: P?, with parser: ((P) throws -> T), overrideWith overrideValue: T?) -> T? { if let value = overrideValue { return value } if let concrete = item { - guard let value = parser(concrete) else { + guard let value = try? parser(concrete) else { return nil } return value @@ -217,9 +240,9 @@ private func parse(_ item: P?, with parser: ((P) -> T?), overrideWith over return nil } -private func parse(_ item: P?, with parser: ((P) -> T?), defaultTo defaultValue: T?) -> T? { +private func parse(_ item: P?, with parser: ((P) throws -> T), defaultTo defaultValue: T?) -> T? { if let concrete = item { - guard let value = parser(concrete) else { + guard let value = try? parser(concrete) else { return nil } return value From 3b0f8c349e4c4149e7492bb00d79d21cf03675c3 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 23:19:43 -0400 Subject: [PATCH 25/82] Rethrow errors from parse() functions --- Sources/Token+URL.swift | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 98c4b09b..330ee5d4 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -147,13 +147,13 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? let factorParser: (String) throws -> Generator.Factor = { string in if string == kFactorCounterKey { - if let counter: UInt64 = parse(queryDictionary[kQueryCounterKey], + if let counter: UInt64 = try parse(queryDictionary[kQueryCounterKey], with: parseCounterValue, defaultTo: defaultCounter) { return .counter(counter) } } else if string == kFactorTimerKey { - if let period: TimeInterval = parse(queryDictionary[kQueryPeriodKey], + if let period: TimeInterval = try parse(queryDictionary[kQueryPeriodKey], with: parseTimerPeriod, defaultTo: defaultPeriod) { return .timer(period: period) @@ -162,15 +162,17 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? throw DeserializationError.factor } - guard let factor = parse(url.host, with: factorParser, defaultTo: nil), - let secret = parse(queryDictionary[kQuerySecretKey], with: parseSecret, + // swiftlint:disable force_try + guard let factor = try! parse(url.host, with: factorParser, defaultTo: nil), + let secret = try! parse(queryDictionary[kQuerySecretKey], with: parseSecret, overrideWith: externalSecret), - let algorithm = parse(queryDictionary[kQueryAlgorithmKey], with: algorithmFromString, + let algorithm = try! parse(queryDictionary[kQueryAlgorithmKey], with: algorithmFromString, defaultTo: defaultAlgorithm), - let digits = parse(queryDictionary[kQueryDigitsKey], with: parseDigits, defaultTo: defaultDigits), + let digits = try! parse(queryDictionary[kQueryDigitsKey], with: parseDigits, defaultTo: defaultDigits), let generator = Generator(factor: factor, secret: secret, algorithm: algorithm, digits: digits) else { return nil } + // swiftlint:enable force_try // Skip the leading "/" var name = String(url.path.dropFirst()) @@ -226,25 +228,21 @@ private func parseDigits(_ rawValue: String) throws -> Int { return intValue } -private func parse(_ item: P?, with parser: ((P) throws -> T), overrideWith overrideValue: T?) -> T? { +private func parse(_ item: P?, with parser: ((P) throws -> T), overrideWith overrideValue: T?) rethrows -> T? { if let value = overrideValue { return value } if let concrete = item { - guard let value = try? parser(concrete) else { - return nil - } + let value = try parser(concrete) return value } return nil } -private func parse(_ item: P?, with parser: ((P) throws -> T), defaultTo defaultValue: T?) -> T? { +private func parse(_ item: P?, with parser: ((P) throws -> T), defaultTo defaultValue: T?) rethrows -> T? { if let concrete = item { - guard let value = try? parser(concrete) else { - return nil - } + let value = try parser(concrete) return value } return defaultValue From 0961d80d3a19eb00913deb8c83c40e90ee13477e Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 23:27:37 -0400 Subject: [PATCH 26/82] Remove uses of force-try --- Sources/Token+URL.swift | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 330ee5d4..4961cc77 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -42,11 +42,15 @@ extension Token { /// Attempts to initialize a token represented by the give URL. public init?(url: URL, secret: Data? = nil) { - if let token = token(from: url, secret: secret) { + do { + if let token = try token(from: url, secret: secret) { self = token } else { return nil } + } catch { + return nil + } } } @@ -135,7 +139,7 @@ private func urlForToken(name: String, issuer: String, factor: Generator.Factor, return url } -private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? { +private func token(from url: URL, secret externalSecret: Data? = nil) throws -> Token? { guard url.scheme == kOTPAuthScheme else { return nil } @@ -162,17 +166,15 @@ private func token(from url: URL, secret externalSecret: Data? = nil) -> Token? throw DeserializationError.factor } - // swiftlint:disable force_try - guard let factor = try! parse(url.host, with: factorParser, defaultTo: nil), - let secret = try! parse(queryDictionary[kQuerySecretKey], with: parseSecret, + guard let factor = try parse(url.host, with: factorParser, defaultTo: nil), + let secret = try parse(queryDictionary[kQuerySecretKey], with: parseSecret, overrideWith: externalSecret), - let algorithm = try! parse(queryDictionary[kQueryAlgorithmKey], with: algorithmFromString, + let algorithm = try parse(queryDictionary[kQueryAlgorithmKey], with: algorithmFromString, defaultTo: defaultAlgorithm), - let digits = try! parse(queryDictionary[kQueryDigitsKey], with: parseDigits, defaultTo: defaultDigits), + let digits = try parse(queryDictionary[kQueryDigitsKey], with: parseDigits, defaultTo: defaultDigits), let generator = Generator(factor: factor, secret: secret, algorithm: algorithm, digits: digits) else { return nil } - // swiftlint:enable force_try // Skip the leading "/" var name = String(url.path.dropFirst()) From 4e1fe8744efeabd9b68d9d6375018011e190702b Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 23:33:58 -0400 Subject: [PATCH 27/82] Remove the parse(_:with:defaultTo:) helper function --- Sources/Token+URL.swift | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 4961cc77..e383c2c0 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -151,27 +151,21 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> let factorParser: (String) throws -> Generator.Factor = { string in if string == kFactorCounterKey { - if let counter: UInt64 = try parse(queryDictionary[kQueryCounterKey], - with: parseCounterValue, - defaultTo: defaultCounter) { - return .counter(counter) - } + let counter = try queryDictionary[kQueryCounterKey].map(parseCounterValue) ?? defaultCounter + return .counter(counter) } else if string == kFactorTimerKey { - if let period: TimeInterval = try parse(queryDictionary[kQueryPeriodKey], - with: parseTimerPeriod, - defaultTo: defaultPeriod) { - return .timer(period: period) - } + let period = try queryDictionary[kQueryPeriodKey].map(parseTimerPeriod) ?? defaultPeriod + return .timer(period: period) } throw DeserializationError.factor } - guard let factor = try parse(url.host, with: factorParser, defaultTo: nil), + let algorithm = try queryDictionary[kQueryAlgorithmKey].map(algorithmFromString) ?? defaultAlgorithm + let digits = try queryDictionary[kQueryDigitsKey].map(parseDigits) ?? defaultDigits + + guard let factor = try url.host.map(factorParser), let secret = try parse(queryDictionary[kQuerySecretKey], with: parseSecret, overrideWith: externalSecret), - let algorithm = try parse(queryDictionary[kQueryAlgorithmKey], with: algorithmFromString, - defaultTo: defaultAlgorithm), - let digits = try parse(queryDictionary[kQueryDigitsKey], with: parseDigits, defaultTo: defaultDigits), let generator = Generator(factor: factor, secret: secret, algorithm: algorithm, digits: digits) else { return nil } @@ -241,11 +235,3 @@ private func parse(_ item: P?, with parser: ((P) throws -> T), overrideWit } return nil } - -private func parse(_ item: P?, with parser: ((P) throws -> T), defaultTo defaultValue: T?) rethrows -> T? { - if let concrete = item { - let value = try parser(concrete) - return value - } - return defaultValue -} From a1800db88d3f6c1a4b940650f8a57ded8a9bdc14 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 23:42:08 -0400 Subject: [PATCH 28/82] Remove the parse(_:with:overrideWith:) helper function --- Sources/Token+URL.swift | 15 +-------------- 1 file changed, 1 insertion(+), 14 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index e383c2c0..a3491a57 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -164,8 +164,7 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> let digits = try queryDictionary[kQueryDigitsKey].map(parseDigits) ?? defaultDigits guard let factor = try url.host.map(factorParser), - let secret = try parse(queryDictionary[kQuerySecretKey], with: parseSecret, - overrideWith: externalSecret), + let secret = try externalSecret ?? queryDictionary[kQuerySecretKey].map(parseSecret), let generator = Generator(factor: factor, secret: secret, algorithm: algorithm, digits: digits) else { return nil } @@ -223,15 +222,3 @@ private func parseDigits(_ rawValue: String) throws -> Int { } return intValue } - -private func parse(_ item: P?, with parser: ((P) throws -> T), overrideWith overrideValue: T?) rethrows -> T? { - if let value = overrideValue { - return value - } - - if let concrete = item { - let value = try parser(concrete) - return value - } - return nil -} From 65329f160605887279da0599982a2eeb21b63247 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 23:45:57 -0400 Subject: [PATCH 29/82] Add associated values to DeserializationError cases --- Sources/Token+URL.swift | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index a3491a57..e7781585 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -59,12 +59,12 @@ internal enum SerializationError: Swift.Error { } internal enum DeserializationError: Swift.Error { - case factor - case counterValue - case timerPeriod - case secret - case algorithm - case digits + case invalidFactor(String) + case invalidCounterValue(String) + case invalidTimerPeriod(String) + case invalidSecret(String) + case invalidAlgorithm(String) + case invalidDigits(String) } private let defaultAlgorithm: Generator.Algorithm = .sha1 @@ -107,7 +107,7 @@ private func algorithmFromString(_ string: String) throws -> Generator.Algorithm case kAlgorithmSHA512: return .sha512 default: - throw DeserializationError.algorithm + throw DeserializationError.invalidAlgorithm(string) } } @@ -156,8 +156,9 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> } else if string == kFactorTimerKey { let period = try queryDictionary[kQueryPeriodKey].map(parseTimerPeriod) ?? defaultPeriod return .timer(period: period) + } else { + throw DeserializationError.invalidFactor(string) } - throw DeserializationError.factor } let algorithm = try queryDictionary[kQueryAlgorithmKey].map(algorithmFromString) ?? defaultAlgorithm @@ -197,28 +198,28 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> private func parseCounterValue(_ rawValue: String) throws -> UInt64 { guard let counterValue = UInt64(rawValue, radix: 10) else { - throw DeserializationError.counterValue + throw DeserializationError.invalidCounterValue(rawValue) } return counterValue } private func parseTimerPeriod(_ rawValue: String) throws -> TimeInterval { guard let int = Int(rawValue) else { - throw DeserializationError.timerPeriod + throw DeserializationError.invalidTimerPeriod(rawValue) } return TimeInterval(int) } private func parseSecret(_ rawValue: String) throws -> Data { guard let data = MF_Base32Codec.data(fromBase32String: rawValue) else { - throw DeserializationError.secret + throw DeserializationError.invalidSecret(rawValue) } return data } private func parseDigits(_ rawValue: String) throws -> Int { guard let intValue = Int(rawValue) else { - throw DeserializationError.digits + throw DeserializationError.invalidDigits(rawValue) } return intValue } From 2598810a56030d689a208ee00ec2fe629854c063 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 23:52:36 -0400 Subject: [PATCH 30/82] Throw deserialization errors for missing factor or secret --- Sources/Token+URL.swift | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index e7781585..027a3184 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -59,9 +59,11 @@ internal enum SerializationError: Swift.Error { } internal enum DeserializationError: Swift.Error { + case missingFactor case invalidFactor(String) case invalidCounterValue(String) case invalidTimerPeriod(String) + case missingSecret case invalidSecret(String) case invalidAlgorithm(String) case invalidDigits(String) @@ -164,10 +166,14 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> let algorithm = try queryDictionary[kQueryAlgorithmKey].map(algorithmFromString) ?? defaultAlgorithm let digits = try queryDictionary[kQueryDigitsKey].map(parseDigits) ?? defaultDigits - guard let factor = try url.host.map(factorParser), - let secret = try externalSecret ?? queryDictionary[kQuerySecretKey].map(parseSecret), - let generator = Generator(factor: factor, secret: secret, algorithm: algorithm, digits: digits) else { - return nil + guard let factor = try url.host.map(factorParser) else { + throw DeserializationError.missingFactor + } + guard let secret = try externalSecret ?? queryDictionary[kQuerySecretKey].map(parseSecret) else { + throw DeserializationError.missingSecret + } + guard let generator = Generator(factor: factor, secret: secret, algorithm: algorithm, digits: digits) else { + return nil } // Skip the leading "/" From 3a60b081b83261664c7e5dac020fd208be5f29a0 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 25 Sep 2017 23:57:33 -0400 Subject: [PATCH 31/82] Remove explicit radix parameter --- Sources/Token+URL.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 027a3184..f5898e58 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -203,7 +203,7 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> } private func parseCounterValue(_ rawValue: String) throws -> UInt64 { - guard let counterValue = UInt64(rawValue, radix: 10) else { + guard let counterValue = UInt64(rawValue) else { throw DeserializationError.invalidCounterValue(rawValue) } return counterValue From 8342327e73fa21ae0be9226fe517994d19149c6d Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 26 Sep 2017 00:02:46 -0400 Subject: [PATCH 32/82] Refactor factor parsing to not use a closure --- Sources/Token+URL.swift | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index f5898e58..3e2ab09a 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -151,24 +151,22 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> queryDictionary[item.name] = item.value } - let factorParser: (String) throws -> Generator.Factor = { string in - if string == kFactorCounterKey { - let counter = try queryDictionary[kQueryCounterKey].map(parseCounterValue) ?? defaultCounter - return .counter(counter) - } else if string == kFactorTimerKey { - let period = try queryDictionary[kQueryPeriodKey].map(parseTimerPeriod) ?? defaultPeriod - return .timer(period: period) - } else { - throw DeserializationError.invalidFactor(string) - } + guard let factorString = url.host else { + throw DeserializationError.missingFactor + } + let factor: Generator.Factor + if factorString == kFactorCounterKey { + let counter = try queryDictionary[kQueryCounterKey].map(parseCounterValue) ?? defaultCounter + factor = .counter(counter) + } else if factorString == kFactorTimerKey { + let period = try queryDictionary[kQueryPeriodKey].map(parseTimerPeriod) ?? defaultPeriod + factor = .timer(period: period) + } else { + throw DeserializationError.invalidFactor(factorString) } let algorithm = try queryDictionary[kQueryAlgorithmKey].map(algorithmFromString) ?? defaultAlgorithm let digits = try queryDictionary[kQueryDigitsKey].map(parseDigits) ?? defaultDigits - - guard let factor = try url.host.map(factorParser) else { - throw DeserializationError.missingFactor - } guard let secret = try externalSecret ?? queryDictionary[kQuerySecretKey].map(parseSecret) else { throw DeserializationError.missingSecret } From 29b771f59830277e55a13524c2432dfe399646d9 Mon Sep 17 00:00:00 2001 From: Chris Ziogas Date: Sun, 8 Oct 2017 23:29:04 +0300 Subject: [PATCH 33/82] Fix String warnings for Xcode 9.1 --- Sources/Generator.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Generator.swift b/Sources/Generator.swift index c9c0b467..87bacc5d 100644 --- a/Sources/Generator.swift +++ b/Sources/Generator.swift @@ -250,7 +250,7 @@ private extension String { /// /// - returns: A new string padded to the given length. func padded(with character: Character, toLength length: Int) -> String { - let paddingCount = length - characters.count + let paddingCount = length - self.count guard paddingCount > 0 else { return self } From 5d6b0c42c2a357b351eb57e931450c89152885a3 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 9 Oct 2017 21:16:20 -0400 Subject: [PATCH 34/82] Refactor parsing a factor from url.host --- Sources/Token+URL.swift | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 3e2ab09a..cb236e31 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -151,18 +151,18 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> queryDictionary[item.name] = item.value } - guard let factorString = url.host else { - throw DeserializationError.missingFactor - } let factor: Generator.Factor - if factorString == kFactorCounterKey { + switch url.host { + case .some(kFactorCounterKey): let counter = try queryDictionary[kQueryCounterKey].map(parseCounterValue) ?? defaultCounter factor = .counter(counter) - } else if factorString == kFactorTimerKey { + case .some(kFactorTimerKey): let period = try queryDictionary[kQueryPeriodKey].map(parseTimerPeriod) ?? defaultPeriod factor = .timer(period: period) - } else { - throw DeserializationError.invalidFactor(factorString) + case let .some(rawValue): + throw DeserializationError.invalidFactor(rawValue) + case .none: + throw DeserializationError.missingFactor } let algorithm = try queryDictionary[kQueryAlgorithmKey].map(algorithmFromString) ?? defaultAlgorithm From dcafd0b26acb373af9c084490e3d213e73d412cb Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 9 Oct 2017 21:17:23 -0400 Subject: [PATCH 35/82] Rename counterValue for clarity --- Sources/Token+URL.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index cb236e31..79601700 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -154,8 +154,8 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> let factor: Generator.Factor switch url.host { case .some(kFactorCounterKey): - let counter = try queryDictionary[kQueryCounterKey].map(parseCounterValue) ?? defaultCounter - factor = .counter(counter) + let counterValue = try queryDictionary[kQueryCounterKey].map(parseCounterValue) ?? defaultCounter + factor = .counter(counterValue) case .some(kFactorTimerKey): let period = try queryDictionary[kQueryPeriodKey].map(parseTimerPeriod) ?? defaultPeriod factor = .timer(period: period) From e18d1a5480f61578f028e5fcf7756cfd6c6f2d4a Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 9 Oct 2017 21:22:32 -0400 Subject: [PATCH 36/82] Simplify TimeInterval parsing and rename parsed constants for clarity --- Sources/Token+URL.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 79601700..d521db03 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -208,22 +208,22 @@ private func parseCounterValue(_ rawValue: String) throws -> UInt64 { } private func parseTimerPeriod(_ rawValue: String) throws -> TimeInterval { - guard let int = Int(rawValue) else { + guard let period = TimeInterval(rawValue) else { throw DeserializationError.invalidTimerPeriod(rawValue) } - return TimeInterval(int) + return period } private func parseSecret(_ rawValue: String) throws -> Data { - guard let data = MF_Base32Codec.data(fromBase32String: rawValue) else { + guard let secret = MF_Base32Codec.data(fromBase32String: rawValue) else { throw DeserializationError.invalidSecret(rawValue) } - return data + return secret } private func parseDigits(_ rawValue: String) throws -> Int { - guard let intValue = Int(rawValue) else { + guard let digits = Int(rawValue) else { throw DeserializationError.invalidDigits(rawValue) } - return intValue + return digits } From c70b95bff6d80f5b94a78a76ea0f31a1ecf6deaf Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 9 Oct 2017 21:32:40 -0400 Subject: [PATCH 37/82] Convert token(from: URL) to return a non-null Token, or throw an error --- Sources/Token+URL.swift | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index d521db03..7e82618c 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -43,11 +43,7 @@ extension Token { /// Attempts to initialize a token represented by the give URL. public init?(url: URL, secret: Data? = nil) { do { - if let token = try token(from: url, secret: secret) { - self = token - } else { - return nil - } + self = try token(from: url, secret: secret) } catch { return nil } @@ -59,6 +55,7 @@ internal enum SerializationError: Swift.Error { } internal enum DeserializationError: Swift.Error { + case invalidURLScheme case missingFactor case invalidFactor(String) case invalidCounterValue(String) @@ -67,6 +64,7 @@ internal enum DeserializationError: Swift.Error { case invalidSecret(String) case invalidAlgorithm(String) case invalidDigits(String) + case invalidGenerator } private let defaultAlgorithm: Generator.Algorithm = .sha1 @@ -141,9 +139,9 @@ private func urlForToken(name: String, issuer: String, factor: Generator.Factor, return url } -private func token(from url: URL, secret externalSecret: Data? = nil) throws -> Token? { +private func token(from url: URL, secret externalSecret: Data? = nil) throws -> Token { guard url.scheme == kOTPAuthScheme else { - return nil + throw DeserializationError.invalidURLScheme } var queryDictionary = Dictionary() @@ -171,7 +169,8 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> throw DeserializationError.missingSecret } guard let generator = Generator(factor: factor, secret: secret, algorithm: algorithm, digits: digits) else { - return nil + // TODO: Convert Generator's failable initializer to a throwable initializer, and rethrow its errors. + throw DeserializationError.invalidGenerator } // Skip the leading "/" From b8fd179568e4d76f9e6aa123dea705fd9cc5f0ba Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 9 Oct 2017 21:35:41 -0400 Subject: [PATCH 38/82] Add a TODO --- Sources/Token+URL.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 7e82618c..25e65d0c 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -146,6 +146,7 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> var queryDictionary = Dictionary() URLComponents(url: url, resolvingAgainstBaseURL: false)?.queryItems?.forEach { item in + // TODO: guard against duplicate query items for the same key. queryDictionary[item.name] = item.value } From b0d62d92b3d60beab4b7ffc8bb97d3e97170b8f0 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 9 Oct 2017 21:40:08 -0400 Subject: [PATCH 39/82] Remove explicit `self.` --- Sources/Generator.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Generator.swift b/Sources/Generator.swift index 87bacc5d..aeb766d1 100644 --- a/Sources/Generator.swift +++ b/Sources/Generator.swift @@ -250,7 +250,7 @@ private extension String { /// /// - returns: A new string padded to the given length. func padded(with character: Character, toLength length: Int) -> String { - let paddingCount = length - self.count + let paddingCount = length - count guard paddingCount > 0 else { return self } From 4778fbb21df351ec5bd0ee143e91e928cfefffb0 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 9 Oct 2017 22:16:54 -0400 Subject: [PATCH 40/82] Improve query string parsing --- Sources/Token+URL.swift | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 25e65d0c..b2d474f5 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -56,6 +56,7 @@ internal enum SerializationError: Swift.Error { internal enum DeserializationError: Swift.Error { case invalidURLScheme + case duplicateQueryItem(String) case missingFactor case invalidFactor(String) case invalidCounterValue(String) @@ -144,19 +145,15 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> throw DeserializationError.invalidURLScheme } - var queryDictionary = Dictionary() - URLComponents(url: url, resolvingAgainstBaseURL: false)?.queryItems?.forEach { item in - // TODO: guard against duplicate query items for the same key. - queryDictionary[item.name] = item.value - } + let queryItems = URLComponents(url: url, resolvingAgainstBaseURL: false)?.queryItems ?? [] let factor: Generator.Factor switch url.host { case .some(kFactorCounterKey): - let counterValue = try queryDictionary[kQueryCounterKey].map(parseCounterValue) ?? defaultCounter + let counterValue = try queryItems.value(for: kQueryCounterKey).map(parseCounterValue) ?? defaultCounter factor = .counter(counterValue) case .some(kFactorTimerKey): - let period = try queryDictionary[kQueryPeriodKey].map(parseTimerPeriod) ?? defaultPeriod + let period = try queryItems.value(for: kQueryPeriodKey).map(parseTimerPeriod) ?? defaultPeriod factor = .timer(period: period) case let .some(rawValue): throw DeserializationError.invalidFactor(rawValue) @@ -164,9 +161,9 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> throw DeserializationError.missingFactor } - let algorithm = try queryDictionary[kQueryAlgorithmKey].map(algorithmFromString) ?? defaultAlgorithm - let digits = try queryDictionary[kQueryDigitsKey].map(parseDigits) ?? defaultDigits - guard let secret = try externalSecret ?? queryDictionary[kQuerySecretKey].map(parseSecret) else { + let algorithm = try queryItems.value(for: kQueryAlgorithmKey).map(algorithmFromString) ?? defaultAlgorithm + let digits = try queryItems.value(for: kQueryDigitsKey).map(parseDigits) ?? defaultDigits + guard let secret = try externalSecret ?? queryItems.value(for: kQuerySecretKey).map(parseSecret) else { throw DeserializationError.missingSecret } guard let generator = Generator(factor: factor, secret: secret, algorithm: algorithm, digits: digits) else { @@ -178,7 +175,7 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> var name = String(url.path.dropFirst()) let issuer: String - if let issuerString = queryDictionary[kQueryIssuerKey] { + if let issuerString = try queryItems.value(for: kQueryIssuerKey) { issuer = issuerString } else if let separatorRange = name.range(of: ":") { // If there is no issuer string, try to extract one from the name @@ -227,3 +224,15 @@ private func parseDigits(_ rawValue: String) throws -> Int { } return digits } + +extension Array where Element == URLQueryItem { + func value(for name: String) throws -> String? { + let matchingQueryItems = self.filter({ + $0.name == name + }) + guard matchingQueryItems.count <= 1 else { + throw DeserializationError.duplicateQueryItem(name) + } + return matchingQueryItems.first?.value + } +} From 91625ea0d79248faf7a699b73f808c46a508d9be Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 9 Oct 2017 22:40:27 -0400 Subject: [PATCH 41/82] Factor out shortName(byTrimming issuer: String, from fullName: String) --- Sources/Token+URL.swift | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index b2d474f5..c8d9fc8a 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -172,27 +172,21 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> } // Skip the leading "/" - var name = String(url.path.dropFirst()) + let fullName = String(url.path.dropFirst()) let issuer: String if let issuerString = try queryItems.value(for: kQueryIssuerKey) { issuer = issuerString - } else if let separatorRange = name.range(of: ":") { + } else if let separatorRange = fullName.range(of: ":") { // If there is no issuer string, try to extract one from the name - issuer = String(name[.. Int { return digits } +private func shortName(byTrimming issuer: String, from fullName: String) -> String { + if !issuer.isEmpty { + let prefix = issuer + ":" + if fullName.hasPrefix(prefix), let prefixRange = fullName.range(of: prefix) { + let substringAfterSeparator = fullName[prefixRange.upperBound...] + return substringAfterSeparator.trimmingCharacters(in: CharacterSet.whitespaces) + } + } + return String(fullName) +} + extension Array where Element == URLQueryItem { func value(for name: String) throws -> String? { let matchingQueryItems = self.filter({ From f51845089285c81e3a316f8eba62b59edba1c00c Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 9 Oct 2017 23:09:57 -0400 Subject: [PATCH 42/82] Improve test coverage by testing URL parsing edge cases --- .../OTPTokenSerializationTests.m | 2 ++ Tests/TokenSerializationTests.swift | 13 +++++++++++++ 2 files changed, 15 insertions(+) diff --git a/OneTimePasswordLegacyTests/OTPTokenSerializationTests.m b/OneTimePasswordLegacyTests/OTPTokenSerializationTests.m index 01ac3521..449e2e98 100644 --- a/OneTimePasswordLegacyTests/OTPTokenSerializationTests.m +++ b/OneTimePasswordLegacyTests/OTPTokenSerializationTests.m @@ -372,9 +372,11 @@ - (void)testTokenWithInvalidURLs { NSArray *badURLs = @[@"http://foo", // invalid scheme @"otpauth://foo", // invalid type + @"otpauth:///bar?secret=AAAQEAYEAUDAOCAJBIFQYDIOB4", // missing type @"otpauth://totp/bar", // missing secret @"otpauth://totp/bar?secret=AAAQEAYEAUDAOCAJBIFQYDIOB4&period=0", // invalid period @"otpauth://totp/bar?secret=AAAQEAYEAUDAOCAJBIFQYDIOB4&period=x", // non-numeric period + @"otpauth://totp/bar?secret=AAAQEAYEAUDAOCAJBIFQYDIOB4&period=30&period=60", // multiple period @"otpauth://totp/bar?secret=AAAQEAYEAUDAOCAJBIFQYDIOB4&algorithm=MD5", // invalid algorithm @"otpauth://totp/bar?secret=AAAQEAYEAUDAOCAJBIFQYDIOB4&digits=2", // invalid digits @"otpauth://totp/bar?secret=AAAQEAYEAUDAOCAJBIFQYDIOB4&digits=x", // non-numeric digits diff --git a/Tests/TokenSerializationTests.swift b/Tests/TokenSerializationTests.swift index b95a6ffb..d2e03501 100644 --- a/Tests/TokenSerializationTests.swift +++ b/Tests/TokenSerializationTests.swift @@ -162,4 +162,17 @@ class TokenSerializationTests: XCTestCase { } } } + + func testTokenWithDefaultCounter() { + let tokenURLString = "otpauth://hotp/bar?secret=AAAQEAYEAUDAOCAJBIFQYDIOB4" + guard let tokenURL = URL(string: tokenURLString) else { + XCTFail("Failed to initialize a URL from String \"\(tokenURLString)\"") + return + } + guard let token = Token(url: tokenURL) else { + XCTFail("Failed to initialize a Token from URL \"\(tokenURL)\"") + return + } + XCTAssertEqual(token.generator.factor, .counter(0)) + } } From 950e8a9c6e7d0a789a7fab4746e226442b2f2385 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 9 Oct 2017 23:28:37 -0400 Subject: [PATCH 43/82] Update xcconfigs for Xcode 9 --- Cartfile.private | 2 +- Cartfile.resolved | 2 +- Carthage/Checkouts/xcconfigs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cartfile.private b/Cartfile.private index 3291dc67..3269cf46 100644 --- a/Cartfile.private +++ b/Cartfile.private @@ -1,3 +1,3 @@ # Configuration for Carthage (https://github.com/Carthage/Carthage) -github "jspahrsummers/xcconfigs" ~> 0.10 +github "jspahrsummers/xcconfigs" ~> 0.11 diff --git a/Cartfile.resolved b/Cartfile.resolved index 333b0889..1e1e8b19 100644 --- a/Cartfile.resolved +++ b/Cartfile.resolved @@ -1,2 +1,2 @@ +github "jspahrsummers/xcconfigs" "0.11" github "mattrubin/Base32" "1.1.2+carthage" -github "jspahrsummers/xcconfigs" "0.10" diff --git a/Carthage/Checkouts/xcconfigs b/Carthage/Checkouts/xcconfigs index cc451b08..40f9bcc6 160000 --- a/Carthage/Checkouts/xcconfigs +++ b/Carthage/Checkouts/xcconfigs @@ -1 +1 @@ -Subproject commit cc451b08e052b6146f5caf66bc1120420c529c7b +Subproject commit 40f9bcc63752cdd95deee267d2fbf9da09a9f6f2 From 0b8f2953239cc0d65629651d7ce88d7ee955c93d Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Mon, 9 Oct 2017 23:29:26 -0400 Subject: [PATCH 44/82] Delete duplicate build settings --- OneTimePassword.xcodeproj/project.pbxproj | 8 -------- 1 file changed, 8 deletions(-) diff --git a/OneTimePassword.xcodeproj/project.pbxproj b/OneTimePassword.xcodeproj/project.pbxproj index 5d0e8520..ce95a0d1 100644 --- a/OneTimePassword.xcodeproj/project.pbxproj +++ b/OneTimePassword.xcodeproj/project.pbxproj @@ -731,10 +731,6 @@ isa = XCBuildConfiguration; baseConfigurationReference = C996EC2C1A74D5830076B105 /* Debug.xcconfig */; buildSettings = { - CLANG_WARN_BLOCK_CAPTURE_AUTORELEASING = YES; - CLANG_WARN_COMMA = YES; - CLANG_WARN_RANGE_LOOP_ANALYSIS = YES; - CLANG_WARN_STRICT_PROTOTYPES = YES; IPHONEOS_DEPLOYMENT_TARGET = 8.0; SWIFT_VERSION = 4.0; WATCHOS_DEPLOYMENT_TARGET = 2.0; @@ -745,10 +741,6 @@ isa = XCBuildConfiguration; baseConfigurationReference = C996EC2E1A74D5830076B105 /* Release.xcconfig */; buildSettings = { - CLANG_WARN_BLOCK_CAPTURE_AUTORELEASING = YES; - CLANG_WARN_COMMA = YES; - CLANG_WARN_RANGE_LOOP_ANALYSIS = YES; - CLANG_WARN_STRICT_PROTOTYPES = YES; IPHONEOS_DEPLOYMENT_TARGET = 8.0; SWIFT_VERSION = 4.0; WATCHOS_DEPLOYMENT_TARGET = 2.0; From 1e06e73298bf87303a65869515702937a7e897ed Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 10 Oct 2017 15:12:56 -0400 Subject: [PATCH 45/82] Selectively exclude legacy tests from nullability conversion warnings --- OneTimePassword.xcodeproj/project.pbxproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/OneTimePassword.xcodeproj/project.pbxproj b/OneTimePassword.xcodeproj/project.pbxproj index ce95a0d1..6b08df04 100644 --- a/OneTimePassword.xcodeproj/project.pbxproj +++ b/OneTimePassword.xcodeproj/project.pbxproj @@ -40,7 +40,7 @@ C97142361C1155FC0063B37E /* OTPToken.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9DC7ECC196C4D3D00B50C82 /* OTPToken.swift */; }; C97142371C1156DB0063B37E /* OneTimePassword.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = C97C82381946E51D00FD9F4C /* OneTimePassword.framework */; }; C97C82441946E51D00FD9F4C /* OneTimePassword.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = C97C82381946E51D00FD9F4C /* OneTimePassword.framework */; }; - C9A486C8196F38C800524F51 /* OTPTokenSerializationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = C93A2515196AFE1100F86892 /* OTPTokenSerializationTests.m */; }; + C9A486C8196F38C800524F51 /* OTPTokenSerializationTests.m in Sources */ = {isa = PBXBuildFile; fileRef = C93A2515196AFE1100F86892 /* OTPTokenSerializationTests.m */; settings = {COMPILER_FLAGS = "-Wno-nullable-to-nonnull-conversion"; }; }; C9A486C9196F38CD00524F51 /* OTPTypeStrings.m in Sources */ = {isa = PBXBuildFile; fileRef = C9C9FE25196D181800C7ACEE /* OTPTypeStrings.m */; }; C9B2A19C199A7F1B00BC4A8A /* EquatableTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C9B2A19B199A7F1B00BC4A8A /* EquatableTests.swift */; }; C9B77D771C03078B00BAF6BF /* KeychainTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = C93A2514196AFE1100F86892 /* KeychainTests.swift */; }; From ba063cb8372ae6696f0e1a8da3ffb179d79134db Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 10 Oct 2017 23:00:52 -0400 Subject: [PATCH 46/82] Refactor Generator initialization to create an internal throwing initializer --- Sources/Generator.swift | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/Sources/Generator.swift b/Sources/Generator.swift index aeb766d1..df88e164 100644 --- a/Sources/Generator.swift +++ b/Sources/Generator.swift @@ -49,8 +49,15 @@ public struct Generator: Equatable { /// - returns: A new password generator with the given parameters, or `nil` if the parameters /// are invalid. public init?(factor: Factor, secret: Data, algorithm: Algorithm, digits: Int) { - guard Generator.validateFactor(factor) && Generator.validateDigits(digits) else { - return nil + try? self.init(_factor: factor, secret: secret, algorithm: algorithm, digits: digits) + } + + internal init(_factor factor: Factor, secret: Data, algorithm: Algorithm, digits: Int) throws { + guard Generator.validateFactor(factor) else { + throw Generator.Error.invalidPeriod + } + guard Generator.validateDigits(digits) else { + throw Generator.Error.invalidDigits } self.factor = factor self.secret = secret From 570aac6214e72cc61c3da9098ce88e21efe68b83 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 10 Oct 2017 23:02:38 -0400 Subject: [PATCH 47/82] Rename private validation methods --- Sources/Generator.swift | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/Sources/Generator.swift b/Sources/Generator.swift index df88e164..d641e802 100644 --- a/Sources/Generator.swift +++ b/Sources/Generator.swift @@ -53,10 +53,10 @@ public struct Generator: Equatable { } internal init(_factor factor: Factor, secret: Data, algorithm: Algorithm, digits: Int) throws { - guard Generator.validateFactor(factor) else { + guard Generator.isValidFactor(factor) else { throw Generator.Error.invalidPeriod } - guard Generator.validateDigits(digits) else { + guard Generator.isValidDigits(digits) else { throw Generator.Error.invalidDigits } self.factor = factor @@ -75,7 +75,7 @@ public struct Generator: Equatable { /// - throws: A `Generator.Error` if a valid password cannot be generated for the given time. /// - returns: The generated password, or throws an error if a password could not be generated. public func password(at time: Date) throws -> String { - guard Generator.validateDigits(digits) else { + guard Generator.isValidDigits(digits) else { throw Error.invalidDigits } @@ -163,10 +163,10 @@ public struct Generator: Equatable { return counter case .timer(let period): let timeSinceEpoch = time.timeIntervalSince1970 - guard Generator.validateTime(timeSinceEpoch) else { + guard Generator.isValidTime(timeSinceEpoch) else { throw Error.invalidTime } - guard Generator.validatePeriod(period) else { + guard Generator.isValidPeriod(period) else { throw Error.invalidPeriod } return UInt64(timeSinceEpoch / period) @@ -222,28 +222,28 @@ public func == (lhs: Generator.Factor, rhs: Generator.Factor) -> Bool { private extension Generator { // MARK: Validation - static func validateDigits(_ digits: Int) -> Bool { + static func isValidDigits(_ digits: Int) -> Bool { // https://tools.ietf.org/html/rfc4226#section-5.3 states "Implementations MUST extract a // 6-digit code at a minimum and possibly 7 and 8-digit codes." let acceptableDigits = 6...8 return acceptableDigits.contains(digits) } - static func validateFactor(_ factor: Factor) -> Bool { + static func isValidFactor(_ factor: Factor) -> Bool { switch factor { case .counter: return true case .timer(let period): - return validatePeriod(period) + return isValidPeriod(period) } } - static func validatePeriod(_ period: TimeInterval) -> Bool { + static func isValidPeriod(_ period: TimeInterval) -> Bool { // The period must be positive and non-zero to produce a valid counter value. return (period > 0) } - static func validateTime(_ timeSinceEpoch: TimeInterval) -> Bool { + static func isValidTime(_ timeSinceEpoch: TimeInterval) -> Bool { // The time must be positive to produce a valid counter value. return (timeSinceEpoch >= 0) } From 949abd7fbd26d0c1751e6f0275900f19f90947e0 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 10 Oct 2017 23:08:00 -0400 Subject: [PATCH 48/82] Add throwing validate helpers --- Sources/Generator.swift | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/Sources/Generator.swift b/Sources/Generator.swift index d641e802..cb4d09d0 100644 --- a/Sources/Generator.swift +++ b/Sources/Generator.swift @@ -56,9 +56,8 @@ public struct Generator: Equatable { guard Generator.isValidFactor(factor) else { throw Generator.Error.invalidPeriod } - guard Generator.isValidDigits(digits) else { - throw Generator.Error.invalidDigits - } + try Generator.validateDigits(digits) + self.factor = factor self.secret = secret self.algorithm = algorithm @@ -75,9 +74,7 @@ public struct Generator: Equatable { /// - throws: A `Generator.Error` if a valid password cannot be generated for the given time. /// - returns: The generated password, or throws an error if a password could not be generated. public func password(at time: Date) throws -> String { - guard Generator.isValidDigits(digits) else { - throw Error.invalidDigits - } + try Generator.validateDigits(digits) let counter = try factor.counterValue(at: time) // Ensure the counter value is big-endian @@ -163,12 +160,8 @@ public struct Generator: Equatable { return counter case .timer(let period): let timeSinceEpoch = time.timeIntervalSince1970 - guard Generator.isValidTime(timeSinceEpoch) else { - throw Error.invalidTime - } - guard Generator.isValidPeriod(period) else { - throw Error.invalidPeriod - } + try Generator.validateTime(timeSinceEpoch) + try Generator.validatePeriod(period) return UInt64(timeSinceEpoch / period) } } @@ -222,6 +215,12 @@ public func == (lhs: Generator.Factor, rhs: Generator.Factor) -> Bool { private extension Generator { // MARK: Validation + static func validateDigits(_ digits: Int) throws { + guard isValidDigits(digits) else { + throw Error.invalidDigits + } + } + static func isValidDigits(_ digits: Int) -> Bool { // https://tools.ietf.org/html/rfc4226#section-5.3 states "Implementations MUST extract a // 6-digit code at a minimum and possibly 7 and 8-digit codes." @@ -238,11 +237,23 @@ private extension Generator { } } + static func validatePeriod(_ period: TimeInterval) throws { + guard isValidPeriod(period) else { + throw Error.invalidPeriod + } + } + static func isValidPeriod(_ period: TimeInterval) -> Bool { // The period must be positive and non-zero to produce a valid counter value. return (period > 0) } + static func validateTime(_ timeSinceEpoch: TimeInterval) throws { + guard isValidTime(timeSinceEpoch) else { + throw Error.invalidTime + } + } + static func isValidTime(_ timeSinceEpoch: TimeInterval) -> Bool { // The time must be positive to produce a valid counter value. return (timeSinceEpoch >= 0) From 37b80a1264a2d5f0b3ca5115c25f63df567f853f Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 10 Oct 2017 23:10:52 -0400 Subject: [PATCH 49/82] Refactor fator validation helper --- Sources/Generator.swift | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Sources/Generator.swift b/Sources/Generator.swift index cb4d09d0..2798cd39 100644 --- a/Sources/Generator.swift +++ b/Sources/Generator.swift @@ -53,9 +53,7 @@ public struct Generator: Equatable { } internal init(_factor factor: Factor, secret: Data, algorithm: Algorithm, digits: Int) throws { - guard Generator.isValidFactor(factor) else { - throw Generator.Error.invalidPeriod - } + try Generator.validateFactor(factor) try Generator.validateDigits(digits) self.factor = factor @@ -228,12 +226,12 @@ private extension Generator { return acceptableDigits.contains(digits) } - static func isValidFactor(_ factor: Factor) -> Bool { + static func validateFactor(_ factor: Factor) throws { switch factor { case .counter: - return true + return case .timer(let period): - return isValidPeriod(period) + try validatePeriod(period) } } From 9fa6fbbceef15c9080cdd5838521e4b18e1e45b7 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 10 Oct 2017 23:16:46 -0400 Subject: [PATCH 50/82] Merge boolean validators into throwing validators --- Sources/Generator.swift | 26 +++++++------------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/Sources/Generator.swift b/Sources/Generator.swift index 2798cd39..110e3ece 100644 --- a/Sources/Generator.swift +++ b/Sources/Generator.swift @@ -214,16 +214,12 @@ private extension Generator { // MARK: Validation static func validateDigits(_ digits: Int) throws { - guard isValidDigits(digits) else { - throw Error.invalidDigits - } - } - - static func isValidDigits(_ digits: Int) -> Bool { // https://tools.ietf.org/html/rfc4226#section-5.3 states "Implementations MUST extract a // 6-digit code at a minimum and possibly 7 and 8-digit codes." let acceptableDigits = 6...8 - return acceptableDigits.contains(digits) + guard acceptableDigits.contains(digits) else { + throw Error.invalidDigits + } } static func validateFactor(_ factor: Factor) throws { @@ -236,26 +232,18 @@ private extension Generator { } static func validatePeriod(_ period: TimeInterval) throws { - guard isValidPeriod(period) else { + // The period must be positive and non-zero to produce a valid counter value. + guard (period > 0) else { throw Error.invalidPeriod } } - static func isValidPeriod(_ period: TimeInterval) -> Bool { - // The period must be positive and non-zero to produce a valid counter value. - return (period > 0) - } - static func validateTime(_ timeSinceEpoch: TimeInterval) throws { - guard isValidTime(timeSinceEpoch) else { + // The time must be positive to produce a valid counter value. + guard (timeSinceEpoch >= 0) else { throw Error.invalidTime } } - - static func isValidTime(_ timeSinceEpoch: TimeInterval) -> Bool { - // The time must be positive to produce a valid counter value. - return (timeSinceEpoch >= 0) - } } private extension String { From d5f99cf8a840c8a67fe294623e3e248a14fb4551 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 10 Oct 2017 23:21:27 -0400 Subject: [PATCH 51/82] Use throwing Generator.init in token deserialization --- Sources/Token+URL.swift | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index c8d9fc8a..758b99f8 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -65,7 +65,6 @@ internal enum DeserializationError: Swift.Error { case invalidSecret(String) case invalidAlgorithm(String) case invalidDigits(String) - case invalidGenerator } private let defaultAlgorithm: Generator.Algorithm = .sha1 @@ -166,10 +165,7 @@ private func token(from url: URL, secret externalSecret: Data? = nil) throws -> guard let secret = try externalSecret ?? queryItems.value(for: kQuerySecretKey).map(parseSecret) else { throw DeserializationError.missingSecret } - guard let generator = Generator(factor: factor, secret: secret, algorithm: algorithm, digits: digits) else { - // TODO: Convert Generator's failable initializer to a throwable initializer, and rethrow its errors. - throw DeserializationError.invalidGenerator - } + let generator = try Generator(_factor: factor, secret: secret, algorithm: algorithm, digits: digits) // Skip the leading "/" let fullName = String(url.path.dropFirst()) From f16fd4489943da5c5917d2b41416c2cfa3eb0b3f Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 10 Oct 2017 23:22:27 -0400 Subject: [PATCH 52/82] Remove unnecessary parentheses --- Sources/Generator.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/Generator.swift b/Sources/Generator.swift index 110e3ece..d194e839 100644 --- a/Sources/Generator.swift +++ b/Sources/Generator.swift @@ -233,14 +233,14 @@ private extension Generator { static func validatePeriod(_ period: TimeInterval) throws { // The period must be positive and non-zero to produce a valid counter value. - guard (period > 0) else { + guard period > 0 else { throw Error.invalidPeriod } } static func validateTime(_ timeSinceEpoch: TimeInterval) throws { // The time must be positive to produce a valid counter value. - guard (timeSinceEpoch >= 0) else { + guard timeSinceEpoch >= 0 else { throw Error.invalidTime } } From 78efeb38fdabd966bb1f3c84e77a6311aeeef28f Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Dec 2017 08:10:43 -0500 Subject: [PATCH 53/82] Upgrade Base32 to an intermediate version configured for Xcode 9 --- Cartfile | 2 +- Cartfile.resolved | 2 +- Carthage/Checkouts/Base32 | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Cartfile b/Cartfile index 9f92ec46..feeca3b0 100644 --- a/Cartfile +++ b/Cartfile @@ -1,3 +1,3 @@ # Configuration for Carthage (https://github.com/Carthage/Carthage) -github "mattrubin/Base32" "1.1.2+carthage" +github "mattrubin/Base32" "24f64bf81b15c815019f37cf64cfbb3a253e2bea" diff --git a/Cartfile.resolved b/Cartfile.resolved index 1e1e8b19..bf402e80 100644 --- a/Cartfile.resolved +++ b/Cartfile.resolved @@ -1,2 +1,2 @@ github "jspahrsummers/xcconfigs" "0.11" -github "mattrubin/Base32" "1.1.2+carthage" +github "mattrubin/Base32" "24f64bf81b15c815019f37cf64cfbb3a253e2bea" diff --git a/Carthage/Checkouts/Base32 b/Carthage/Checkouts/Base32 index 1af92986..24f64bf8 160000 --- a/Carthage/Checkouts/Base32 +++ b/Carthage/Checkouts/Base32 @@ -1 +1 @@ -Subproject commit 1af92986863c852c2343e9ae3b427b81abc6a5d6 +Subproject commit 24f64bf81b15c815019f37cf64cfbb3a253e2bea From 731b0a2fb49c8642ae78731da8d7cbe4fae026e9 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Dec 2017 08:16:15 -0500 Subject: [PATCH 54/82] Enable several new SwiftLint rules --- .swiftlint.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.swiftlint.yml b/.swiftlint.yml index 1a20006c..56602d96 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -3,10 +3,12 @@ excluded: - Carthage opt_in_rules: + - array_init - attributes - closure_end_indentation - closure_spacing - conditional_returns_on_newline + - contains_over_first_not_nil - empty_count - explicit_init - fatal_error_message @@ -20,11 +22,13 @@ opt_in_rules: - nimble_operator - operator_usage_whitespace - overridden_super_call + - override_in_extension - pattern_matching_keywords - private_outlet - prohibited_super_call - redundant_nil_coalescing - single_test_class + - sorted_first_last - switch_case_on_newline - unneeded_parentheses_in_closure_argument - vertical_whitespace From eb0aaa07cab3b854833131dca900406d3f83747a Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Dec 2017 08:17:16 -0500 Subject: [PATCH 55/82] Remove opt-in for SwiftLint vertical_whitespace rule This rule is already enabled by default --- .swiftlint.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index 56602d96..170ab5a4 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -31,7 +31,6 @@ opt_in_rules: - sorted_first_last - switch_case_on_newline - unneeded_parentheses_in_closure_argument - - vertical_whitespace disabled_rules: - colon - comma From e7057cb93766ac1757614e80c22cedfbd5589f0e Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Dec 2017 08:19:45 -0500 Subject: [PATCH 56/82] Enable explicit_enum_raw_value lint warnings Disable these warnings in the legacy test shim. --- .swiftlint.yml | 1 + OneTimePasswordLegacyTests/OTPToken.swift | 2 ++ 2 files changed, 3 insertions(+) diff --git a/.swiftlint.yml b/.swiftlint.yml index 170ab5a4..e4fa3e95 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -10,6 +10,7 @@ opt_in_rules: - conditional_returns_on_newline - contains_over_first_not_nil - empty_count + - explicit_enum_raw_value - explicit_init - fatal_error_message - file_header diff --git a/OneTimePasswordLegacyTests/OTPToken.swift b/OneTimePasswordLegacyTests/OTPToken.swift index c4df1104..195bf2cb 100644 --- a/OneTimePasswordLegacyTests/OTPToken.swift +++ b/OneTimePasswordLegacyTests/OTPToken.swift @@ -102,6 +102,7 @@ public extension OTPToken { // MARK: Enums +// swiftlint:disable explicit_enum_raw_value @objc public enum OTPTokenType: UInt8 { case counter @@ -114,6 +115,7 @@ public enum OTPAlgorithm: UInt32 { @objc(OTPAlgorithmSHA256) case sha256 @objc(OTPAlgorithmSHA512) case sha512 } +// swiftlint:enable explicit_enum_raw_value // MARK: Conversion From f544c886f3ea4f6749431ce2cc0abbf97d5528de Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Dec 2017 08:40:13 -0500 Subject: [PATCH 57/82] Prefer access control modifiers at the extension level --- .swiftlint.yml | 1 + Sources/Token+URL.swift | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index e4fa3e95..c00b96a1 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -12,6 +12,7 @@ opt_in_rules: - empty_count - explicit_enum_raw_value - explicit_init + - extension_access_modifier - fatal_error_message - file_header - first_where diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index c8d9fc8a..c30a7822 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -26,11 +26,11 @@ import Foundation import Base32 -extension Token { +public extension Token { // MARK: Serialization /// Serializes the token to a URL. - public func toURL() throws -> URL { + func toURL() throws -> URL { return try urlForToken( name: name, issuer: issuer, @@ -41,7 +41,7 @@ extension Token { } /// Attempts to initialize a token represented by the give URL. - public init?(url: URL, secret: Data? = nil) { + init?(url: URL, secret: Data? = nil) { do { self = try token(from: url, secret: secret) } catch { From 31bd62e46f3688fcb383a2d29070247b21119a37 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Dec 2017 08:41:37 -0500 Subject: [PATCH 58/82] Enable the literal_expression_end_indentation lint rule --- .swiftlint.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.swiftlint.yml b/.swiftlint.yml index c00b96a1..653324fc 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -20,6 +20,7 @@ opt_in_rules: - implicitly_unwrapped_optional - joined_default_parameter - let_var_whitespace + - literal_expression_end_indentation - multiline_parameters - nimble_operator - operator_usage_whitespace From 529b8f08f4b4c020160602086c645b1a98a76e5c Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Dec 2017 08:45:23 -0500 Subject: [PATCH 59/82] Re-enable syntactic_sugar lint warnings --- .swiftlint.yml | 1 - Tests/TokenSerializationTests.swift | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index 653324fc..7f71316a 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -39,7 +39,6 @@ disabled_rules: - comma - cyclomatic_complexity - function_body_length - - syntactic_sugar trailing_comma: mandatory_comma: true diff --git a/Tests/TokenSerializationTests.swift b/Tests/TokenSerializationTests.swift index d2e03501..6f5d953a 100644 --- a/Tests/TokenSerializationTests.swift +++ b/Tests/TokenSerializationTests.swift @@ -100,7 +100,7 @@ class TokenSerializationTests: XCTestCase { XCTAssertEqual(items?.count, expectedItemCount, "There shouldn't be any unexpected query arguments: \(url)") - var queryArguments = Dictionary() + var queryArguments: [String: String] = [:] for item in items ?? [] { queryArguments[item.name] = item.value } From 0439077705ef30c769ba9ff03c81e7cb2c940033 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Dec 2017 08:54:45 -0500 Subject: [PATCH 60/82] Use the default line length warning threshold --- .swiftlint.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index 7f71316a..74950286 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -44,7 +44,6 @@ trailing_comma: mandatory_comma: true line_length: - warning: 120 ignores_function_declarations: true file_header: From c9ffa7a835126ea14bfb8f37acf6b977c3aeaa2f Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Dec 2017 10:52:21 -0500 Subject: [PATCH 61/82] Add a comment to the new Generator initializer --- Sources/Generator.swift | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Sources/Generator.swift b/Sources/Generator.swift index d194e839..0203e9d9 100644 --- a/Sources/Generator.swift +++ b/Sources/Generator.swift @@ -52,6 +52,10 @@ public struct Generator: Equatable { try? self.init(_factor: factor, secret: secret, algorithm: algorithm, digits: digits) } + // Eventually, this throwing initializer will replace the failable initializer above. For now, the failable + // initializer remains to maintain a consistent public API. Since two different initializers cannot overload the + // same initializer signature with both throwing an failable versions, this new initializer is currently prefixed + // with an underscore and marked as internal. internal init(_factor factor: Factor, secret: Data, algorithm: Algorithm, digits: Int) throws { try Generator.validateFactor(factor) try Generator.validateDigits(digits) From edecc8cefff44cf6b12a778def91bbbf2730174d Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Dec 2017 11:14:01 -0500 Subject: [PATCH 62/82] Use the new Generator initializer in Generator.successor() --- Sources/Generator.swift | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/Generator.swift b/Sources/Generator.swift index 0203e9d9..664ca289 100644 --- a/Sources/Generator.swift +++ b/Sources/Generator.swift @@ -116,16 +116,16 @@ public struct Generator: Equatable { /// - requires: The next generator is valid. public func successor() -> Generator { switch factor { - case .counter(let counter): - // Update a counter-based generator by incrementing the counter. Force-unwrapping should - // be safe here, since any valid generator should have a valid successor. - let nextGenerator = Generator( - factor: .counter(counter + 1), + case .counter(let counterValue): + // Update a counter-based generator by incrementing the counter. + // Force-trying should be safe here, since any valid generator should have a valid successor. + // swiftlint:disable:next force_try + return try! Generator( + _factor: .counter(counterValue + 1), secret: secret, algorithm: algorithm, digits: digits ) - return nextGenerator! case .timer: // A timer-based generator does not need to be updated. return self From c7fcc23029a1cf47458bb91de1452cf35985ba59 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Dec 2017 11:19:48 -0500 Subject: [PATCH 63/82] Add an internal throwing initializer for creating a Token from a url --- Sources/Keychain.swift | 2 +- Sources/Token+URL.swift | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/Sources/Keychain.swift b/Sources/Keychain.swift index f72dce08..6cfbd0fe 100644 --- a/Sources/Keychain.swift +++ b/Sources/Keychain.swift @@ -129,7 +129,7 @@ private extension PersistentToken { let secret = keychainDictionary[kSecValueData as String] as? Data, let keychainItemRef = keychainDictionary[kSecValuePersistentRef as String] as? Data, let url = URL(string: urlString as String), - let token = Token(url: url, secret: secret) else { + let token = try? Token(_url: url, secret: secret) else { return nil } self.init(token: token, identifier: keychainItemRef) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index 758b99f8..b286eb94 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -48,6 +48,10 @@ extension Token { return nil } } + + internal init(_url url: URL, secret: Data? = nil) throws { + self = try token(from: url, secret: secret) + } } internal enum SerializationError: Swift.Error { From 3d5733181f017de52193b6df8f20a1409ecca98f Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 22 Dec 2017 11:27:35 -0500 Subject: [PATCH 64/82] Convert PersistentToken's failable initializer to throw errors This allows persistentToken(withIdentifier:) and allPersistentTokens() to be refactored to not ignore deserialization errors. --- Sources/Keychain.swift | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Sources/Keychain.swift b/Sources/Keychain.swift index 6cfbd0fe..57321793 100644 --- a/Sources/Keychain.swift +++ b/Sources/Keychain.swift @@ -40,14 +40,14 @@ public final class Keychain { /// - throws: A `Keychain.Error` if an error occurred. /// - returns: The persistent token, or `nil` if no token matched the given identifier. public func persistentToken(withIdentifier identifier: Data) throws -> PersistentToken? { - return try keychainItem(forPersistentRef: identifier).flatMap(PersistentToken.init(keychainDictionary:)) + return try keychainItem(forPersistentRef: identifier).map(PersistentToken.init(keychainDictionary:)) } /// Returns the set of all persistent tokens found in the keychain. /// /// - throws: A `Keychain.Error` if an error occurred. public func allPersistentTokens() throws -> Set { - return Set(try allKeychainItems().flatMap(PersistentToken.init(keychainDictionary:))) + return Set(try allKeychainItems().map(PersistentToken.init(keychainDictionary:))) } // MARK: Write @@ -123,15 +123,18 @@ private extension Token { } private extension PersistentToken { - init?(keychainDictionary: NSDictionary) { + struct DeserializationError: Error {} + + init(keychainDictionary: NSDictionary) throws { guard let urlData = keychainDictionary[kSecAttrGeneric as String] as? Data, let urlString = String(data: urlData, encoding: urlStringEncoding), let secret = keychainDictionary[kSecValueData as String] as? Data, let keychainItemRef = keychainDictionary[kSecValuePersistentRef as String] as? Data, - let url = URL(string: urlString as String), - let token = try? Token(_url: url, secret: secret) else { - return nil + let url = URL(string: urlString as String) + else { + throw DeserializationError() } + let token = try Token(_url: url, secret: secret) self.init(token: token, identifier: keychainItemRef) } } From 0eeefc467a615fe76d9d906cc08db2d40a90319b Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 8 Mar 2018 01:07:23 -0500 Subject: [PATCH 65/82] Add enum cases to PersistentToken.DeserializationError --- Sources/Keychain.swift | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/Sources/Keychain.swift b/Sources/Keychain.swift index 57321793..1e140423 100644 --- a/Sources/Keychain.swift +++ b/Sources/Keychain.swift @@ -123,16 +123,26 @@ private extension Token { } private extension PersistentToken { - struct DeserializationError: Error {} + enum DeserializationError: Error { + case missingData + case missingSecret + case missingPersistentRef + case unreadableData + } init(keychainDictionary: NSDictionary) throws { - guard let urlData = keychainDictionary[kSecAttrGeneric as String] as? Data, - let urlString = String(data: urlData, encoding: urlStringEncoding), - let secret = keychainDictionary[kSecValueData as String] as? Data, - let keychainItemRef = keychainDictionary[kSecValuePersistentRef as String] as? Data, - let url = URL(string: urlString as String) - else { - throw DeserializationError() + guard let urlData = keychainDictionary[kSecAttrGeneric as String] as? Data else { + throw DeserializationError.missingData + } + guard let secret = keychainDictionary[kSecValueData as String] as? Data else { + throw DeserializationError.missingSecret + } + guard let keychainItemRef = keychainDictionary[kSecValuePersistentRef as String] as? Data else { + throw DeserializationError.missingPersistentRef + } + guard let urlString = String(data: urlData, encoding: urlStringEncoding), + let url = URL(string: urlString as String) else { + throw DeserializationError.unreadableData } let token = try Token(_url: url, secret: secret) self.init(token: token, identifier: keychainItemRef) From 3a112d17a548e79d44129955a659fd7e4d4f5b54 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 8 Mar 2018 04:49:43 -0500 Subject: [PATCH 66/82] Enable several new linter rules --- .swiftlint.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.swiftlint.yml b/.swiftlint.yml index 74950286..2b9781e5 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -9,6 +9,8 @@ opt_in_rules: - closure_spacing - conditional_returns_on_newline - contains_over_first_not_nil + - discouraged_object_literal + - discouraged_optional_boolean - empty_count - explicit_enum_raw_value - explicit_init @@ -34,6 +36,7 @@ opt_in_rules: - sorted_first_last - switch_case_on_newline - unneeded_parentheses_in_closure_argument + - yoda_condition disabled_rules: - colon - comma From 32bd750464997101e46d7a5b1dae5ee82c2683da Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 8 Mar 2018 16:31:04 -0500 Subject: [PATCH 67/82] Enable SwiftLint's vertical_parameter_alignment_on_call rule Add a comment and temporarily disable the rule in one spot to prevent a false positive. --- .swiftlint.yml | 1 + Sources/Keychain.swift | 2 +- Tests/GeneratorTests.swift | 8 ++++---- Tests/TokenSerializationTests.swift | 4 ++++ 4 files changed, 10 insertions(+), 5 deletions(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index 2b9781e5..617fdb87 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -36,6 +36,7 @@ opt_in_rules: - sorted_first_last - switch_case_on_newline - unneeded_parentheses_in_closure_argument + - vertical_parameter_alignment_on_call - yoda_condition disabled_rules: - colon diff --git a/Sources/Keychain.swift b/Sources/Keychain.swift index f72dce08..80c75a4d 100644 --- a/Sources/Keychain.swift +++ b/Sources/Keychain.swift @@ -74,7 +74,7 @@ public final class Keychain { public func update(_ persistentToken: PersistentToken, with token: Token) throws -> PersistentToken { let attributes = try token.keychainAttributes() try updateKeychainItem(forPersistentRef: persistentToken.identifier, - withAttributes: attributes) + withAttributes: attributes) return PersistentToken(token: token, identifier: persistentToken.identifier) } diff --git a/Tests/GeneratorTests.swift b/Tests/GeneratorTests.swift index 9e9a4d14..f82c9ec9 100644 --- a/Tests/GeneratorTests.swift +++ b/Tests/GeneratorTests.swift @@ -90,7 +90,7 @@ class GeneratorTests: XCTestCase { let totp = Generator(factor: timer, secret: secret, algorithm: .sha1, digits: 6) .flatMap { try? $0.password(at: time) } XCTAssertEqual(hotp, totp, - "TOTP with \(timer) should match HOTP with counter \(counter) at time \(time).") + "TOTP with \(timer) should match HOTP with counter \(counter) at time \(time).") } } @@ -226,7 +226,7 @@ class GeneratorTests: XCTestCase { let time = Date(timeIntervalSince1970: 0) let password = generator.flatMap { try? $0.password(at: time) } XCTAssertEqual(password, expectedPassword, - "The generator did not produce the expected OTP.") + "The generator did not produce the expected OTP.") } } @@ -255,7 +255,7 @@ class GeneratorTests: XCTestCase { let time = Date(timeIntervalSince1970: timeSinceEpoch) let password = generator.flatMap { try? $0.password(at: time) } XCTAssertEqual(password, expectedPassword, - "Incorrect result for \(algorithm) at \(timeSinceEpoch)") + "Incorrect result for \(algorithm) at \(timeSinceEpoch)") } } } @@ -278,7 +278,7 @@ class GeneratorTests: XCTestCase { let time = Date(timeIntervalSince1970: timeSinceEpoch) let password = generator.flatMap { try? $0.password(at: time) } XCTAssertEqual(password, expectedPassword, - "Incorrect result for \(algorithm) at \(timeSinceEpoch)") + "Incorrect result for \(algorithm) at \(timeSinceEpoch)") } } } diff --git a/Tests/TokenSerializationTests.swift b/Tests/TokenSerializationTests.swift index 6f5d953a..d71ae10d 100644 --- a/Tests/TokenSerializationTests.swift +++ b/Tests/TokenSerializationTests.swift @@ -97,8 +97,12 @@ class TokenSerializationTests: XCTestCase { let urlComponents = URLComponents(url: url, resolvingAgainstBaseURL: false) let items = urlComponents?.queryItems let expectedItemCount = 4 + // SwiftLint gives a false positive here because of a Swift/SourceKit bug. + // See https://github.com/realm/SwiftLint/issues/1785 + // swiftlint:disable vertical_parameter_alignment_on_call XCTAssertEqual(items?.count, expectedItemCount, "There shouldn't be any unexpected query arguments: \(url)") + // swiftlint:enable vertical_parameter_alignment_on_call var queryArguments: [String: String] = [:] for item in items ?? [] { From e2f3a06c64ddfb22c36d160e8def9c50c79beb88 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 8 Mar 2018 16:44:20 -0500 Subject: [PATCH 68/82] Re-enable Swiftlint's function_body_length rule Add targeted exceptions for two long test functions. --- .swiftlint.yml | 1 - Tests/KeychainTests.swift | 1 + Tests/TokenSerializationTests.swift | 1 + 3 files changed, 2 insertions(+), 1 deletion(-) diff --git a/.swiftlint.yml b/.swiftlint.yml index 617fdb87..7b047986 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -42,7 +42,6 @@ disabled_rules: - colon - comma - cyclomatic_complexity - - function_body_length trailing_comma: mandatory_comma: true diff --git a/Tests/KeychainTests.swift b/Tests/KeychainTests.swift index 0826af8d..7d606428 100644 --- a/Tests/KeychainTests.swift +++ b/Tests/KeychainTests.swift @@ -101,6 +101,7 @@ class KeychainTests: XCTestCase { } } + // swiftlint:disable:next function_body_length func testDuplicateTokens() { let token1 = testToken, token2 = testToken diff --git a/Tests/TokenSerializationTests.swift b/Tests/TokenSerializationTests.swift index d71ae10d..4ef313ba 100644 --- a/Tests/TokenSerializationTests.swift +++ b/Tests/TokenSerializationTests.swift @@ -50,6 +50,7 @@ class TokenSerializationTests: XCTestCase { let algorithms: [OneTimePassword.Generator.Algorithm] = [.sha1, .sha256, .sha512] let digits = [6, 7, 8] + // swiftlint:disable:next function_body_length func testSerialization() { for factor in factors { for name in names { From 0103eea2cbe3dcdd24199519d58abc66bb4caa19 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 8 Mar 2018 17:39:50 -0500 Subject: [PATCH 69/82] Depend on a tagged version of Base32, rather than a specific commit hash --- Cartfile | 2 +- Cartfile.resolved | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cartfile b/Cartfile index feeca3b0..dda7e5c9 100644 --- a/Cartfile +++ b/Cartfile @@ -1,3 +1,3 @@ # Configuration for Carthage (https://github.com/Carthage/Carthage) -github "mattrubin/Base32" "24f64bf81b15c815019f37cf64cfbb3a253e2bea" +github "mattrubin/Base32" "xcode9" diff --git a/Cartfile.resolved b/Cartfile.resolved index bf402e80..0f12af37 100644 --- a/Cartfile.resolved +++ b/Cartfile.resolved @@ -1,2 +1,2 @@ github "jspahrsummers/xcconfigs" "0.11" -github "mattrubin/Base32" "24f64bf81b15c815019f37cf64cfbb3a253e2bea" +github "mattrubin/Base32" "xcode9" From 06df8768d74b6db95a8f18d41f6efc3653cb2781 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 8 Mar 2018 19:51:06 -0500 Subject: [PATCH 70/82] Remove unnecessary String cast --- Sources/Keychain.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Keychain.swift b/Sources/Keychain.swift index 1e140423..0740fdc0 100644 --- a/Sources/Keychain.swift +++ b/Sources/Keychain.swift @@ -141,7 +141,7 @@ private extension PersistentToken { throw DeserializationError.missingPersistentRef } guard let urlString = String(data: urlData, encoding: urlStringEncoding), - let url = URL(string: urlString as String) else { + let url = URL(string: urlString) else { throw DeserializationError.unreadableData } let token = try Token(_url: url, secret: secret) From b0f057331d63d52ee81b93ee2ed68e6a5bd60cad Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Fri, 9 Mar 2018 23:20:59 -0500 Subject: [PATCH 71/82] Add a comment to the new throwing Token initializer --- Sources/Token+URL.swift | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Sources/Token+URL.swift b/Sources/Token+URL.swift index fac4f4b1..fbc0aa26 100644 --- a/Sources/Token+URL.swift +++ b/Sources/Token+URL.swift @@ -42,13 +42,13 @@ public extension Token { /// Attempts to initialize a token represented by the give URL. init?(url: URL, secret: Data? = nil) { - do { - self = try token(from: url, secret: secret) - } catch { - return nil - } + try? self.init(_url: url, secret: secret) } + // Eventually, this throwing initializer will replace the failable initializer above. For now, the failable + // initializer remains to maintain a consistent public API. Since two different initializers cannot overload the + // same initializer signature with both throwing an failable versions, this new initializer is currently prefixed + // with an underscore and marked as internal. internal init(_url url: URL, secret: Data? = nil) throws { self = try token(from: url, secret: secret) } From d58b404f4e6ba9a7cf03a9639e584c425fc469ff Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 15 Mar 2018 14:55:45 -0400 Subject: [PATCH 72/82] Add tests for the failure cases of Keychain's init(keychainDictionary:) --- Tests/KeychainTests.swift | 78 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/Tests/KeychainTests.swift b/Tests/KeychainTests.swift index 7d606428..3c6661a5 100644 --- a/Tests/KeychainTests.swift +++ b/Tests/KeychainTests.swift @@ -222,4 +222,82 @@ class KeychainTests: XCTestCase { XCTFail("allPersistentTokens() failed with error: \(error)") } } + + func testMissingData() throws { + let keychainAttributes: [String: AnyObject] = [ + kSecValueData as String: testToken.generator.secret as NSData, + ] + + let persistentRef = try addKeychainItem(withAttributes: keychainAttributes) + + XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef)) + XCTAssertThrowsError(try keychain.allPersistentTokens()) + } + + func testMissingSecret() throws { + let data = try testToken.toURL().absoluteString.data(using: .utf8)! + + let keychainAttributes: [String: AnyObject] = [ + kSecAttrGeneric as String: data as NSData, + ] + + let persistentRef = try addKeychainItem(withAttributes: keychainAttributes) + + XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef)) + XCTAssertThrowsError(try keychain.allPersistentTokens()) + } + + func testBadData() throws { + let badData = " ".data(using: .utf8)! + + let keychainAttributes: [String: AnyObject] = [ + kSecAttrGeneric as String: badData as NSData, + kSecValueData as String: testToken.generator.secret as NSData, + ] + + let persistentRef = try addKeychainItem(withAttributes: keychainAttributes) + + XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef), "") { error in print(error) } + XCTAssertThrowsError(try keychain.allPersistentTokens()) + } + + func testBadURL() throws { + let badData = "http://example.com".data(using: .utf8)! + + let keychainAttributes: [String: AnyObject] = [ + kSecAttrGeneric as String: badData as NSData, + kSecValueData as String: testToken.generator.secret as NSData, + ] + + let persistentRef = try addKeychainItem(withAttributes: keychainAttributes) + + XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef), "") { error in print(error) } + XCTAssertThrowsError(try keychain.allPersistentTokens()) + } + + // MARK: Keychain helpers + + private func addKeychainItem(withAttributes attributes: [String: AnyObject]) throws -> Data { + var mutableAttributes = attributes + mutableAttributes[kSecClass as String] = kSecClassGenericPassword + mutableAttributes[kSecReturnPersistentRef as String] = kCFBooleanTrue + // Set a random string for the account name. + // We never query by or display this value, but the keychain requires it to be unique. + if mutableAttributes[kSecAttrAccount as String] == nil { + mutableAttributes[kSecAttrAccount as String] = UUID().uuidString as NSString + } + + var result: AnyObject? + let resultCode: OSStatus = withUnsafeMutablePointer(to: &result) { + SecItemAdd(mutableAttributes as CFDictionary, $0) + } + + guard resultCode == errSecSuccess else { + throw Keychain.Error.systemError(resultCode) + } + guard let persistentRef = result as? Data else { + throw Keychain.Error.incorrectReturnType + } + return persistentRef + } } From 0ac6dbec7fe23f2b4108a6c822ee15551ee66d6b Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 15 Mar 2018 14:59:33 -0400 Subject: [PATCH 73/82] Clean up the CodeCov ignore list --- .codecov.yml | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/.codecov.yml b/.codecov.yml index 930c7dbd..059c9f54 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -1,6 +1,5 @@ # Configuration for Codecov (https://codecov.io) -coverage: - ignore: - - "Tests" - - "OneTimePasswordLegacyTests" +ignore: + - Tests + - OneTimePasswordLegacyTests From 77608baa2f806b39ef9f6e260dadd5b36f83f6b7 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 15 Mar 2018 15:00:00 -0400 Subject: [PATCH 74/82] Configure CodeCov to use `develop` as the default branch --- .codecov.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.codecov.yml b/.codecov.yml index 059c9f54..6a957cc7 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -1,5 +1,9 @@ # Configuration for Codecov (https://codecov.io) +codecov: + # Use `develop` as the default branch + branch: develop + ignore: - Tests - OneTimePasswordLegacyTests From afdaef546e6b89fa793818cc6ea44ad81b7a5b74 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Thu, 15 Mar 2018 22:06:11 -0400 Subject: [PATCH 75/82] Allow patch to be 0% covered without marking a PR with a failing status --- .codecov.yml | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/.codecov.yml b/.codecov.yml index 6a957cc7..46fd814f 100644 --- a/.codecov.yml +++ b/.codecov.yml @@ -7,3 +7,10 @@ codecov: ignore: - Tests - OneTimePasswordLegacyTests + +coverage: + status: + patch: + default: + # Allow patch to be 0% covered without marking a PR with a failing status. + target: 0 From b7c419d6cdf837ff01d477e8af4ae77b170f0cd1 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Sat, 17 Mar 2018 13:21:40 -0400 Subject: [PATCH 76/82] Remove debug logging in keychain tests --- Tests/KeychainTests.swift | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Tests/KeychainTests.swift b/Tests/KeychainTests.swift index 3c6661a5..1e99800b 100644 --- a/Tests/KeychainTests.swift +++ b/Tests/KeychainTests.swift @@ -257,7 +257,7 @@ class KeychainTests: XCTestCase { let persistentRef = try addKeychainItem(withAttributes: keychainAttributes) - XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef), "") { error in print(error) } + XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef)) XCTAssertThrowsError(try keychain.allPersistentTokens()) } @@ -271,7 +271,7 @@ class KeychainTests: XCTestCase { let persistentRef = try addKeychainItem(withAttributes: keychainAttributes) - XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef), "") { error in print(error) } + XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef)) XCTAssertThrowsError(try keychain.allPersistentTokens()) } From 5a3c36a8574fc39afa82c1c14861e9a48f596dd6 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Sat, 17 Mar 2018 13:30:10 -0400 Subject: [PATCH 77/82] Delete bad tokens from the keychain to prevent breaking future test runs --- Tests/KeychainTests.swift | 67 +++++++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 21 deletions(-) diff --git a/Tests/KeychainTests.swift b/Tests/KeychainTests.swift index 1e99800b..c2068368 100644 --- a/Tests/KeychainTests.swift +++ b/Tests/KeychainTests.swift @@ -232,6 +232,9 @@ class KeychainTests: XCTestCase { XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef)) XCTAssertThrowsError(try keychain.allPersistentTokens()) + + XCTAssertNoThrow(try deleteKeychainItem(forPersistentRef: persistentRef), + "Failed to delete the test token from the keychain. This may cause future test runs to fail.") } func testMissingSecret() throws { @@ -245,6 +248,9 @@ class KeychainTests: XCTestCase { XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef)) XCTAssertThrowsError(try keychain.allPersistentTokens()) + + XCTAssertNoThrow(try deleteKeychainItem(forPersistentRef: persistentRef), + "Failed to delete the test token from the keychain. This may cause future test runs to fail.") } func testBadData() throws { @@ -259,6 +265,9 @@ class KeychainTests: XCTestCase { XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef)) XCTAssertThrowsError(try keychain.allPersistentTokens()) + + XCTAssertNoThrow(try deleteKeychainItem(forPersistentRef: persistentRef), + "Failed to delete the test token from the keychain. This may cause future test runs to fail.") } func testBadURL() throws { @@ -273,31 +282,47 @@ class KeychainTests: XCTestCase { XCTAssertThrowsError(try keychain.persistentToken(withIdentifier: persistentRef)) XCTAssertThrowsError(try keychain.allPersistentTokens()) + + XCTAssertNoThrow(try deleteKeychainItem(forPersistentRef: persistentRef), + "Failed to delete the test token from the keychain. This may cause future test runs to fail.") } +} - // MARK: Keychain helpers +// MARK: Keychain helpers - private func addKeychainItem(withAttributes attributes: [String: AnyObject]) throws -> Data { - var mutableAttributes = attributes - mutableAttributes[kSecClass as String] = kSecClassGenericPassword - mutableAttributes[kSecReturnPersistentRef as String] = kCFBooleanTrue - // Set a random string for the account name. - // We never query by or display this value, but the keychain requires it to be unique. - if mutableAttributes[kSecAttrAccount as String] == nil { - mutableAttributes[kSecAttrAccount as String] = UUID().uuidString as NSString - } +private func addKeychainItem(withAttributes attributes: [String: AnyObject]) throws -> Data { + var mutableAttributes = attributes + mutableAttributes[kSecClass as String] = kSecClassGenericPassword + mutableAttributes[kSecReturnPersistentRef as String] = kCFBooleanTrue + // Set a random string for the account name. + // We never query by or display this value, but the keychain requires it to be unique. + if mutableAttributes[kSecAttrAccount as String] == nil { + mutableAttributes[kSecAttrAccount as String] = UUID().uuidString as NSString + } - var result: AnyObject? - let resultCode: OSStatus = withUnsafeMutablePointer(to: &result) { - SecItemAdd(mutableAttributes as CFDictionary, $0) - } + var result: AnyObject? + let resultCode: OSStatus = withUnsafeMutablePointer(to: &result) { + SecItemAdd(mutableAttributes as CFDictionary, $0) + } - guard resultCode == errSecSuccess else { - throw Keychain.Error.systemError(resultCode) - } - guard let persistentRef = result as? Data else { - throw Keychain.Error.incorrectReturnType - } - return persistentRef + guard resultCode == errSecSuccess else { + throw Keychain.Error.systemError(resultCode) + } + guard let persistentRef = result as? Data else { + throw Keychain.Error.incorrectReturnType + } + return persistentRef +} + +public func deleteKeychainItem(forPersistentRef persistentRef: Data) throws { + let queryDict: [String : AnyObject] = [ + kSecClass as String: kSecClassGenericPassword, + kSecValuePersistentRef as String: persistentRef as NSData, + ] + + let resultCode = SecItemDelete(queryDict as CFDictionary) + + guard resultCode == errSecSuccess else { + throw Keychain.Error.systemError(resultCode) } } From 3c130e3164cd62fe8d6c0834bd02e11d174ea9d2 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Sun, 18 Mar 2018 19:38:34 -0400 Subject: [PATCH 78/82] Add recent changes to the changelog --- CHANGELOG.md | 22 +++++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 164335de..68b0eba8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,25 @@ # OneTimePassword Changelog - +## [In development][develop] +- Upgrade to Swift 4 and Xcode 9. +([#147](https://github.com/mattrubin/OneTimePassword/pull/147), +[#149](https://github.com/mattrubin/OneTimePassword/pull/149), +[#151](https://github.com/mattrubin/OneTimePassword/pull/151), +[#153](https://github.com/mattrubin/OneTimePassword/pull/153), +[#160](https://github.com/mattrubin/OneTimePassword/pull/160)) +- Handle keychain deserialization errors. +([#161](https://github.com/mattrubin/OneTimePassword/pull/161)) +- Refactor token URL parsing. +([#150](https://github.com/mattrubin/OneTimePassword/pull/150)) +- Refactor Generator validation. +([#155](https://github.com/mattrubin/OneTimePassword/pull/155)) +- Update SwiftLint configuration and improve code formatting. +([#148](https://github.com/mattrubin/OneTimePassword/pull/148), +[#154](https://github.com/mattrubin/OneTimePassword/pull/154), +[#159](https://github.com/mattrubin/OneTimePassword/pull/159)) +- Update CodeCov configuration. +([#162](https://github.com/mattrubin/OneTimePassword/pull/162)) + ## [3.0.1][] (2018-03-08) - Fix an issue where CocoaPods was trying to build OneTimePassword with Swift 4. ([#157](https://github.com/mattrubin/OneTimePassword/pull/157)) @@ -8,6 +27,7 @@ - Tweak the Travis CI configuration to work around test timeout flakiness. ([#131](https://github.com/mattrubin/OneTimePassword/pull/131)) - Clean up some old Keychain code which was used to provide Xcode 7 backwards compatibility. ([#133](https://github.com/mattrubin/OneTimePassword/pull/133)) + ## [3.0][] (2017-02-07) - Convert to Swift 3 and update the library API to follow the Swift [API Design Guidelines](https://swift.org/documentation/api-design-guidelines/). ([#74](https://github.com/mattrubin/OneTimePassword/pull/74), From 1c0d2de420847abdc1b9cb9caffa04da59afdd20 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 27 Mar 2018 13:32:17 -0400 Subject: [PATCH 79/82] Bump the version number to 3.1 --- OneTimePassword.podspec | 2 +- Sources/Info.plist | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/OneTimePassword.podspec b/OneTimePassword.podspec index 0683de89..710c9760 100644 --- a/OneTimePassword.podspec +++ b/OneTimePassword.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = "OneTimePassword" - s.version = "3.0.1" + s.version = "3.1" s.summary = "A small library for generating TOTP and HOTP one-time passwords." s.homepage = "https://github.com/mattrubin/OneTimePassword" s.license = "MIT" diff --git a/Sources/Info.plist b/Sources/Info.plist index bf64edb3..54106c8d 100644 --- a/Sources/Info.plist +++ b/Sources/Info.plist @@ -15,11 +15,11 @@ CFBundlePackageType FMWK CFBundleShortVersionString - 3.0.1 + 3.1 CFBundleSignature ???? CFBundleVersion - 3.0.1 + 3.1 NSPrincipalClass From c0ed8b973cc77815c444bbd33c2619b049ecabf6 Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 27 Mar 2018 13:35:28 -0400 Subject: [PATCH 80/82] Bump the version number in the installation instructions --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 62ea3e3c..f22a3e7e 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ The OneTimePassword library is the core of [Authenticator][]. It can generate bo Add the following line to your [Cartfile][]: ````config -github "mattrubin/OneTimePassword" ~> 3.0 +github "mattrubin/OneTimePassword" ~> 3.1 ```` Then run `carthage update OneTimePassword` to install the latest version of the framework. @@ -39,7 +39,7 @@ Be sure to check the Carthage README file for the latest instructions on [adding Add the following line to your [Podfile][]: ````ruby -pod 'OneTimePassword', '~> 3.0' +pod 'OneTimePassword', '~> 3.1' ```` OneTimePassword, like all pods written in Swift, can only be integrated as a framework. Make sure to add the line `use_frameworks!` to your Podfile or target to opt into frameworks instead of static libraries. From 97e09acfed6dc1bbd5567c99c9e1497af076fe2b Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 27 Mar 2018 13:59:20 -0400 Subject: [PATCH 81/82] Update the usage note about Swift version compatibility in the README --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index f22a3e7e..8d98ce0f 100644 --- a/README.md +++ b/README.md @@ -52,8 +52,9 @@ Then run `pod install` to install the latest version of the framework. ## Usage -> The [latest version][swift-3] of OneTimePassword targets Swift 3. To use OneTimePassword with Swift 2.3, check out the [`swift-2.3` branch][swift-2.3] and the [2.x releases][releases]. To use OneTimePassword in an Objective-C based project, check out the [`objc` branch][objc] and the [1.x releases][releases]. +> The [latest version][swift-4] of OneTimePassword uses Swift 4, and can be linked with Swift 3.2 projects using the Swift compiler's [compatibility mode](https://swift.org/blog/swift-4-0-released/#new-compatibility-modes). To use OneTimePassword with earlier versions of Swift, check out the [`swift-3`][swift-3] and [`swift-2.3`][swift-2.3] branches. To use OneTimePassword in an Objective-C based project, check out the [`objc` branch][objc] and the [1.x releases][releases]. +[swift-4]: https://github.com/mattrubin/OneTimePassword/tree/swift-4 [swift-3]: https://github.com/mattrubin/OneTimePassword/tree/swift-3 [swift-2.3]: https://github.com/mattrubin/OneTimePassword/tree/swift-2.3 [objc]: https://github.com/mattrubin/OneTimePassword/tree/objc From 789de062b2bb7774e3fc7e31752e5ba6a96f807d Mon Sep 17 00:00:00 2001 From: Matt Rubin Date: Tue, 27 Mar 2018 14:14:48 -0400 Subject: [PATCH 82/82] Add 3.1 header to the changelog --- CHANGELOG.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68b0eba8..9815d969 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ # OneTimePassword Changelog -## [In development][develop] + + +## [3.1][] (2018-03-27) - Upgrade to Swift 4 and Xcode 9. ([#147](https://github.com/mattrubin/OneTimePassword/pull/147), [#149](https://github.com/mattrubin/OneTimePassword/pull/149), @@ -141,8 +143,9 @@ Changes between prerelease versions of OneTimePassword version 2 can be found be ## [1.0.0][] (2014-07-17) -[develop]: https://github.com/mattrubin/OneTimePassword/compare/3.0.1...develop +[develop]: https://github.com/mattrubin/OneTimePassword/compare/3.1...develop +[3.1]: https://github.com/mattrubin/OneTimePassword/compare/3.0.1...3.1 [3.0.1]: https://github.com/mattrubin/OneTimePassword/compare/3.0...3.0.1 [3.0]: https://github.com/mattrubin/OneTimePassword/compare/2.1.1...3.0 [2.1.1]: https://github.com/mattrubin/OneTimePassword/compare/2.1...2.1.1