From 55faf550d71281171f8bb557268ddc29224b91b5 Mon Sep 17 00:00:00 2001 From: Kiel Gillard Date: Fri, 15 Nov 2019 19:09:14 +1100 Subject: [PATCH] Implements logout for Feedly accounts. --- .../Account/Account.xcodeproj/project.pbxproj | 8 + .../Feedly/FeedlyLogoutOperationTests.swift | 216 ++++++++++++++++++ .../Feedly/FeedlyTestSupport.swift | 1 + .../Account/Feedly/FeedlyAPICaller.swift | 36 +++ .../Feedly/FeedlyAccountDelegate.swift | 4 +- .../Operations/FeedlyLogoutOperation.swift | 54 +++++ 6 files changed, 318 insertions(+), 1 deletion(-) create mode 100644 Frameworks/Account/AccountTests/Feedly/FeedlyLogoutOperationTests.swift create mode 100644 Frameworks/Account/Feedly/Operations/FeedlyLogoutOperation.swift diff --git a/Frameworks/Account/Account.xcodeproj/project.pbxproj b/Frameworks/Account/Account.xcodeproj/project.pbxproj index 090d9d563..af49c7db9 100644 --- a/Frameworks/Account/Account.xcodeproj/project.pbxproj +++ b/Frameworks/Account/Account.xcodeproj/project.pbxproj @@ -111,6 +111,8 @@ 9E713653233AD63E00765C84 /* FeedlySetUnreadArticlesOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E713652233AD63E00765C84 /* FeedlySetUnreadArticlesOperation.swift */; }; 9E7299D723505E9600DAEFB7 /* FeedlyAddFeedOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E7299D623505E9600DAEFB7 /* FeedlyAddFeedOperation.swift */; }; 9E7299D9235062A200DAEFB7 /* FeedlyResourceProviding.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E7299D8235062A200DAEFB7 /* FeedlyResourceProviding.swift */; }; + 9E784EBE237E890600099B1B /* FeedlyLogoutOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E784EBD237E890600099B1B /* FeedlyLogoutOperation.swift */; }; + 9E784EC0237E8BE100099B1B /* FeedlyLogoutOperationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E784EBF237E8BE100099B1B /* FeedlyLogoutOperationTests.swift */; }; 9E7F88AC235EDDC2009AB9DF /* FeedlyCreateFeedsForCollectionFoldersOperationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E7F88AB235EDDC2009AB9DF /* FeedlyCreateFeedsForCollectionFoldersOperationTests.swift */; }; 9E7F88AE235FBB11009AB9DF /* FeedlyGetStreamContentsOperationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E7F88AD235FBB11009AB9DF /* FeedlyGetStreamContentsOperationTests.swift */; }; 9E84DC472359A23200D6E809 /* FeedlySyncUnreadStatusesOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E84DC462359A23200D6E809 /* FeedlySyncUnreadStatusesOperation.swift */; }; @@ -313,6 +315,8 @@ 9E713652233AD63E00765C84 /* FeedlySetUnreadArticlesOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlySetUnreadArticlesOperation.swift; sourceTree = ""; }; 9E7299D623505E9600DAEFB7 /* FeedlyAddFeedOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyAddFeedOperation.swift; sourceTree = ""; }; 9E7299D8235062A200DAEFB7 /* FeedlyResourceProviding.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyResourceProviding.swift; sourceTree = ""; }; + 9E784EBD237E890600099B1B /* FeedlyLogoutOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyLogoutOperation.swift; sourceTree = ""; }; + 9E784EBF237E8BE100099B1B /* FeedlyLogoutOperationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyLogoutOperationTests.swift; sourceTree = ""; }; 9E7F88AB235EDDC2009AB9DF /* FeedlyCreateFeedsForCollectionFoldersOperationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyCreateFeedsForCollectionFoldersOperationTests.swift; sourceTree = ""; }; 9E7F88AD235FBB11009AB9DF /* FeedlyGetStreamContentsOperationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyGetStreamContentsOperationTests.swift; sourceTree = ""; }; 9E84DC462359A23200D6E809 /* FeedlySyncUnreadStatusesOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlySyncUnreadStatusesOperation.swift; sourceTree = ""; }; @@ -604,6 +608,7 @@ 9EC804E2236C18AB0057CFCB /* FeedlySyncAllMockResponseProvider.swift */, 9E1773DA234593CF0056A5A8 /* FeedlyResourceIdTests.swift */, 9E0260CA236FF99A00D122D3 /* FeedlyRefreshAccessTokenOperationTests.swift */, + 9E784EBF237E8BE100099B1B /* FeedlyLogoutOperationTests.swift */, 9E5ABE99236BE6BC00B5DE9F /* feedly-1-initial */, 9EC804E4236C1A7F0057CFCB /* feedly-2-changestatuses */, 9EC804E6236C1BA60057CFCB /* feedly-3-changestatusesagain */, @@ -658,6 +663,7 @@ 9E84DC462359A23200D6E809 /* FeedlySyncUnreadStatusesOperation.swift */, 9E1D154C233370D800F4944C /* FeedlySyncAllOperation.swift */, 9E672393236F7CA0000BE141 /* FeedlyRefreshAccessTokenOperation.swift */, + 9E784EBD237E890600099B1B /* FeedlyLogoutOperation.swift */, ); path = Operations; sourceTree = ""; @@ -966,6 +972,7 @@ 841974251F6DDCE4006346C4 /* AccountDelegate.swift in Sources */, 510BD113232C3E9D002692E4 /* WebFeedMetadataFile.swift in Sources */, 5165D73122837F3400D9D53D /* InitialFeedDownloader.swift in Sources */, + 9E784EBE237E890600099B1B /* FeedlyLogoutOperation.swift in Sources */, 9EEEF71F23545CB4009E9D80 /* FeedlySendArticleStatusesOperation.swift in Sources */, 846E77541F6F00E300A165E2 /* AccountManager.swift in Sources */, 515E4EB72324FF8C0057B0E7 /* Credentials.swift in Sources */, @@ -1043,6 +1050,7 @@ 9EC228572362C7F900766EF8 /* FeedlyCheckpointOperationTests.swift in Sources */, 9E03C122235E62E100FB6D9E /* FeedlyTestSupport.swift in Sources */, 9E3CFFFD2368202000BA7365 /* FeedlySyncUnreadStatusesOperationTests.swift in Sources */, + 9E784EC0237E8BE100099B1B /* FeedlyLogoutOperationTests.swift in Sources */, 9EC228552362C17F00766EF8 /* FeedlySetStarredArticlesOperationTests.swift in Sources */, 9E03C120235E62A500FB6D9E /* FeedlyMirrorCollectionsAsFoldersOperationTests.swift in Sources */, 9E489E912360ED30004372EE /* FeedlyOrganiseParsedItemsByFeedOperationTests.swift in Sources */, diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlyLogoutOperationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlyLogoutOperationTests.swift new file mode 100644 index 000000000..95c3c3156 --- /dev/null +++ b/Frameworks/Account/AccountTests/Feedly/FeedlyLogoutOperationTests.swift @@ -0,0 +1,216 @@ +// +// FeedlyLogoutOperationTests.swift +// AccountTests +// +// Created by Kiel Gillard on 15/11/19. +// Copyright © 2019 Ranchero Software, LLC. All rights reserved. +// + +import XCTest +@testable import Account + +class FeedlyLogoutOperationTests: XCTestCase { + + private var account: Account! + private let support = FeedlyTestSupport() + + override func setUp() { + super.setUp() + account = support.makeTestAccount() + } + + override func tearDown() { + if let account = account { + support.destroy(account) + } + super.tearDown() + } + + private func getTokens(for account: Account) throws -> (accessToken: Credentials, refreshToken: Credentials) { + guard let accessToken = try account.retrieveCredentials(type: .oauthAccessToken), let refreshToken = try account.retrieveCredentials(type: .oauthRefreshToken) else { + XCTFail("Unable to retrieve access and/or refresh token from account.") + throw CredentialsError.incompleteCredentials + } + return (accessToken, refreshToken) + } + + class TestFeedlyLogoutService: FeedlyLogoutService { + var mockResult: Result? + var logoutExpectation: XCTestExpectation? + + func logout(completionHandler: @escaping (Result) -> ()) { + guard let result = mockResult else { + XCTFail("Missing mock result. Test may time out because the completion will not be called.") + return + } + DispatchQueue.main.async { + completionHandler(result) + self.logoutExpectation?.fulfill() + } + } + } + + func testCancel() { + let service = TestFeedlyLogoutService() + service.logoutExpectation = expectation(description: "Did Call Logout") + service.logoutExpectation?.isInverted = true + + let accessToken: Credentials + let refreshToken: Credentials + do { + (accessToken, refreshToken) = try getTokens(for: account) + } catch { + XCTFail("Could not retrieve credentials to verify their integrity later.") + return + } + + let logout = FeedlyLogoutOperation(account: account, service: service, log: support.log) + + // If this expectation is not fulfilled, the operation is not calling `didFinish`. + let completionExpectation = expectation(description: "Did Finish") + logout.completionBlock = { + completionExpectation.fulfill() + } + + OperationQueue.main.addOperation(logout) + + logout.cancel() + + waitForExpectations(timeout: 1) + + XCTAssertTrue(logout.isCancelled) + XCTAssertTrue(logout.isFinished) + + do { + let accountAccessToken = try account.retrieveCredentials(type: .oauthAccessToken) + let accountRefreshToken = try account.retrieveCredentials(type: .oauthRefreshToken) + + XCTAssertEqual(accountAccessToken, accessToken) + XCTAssertEqual(accountRefreshToken, refreshToken) + } catch { + XCTFail("Could not verify tokens were left intact. Did the operation delete them?") + } + } + + func testLogoutSuccess() { + let service = TestFeedlyLogoutService() + service.logoutExpectation = expectation(description: "Did Call Logout") + service.mockResult = .success(()) + + let logout = FeedlyLogoutOperation(account: account, service: service, log: support.log) + + // If this expectation is not fulfilled, the operation is not calling `didFinish`. + let completionExpectation = expectation(description: "Did Finish") + logout.completionBlock = { + completionExpectation.fulfill() + } + + OperationQueue.main.addOperation(logout) + + waitForExpectations(timeout: 1) + + XCTAssertFalse(logout.isCancelled) + + do { + let accountAccessToken = try account.retrieveCredentials(type: .oauthAccessToken) + let accountRefreshToken = try account.retrieveCredentials(type: .oauthRefreshToken) + + XCTAssertNil(accountAccessToken) + XCTAssertNil(accountRefreshToken) + } catch { + XCTFail("Could not verify tokens were deleted.") + } + } + + class TestLogoutDelegate: FeedlyOperationDelegate { + var error: Error? + var didFailExpectation: XCTestExpectation? + + func feedlyOperation(_ operation: FeedlyOperation, didFailWith error: Error) { + self.error = error + didFailExpectation?.fulfill() + } + } + + func testLogoutMissingAccessToken() { + support.removeCredentials(matching: .oauthAccessToken, from: account) + + let (_, service) = support.makeMockNetworkStack() + service.credentials = nil + + let logout = FeedlyLogoutOperation(account: account, service: service, log: support.log) + + let delegate = TestLogoutDelegate() + delegate.didFailExpectation = expectation(description: "Did Fail") + + logout.delegate = delegate + + // If this expectation is not fulfilled, the operation is not calling `didFinish`. + let completionExpectation = expectation(description: "Did Finish") + logout.completionBlock = { + completionExpectation.fulfill() + } + + OperationQueue.main.addOperation(logout) + + waitForExpectations(timeout: 1) + + XCTAssertFalse(logout.isCancelled) + + do { + let accountAccessToken = try account.retrieveCredentials(type: .oauthAccessToken) + XCTAssertNil(accountAccessToken) + } catch { + XCTFail("Could not verify tokens were deleted.") + } + + XCTAssertNotNil(delegate.error, "Should have failed with error.") + if let error = delegate.error { + switch error { + case CredentialsError.incompleteCredentials: + break + default: + XCTFail("Expected \(CredentialsError.incompleteCredentials)") + } + } + } + + func testLogoutFailure() { + let service = TestFeedlyLogoutService() + service.logoutExpectation = expectation(description: "Did Call Logout") + service.mockResult = .failure(URLError(.timedOut)) + + let accessToken: Credentials + let refreshToken: Credentials + do { + (accessToken, refreshToken) = try getTokens(for: account) + } catch { + XCTFail("Could not retrieve credentials to verify their integrity later.") + return + } + + let logout = FeedlyLogoutOperation(account: account, service: service, log: support.log) + + // If this expectation is not fulfilled, the operation is not calling `didFinish`. + let completionExpectation = expectation(description: "Did Finish") + logout.completionBlock = { + completionExpectation.fulfill() + } + + OperationQueue.main.addOperation(logout) + + waitForExpectations(timeout: 1) + + XCTAssertFalse(logout.isCancelled) + + do { + let accountAccessToken = try account.retrieveCredentials(type: .oauthAccessToken) + let accountRefreshToken = try account.retrieveCredentials(type: .oauthRefreshToken) + + XCTAssertEqual(accountAccessToken, accessToken) + XCTAssertEqual(accountRefreshToken, refreshToken) + } catch { + XCTFail("Could not verify tokens were left intact. Did the operation delete them?") + } + } +} diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlyTestSupport.swift b/Frameworks/Account/AccountTests/Feedly/FeedlyTestSupport.swift index ea1e79f26..afa84b3c2 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlyTestSupport.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlyTestSupport.swift @@ -77,6 +77,7 @@ class FeedlyTestSupport { func destroy(_ testAccount: Account) { do { + // These should not throw when the keychain items are not found. try testAccount.removeCredentials(type: .oauthAccessToken) try testAccount.removeCredentials(type: .oauthRefreshToken) } catch { diff --git a/Frameworks/Account/Feedly/FeedlyAPICaller.swift b/Frameworks/Account/Feedly/FeedlyAPICaller.swift index 38fd68734..29cfa8ff1 100644 --- a/Frameworks/Account/Feedly/FeedlyAPICaller.swift +++ b/Frameworks/Account/Feedly/FeedlyAPICaller.swift @@ -680,3 +680,39 @@ extension FeedlyAPICaller: FeedlyMarkArticlesService { } } } + +extension FeedlyAPICaller: FeedlyLogoutService { + + func logout(completionHandler: @escaping (Result) -> ()) { + guard let accessToken = credentials?.secret else { + return DispatchQueue.main.async { + completionHandler(.failure(CredentialsError.incompleteCredentials)) + } + } + var components = baseUrlComponents + components.path = "/v3/auth/logout" + + guard let url = components.url else { + fatalError("\(components) does not produce a valid URL.") + } + + var request = URLRequest(url: url) + request.httpMethod = "POST" + request.addValue("application/json", forHTTPHeaderField: HTTPRequestHeader.contentType) + request.addValue("application/json", forHTTPHeaderField: "Accept-Type") + request.addValue("OAuth \(accessToken)", forHTTPHeaderField: HTTPRequestHeader.authorization) + + transport.send(request: request, resultType: String.self, dateDecoding: .millisecondsSince1970, keyDecoding: .convertFromSnakeCase) { result in + switch result { + case .success(let (httpResponse, _)): + if httpResponse.statusCode == 200 { + completionHandler(.success(())) + } else { + completionHandler(.failure(URLError(.cannotDecodeContentData))) + } + case .failure(let error): + completionHandler(.failure(error)) + } + } + } +} diff --git a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift index dcca708d0..8f8a0e588 100644 --- a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift @@ -487,7 +487,9 @@ final class FeedlyAccountDelegate: AccountDelegate { } func accountWillBeDeleted(_ account: Account) { - + let logout = FeedlyLogoutOperation(account: account, service: caller, log: log) + // Dispatch on the main queue because the lifetime of the account delegate is uncertain. + OperationQueue.main.addOperation(logout) } static func validateCredentials(transport: Transport, credentials: Credentials, endpoint: URL?, completion: @escaping (Result) -> Void) { diff --git a/Frameworks/Account/Feedly/Operations/FeedlyLogoutOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyLogoutOperation.swift new file mode 100644 index 000000000..53e640799 --- /dev/null +++ b/Frameworks/Account/Feedly/Operations/FeedlyLogoutOperation.swift @@ -0,0 +1,54 @@ +// +// FeedlyLogoutOperation.swift +// Account +// +// Created by Kiel Gillard on 15/11/19. +// Copyright © 2019 Ranchero Software, LLC. All rights reserved. +// + +import Foundation +import os.log + +protocol FeedlyLogoutService { + func logout(completionHandler: @escaping (Result) -> ()) +} + +final class FeedlyLogoutOperation: FeedlyOperation { + let service: FeedlyLogoutService + let account: Account + let log: OSLog + + init(account: Account, service: FeedlyLogoutService, log: OSLog) { + self.service = service + self.account = account + self.log = log + } + + override func main() { + guard !isCancelled else { + didFinish() + return + } + os_log("Requesting logout of %{public}@ account.", "\(account.type)") + service.logout(completionHandler: didCompleteLogout(_:)) + } + + func didCompleteLogout(_ result: Result) { + assert(Thread.isMainThread) + switch result { + case .success: + os_log("Logged out of %{public}@ account.", "\(account.type)") + do { + try account.removeCredentials(type: .oauthAccessToken) + try account.removeCredentials(type: .oauthRefreshToken) + } catch { + // oh well, we tried our best. + } + didFinish() + + case .failure(let error): + os_log("Logout failed because %{public}@.", error as NSError) + didFinish(error) + } + } +}