refactor: updated the key logic for storing MRRT token (#1031)

Author: pmathew92

Auth0/CredentialsManager.swift

@@ -37,7 +37,7 @@ public struct CredentialsManager {
         let noSession: TimeInterval = -1
         var lastBiometricAuthTime: TimeInterval = -1
         let lock = NSLock()
-        
+
         init() {
             lastBiometricAuthTime = noSession
         }
@@ -163,9 +163,12 @@ public struct CredentialsManager {
     /// ```
     ///
     /// - Parameter audience: Identifier of the API the stored API credentials are for.
+    /// - Parameter scope: Optional scope for which the  API Credentials are stored. If the credentials were initially fetched/stored with scope,
+    ///   it is recommended to pass scope also while clearing them.
     /// - Returns: If the API credentials were removed.
-    public func clear(forAudience audience: String) -> Bool {
-        return self.storage.deleteEntry(forKey: audience)
+    public func clear(forAudience audience: String, scope: String? = nil) -> Bool {
+        let key = getAPICredentialsStorageKey(audience: audience, scope: scope)
+        return self.storage.deleteEntry(forKey: key)
     }
 
     #if WEB_AUTH_PLATFORM
@@ -180,13 +183,13 @@ public struct CredentialsManager {
     /// - Returns: `true` if the session is valid and biometric authentication can be skipped, `false` otherwise.
     public func isBiometricSessionValid() -> Bool {
         guard let bioAuth = self.bioAuth else { return false }
-        
+
         self.biometricSession.lock.lock()
         defer { self.biometricSession.lock.unlock() }
-        
+
         let lastAuth = self.biometricSession.lastBiometricAuthTime
         if lastAuth == self.biometricSession.noSession { return false }
-        
+
         switch bioAuth.policy {
         case .session(let timeoutInSeconds), .appLifecycle(let timeoutInSeconds):
             let timeoutInterval = TimeInterval(timeoutInSeconds)
@@ -694,24 +697,39 @@ public struct CredentialsManager {
                                  callback: callback)
     }
 
-    public func store(apiCredentials: APICredentials, forAudience audience: String) -> Bool {
+    public func store(apiCredentials: APICredentials, forAudience audience: String, forScope scope: String? = nil) -> Bool {
         guard let data = try? apiCredentials.encode() else {
             return false
         }
 
-        return self.storage.setEntry(data, forKey: audience)
+        let key = getAPICredentialsStorageKey(audience: audience, scope: scope)
+        return self.storage.setEntry(data, forKey: key)
     }
 
     private func retrieveCredentials() -> Credentials? {
         guard let data = self.storage.getEntry(forKey: self.storeKey) else { return nil }
         return try? NSKeyedUnarchiver.unarchivedObject(ofClass: Credentials.self, from: data)
     }
 
-    private func retrieveAPICredentials(audience: String) -> APICredentials? {
-        guard let data = self.storage.getEntry(forKey: audience) else { return nil }
+    private func retrieveAPICredentials(audience: String, scope: String?) -> APICredentials? {
+        let key = getAPICredentialsStorageKey(audience: audience, scope: scope)
+        guard let data = self.storage.getEntry(forKey: key) else { return nil }
         return try? APICredentials(from: data)
     }
 
+    private func getAPICredentialsStorageKey(audience: String, scope: String?) -> String {
+        // Use audience if scope is null else use a combination of audience and scope
+        if let scope = scope {
+            let normalisedScopes = scope
+            .split(separator: " ")
+            .sorted()
+            .joined(separator: "::")
+            return "\(audience)::\(normalisedScopes)"
+        } else {
+            return audience
+        }
+    }
+
     // swiftlint:disable:next function_parameter_count
     private func retrieveCredentials(scope: String?,
                                      minTTL: Int,
@@ -832,10 +850,10 @@ public struct CredentialsManager {
             dispatchGroup.enter()
 
             DispatchQueue.global(qos: .userInitiated).async {
-                if let apiCredentials = self.retrieveAPICredentials(audience: audience),
+                if let apiCredentials = self.retrieveAPICredentials(audience: audience, scope: scope),
                       !self.hasExpired(apiCredentials.expiresIn),
                       !self.willExpire(apiCredentials.expiresIn, within: minTTL),
-                      !self.hasScopeChanged(from: apiCredentials.scope, to: scope) {
+                   !self.hasScopeChanged(from: apiCredentials.scope, to: scope, ignoreOpenid: scope?.contains("openid") == false) {
                     dispatchGroup.leave()
                     return callback(.success(apiCredentials))
                 }
@@ -867,7 +885,7 @@ public struct CredentialsManager {
                             } else if !self.store(credentials: newCredentials) {
                                 dispatchGroup.leave()
                                 callback(.failure(CredentialsManagerError(code: .storeFailed)))
-                            } else if !self.store(apiCredentials: newAPICredentials, forAudience: audience) {
+                            } else if !self.store(apiCredentials: newAPICredentials, forAudience: audience, forScope: scope) {
                                 dispatchGroup.leave()
                                 callback(.failure(CredentialsManagerError(code: .storeFailed)))
                             } else {
@@ -893,14 +911,30 @@ public struct CredentialsManager {
         return expiresIn < Date()
     }
 
-    func hasScopeChanged(from lastScope: String?, to newScope: String?) -> Bool {
+    func hasScopeChanged(from lastScope: String?, to newScope: String?, ignoreOpenid: Bool = false) -> Bool {
+
         if let lastScope = lastScope, let newScope = newScope {
-            let lastScopeList = lastScope.lowercased().split(separator: " ").sorted()
-            let newScopeList = newScope.lowercased().split(separator: " ").sorted()
 
-            return lastScopeList != newScopeList
-        }
+            var storedScopes = Set(
+                    lastScope
+                    .split(separator: " ")
+                    .filter { !$0.isEmpty }
+                    .map { String($0).lowercased() }
+            )
 
+            if ignoreOpenid {
+                storedScopes.remove("openid")
+            }
+
+            let requiredScopes = Set(
+                    newScope
+                    .split(separator: " ")
+                    .filter { !$0.isEmpty }
+                    .map { String($0).lowercased() }
+            )
+
+            return storedScopes != requiredScopes
+        }
         return false
     }
 

Auth0Tests/CredentialsManagerSpec.swift

@@ -127,6 +127,79 @@ class CredentialsManagerSpec: QuickSpec {
 
         }
 
+        describe("storage with scoped keys") {
+
+            afterEach {
+                _ = credentialsManager.clear(forAudience: Audience)
+                _ = credentialsManager.clear(forAudience: Audience, scope: Scope)
+                _ = credentialsManager.clear(forAudience: Audience, scope: NewScope)
+                _ = credentialsManager.clear(forAudience: Audience, scope: "read write")
+                _ = credentialsManager.clear(forAudience: Audience, scope: "read")
+            }
+
+            it("should store api credentials without scope using audience as key") {
+                let store = SimpleKeychain()
+                credentialsManager = CredentialsManager(authentication: authentication, storage: store)
+
+                expect(credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience)).to(beTrue())
+                expect(fetchAPICredentials(forAudience: Audience, from: store)).toNot(beNil())
+            }
+
+            it("should store api credentials with scope using compound key") {
+                let store = SimpleKeychain()
+                credentialsManager = CredentialsManager(authentication: authentication, storage: store)
+
+                expect(credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience, forScope: "read write")).to(beTrue())
+              
+                expect(fetchAPICredentials(forAudience: Audience, forScope: "read write", from: store)).toNot(beNil())
+            }
+
+            it("should store credentials for same audience with different scopes separately") {
+                let store = SimpleKeychain()
+                credentialsManager = CredentialsManager(authentication: authentication, storage: store)
+
+                let apiCredentials1 = APICredentials(accessToken: "token1", tokenType: TokenType, expiresIn: Date(timeIntervalSinceNow: ExpiresIn), scope: "read")
+                let apiCredentials2 = APICredentials(accessToken: "token2", tokenType: TokenType, expiresIn: Date(timeIntervalSinceNow: ExpiresIn), scope: "write")
+
+                expect(credentialsManager.store(apiCredentials: apiCredentials1, forAudience: Audience, forScope: "read")).to(beTrue())
+                expect(credentialsManager.store(apiCredentials: apiCredentials2, forAudience: Audience, forScope: "write")).to(beTrue())
+
+                // Both should exist
+                let retrieved1 = fetchAPICredentials(forAudience: Audience, forScope: "read", from: store)
+                let retrieved2 = fetchAPICredentials(forAudience: Audience, forScope: "write", from: store)
+                expect(retrieved1?.accessToken) == "token1"
+                expect(retrieved2?.accessToken) == "token2"
+            }
+
+            it("should clear api credentials only for specified scope") {
+                let store = SimpleKeychain()
+                credentialsManager = CredentialsManager(authentication: authentication, storage: store)
+
+                _ = credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience)
+                _ = credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience, forScope: "read")
+
+                _ = credentialsManager.clear(forAudience: Audience, scope: "read")
+
+                expect(fetchAPICredentials(forAudience: Audience, from: store)).toNot(beNil())
+                
+                expect(fetchAPICredentials(forAudience: Audience, forScope: "read", from: store)).to(beNil())
+            }
+
+            it("should not clear scoped credentials when clearing without scope") {
+                let store = SimpleKeychain()
+                credentialsManager = CredentialsManager(authentication: authentication, storage: store)
+
+                _ = credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience)
+                _ = credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience, forScope: "read")
+
+                _ = credentialsManager.clear(forAudience: Audience)
+
+                expect(fetchAPICredentials(forAudience: Audience, from: store)).to(beNil())
+                expect(fetchAPICredentials(forAudience: Audience, forScope: "read", from: store)).toNot(beNil())
+            }
+
+        }
+
         describe("custom storage") {
 
             class CustomStore: CredentialsStorage {
@@ -1122,7 +1195,7 @@ class CredentialsManagerSpec: QuickSpec {
                                                     tokenType: TokenType,
                                                     expiresIn: Date(timeIntervalSinceNow: ExpiresIn),
                                                     scope: "openid phone")
-                    _ = credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience)
+                    _ = credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience,forScope: "openid phone")
                     waitUntil(timeout: Timeout) { done in
                         credentialsManager.apiCredentials(forAudience: Audience, scope: "openid phone") { result in
                             expect(result).to(haveAPICredentials(AccessToken))
@@ -1140,7 +1213,7 @@ class CredentialsManagerSpec: QuickSpec {
                                                     tokenType: TokenType,
                                                     expiresIn: Date(timeIntervalSinceNow: ExpiresIn),
                                                     scope: "openid phone")
-                    _ = credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience)
+                    _ = credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience, forScope: "openid phone")
                     waitUntil(timeout: Timeout) { done in
                         credentialsManager.apiCredentials(forAudience: Audience, scope: "openid email") { result in
                             expect(result).to(haveAPICredentials(NewAccessToken))
@@ -1185,6 +1258,64 @@ class CredentialsManagerSpec: QuickSpec {
 
             }
 
+            context("retrieval of api credentials with scope") {
+
+                beforeEach {
+                    _ = credentialsManager.store(credentials: credentials)
+                }
+
+                afterEach {
+                    _ = credentialsManager.clear()
+                    _ = credentialsManager.clear(forAudience: Audience)
+                    _ = credentialsManager.clear(forAudience: Audience, scope: Scope)
+                    _ = credentialsManager.clear(forAudience: Audience, scope: "openid phone")
+                    _ = credentialsManager.clear(forAudience: Audience, scope: "different")
+                    _ = credentialsManager.clear(forAudience: Audience, scope: "read write")
+                }
+
+                it("should retrieve api credentials stored with matching scope") {
+                    apiCredentials = APICredentials(accessToken: AccessToken, tokenType: TokenType, expiresIn: Date(timeIntervalSinceNow: ExpiresIn), scope: Scope)
+                    _ = credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience, forScope: Scope)
+
+                    waitUntil(timeout: Timeout) { done in
+                        credentialsManager.apiCredentials(forAudience: Audience, scope: Scope) { result in
+                            expect(result).to(haveAPICredentials(AccessToken))
+                            done()
+                        }
+                    }
+                }
+
+                it("should renew api credentials when scope does not match stored scope") {
+                    NetworkStub.clearStubs()
+                    NetworkStub.addStub(condition: {
+                        $0.isToken(Domain) && $0.hasAtLeast(["refresh_token": RefreshToken, "audience": Audience])
+                    }, response: authResponse(accessToken: NewAccessToken, idToken: NewIdToken, expiresIn: ExpiresIn, scope: "different"))
+
+                    apiCredentials = APICredentials(accessToken: AccessToken, tokenType: TokenType, expiresIn: Date(timeIntervalSinceNow: ExpiresIn), scope: Scope)
+                    _ = credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience, forScope: Scope)
+
+                    waitUntil(timeout: Timeout) { done in
+                        credentialsManager.apiCredentials(forAudience: Audience, scope: "different") { result in
+                            expect(result).to(haveAPICredentials(NewAccessToken))
+                            done()
+                        }
+                    }
+                }
+
+                it("should retrieve api credentials with scopes in different order") {
+                    apiCredentials = APICredentials(accessToken: AccessToken, tokenType: TokenType, expiresIn: Date(timeIntervalSinceNow: ExpiresIn), scope: "read write")
+                    _ = credentialsManager.store(apiCredentials: apiCredentials, forAudience: Audience, forScope: "read write")
+
+                    waitUntil(timeout: Timeout) { done in
+                        credentialsManager.apiCredentials(forAudience: Audience, scope: "write read") { result in
+                            expect(result).to(haveAPICredentials(AccessToken))
+                            done()
+                        }
+                    }
+                }
+
+            }
+
             context("serial exchange for api credentials from same thread") {
 
                 it("should yield the stored api credentials after the previous renewal operation succeeded") {
@@ -2442,7 +2573,14 @@ private func fetchCredentials(from store: CredentialsStorage) -> Credentials? {
     return try? NSKeyedUnarchiver.unarchivedObject(ofClass: Credentials.self, from: data)
 }
 
-private func fetchAPICredentials(forAudience audience: String = Audience, from store: CredentialsStorage) -> APICredentials? {
-    guard let data = store.getEntry(forKey: audience) else { return nil }
+private func fetchAPICredentials(forAudience audience: String = Audience, forScope scope: String? = nil, from store: CredentialsStorage) -> APICredentials? {
+    let key: String
+    if let scope = scope {
+        let normalisedScopes = scope.split(separator: " ").sorted().joined(separator: "::")
+        key = "\(audience)::\(normalisedScopes)"
+    } else {
+        key = audience
+    }
+    guard let data = store.getEntry(forKey: key) else { return nil }
     return try? APICredentials(from: data)
 }