From d0514a7f60ed516376cceedbb56f24dcaccab056 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Wed, 2 Oct 2019 14:03:14 +0100 Subject: [PATCH 01/13] Added CredentialsManager.clearAndRevokeToken(), superceding .clear() --- .../AppIcon.appiconset/Contents.json | 5 ++ App/Base.lproj/Main.storyboard | 10 ++-- Auth0/CredentialsManager.swift | 26 +++++++++ Auth0/CredentialsManagerError.swift | 1 + Auth0Tests/CredentialsManagerSpec.swift | 58 +++++++++++++++++++ 5 files changed, 95 insertions(+), 5 deletions(-) diff --git a/App/Assets.xcassets/AppIcon.appiconset/Contents.json b/App/Assets.xcassets/AppIcon.appiconset/Contents.json index b8236c65..19882d56 100644 --- a/App/Assets.xcassets/AppIcon.appiconset/Contents.json +++ b/App/Assets.xcassets/AppIcon.appiconset/Contents.json @@ -39,6 +39,11 @@ "idiom" : "iphone", "size" : "60x60", "scale" : "3x" + }, + { + "idiom" : "ios-marketing", + "size" : "1024x1024", + "scale" : "1x" } ], "info" : { diff --git a/App/Base.lproj/Main.storyboard b/App/Base.lproj/Main.storyboard index 527c1b51..ef646f3f 100644 --- a/App/Base.lproj/Main.storyboard +++ b/App/Base.lproj/Main.storyboard @@ -1,10 +1,9 @@ - - - - + + - + + @@ -99,6 +98,7 @@ + diff --git a/Auth0/CredentialsManager.swift b/Auth0/CredentialsManager.swift index 1b024262..82abe671 100644 --- a/Auth0/CredentialsManager.swift +++ b/Auth0/CredentialsManager.swift @@ -88,6 +88,32 @@ public struct CredentialsManager { return self.storage.deleteEntry(forKey: storeKey) } + /// Revokes the refresh token and, if successful, the credentials are cleared. Otherwise, + /// the credentials are not cleared and an error is raised through the callback. + /// + /// - Parameter callback: callback with an error if the refresh token could not be revoked + public func clearAndRevokeToken(_ callback: @escaping (CredentialsManagerError?) -> Void) { + guard + let data = self.storage.data(forKey: self.storeKey), + let credentials = NSKeyedUnarchiver.unarchiveObject(with: data) as? Credentials, + let refreshToken = credentials.refreshToken else { + self.storage.deleteEntry(forKey: self.storeKey) + return callback(nil) + } + + self.authentication + .revoke(refreshToken: refreshToken) + .start { result in + switch result { + case .failure(let error): + callback(CredentialsManagerError.revokeFailed(error)) + default: + self.storage.deleteEntry(forKey: self.storeKey) + callback(nil) + } + } + } + /// Checks if a non-expired set of credentials are stored /// /// - Returns: if there are valid and non-expired credentials stored diff --git a/Auth0/CredentialsManagerError.swift b/Auth0/CredentialsManagerError.swift index 7495c93e..8a78fc3a 100644 --- a/Auth0/CredentialsManagerError.swift +++ b/Auth0/CredentialsManagerError.swift @@ -27,4 +27,5 @@ public enum CredentialsManagerError: Error { case noRefreshToken case failedRefresh(Error) case touchFailed(Error) + case revokeFailed(Error) } diff --git a/Auth0Tests/CredentialsManagerSpec.swift b/Auth0Tests/CredentialsManagerSpec.swift index 78cb99da..10a459c7 100644 --- a/Auth0Tests/CredentialsManagerSpec.swift +++ b/Auth0Tests/CredentialsManagerSpec.swift @@ -73,6 +73,64 @@ class CredentialsManagerSpec: QuickSpec { } } + describe("clearing and revoking refresh token") { + + beforeEach { + _ = credentialsManager.store(credentials: credentials) + + stub(condition: isRevokeToken(Domain) && hasAtLeast(["refresh_token": RefreshToken])) { _ in return revokeTokenResponse() }.name = "revoke success" + } + + afterEach { + _ = credentialsManager.clear() + } + + it("should clear credentials and revoke the refresh token") { + credentialsManager.clearAndRevokeToken { + expect($0).to(beNil()) + expect(credentialsManager.hasValid()).to(beFalse()) + } + } + + it("should not return an error if there were no credentials stored") { + _ = credentialsManager.clear() + + credentialsManager.clearAndRevokeToken { + expect($0).to(beNil()) + expect(credentialsManager.hasValid()).to(beFalse()) + } + } + + it("should not return an error if there is no refresh token") { + let credentials = Credentials( + accessToken: AccessToken, + idToken: IdToken, + expiresIn: Date(timeIntervalSinceNow: ExpiresIn) + ) + + _ = credentialsManager.store(credentials: credentials) + + credentialsManager.clearAndRevokeToken { + expect($0).to(beNil()) + expect(credentialsManager.hasValid()).to(beFalse()) + } + } + + it("should return the failure if the token could not be revoked, and not clear credentials") { + stub(condition: isRevokeToken(Domain) && hasAtLeast(["token": RefreshToken])) { _ in + return authFailure(code: "400", description: "Revoke failed") + } + + credentialsManager.clearAndRevokeToken { + expect($0).to(matchError( + CredentialsManagerError.revokeFailed(AuthenticationError(string: "Revoke failed", statusCode: 400)) + )) + + expect(credentialsManager.hasValid()).to(beTrue()) + } + } + } + describe("multi instances of credentials manager") { var secondaryCredentialsManager: CredentialsManager! From 8702c6ca70cd4ef912724dd8d895b6d3406905dc Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Wed, 2 Oct 2019 14:06:19 +0100 Subject: [PATCH 02/13] Added deprecation notice above the old clear() method --- Auth0/CredentialsManager.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/Auth0/CredentialsManager.swift b/Auth0/CredentialsManager.swift index 82abe671..36d41492 100644 --- a/Auth0/CredentialsManager.swift +++ b/Auth0/CredentialsManager.swift @@ -84,6 +84,7 @@ public struct CredentialsManager { /// Clear credentials stored in keychain /// /// - Returns: if credentials were removed + @available(*, deprecated, message: "use clearAndRevokeToken(callback) instead") public func clear() -> Bool { return self.storage.deleteEntry(forKey: storeKey) } From 8fa03e5aea07eab67fd2935e6937ff8677bcc0f9 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Wed, 2 Oct 2019 14:28:05 +0100 Subject: [PATCH 03/13] Added an example of `clearAndRevokeToken` to the readme --- README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/README.md b/README.md index 94bd504c..abe3d619 100644 --- a/README.md +++ b/README.md @@ -214,6 +214,20 @@ credentialsManager.credentials { error, credentials in } ``` +#### Clearing credentials + +Credentials can be cleared by using the `clearAndRevokeToken` function. This function will attempt to revoke any associated refresh token that has been stored, before clearing the credentials from memory: + +```swift +credentialsManager.clearAndRevokeToken { error in + guard error == nil else { + return print("Failed to clear credentials") + } + + print("Credentials cleared") +} +``` + #### Biometric authentication You can enable an additional level of user authentication before retrieving credentials using the biometric authentication supported by your device e.g. Face ID or Touch ID. From 1632964f910221fc69a3fca753b469f15b505ec5 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Fri, 4 Oct 2019 11:18:53 +0100 Subject: [PATCH 04/13] Fixed unit tests They needed a sprinkling of `waitUntil` blocks so that the tests didn't trip over themselves with clearing credentials asynchronously while other tests were running. --- Auth0Tests/CredentialsManagerSpec.swift | 89 ++++++++++++++----------- 1 file changed, 49 insertions(+), 40 deletions(-) diff --git a/Auth0Tests/CredentialsManagerSpec.swift b/Auth0Tests/CredentialsManagerSpec.swift index 10a459c7..4a1750aa 100644 --- a/Auth0Tests/CredentialsManagerSpec.swift +++ b/Auth0Tests/CredentialsManagerSpec.swift @@ -50,13 +50,17 @@ class CredentialsManagerSpec: QuickSpec { beforeEach { credentialsManager = CredentialsManager(authentication: authentication) + credentials = Credentials(accessToken: AccessToken, tokenType: TokenType, idToken: IdToken, refreshToken: RefreshToken, expiresIn: Date(timeIntervalSinceNow: ExpiresIn)) + + stub(condition: isRevokeToken(Domain) && hasAtLeast(["token": RefreshToken])) { _ in return revokeTokenResponse() }.name = "revoke success" } describe("storage") { - afterEach { - _ = credentialsManager.clear() + waitUntil { done in + credentialsManager.clearAndRevokeToken { _ in done() } + } } it("should store credentials in keychain") { @@ -65,39 +69,28 @@ class CredentialsManagerSpec: QuickSpec { it("should clear credentials in keychain") { expect(credentialsManager.store(credentials: credentials)).to(beTrue()) - expect(credentialsManager.clear()).to(beTrue()) - } - - it("should fail to clear credentials") { - expect(credentialsManager.clear()).to(beFalse()) + + waitUntil { done in + credentialsManager.clearAndRevokeToken { error in + expect(error).to(beNil()) + done() + } + } } } describe("clearing and revoking refresh token") { - beforeEach { _ = credentialsManager.store(credentials: credentials) - - stub(condition: isRevokeToken(Domain) && hasAtLeast(["refresh_token": RefreshToken])) { _ in return revokeTokenResponse() }.name = "revoke success" - } - - afterEach { - _ = credentialsManager.clear() } it("should clear credentials and revoke the refresh token") { - credentialsManager.clearAndRevokeToken { - expect($0).to(beNil()) - expect(credentialsManager.hasValid()).to(beFalse()) - } - } - - it("should not return an error if there were no credentials stored") { - _ = credentialsManager.clear() - - credentialsManager.clearAndRevokeToken { - expect($0).to(beNil()) - expect(credentialsManager.hasValid()).to(beFalse()) + waitUntil { done in + credentialsManager.clearAndRevokeToken { + expect($0).to(beNil()) + expect(credentialsManager.hasValid()).to(beFalse()) + done() + } } } @@ -110,9 +103,12 @@ class CredentialsManagerSpec: QuickSpec { _ = credentialsManager.store(credentials: credentials) - credentialsManager.clearAndRevokeToken { - expect($0).to(beNil()) - expect(credentialsManager.hasValid()).to(beFalse()) + waitUntil { done in + credentialsManager.clearAndRevokeToken { + expect($0).to(beNil()) + expect(credentialsManager.hasValid()).to(beFalse()) + done() + } } } @@ -121,12 +117,15 @@ class CredentialsManagerSpec: QuickSpec { return authFailure(code: "400", description: "Revoke failed") } - credentialsManager.clearAndRevokeToken { - expect($0).to(matchError( - CredentialsManagerError.revokeFailed(AuthenticationError(string: "Revoke failed", statusCode: 400)) - )) - - expect(credentialsManager.hasValid()).to(beTrue()) + waitUntil { done in + credentialsManager.clearAndRevokeToken { + expect($0).to(matchError( + CredentialsManagerError.revokeFailed(AuthenticationError(string: "Revoke failed", statusCode: 400)) + )) + + expect(credentialsManager.hasValid()).to(beTrue()) + done() + } } } } @@ -201,8 +200,11 @@ class CredentialsManagerSpec: QuickSpec { } afterEach { - _ = credentialsManager.clear() - _ = secondaryCredentialsManager.clear() + waitUntil { done in + credentialsManager.clearAndRevokeToken { _ in + secondaryCredentialsManager.clearAndRevokeToken { _ in done() } + } + } } } @@ -210,7 +212,9 @@ class CredentialsManagerSpec: QuickSpec { describe("valididity") { afterEach { - _ = credentialsManager.clear() + waitUntil { done in + credentialsManager.clearAndRevokeToken { _ in done() } + } } it("should have valid credentials when stored and not expired") { @@ -370,8 +374,13 @@ class CredentialsManagerSpec: QuickSpec { it("custom keychain should successfully set and clear credentials") { _ = credentialsManager.store(credentials: credentials) expect(storage.data(forKey: "credentials")).toNot(beNil()) - _ = credentialsManager.clear() - expect(storage.data(forKey: "credentials")).to(beNil()) + + waitUntil { done in + credentialsManager.clearAndRevokeToken { _ in + expect(storage.data(forKey: "credentials")).to(beNil()) + done() + } + } } } } From 351f2d2abf636f2307d8405f454bd19e64e78942 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Fri, 4 Oct 2019 11:26:53 +0100 Subject: [PATCH 05/13] Reverted changes to icons and storyboard --- App/Assets.xcassets/AppIcon.appiconset/Contents.json | 5 ----- App/Base.lproj/Main.storyboard | 10 +++++----- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/App/Assets.xcassets/AppIcon.appiconset/Contents.json b/App/Assets.xcassets/AppIcon.appiconset/Contents.json index 19882d56..b8236c65 100644 --- a/App/Assets.xcassets/AppIcon.appiconset/Contents.json +++ b/App/Assets.xcassets/AppIcon.appiconset/Contents.json @@ -39,11 +39,6 @@ "idiom" : "iphone", "size" : "60x60", "scale" : "3x" - }, - { - "idiom" : "ios-marketing", - "size" : "1024x1024", - "scale" : "1x" } ], "info" : { diff --git a/App/Base.lproj/Main.storyboard b/App/Base.lproj/Main.storyboard index ef646f3f..527c1b51 100644 --- a/App/Base.lproj/Main.storyboard +++ b/App/Base.lproj/Main.storyboard @@ -1,9 +1,10 @@ - - + + + + - - + @@ -98,7 +99,6 @@ - From 3c58e88587ce4e43ec28e6e6553a243d9c44f266 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Fri, 4 Oct 2019 11:43:36 +0100 Subject: [PATCH 06/13] Removed the default case in favour of .success --- Auth0/CredentialsManager.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Auth0/CredentialsManager.swift b/Auth0/CredentialsManager.swift index 36d41492..a3abdd40 100644 --- a/Auth0/CredentialsManager.swift +++ b/Auth0/CredentialsManager.swift @@ -108,7 +108,7 @@ public struct CredentialsManager { switch result { case .failure(let error): callback(CredentialsManagerError.revokeFailed(error)) - default: + case .success: self.storage.deleteEntry(forKey: self.storeKey) callback(nil) } From aa63489029a570c03845d4f51531b7869df5315e Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Mon, 7 Oct 2019 11:43:21 +0100 Subject: [PATCH 07/13] Bumped timeout values inline with other waitUntil calls May fix timeout issue when running in CI environment --- Auth0Tests/CredentialsManagerSpec.swift | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Auth0Tests/CredentialsManagerSpec.swift b/Auth0Tests/CredentialsManagerSpec.swift index 4a1750aa..88a7dc93 100644 --- a/Auth0Tests/CredentialsManagerSpec.swift +++ b/Auth0Tests/CredentialsManagerSpec.swift @@ -58,7 +58,7 @@ class CredentialsManagerSpec: QuickSpec { describe("storage") { afterEach { - waitUntil { done in + waitUntil(timeout: 2) { done in credentialsManager.clearAndRevokeToken { _ in done() } } } @@ -70,7 +70,7 @@ class CredentialsManagerSpec: QuickSpec { it("should clear credentials in keychain") { expect(credentialsManager.store(credentials: credentials)).to(beTrue()) - waitUntil { done in + waitUntil(timeout: 2) { done in credentialsManager.clearAndRevokeToken { error in expect(error).to(beNil()) done() @@ -85,7 +85,7 @@ class CredentialsManagerSpec: QuickSpec { } it("should clear credentials and revoke the refresh token") { - waitUntil { done in + waitUntil(timeout: 2) { done in credentialsManager.clearAndRevokeToken { expect($0).to(beNil()) expect(credentialsManager.hasValid()).to(beFalse()) @@ -103,7 +103,7 @@ class CredentialsManagerSpec: QuickSpec { _ = credentialsManager.store(credentials: credentials) - waitUntil { done in + waitUntil(timeout: 2) { done in credentialsManager.clearAndRevokeToken { expect($0).to(beNil()) expect(credentialsManager.hasValid()).to(beFalse()) @@ -117,7 +117,7 @@ class CredentialsManagerSpec: QuickSpec { return authFailure(code: "400", description: "Revoke failed") } - waitUntil { done in + waitUntil(timeout: 2) { done in credentialsManager.clearAndRevokeToken { expect($0).to(matchError( CredentialsManagerError.revokeFailed(AuthenticationError(string: "Revoke failed", statusCode: 400)) @@ -200,7 +200,7 @@ class CredentialsManagerSpec: QuickSpec { } afterEach { - waitUntil { done in + waitUntil(timeout: 2) { done in credentialsManager.clearAndRevokeToken { _ in secondaryCredentialsManager.clearAndRevokeToken { _ in done() } } @@ -212,7 +212,7 @@ class CredentialsManagerSpec: QuickSpec { describe("valididity") { afterEach { - waitUntil { done in + waitUntil(timeout: 2) { done in credentialsManager.clearAndRevokeToken { _ in done() } } } @@ -375,7 +375,7 @@ class CredentialsManagerSpec: QuickSpec { _ = credentialsManager.store(credentials: credentials) expect(storage.data(forKey: "credentials")).toNot(beNil()) - waitUntil { done in + waitUntil(timeout: 2) { done in credentialsManager.clearAndRevokeToken { _ in expect(storage.data(forKey: "credentials")).to(beNil()) done() From f4df6576f53928125b8fa488d9b230705e493ddd Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Mon, 7 Oct 2019 11:51:56 +0100 Subject: [PATCH 08/13] Bumped timeout for clearAndRevoke for multi manager tests --- Auth0Tests/CredentialsManagerSpec.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Auth0Tests/CredentialsManagerSpec.swift b/Auth0Tests/CredentialsManagerSpec.swift index 88a7dc93..2a2a026e 100644 --- a/Auth0Tests/CredentialsManagerSpec.swift +++ b/Auth0Tests/CredentialsManagerSpec.swift @@ -200,7 +200,7 @@ class CredentialsManagerSpec: QuickSpec { } afterEach { - waitUntil(timeout: 2) { done in + waitUntil(timeout: 4) { done in credentialsManager.clearAndRevokeToken { _ in secondaryCredentialsManager.clearAndRevokeToken { _ in done() } } From d4a90e0672f00a643ffc7f5092f4c89370e3e639 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Mon, 7 Oct 2019 12:31:54 +0100 Subject: [PATCH 09/13] Reversed 'clear' deprecation, reverted tests * Tests have been reverted to a previous state where they used `clear` to remove credentials between tests, now that `clear` is no longer depcreated * Appropriate use of `waitUntil` has been restored into the tests so that they run properly --- Auth0/CredentialsManager.swift | 5 +-- Auth0Tests/CredentialsManagerSpec.swift | 59 +++++++++++++------------ 2 files changed, 33 insertions(+), 31 deletions(-) diff --git a/Auth0/CredentialsManager.swift b/Auth0/CredentialsManager.swift index a3abdd40..b0e3bad4 100644 --- a/Auth0/CredentialsManager.swift +++ b/Auth0/CredentialsManager.swift @@ -84,7 +84,6 @@ public struct CredentialsManager { /// Clear credentials stored in keychain /// /// - Returns: if credentials were removed - @available(*, deprecated, message: "use clearAndRevokeToken(callback) instead") public func clear() -> Bool { return self.storage.deleteEntry(forKey: storeKey) } @@ -98,7 +97,7 @@ public struct CredentialsManager { let data = self.storage.data(forKey: self.storeKey), let credentials = NSKeyedUnarchiver.unarchiveObject(with: data) as? Credentials, let refreshToken = credentials.refreshToken else { - self.storage.deleteEntry(forKey: self.storeKey) + _ = self.clear() return callback(nil) } @@ -109,7 +108,7 @@ public struct CredentialsManager { case .failure(let error): callback(CredentialsManagerError.revokeFailed(error)) case .success: - self.storage.deleteEntry(forKey: self.storeKey) + _ = self.clear() callback(nil) } } diff --git a/Auth0Tests/CredentialsManagerSpec.swift b/Auth0Tests/CredentialsManagerSpec.swift index 2a2a026e..5b7e06e5 100644 --- a/Auth0Tests/CredentialsManagerSpec.swift +++ b/Auth0Tests/CredentialsManagerSpec.swift @@ -50,17 +50,13 @@ class CredentialsManagerSpec: QuickSpec { beforeEach { credentialsManager = CredentialsManager(authentication: authentication) - credentials = Credentials(accessToken: AccessToken, tokenType: TokenType, idToken: IdToken, refreshToken: RefreshToken, expiresIn: Date(timeIntervalSinceNow: ExpiresIn)) - - stub(condition: isRevokeToken(Domain) && hasAtLeast(["token": RefreshToken])) { _ in return revokeTokenResponse() }.name = "revoke success" } describe("storage") { + afterEach { - waitUntil(timeout: 2) { done in - credentialsManager.clearAndRevokeToken { _ in done() } - } + _ = credentialsManager.clear() } it("should store credentials in keychain") { @@ -69,19 +65,24 @@ class CredentialsManagerSpec: QuickSpec { it("should clear credentials in keychain") { expect(credentialsManager.store(credentials: credentials)).to(beTrue()) - - waitUntil(timeout: 2) { done in - credentialsManager.clearAndRevokeToken { error in - expect(error).to(beNil()) - done() - } - } + expect(credentialsManager.clear()).to(beTrue()) + } + + it("should fail to clear credentials") { + expect(credentialsManager.clear()).to(beFalse()) } } describe("clearing and revoking refresh token") { + beforeEach { _ = credentialsManager.store(credentials: credentials) + + stub(condition: isRevokeToken(Domain) && hasAtLeast(["token": RefreshToken])) { _ in return revokeTokenResponse() }.name = "revoke success" + } + + afterEach { + _ = credentialsManager.clear() } it("should clear credentials and revoke the refresh token") { @@ -94,6 +95,18 @@ class CredentialsManagerSpec: QuickSpec { } } + it("should not return an error if there were no credentials stored") { + _ = credentialsManager.clear() + + waitUntil(timeout: 2) { done in + credentialsManager.clearAndRevokeToken { + expect($0).to(beNil()) + expect(credentialsManager.hasValid()).to(beFalse()) + done() + } + } + } + it("should not return an error if there is no refresh token") { let credentials = Credentials( accessToken: AccessToken, @@ -200,11 +213,8 @@ class CredentialsManagerSpec: QuickSpec { } afterEach { - waitUntil(timeout: 4) { done in - credentialsManager.clearAndRevokeToken { _ in - secondaryCredentialsManager.clearAndRevokeToken { _ in done() } - } - } + _ = credentialsManager.clear() + _ = secondaryCredentialsManager.clear() } } @@ -212,9 +222,7 @@ class CredentialsManagerSpec: QuickSpec { describe("valididity") { afterEach { - waitUntil(timeout: 2) { done in - credentialsManager.clearAndRevokeToken { _ in done() } - } + _ = credentialsManager.clear() } it("should have valid credentials when stored and not expired") { @@ -374,13 +382,8 @@ class CredentialsManagerSpec: QuickSpec { it("custom keychain should successfully set and clear credentials") { _ = credentialsManager.store(credentials: credentials) expect(storage.data(forKey: "credentials")).toNot(beNil()) - - waitUntil(timeout: 2) { done in - credentialsManager.clearAndRevokeToken { _ in - expect(storage.data(forKey: "credentials")).to(beNil()) - done() - } - } + _ = credentialsManager.clear() + expect(storage.data(forKey: "credentials")).to(beNil()) } } } From 9d65c7f6538fbcd984f5e2557e21444fbbe3935c Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Mon, 7 Oct 2019 12:41:03 +0100 Subject: [PATCH 10/13] Renamed clearAndRevokeToken to revoke, updated README --- Auth0/CredentialsManager.swift | 2 +- Auth0Tests/CredentialsManagerSpec.swift | 8 ++++---- README.md | 14 +++++++++++--- 3 files changed, 16 insertions(+), 8 deletions(-) diff --git a/Auth0/CredentialsManager.swift b/Auth0/CredentialsManager.swift index b0e3bad4..13bb6095 100644 --- a/Auth0/CredentialsManager.swift +++ b/Auth0/CredentialsManager.swift @@ -92,7 +92,7 @@ public struct CredentialsManager { /// the credentials are not cleared and an error is raised through the callback. /// /// - Parameter callback: callback with an error if the refresh token could not be revoked - public func clearAndRevokeToken(_ callback: @escaping (CredentialsManagerError?) -> Void) { + public func revoke(_ callback: @escaping (CredentialsManagerError?) -> Void) { guard let data = self.storage.data(forKey: self.storeKey), let credentials = NSKeyedUnarchiver.unarchiveObject(with: data) as? Credentials, diff --git a/Auth0Tests/CredentialsManagerSpec.swift b/Auth0Tests/CredentialsManagerSpec.swift index 5b7e06e5..0166d29b 100644 --- a/Auth0Tests/CredentialsManagerSpec.swift +++ b/Auth0Tests/CredentialsManagerSpec.swift @@ -87,7 +87,7 @@ class CredentialsManagerSpec: QuickSpec { it("should clear credentials and revoke the refresh token") { waitUntil(timeout: 2) { done in - credentialsManager.clearAndRevokeToken { + credentialsManager.revoke { expect($0).to(beNil()) expect(credentialsManager.hasValid()).to(beFalse()) done() @@ -99,7 +99,7 @@ class CredentialsManagerSpec: QuickSpec { _ = credentialsManager.clear() waitUntil(timeout: 2) { done in - credentialsManager.clearAndRevokeToken { + credentialsManager.revoke { expect($0).to(beNil()) expect(credentialsManager.hasValid()).to(beFalse()) done() @@ -117,7 +117,7 @@ class CredentialsManagerSpec: QuickSpec { _ = credentialsManager.store(credentials: credentials) waitUntil(timeout: 2) { done in - credentialsManager.clearAndRevokeToken { + credentialsManager.revoke { expect($0).to(beNil()) expect(credentialsManager.hasValid()).to(beFalse()) done() @@ -131,7 +131,7 @@ class CredentialsManagerSpec: QuickSpec { } waitUntil(timeout: 2) { done in - credentialsManager.clearAndRevokeToken { + credentialsManager.revoke { expect($0).to(matchError( CredentialsManagerError.revokeFailed(AuthenticationError(string: "Revoke failed", statusCode: 400)) )) diff --git a/README.md b/README.md index abe3d619..45f769e9 100644 --- a/README.md +++ b/README.md @@ -214,12 +214,20 @@ credentialsManager.credentials { error, credentials in } ``` -#### Clearing credentials +#### Clearing credentials and revoking refresh tokens -Credentials can be cleared by using the `clearAndRevokeToken` function. This function will attempt to revoke any associated refresh token that has been stored, before clearing the credentials from memory: +Credentials can be cleared by using the `clear` function, which clears credentials from memory: ```swift -credentialsManager.clearAndRevokeToken { error in +let didClear = credentialsManager.clear() +``` + +In addition, credentials can be cleared and the refresh token revoked using a single call to `revoke`. This function will attempt to revoke any associated refresh token that has been stored, before clearing the credentials from memory. If revoking the token results in an error, then the credentials are not cleared. + +This method is asynchronous, so a callback should be specified to handle the result: + +```swift +credentialsManager.revoke { error in guard error == nil else { return print("Failed to clear credentials") } From 4a93f1a5bbf1afce9cec2b0ee7b42fca097ec591 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Wed, 9 Oct 2019 09:41:20 +0100 Subject: [PATCH 11/13] Applied readme changes based on review feedback --- README.md | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index 45f769e9..004f1408 100644 --- a/README.md +++ b/README.md @@ -216,23 +216,21 @@ credentialsManager.credentials { error, credentials in #### Clearing credentials and revoking refresh tokens -Credentials can be cleared by using the `clear` function, which clears credentials from memory: +Credentials can be cleared by using the `clear` function, which clears credentials from the keychain: ```swift let didClear = credentialsManager.clear() ``` -In addition, credentials can be cleared and the refresh token revoked using a single call to `revoke`. This function will attempt to revoke any associated refresh token that has been stored, before clearing the credentials from memory. If revoking the token results in an error, then the credentials are not cleared. - -This method is asynchronous, so a callback should be specified to handle the result: +In addition, credentials can be cleared and the refresh token revoked using a single call to `revoke`. This function will attempt to revoke the current refresh token stored by the credential manager and then clear credentials from the keychain. If revoking the token results in an error, then the credentials are not cleared: ```swift credentialsManager.revoke { error in guard error == nil else { - return print("Failed to clear credentials") + return print("Failed to revoke refresh token: \(error)") } - print("Credentials cleared") + print("Success") } ``` From e1b8eb90fd6e6a6a66cdbeddd4aa735915fee9f6 Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Thu, 10 Oct 2019 12:45:07 +0100 Subject: [PATCH 12/13] Added more detail to doc comments for revoke method --- Auth0/CredentialsManager.swift | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Auth0/CredentialsManager.swift b/Auth0/CredentialsManager.swift index 13bb6095..a4eaae2c 100644 --- a/Auth0/CredentialsManager.swift +++ b/Auth0/CredentialsManager.swift @@ -88,9 +88,11 @@ public struct CredentialsManager { return self.storage.deleteEntry(forKey: storeKey) } - /// Revokes the refresh token and, if successful, the credentials are cleared. Otherwise, + /// Calls the revoke token endpoint to revoke the refresh token and, if successful, the credentials are cleared. Otherwise, /// the credentials are not cleared and an error is raised through the callback. /// + /// If no refresh token is available the endpoint is not called, the credentials are cleared, and the callback is invoked without an error. + /// /// - Parameter callback: callback with an error if the refresh token could not be revoked public func revoke(_ callback: @escaping (CredentialsManagerError?) -> Void) { guard From 6538abfd349b6ae7976520ddbc01f6fbf564382a Mon Sep 17 00:00:00 2001 From: Steve Hobbs Date: Thu, 10 Oct 2019 12:46:23 +0100 Subject: [PATCH 13/13] Reworded test spec for clarity --- Auth0Tests/CredentialsManagerSpec.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Auth0Tests/CredentialsManagerSpec.swift b/Auth0Tests/CredentialsManagerSpec.swift index 0166d29b..7a1d611f 100644 --- a/Auth0Tests/CredentialsManagerSpec.swift +++ b/Auth0Tests/CredentialsManagerSpec.swift @@ -107,7 +107,7 @@ class CredentialsManagerSpec: QuickSpec { } } - it("should not return an error if there is no refresh token") { + it("should not return an error if there is no refresh token, and clear credentials anyway") { let credentials = Credentials( accessToken: AccessToken, idToken: IdToken,