From 508967a4158d7ad121d2f0597c2c4fd456cbb217 Mon Sep 17 00:00:00 2001 From: adamjmcgrath Date: Thu, 18 Nov 2021 12:45:04 +0000 Subject: [PATCH 1/6] CredentialsStore protocol --- Auth0/CredentialsManager.swift | 30 +++++++++++++++++-- Auth0Tests/CredentialsManagerSpec.swift | 39 ++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/Auth0/CredentialsManager.swift b/Auth0/CredentialsManager.swift index 48ba5925..724c6ea9 100644 --- a/Auth0/CredentialsManager.swift +++ b/Auth0/CredentialsManager.swift @@ -5,10 +5,36 @@ import JWTDecode import LocalAuthentication #endif +/// Generic storage API for storing credentials +public protocol CredentialsStorage { + /// Retreive a storage entry + /// + /// - Parameters: + /// - forKey: The key to get from the store + /// - Returns: The stored data + func data(forKey: String) -> Data? + /// Set a storage entry + /// + /// - Parameters: + /// - _: The data to be stored + /// - forKey: The key to store it to + /// - Returns: if credentials were stored + func setData(_: Data, forKey: String) -> Bool + /// Delete a storage entry + /// + /// - Parameters: + /// - forKey: The key to delete from the store + /// - Returns: if credentials were deleted + func deleteEntry(forKey: String) -> Bool +} + +extension A0SimpleKeychain: CredentialsStorage { +} + /// Credentials management utility public struct CredentialsManager { - private let storage: A0SimpleKeychain + private let storage: CredentialsStorage private let storeKey: String private let authentication: Authentication private let dispatchQueue = DispatchQueue(label: "com.auth0.credentialsmanager.serial") @@ -23,7 +49,7 @@ public struct CredentialsManager { /// - authentication: Auth0 authentication instance /// - storeKey: Key used to store user credentials in the keychain, defaults to "credentials" /// - storage: The A0SimpleKeychain instance used to manage credentials storage. Defaults to a standard A0SimpleKeychain instance - public init(authentication: Authentication, storeKey: String = "credentials", storage: A0SimpleKeychain = A0SimpleKeychain()) { + public init(authentication: Authentication, storeKey: String = "credentials", storage: CredentialsStorage = A0SimpleKeychain()) { self.storeKey = storeKey self.authentication = authentication self.storage = storage diff --git a/Auth0Tests/CredentialsManagerSpec.swift b/Auth0Tests/CredentialsManagerSpec.swift index 36bf1cf3..56eef2bb 100644 --- a/Auth0Tests/CredentialsManagerSpec.swift +++ b/Auth0Tests/CredentialsManagerSpec.swift @@ -59,7 +59,44 @@ class CredentialsManagerSpec: QuickSpec { expect(credentialsManager.clear()).to(beFalse()) } } - + + describe("custom storage") { + + class CustomStore: CredentialsStorage { + var store: [String : Data] = [:] + func setData(_ data: Data, forKey: String) -> Bool { + store[forKey] = data + return true + } + func deleteEntry(forKey: String) -> Bool { + store[forKey] = nil + return true + } + func data(forKey: String) -> Data? { + return store[forKey] + } + } + + beforeEach { + credentialsManager = CredentialsManager(authentication: authentication, storage: CustomStore()); + } + + afterEach { + _ = credentialsManager.clear() + } + + it("should store credentials in custom store") { + expect(credentialsManager.store(credentials: credentials)).to(beTrue()) + expect(credentialsManager.hasValid()).to(beTrue()) + } + + it("should clear credentials from custom store") { + expect(credentialsManager.store(credentials: credentials)).to(beTrue()) + expect(credentialsManager.clear()).to(beTrue()) + expect(credentialsManager.hasValid()).to(beFalse()) + } + } + describe("clearing and revoking refresh token") { beforeEach { From 34ad5e59ff919d470908ce56d39f2595a61da4c9 Mon Sep 17 00:00:00 2001 From: adamjmcgrath Date: Thu, 18 Nov 2021 12:57:09 +0000 Subject: [PATCH 2/6] Add migration guide --- V2_MIGRATION_GUIDE.md | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/V2_MIGRATION_GUIDE.md b/V2_MIGRATION_GUIDE.md index 2a5e9128..38548364 100644 --- a/V2_MIGRATION_GUIDE.md +++ b/V2_MIGRATION_GUIDE.md @@ -79,7 +79,7 @@ The following classes were also removed, as they are no longer in use: - `Profile` - `Identity` -## Metods Removed +## Methods Removed The iOS-only method `resumeAuth(_:options:)` and the macOS-only method `resumeAuth(_:)` were removed from the library, as they are no longer needed. @@ -202,6 +202,32 @@ In the following methods the `scope` parameter became non-optional (with a defau The `multifactorChallenge(mfaToken:types:authenticatorId:)` method lost its `channel` parameter, which is no longer necessary. +### Credentials Manager + +`CredentialsManager` now takes a `CredentialsStorage` protocol as it's storage argument rather than an instance of `SimpleKeyChain`. + +This means you can now provide your own storage layer to `CredentialsManager`. + +```swift +class CustomStore: CredentialsStorage { + var store: [String : Data] = [:] + func setData(_ data: Data, forKey: String) -> Bool { + store[forKey] = data + return true + } + func deleteEntry(forKey: String) -> Bool { + store[forKey] = nil + return true + } + func data(forKey: String) -> Data? { + return store[forKey] + } +} + +let credentialsManager = CredentialsManager(authentication: authentication, + storage: CustomStore()); +``` + ## Behavior changes ### `openid` scope enforced on Web Auth From b198708f47f57fac8e3f8747540503602bb42f57 Mon Sep 17 00:00:00 2001 From: adamjmcgrath Date: Thu, 18 Nov 2021 12:58:28 +0000 Subject: [PATCH 3/6] Add migration guide --- V2_MIGRATION_GUIDE.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/V2_MIGRATION_GUIDE.md b/V2_MIGRATION_GUIDE.md index 38548364..726d8593 100644 --- a/V2_MIGRATION_GUIDE.md +++ b/V2_MIGRATION_GUIDE.md @@ -224,8 +224,7 @@ class CustomStore: CredentialsStorage { } } -let credentialsManager = CredentialsManager(authentication: authentication, - storage: CustomStore()); +let credentialsManager = CredentialsManager(authentication: authentication, storage: CustomStore()); ``` ## Behavior changes From 6bd17f0f2c9af326cd03f007b3fd41e164af6fb7 Mon Sep 17 00:00:00 2001 From: adamjmcgrath Date: Thu, 18 Nov 2021 15:28:29 +0000 Subject: [PATCH 4/6] fix typo --- Auth0/CredentialsManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Auth0/CredentialsManager.swift b/Auth0/CredentialsManager.swift index 724c6ea9..35fa896d 100644 --- a/Auth0/CredentialsManager.swift +++ b/Auth0/CredentialsManager.swift @@ -7,7 +7,7 @@ import LocalAuthentication /// Generic storage API for storing credentials public protocol CredentialsStorage { - /// Retreive a storage entry + /// Retrieve a storage entry /// /// - Parameters: /// - forKey: The key to get from the store From 73a1109cee049f81ed140f226c305b9bf8614965 Mon Sep 17 00:00:00 2001 From: Adam Mcgrath Date: Fri, 19 Nov 2021 08:42:05 +0000 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Rita Zerrizuela --- Auth0Tests/CredentialsManagerSpec.swift | 2 +- V2_MIGRATION_GUIDE.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Auth0Tests/CredentialsManagerSpec.swift b/Auth0Tests/CredentialsManagerSpec.swift index 56eef2bb..b403201d 100644 --- a/Auth0Tests/CredentialsManagerSpec.swift +++ b/Auth0Tests/CredentialsManagerSpec.swift @@ -63,7 +63,7 @@ class CredentialsManagerSpec: QuickSpec { describe("custom storage") { class CustomStore: CredentialsStorage { - var store: [String : Data] = [:] + var store: [String: Data] = [:] func setData(_ data: Data, forKey: String) -> Bool { store[forKey] = data return true diff --git a/V2_MIGRATION_GUIDE.md b/V2_MIGRATION_GUIDE.md index 726d8593..fa4f5fd5 100644 --- a/V2_MIGRATION_GUIDE.md +++ b/V2_MIGRATION_GUIDE.md @@ -204,7 +204,7 @@ The `multifactorChallenge(mfaToken:types:authenticatorId:)` method lost its `cha ### Credentials Manager -`CredentialsManager` now takes a `CredentialsStorage` protocol as it's storage argument rather than an instance of `SimpleKeyChain`. +`CredentialsManager` now takes a `CredentialsStorage` protocol as it's storage argument rather than an instance of `SimpleKeychain`. This means you can now provide your own storage layer to `CredentialsManager`. From 34c5a8b96827f97b62c8ad53dffb3ed5de3edc74 Mon Sep 17 00:00:00 2001 From: adamjmcgrath Date: Fri, 19 Nov 2021 08:55:18 +0000 Subject: [PATCH 6/6] Better custom store API --- Auth0/CredentialsManager.swift | 18 ++++++++++++------ Auth0Tests/CredentialsManagerSpec.swift | 8 ++++---- V2_MIGRATION_GUIDE.md | 8 ++++---- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/Auth0/CredentialsManager.swift b/Auth0/CredentialsManager.swift index 35fa896d..d31cbe3e 100644 --- a/Auth0/CredentialsManager.swift +++ b/Auth0/CredentialsManager.swift @@ -12,14 +12,14 @@ public protocol CredentialsStorage { /// - Parameters: /// - forKey: The key to get from the store /// - Returns: The stored data - func data(forKey: String) -> Data? + func getEntry(forKey: String) -> Data? /// Set a storage entry /// /// - Parameters: /// - _: The data to be stored /// - forKey: The key to store it to /// - Returns: if credentials were stored - func setData(_: Data, forKey: String) -> Bool + func setEntry(_: Data, forKey: String) -> Bool /// Delete a storage entry /// /// - Parameters: @@ -29,6 +29,12 @@ public protocol CredentialsStorage { } extension A0SimpleKeychain: CredentialsStorage { + public func getEntry(forKey: String) -> Data? { + return data(forKey: forKey) + } + public func setEntry(_ data: Data, forKey: String) -> Bool { + return setData(data, forKey: forKey) + } } /// Credentials management utility @@ -89,7 +95,7 @@ public struct CredentialsManager { guard let data = try? NSKeyedArchiver.archivedData(withRootObject: credentials, requiringSecureCoding: true) else { return false } - return self.storage.setData(data, forKey: storeKey) + return self.storage.setEntry(data, forKey: storeKey) } /// Clear credentials stored in keychain @@ -107,7 +113,7 @@ public struct CredentialsManager { /// - Parameter callback: callback with an error if the refresh token could not be revoked public func revoke(_ callback: @escaping (CredentialsManagerError?) -> Void) { guard - let data = self.storage.data(forKey: self.storeKey), + let data = self.storage.getEntry(forKey: self.storeKey), let credentials = try? NSKeyedUnarchiver.unarchivedObject(ofClass: Credentials.self, from: data), let refreshToken = credentials.refreshToken else { _ = self.clear() @@ -132,7 +138,7 @@ public struct CredentialsManager { /// - Parameter minTTL: minimum lifetime in seconds the access token must have left. /// - Returns: if there are valid and non-expired credentials stored. public func hasValid(minTTL: Int = 0) -> Bool { - guard let data = self.storage.data(forKey: self.storeKey), + guard let data = self.storage.getEntry(forKey: self.storeKey), let credentials = try? NSKeyedUnarchiver.unarchivedObject(ofClass: Credentials.self, from: data) else { return false } return (!self.hasExpired(credentials) && !self.willExpire(credentials, within: minTTL)) || credentials.refreshToken != nil } @@ -176,7 +182,7 @@ public struct CredentialsManager { #endif private func retrieveCredentials() -> Credentials? { - guard let data = self.storage.data(forKey: self.storeKey), + guard let data = self.storage.getEntry(forKey: self.storeKey), let credentials = try? NSKeyedUnarchiver.unarchivedObject(ofClass: Credentials.self, from: data) else { return nil } return credentials diff --git a/Auth0Tests/CredentialsManagerSpec.swift b/Auth0Tests/CredentialsManagerSpec.swift index b403201d..44261ae6 100644 --- a/Auth0Tests/CredentialsManagerSpec.swift +++ b/Auth0Tests/CredentialsManagerSpec.swift @@ -64,7 +64,10 @@ class CredentialsManagerSpec: QuickSpec { class CustomStore: CredentialsStorage { var store: [String: Data] = [:] - func setData(_ data: Data, forKey: String) -> Bool { + func getEntry(forKey: String) -> Data? { + return store[forKey] + } + func setEntry(_ data: Data, forKey: String) -> Bool { store[forKey] = data return true } @@ -72,9 +75,6 @@ class CredentialsManagerSpec: QuickSpec { store[forKey] = nil return true } - func data(forKey: String) -> Data? { - return store[forKey] - } } beforeEach { diff --git a/V2_MIGRATION_GUIDE.md b/V2_MIGRATION_GUIDE.md index fa4f5fd5..b51248b6 100644 --- a/V2_MIGRATION_GUIDE.md +++ b/V2_MIGRATION_GUIDE.md @@ -211,7 +211,10 @@ This means you can now provide your own storage layer to `CredentialsManager`. ```swift class CustomStore: CredentialsStorage { var store: [String : Data] = [:] - func setData(_ data: Data, forKey: String) -> Bool { + func getEntry(forKey: String) -> Data? { + return store[forKey] + } + func setEntry(_ data: Data, forKey: String) -> Bool { store[forKey] = data return true } @@ -219,9 +222,6 @@ class CustomStore: CredentialsStorage { store[forKey] = nil return true } - func data(forKey: String) -> Data? { - return store[forKey] - } } let credentialsManager = CredentialsManager(authentication: authentication, storage: CustomStore());