diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index fa49bd0e6..f40d48c43 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -225,7 +225,7 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, case .freshRSS: self.delegate = ReaderAPIAccountDelegate(dataFolder: dataFolder, transport: transport) case .feedly: - self.delegate = FeedlyAccountDelegate(dataFolder: dataFolder, transport: transport) + self.delegate = FeedlyAccountDelegate(dataFolder: dataFolder, transport: transport, api: FeedlyAccountDelegate.environment) default: return nil } @@ -308,6 +308,18 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, } } + public static func oauthAuthorizationClient(for type: AccountType) -> OAuthAuthorizationClient { + let grantingType: OAuthAuthorizationGranting.Type + switch type { + case .feedly: + grantingType = FeedlyAccountDelegate.self + default: + fatalError("\(type) does not support OAuth authorization code granting.") + } + + return grantingType.oauthAuthorizationClient + } + public static func oauthAuthorizationCodeGrantRequest(for type: AccountType, client: OAuthAuthorizationClient) -> URLRequest { let grantingType: OAuthAuthorizationGranting.Type switch type { diff --git a/Frameworks/Account/Account.xcodeproj/project.pbxproj b/Frameworks/Account/Account.xcodeproj/project.pbxproj index 7a9a6a0da..e15262ed9 100644 --- a/Frameworks/Account/Account.xcodeproj/project.pbxproj +++ b/Frameworks/Account/Account.xcodeproj/project.pbxproj @@ -76,6 +76,7 @@ 84EAC4822148CC6300F154AB /* RSDatabase.framework in Frameworks */ = {isa = PBXBuildFile; fileRef = 84EAC4812148CC6300F154AB /* RSDatabase.framework */; }; 84F1F06E2243524700DA0616 /* AccountMetadata.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84AF4EA3222CFDD100F6A800 /* AccountMetadata.swift */; }; 84F73CF1202788D90000BCEF /* ArticleFetcher.swift in Sources */ = {isa = PBXBuildFile; fileRef = 84F73CF0202788D80000BCEF /* ArticleFetcher.swift */; }; + 9E0260CB236FF99A00D122D3 /* FeedlyRefreshAccessTokenOperationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E0260CA236FF99A00D122D3 /* FeedlyRefreshAccessTokenOperationTests.swift */; }; 9E03C11C235D921400FB6D9E /* FeedlyOperationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E03C11B235D921400FB6D9E /* FeedlyOperationTests.swift */; }; 9E03C11E235D976500FB6D9E /* FeedlyGetCollectionsOperationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E03C11D235D976500FB6D9E /* FeedlyGetCollectionsOperationTests.swift */; }; 9E03C120235E62A500FB6D9E /* FeedlyMirrorCollectionsAsFoldersOperationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E03C11F235E62A500FB6D9E /* FeedlyMirrorCollectionsAsFoldersOperationTests.swift */; }; @@ -107,6 +108,8 @@ 9E489E93236101FC004372EE /* FeedlyUpdateAccountFeedsWithItemsOperationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E489E92236101FC004372EE /* FeedlyUpdateAccountFeedsWithItemsOperationTests.swift */; }; 9E510D6E234F16A8002E6F1A /* FeedlyAddFeedRequest.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E510D6D234F16A8002E6F1A /* FeedlyAddFeedRequest.swift */; }; 9E5ABE9A236BE6BD00B5DE9F /* feedly-1-initial in Resources */ = {isa = PBXBuildFile; fileRef = 9E5ABE99236BE6BC00B5DE9F /* feedly-1-initial */; }; + 9E672394236F7CA0000BE141 /* FeedlyRefreshAccessTokenOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E672393236F7CA0000BE141 /* FeedlyRefreshAccessTokenOperation.swift */; }; + 9E672396236F7E68000BE141 /* OAuthAcessTokenRefreshing.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9E672395236F7E68000BE141 /* OAuthAcessTokenRefreshing.swift */; }; 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 */; }; @@ -275,6 +278,7 @@ 84D09622217418DC00D77525 /* FeedbinTagging.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedbinTagging.swift; sourceTree = ""; }; 84EAC4812148CC6300F154AB /* RSDatabase.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; path = RSDatabase.framework; sourceTree = BUILT_PRODUCTS_DIR; }; 84F73CF0202788D80000BCEF /* ArticleFetcher.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ArticleFetcher.swift; sourceTree = ""; }; + 9E0260CA236FF99A00D122D3 /* FeedlyRefreshAccessTokenOperationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyRefreshAccessTokenOperationTests.swift; sourceTree = ""; }; 9E03C11B235D921400FB6D9E /* FeedlyOperationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyOperationTests.swift; sourceTree = ""; }; 9E03C11D235D976500FB6D9E /* FeedlyGetCollectionsOperationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyGetCollectionsOperationTests.swift; sourceTree = ""; }; 9E03C11F235E62A500FB6D9E /* FeedlyMirrorCollectionsAsFoldersOperationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyMirrorCollectionsAsFoldersOperationTests.swift; sourceTree = ""; }; @@ -306,6 +310,8 @@ 9E489E92236101FC004372EE /* FeedlyUpdateAccountFeedsWithItemsOperationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyUpdateAccountFeedsWithItemsOperationTests.swift; sourceTree = ""; }; 9E510D6D234F16A8002E6F1A /* FeedlyAddFeedRequest.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyAddFeedRequest.swift; sourceTree = ""; }; 9E5ABE99236BE6BC00B5DE9F /* feedly-1-initial */ = {isa = PBXFileReference; lastKnownFileType = folder; path = "feedly-1-initial"; sourceTree = ""; }; + 9E672393236F7CA0000BE141 /* FeedlyRefreshAccessTokenOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FeedlyRefreshAccessTokenOperation.swift; sourceTree = ""; }; + 9E672395236F7E68000BE141 /* OAuthAcessTokenRefreshing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OAuthAcessTokenRefreshing.swift; sourceTree = ""; }; 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 = ""; }; @@ -599,6 +605,7 @@ 9E1FF8632368EC2400834C24 /* FeedlySyncAllOperationTests.swift */, 9EC804E2236C18AB0057CFCB /* FeedlySyncAllMockResponseProvider.swift */, 9E1773DA234593CF0056A5A8 /* FeedlyResourceIdTests.swift */, + 9E0260CA236FF99A00D122D3 /* FeedlyRefreshAccessTokenOperationTests.swift */, 9E5ABE99236BE6BC00B5DE9F /* feedly-1-initial */, 9EC804E4236C1A7F0057CFCB /* feedly-2-changestatuses */, 9EC804E6236C1BA60057CFCB /* feedly-3-changestatusesagain */, @@ -616,6 +623,7 @@ 9ECC9A84234DC16E009B5144 /* FeedlyAccountDelegateError.swift */, 9EC688EB232C583300A8D0A2 /* FeedlyAccountDelegate+OAuth.swift */, 9EC688ED232C58E800A8D0A2 /* OAuthAuthorizationCodeGranting.swift */, + 9E672395236F7E68000BE141 /* OAuthAcessTokenRefreshing.swift */, 9EC688E9232B973C00A8D0A2 /* FeedlyAPICaller.swift */, 9E510D6D234F16A8002E6F1A /* FeedlyAddFeedRequest.swift */, 9E7299D8235062A200DAEFB7 /* FeedlyResourceProviding.swift */, @@ -649,6 +657,7 @@ 9EEEF7202355277F009E9D80 /* FeedlySyncStarredArticlesOperation.swift */, 9E84DC462359A23200D6E809 /* FeedlySyncUnreadStatusesOperation.swift */, 9E1D154C233370D800F4944C /* FeedlySyncAllOperation.swift */, + 9E672393236F7CA0000BE141 /* FeedlyRefreshAccessTokenOperation.swift */, ); path = Operations; sourceTree = ""; @@ -901,6 +910,7 @@ 9EC688EE232C58E800A8D0A2 /* OAuthAuthorizationCodeGranting.swift in Sources */, 9EEAE071235D019B00E3FEE4 /* FeedlyGetStreamContentsService.swift in Sources */, 9E7299D9235062A200DAEFB7 /* FeedlyResourceProviding.swift in Sources */, + 9E672394236F7CA0000BE141 /* FeedlyRefreshAccessTokenOperation.swift in Sources */, 9EC688EC232C583300A8D0A2 /* FeedlyAccountDelegate+OAuth.swift in Sources */, 8469F81C1F6DD15E0084783E /* Account.swift in Sources */, 9EAEC60E2332FEC20085D7C9 /* FeedlyFeed.swift in Sources */, @@ -933,6 +943,7 @@ 9E1D15512334282100F4944C /* FeedlyMirrorCollectionsAsFoldersOperation.swift in Sources */, 9E1773D7234575AB0056A5A8 /* FeedlyTag.swift in Sources */, 515E4EB62324FF8C0057B0E7 /* URLRequest+RSWeb.swift in Sources */, + 9E672396236F7E68000BE141 /* OAuthAcessTokenRefreshing.swift in Sources */, 9E7299D723505E9600DAEFB7 /* FeedlyAddFeedOperation.swift in Sources */, 9EEAE075235D01C400E3FEE4 /* FeedlyMarkArticlesService.swift in Sources */, 9EF1B10323584B4C000A486A /* FeedlySyncStreamContentsOperation.swift in Sources */, @@ -997,6 +1008,7 @@ 9EC228552362C17F00766EF8 /* FeedlySetStarredArticlesOperationTests.swift in Sources */, 9E03C120235E62A500FB6D9E /* FeedlyMirrorCollectionsAsFoldersOperationTests.swift in Sources */, 9E489E912360ED30004372EE /* FeedlyOrganiseParsedItemsByFeedOperationTests.swift in Sources */, + 9E0260CB236FF99A00D122D3 /* FeedlyRefreshAccessTokenOperationTests.swift in Sources */, 9E1FF8622368219B00834C24 /* TestGetPagedStreamIdsService.swift in Sources */, 9E7F88AC235EDDC2009AB9DF /* FeedlyCreateFeedsForCollectionFoldersOperationTests.swift in Sources */, 9E03C11E235D976500FB6D9E /* FeedlyGetCollectionsOperationTests.swift in Sources */, diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlyRefreshAccessTokenOperationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlyRefreshAccessTokenOperationTests.swift new file mode 100644 index 000000000..3e1e5f07e --- /dev/null +++ b/Frameworks/Account/AccountTests/Feedly/FeedlyRefreshAccessTokenOperationTests.swift @@ -0,0 +1,217 @@ +// +// FeedlyRefreshAccessTokenOperationTests.swift +// AccountTests +// +// Created by Kiel Gillard on 4/11/19. +// Copyright © 2019 Ranchero Software, LLC. All rights reserved. +// + +import XCTest +@testable import Account +import RSWeb + +class FeedlyRefreshAccessTokenOperationTests: 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() + } + + class TestRefreshTokenService: OAuthAccessTokenRefreshing { + var mockResult: Result? + var refreshAccessTokenExpectation: XCTestExpectation? + var parameterTester: ((String, OAuthAuthorizationClient) -> ())? + + func refreshAccessToken(with refreshToken: String, client: OAuthAuthorizationClient, completionHandler: @escaping (Result) -> ()) { + + guard let result = mockResult else { + XCTFail("Missing mock result. Test may time out because the completion will not be called.") + return + } + parameterTester?(refreshToken, client) + DispatchQueue.main.async { + completionHandler(result) + self.refreshAccessTokenExpectation?.fulfill() + } + } + } + + func testCancel() { + let service = TestRefreshTokenService() + service.refreshAccessTokenExpectation = expectation(description: "Did Call Refresh") + service.refreshAccessTokenExpectation?.isInverted = true + + let client = support.makeMockOAuthClient() + let refresh = FeedlyRefreshAccessTokenOperation(account: account, service: service, oauthClient: client, log: support.log) + + // If this expectation is not fulfilled, the operation is not calling `didFinish`. + let completionExpectation = expectation(description: "Did Finish") + refresh.completionBlock = { + completionExpectation.fulfill() + } + + OperationQueue.main.addOperation(refresh) + + refresh.cancel() + + waitForExpectations(timeout: 1) + + XCTAssertTrue(refresh.isCancelled) + } + + class TestRefreshTokenDelegate: FeedlyOperationDelegate { + var error: Error? + var didFailExpectation: XCTestExpectation? + + func feedlyOperation(_ operation: FeedlyOperation, didFailWith error: Error) { + self.error = error + didFailExpectation?.fulfill() + } + } + + func testMissingRefreshToken() { + support.removeCredentials(matching: .oauthRefreshToken, from: account) + + let service = TestRefreshTokenService() + service.refreshAccessTokenExpectation = expectation(description: "Did Call Refresh") + service.refreshAccessTokenExpectation?.isInverted = true + + let client = support.makeMockOAuthClient() + let refresh = FeedlyRefreshAccessTokenOperation(account: account, service: service, oauthClient: client, log: support.log) + + let delegate = TestRefreshTokenDelegate() + delegate.didFailExpectation = expectation(description: "Did Fail") + refresh.delegate = delegate + + // If this expectation is not fulfilled, the operation is not calling `didFinish`. + let completionExpectation = expectation(description: "Did Finish") + refresh.completionBlock = { + completionExpectation.fulfill() + } + + OperationQueue.main.addOperation(refresh) + + waitForExpectations(timeout: 1) + + XCTAssertNotNil(delegate.error, "Should have failed with error.") + if let error = delegate.error { + switch error { + case let error as TransportError: + switch error { + case .httpError(status: let status): + XCTAssertEqual(status, 403, "Expected 403 Forbidden.") + default: + XCTFail("Expected 403 Forbidden") + } + default: + XCTFail("Expected \(TransportError.httpError(status: 403))") + } + } + } + + func testRefreshTokenSuccess() { + let service = TestRefreshTokenService() + service.refreshAccessTokenExpectation = expectation(description: "Did Call Refresh") + + let mockAccessToken = Credentials(type: .oauthAccessToken, username: "Test", secret: UUID().uuidString) + let mockRefreshToken = Credentials(type: .oauthRefreshToken, username: "Test", secret: UUID().uuidString) + let grant = OAuthAuthorizationGrant(accessToken: mockAccessToken, refreshToken: mockRefreshToken) + service.mockResult = .success(grant) + + let client = support.makeMockOAuthClient() + service.parameterTester = { serviceRefreshToken, serviceClient in + if let accountRefreshToken = try! self.account.retrieveCredentials(type: .oauthRefreshToken) { + XCTAssertEqual(serviceRefreshToken, accountRefreshToken.secret) + } else { + XCTFail("Could not verify correct refresh token used.") + } + XCTAssertEqual(serviceClient, client) + } + + let refresh = FeedlyRefreshAccessTokenOperation(account: account, service: service, oauthClient: client, log: support.log) + + // If this expectation is not fulfilled, the operation is not calling `didFinish`. + let completionExpectation = expectation(description: "Did Finish") + refresh.completionBlock = { + completionExpectation.fulfill() + } + + OperationQueue.main.addOperation(refresh) + + waitForExpectations(timeout: 1) + + do { + let accessToken = try account.retrieveCredentials(type: .oauthAccessToken) + XCTAssertEqual(accessToken, mockAccessToken) + + let refreshToken = try account.retrieveCredentials(type: .oauthRefreshToken) + XCTAssertEqual(refreshToken, mockRefreshToken) + } catch { + XCTFail("Could not verify refresh and access tokens because \(error).") + } + } + + func testRefreshTokenFailure() { + let accessTokenBefore: Credentials + let refreshTokenBefore: Credentials + + do { + guard let accessToken = try account.retrieveCredentials(type: .oauthAccessToken), + let refreshToken = try account.retrieveCredentials(type: .oauthRefreshToken) else { + XCTFail("Initial refresh and/or access token does not exist.") + return + } + accessTokenBefore = accessToken + refreshTokenBefore = refreshToken + } catch { + XCTFail("Caught error getting initial refresh and access tokens because \(error).") + return + } + + let service = TestRefreshTokenService() + service.refreshAccessTokenExpectation = expectation(description: "Did Call Refresh") + service.mockResult = .failure(URLError(.timedOut)) + + let client = support.makeMockOAuthClient() + service.parameterTester = { serviceRefreshToken, serviceClient in + if let accountRefreshToken = try! self.account.retrieveCredentials(type: .oauthRefreshToken) { + XCTAssertEqual(serviceRefreshToken, accountRefreshToken.secret) + } else { + XCTFail("Could not verify correct refresh token used.") + } + XCTAssertEqual(serviceClient, client) + } + + let refresh = FeedlyRefreshAccessTokenOperation(account: account, service: service, oauthClient: client, log: support.log) + + // If this expectation is not fulfilled, the operation is not calling `didFinish`. + let completionExpectation = expectation(description: "Did Finish") + refresh.completionBlock = { + completionExpectation.fulfill() + } + + OperationQueue.main.addOperation(refresh) + + waitForExpectations(timeout: 1) + + do { + let accessToken = try account.retrieveCredentials(type: .oauthAccessToken) + XCTAssertEqual(accessToken, accessTokenBefore) + + let refreshToken = try account.retrieveCredentials(type: .oauthRefreshToken) + XCTAssertEqual(refreshToken, refreshTokenBefore) + } catch { + XCTFail("Could not verify refresh and access tokens because \(error).") + } + } +} diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlySyncAllOperationTests.swift b/Frameworks/Account/AccountTests/Feedly/FeedlySyncAllOperationTests.swift index c9983f7cf..983c28e45 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlySyncAllOperationTests.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlySyncAllOperationTests.swift @@ -50,7 +50,7 @@ class FeedlySyncAllOperationTests: XCTestCase { let container = support.makeTestDatabaseContainer() let syncAll = FeedlySyncAllOperation(account: account, - credentials: support.credentials, + credentials: support.accessToken, lastSuccessfulFetchStartDate: nil, markArticlesService: markArticlesService, getUnreadService: getStreamIdsService, @@ -89,7 +89,7 @@ class FeedlySyncAllOperationTests: XCTestCase { private var transport = TestTransport() lazy var caller: FeedlyAPICaller = { let caller = FeedlyAPICaller(transport: transport, api: .sandbox) - caller.credentials = support.credentials + caller.credentials = support.accessToken return caller }() @@ -116,7 +116,7 @@ class FeedlySyncAllOperationTests: XCTestCase { // lastSuccessfulFetchStartDate does not matter for the test, content will always be the same. // It is tested in `FeedlyGetStreamContentsOperationTests`. let syncAll = FeedlySyncAllOperation(account: account, - credentials: support.credentials, + credentials: support.accessToken, caller: caller, database: databaseContainer.database, lastSuccessfulFetchStartDate: nil, diff --git a/Frameworks/Account/AccountTests/Feedly/FeedlyTestSupport.swift b/Frameworks/Account/AccountTests/Feedly/FeedlyTestSupport.swift index a82b4841e..ffb0e8660 100644 --- a/Frameworks/Account/AccountTests/Feedly/FeedlyTestSupport.swift +++ b/Frameworks/Account/AccountTests/Feedly/FeedlyTestSupport.swift @@ -12,14 +12,15 @@ import RSParser import os.log import SyncDatabase -struct FeedlyTestSupport { +class FeedlyTestSupport { var log = OSLog(subsystem: Bundle.main.bundleIdentifier!, category: "FeedlyTests") - var credentials = Credentials(type: .oauthAccessToken, username: "Test", secret: "t3st") + var accessToken = Credentials(type: .oauthAccessToken, username: "Test", secret: "t3st-access-tok3n") + var refreshToken = Credentials(type: .oauthRefreshToken, username: "Test", secret: "t3st-refresh-tok3n") var transport = TestTransport() func makeMockNetworkStack() -> (TestTransport, FeedlyAPICaller) { let caller = FeedlyAPICaller(transport: transport, api: .sandbox) - caller.credentials = credentials + caller.credentials = accessToken return (transport, caller) } @@ -27,13 +28,27 @@ struct FeedlyTestSupport { let manager = TestAccountManager() let account = manager.createAccount(type: .feedly, transport: transport) do { - try account.storeCredentials(credentials) + try account.storeCredentials(refreshToken) + // This must be done last or the account uses the refresh token for request Authorization! + try account.storeCredentials(accessToken) } catch { XCTFail("Unable to register mock credentials because \(error)") } return account } + func makeMockOAuthClient() -> OAuthAuthorizationClient { + return OAuthAuthorizationClient(id: "test", redirectUri: "test://test/auth", state: nil, secret: "password") + } + + func removeCredentials(matching type: CredentialsType, from account: Account) { + do { + try account.removeCredentials(type: type) + } catch { + XCTFail("Unable to remove \(type)") + } + } + func makeTestDatabaseContainer() -> TestDatabaseContainer { return TestDatabaseContainer() } @@ -63,6 +78,7 @@ struct FeedlyTestSupport { func destroy(_ testAccount: Account) { do { try testAccount.removeCredentials(type: .oauthAccessToken) + try testAccount.removeCredentials(type: .oauthRefreshToken) } catch { XCTFail("Unable to clean up mock credentials because \(error)") } diff --git a/Frameworks/Account/Credentials/Credentials.swift b/Frameworks/Account/Credentials/Credentials.swift index 22a99e8a3..85d549ac9 100644 --- a/Frameworks/Account/Credentials/Credentials.swift +++ b/Frameworks/Account/Credentials/Credentials.swift @@ -21,7 +21,7 @@ public enum CredentialsType: String { case oauthRefreshToken = "oauthRefreshToken" } -public struct Credentials { +public struct Credentials: Equatable { public let type: CredentialsType public let username: String public let secret: String diff --git a/Frameworks/Account/Feedly/FeedlyAPICaller.swift b/Frameworks/Account/Feedly/FeedlyAPICaller.swift index 24b4108d2..dfe4e9c99 100644 --- a/Frameworks/Account/Feedly/FeedlyAPICaller.swift +++ b/Frameworks/Account/Feedly/FeedlyAPICaller.swift @@ -15,15 +15,6 @@ final class FeedlyAPICaller { case sandbox case cloud - static var `default`: API { - // https://developer.feedly.com/v3/developer/ - if let token = ProcessInfo.processInfo.environment["FEEDLY_DEV_ACCESS_TOKEN"], !token.isEmpty { - return .cloud - } - - return .sandbox - } - var baseUrlComponents: URLComponents { var components = URLComponents() components.scheme = "https" @@ -37,6 +28,25 @@ final class FeedlyAPICaller { } return components } + + var oauthAuthorizationClient: OAuthAuthorizationClient { + switch self { + case .cloud: + /// Models private NetNewsWire client secrets. + /// https://developer.feedly.com/v3/auth/#authenticating-a-user-and-obtaining-an-auth-code + return OAuthAuthorizationClient(id: "{FEEDLY-ID}", + redirectUri: "{FEEDLY-REDIRECT-URI}", + state: nil, + secret: "{FEEDLY-SECRET}") + case .sandbox: + /// Models public sandbox API values found at: + /// https://groups.google.com/forum/#!topic/feedly-cloud/WwQWMgDmOuw + return OAuthAuthorizationClient(id: "sandbox", + redirectUri: "http://localhost", + state: nil, + secret: "ReVGXA6WekanCxbf") + } + } } private let transport: Transport @@ -321,13 +331,14 @@ final class FeedlyAPICaller { extension FeedlyAPICaller: OAuthAuthorizationCodeGrantRequesting { - static func authorizationCodeUrlRequest(for request: OAuthAuthorizationRequest) -> URLRequest { - let api = API.default - var components = api.baseUrlComponents + static func authorizationCodeUrlRequest(for request: OAuthAuthorizationRequest, baseUrlComponents: URLComponents) -> URLRequest { + var components = baseUrlComponents components.path = "/v3/auth/auth" components.queryItems = request.queryItems guard let url = components.url else { + assert(components.scheme != nil) + assert(components.host != nil) fatalError("\(components) does not produce a valid URL.") } @@ -378,6 +389,47 @@ extension FeedlyAPICaller: OAuthAuthorizationCodeGrantRequesting { } } +extension FeedlyAPICaller: OAuthAcessTokenRefreshRequesting { + + func refreshAccessToken(_ refreshRequest: OAuthRefreshAccessTokenRequest, completionHandler: @escaping (Result) -> ()) { + var components = baseUrlComponents + components.path = "/v3/auth/token" + + 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: "Content-Type") + request.addValue("application/json", forHTTPHeaderField: "Accept-Type") + + do { + let encoder = JSONEncoder() + encoder.keyEncodingStrategy = .convertToSnakeCase + request.httpBody = try encoder.encode(refreshRequest) + } catch { + DispatchQueue.main.async { + completionHandler(.failure(error)) + } + return + } + + transport.send(request: request, resultType: AccessTokenResponse.self, keyDecoding: .convertFromSnakeCase) { result in + switch result { + case .success(let (_, tokenResponse)): + if let response = tokenResponse { + completionHandler(.success(response)) + } else { + completionHandler(.failure(URLError(.cannotDecodeContentData))) + } + case .failure(let error): + completionHandler(.failure(error)) + } + } + } +} + extension FeedlyAPICaller: FeedlyGetCollectionsService { func getCollections(completionHandler: @escaping (Result<[FeedlyCollection], Error>) -> ()) { diff --git a/Frameworks/Account/Feedly/FeedlyAccountDelegate+OAuth.swift b/Frameworks/Account/Feedly/FeedlyAccountDelegate+OAuth.swift index 8e97e5f2c..abe0c465a 100644 --- a/Frameworks/Account/Feedly/FeedlyAccountDelegate+OAuth.swift +++ b/Frameworks/Account/Feedly/FeedlyAccountDelegate+OAuth.swift @@ -27,19 +27,24 @@ extension FeedlyAccountDelegate: OAuthAuthorizationGranting { private static let oauthAuthorizationGrantScope = "https://cloud.feedly.com/subscriptions" + static var oauthAuthorizationClient: OAuthAuthorizationClient { + return environment.oauthAuthorizationClient + } + static func oauthAuthorizationCodeGrantRequest(for client: OAuthAuthorizationClient) -> URLRequest { let authorizationRequest = OAuthAuthorizationRequest(clientId: client.id, redirectUri: client.redirectUri, scope: oauthAuthorizationGrantScope, state: client.state) - return FeedlyAPICaller.authorizationCodeUrlRequest(for: authorizationRequest) + let baseURLComponents = environment.baseUrlComponents + return FeedlyAPICaller.authorizationCodeUrlRequest(for: authorizationRequest, baseUrlComponents: baseURLComponents) } static func requestOAuthAccessToken(with response: OAuthAuthorizationResponse, client: OAuthAuthorizationClient, transport: Transport, completionHandler: @escaping (Result) -> ()) { let request = OAuthAccessTokenRequest(authorizationResponse: response, scope: oauthAuthorizationGrantScope, client: client) - let caller = FeedlyAPICaller(transport: transport, api: .default) + let caller = FeedlyAPICaller(transport: transport, api: environment) caller.requestAccessToken(request) { result in switch result { case .success(let response): @@ -62,3 +67,30 @@ extension FeedlyAccountDelegate: OAuthAuthorizationGranting { } } } + +extension FeedlyAccountDelegate: OAuthAccessTokenRefreshing { + func refreshAccessToken(with refreshToken: String, client: OAuthAuthorizationClient, completionHandler: @escaping (Result) -> ()) { + let request = OAuthRefreshAccessTokenRequest(refreshToken: refreshToken, scope: nil, client: client) + + caller.refreshAccessToken(request) { result in + switch result { + case .success(let response): + let accessToken = Credentials(type: .oauthAccessToken, username: response.id, secret: response.accessToken) + + let refreshToken: Credentials? = { + guard let token = response.refreshToken else { + return nil + } + return Credentials(type: .oauthRefreshToken, username: response.id, secret: token) + }() + + let grant = OAuthAuthorizationGrant(accessToken: accessToken, refreshToken: refreshToken) + + completionHandler(.success(grant)) + + case .failure(let error): + completionHandler(.failure(error)) + } + } + } +} diff --git a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift index 4fc95fd47..37e33ff16 100644 --- a/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift +++ b/Frameworks/Account/Feedly/FeedlyAccountDelegate.swift @@ -14,6 +14,21 @@ import SyncDatabase import os.log final class FeedlyAccountDelegate: AccountDelegate { + + /// Feedly has a sandbox API and a production API. + /// This property is referred to when clients need to know which environment it should be pointing to. + static var environment: FeedlyAPICaller.API { + #if DEBUG + // https://developer.feedly.com/v3/developer/ + if let token = ProcessInfo.processInfo.environment["FEEDLY_DEV_ACCESS_TOKEN"], !token.isEmpty { + return .cloud + } + return .sandbox + + #else + return .cloud + #endif + } // TODO: Kiel, if you decide not to support OPML import you will have to disallow it in the behaviors // See https://developer.feedly.com/v3/opml/ @@ -42,16 +57,24 @@ final class FeedlyAccountDelegate: AccountDelegate { var refreshProgress = DownloadProgress(numberOfTasks: 0) - private let caller: FeedlyAPICaller + internal let caller: FeedlyAPICaller + private let log = OSLog(subsystem: Bundle.main.bundleIdentifier!, category: "Feedly") private let database: SyncDatabase private weak var currentSyncAllOperation: FeedlySyncAllOperation? + private let operationQueue: OperationQueue - init(dataFolder: String, transport: Transport?, api: FeedlyAPICaller.API = .default) { + init(dataFolder: String, transport: Transport?, api: FeedlyAPICaller.API) { + self.operationQueue = OperationQueue() + // Many operations have their own operation queues, such as the sync all operation. + // Making this a serial queue at this higher level of abstraction means we can ensure, + // for example, a `FeedlyRefreshAccessTokenOperation` occurs before a `FeedlySyncAllOperation`, + // improving our ability to debug, reason about and predict the behaviour of the code. + self.operationQueue.maxConcurrentOperationCount = 1 if let transport = transport { - caller = FeedlyAPICaller(transport: transport, api: api) + self.caller = FeedlyAPICaller(transport: transport, api: api) } else { @@ -69,9 +92,9 @@ final class FeedlyAccountDelegate: AccountDelegate { } let session = URLSession(configuration: sessionConfiguration) - caller = FeedlyAPICaller(transport: session, api: api) + self.caller = FeedlyAPICaller(transport: session, api: api) } - + let databaseFilePath = (dataFolder as NSString).appendingPathComponent("Sync.sqlite3") self.database = SyncDatabase(databaseFilePath: databaseFilePath) } @@ -79,7 +102,7 @@ final class FeedlyAccountDelegate: AccountDelegate { // MARK: Account API func cancelAll(for account: Account) { - // TODO: Implement me please + operationQueue.cancelAllOperations() } func refreshAll(for account: Account, completion: @escaping (Result) -> Void) { @@ -114,7 +137,7 @@ final class FeedlyAccountDelegate: AccountDelegate { currentSyncAllOperation = operation - OperationQueue.main.addOperation(operation) + operationQueue.addOperation(operation) } func sendArticleStatus(for account: Account, completion: @escaping ((Result) -> Void)) { @@ -125,7 +148,7 @@ final class FeedlyAccountDelegate: AccountDelegate { completion(.success(())) } } - OperationQueue.main.addOperation(send) + operationQueue.addOperation(send) } /// Attempts to ensure local articles have the same status as they do remotely. @@ -145,18 +168,18 @@ final class FeedlyAccountDelegate: AccountDelegate { let group = DispatchGroup() - let getUnread = FeedlySyncUnreadStatusesOperation(account: account, credentials: credentials, service: caller, newerThan: nil, log: log) + let syncUnread = FeedlySyncUnreadStatusesOperation(account: account, credentials: credentials, service: caller, newerThan: nil, log: log) group.enter() - getUnread.completionBlock = { + syncUnread.completionBlock = { group.leave() } - let getStarred = FeedlySyncStarredArticlesOperation(account: account, credentials: credentials, service: caller, log: log) + let syncStarred = FeedlySyncStarredArticlesOperation(account: account, credentials: credentials, service: caller, log: log) group.enter() - getStarred.completionBlock = { + syncStarred.completionBlock = { group.leave() } @@ -164,7 +187,7 @@ final class FeedlyAccountDelegate: AccountDelegate { completion(.success(())) } - OperationQueue.main.addOperations([getUnread, getStarred], waitUntilFinished: false) + operationQueue.addOperations([syncUnread, syncStarred], waitUntilFinished: false) } func importOPML(for account: Account, opmlFile: URL, completion: @escaping (Result) -> Void) { @@ -460,9 +483,14 @@ final class FeedlyAccountDelegate: AccountDelegate { func accountDidInitialize(_ account: Account) { credentials = try? account.retrieveCredentials(type: .oauthAccessToken) + + let client = FeedlyAccountDelegate.oauthAuthorizationClient + let refreshAccessToken = FeedlyRefreshAccessTokenOperation(account: account, service: self, oauthClient: client, log: log) + operationQueue.addOperation(refreshAccessToken) } static func validateCredentials(transport: Transport, credentials: Credentials, endpoint: URL?, completion: @escaping (Result) -> Void) { - fatalError() + assertionFailure("An `account` instance should enqueue an \(FeedlyRefreshAccessTokenOperation.self) instead.") + completion(.success(credentials)) } } diff --git a/Frameworks/Account/Feedly/OAuthAcessTokenRefreshing.swift b/Frameworks/Account/Feedly/OAuthAcessTokenRefreshing.swift new file mode 100644 index 000000000..811f004fa --- /dev/null +++ b/Frameworks/Account/Feedly/OAuthAcessTokenRefreshing.swift @@ -0,0 +1,46 @@ +// +// OAuthAcessTokenRefreshing.swift +// Account +// +// Created by Kiel Gillard on 4/11/19. +// Copyright © 2019 Ranchero Software, LLC. All rights reserved. +// + +import Foundation +import RSWeb + +/// Models section 6 of the OAuth 2.0 Authorization Framework +/// https://tools.ietf.org/html/rfc6749#section-6 +public struct OAuthRefreshAccessTokenRequest: Encodable { + public let grantType = "refresh_token" + public var refreshToken: String + public var scope: String? + + // Possibly not part of the standard but specific to certain implementations (e.g.: Feedly). + public var clientId: String + public var clientSecret: String + + public init(refreshToken: String, scope: String?, client: OAuthAuthorizationClient) { + self.refreshToken = refreshToken + self.scope = scope + self.clientId = client.id + self.clientSecret = client.secret + } +} + +/// Conformed to by API callers to provide a consistent interface for `AccountDelegate` types to refresh OAuth Access Tokens. Conformers provide an associated type that models any custom parameters/properties, as well as the standard ones, in the response to a request for an access token. +/// https://tools.ietf.org/html/rfc6749#section-6 +public protocol OAuthAcessTokenRefreshRequesting { + associatedtype AccessTokenResponse: OAuthAccessTokenResponse + + /// Access tokens expire. Perform a request for a fresh access token given the long life refresh token received when authorization was granted. + /// - Parameter refreshRequest: The refresh token and other information the authorization server requires to grant the client fresh access tokens on the user's behalf. + /// - Parameter completionHandler: On success, the access token response appropriate for concrete type's service. Both the access and refresh token should be stored, preferrably on the Keychain. On failure, possibly a `URLError` or `OAuthAuthorizationErrorResponse` value. + func refreshAccessToken(_ refreshRequest: OAuthRefreshAccessTokenRequest, completionHandler: @escaping (Result) -> ()) +} + +/// Implemented by concrete types to perform the actual request. +protocol OAuthAccessTokenRefreshing: class { + + func refreshAccessToken(with refreshToken: String, client: OAuthAuthorizationClient, completionHandler: @escaping (Result) -> ()) +} diff --git a/Frameworks/Account/Feedly/OAuthAuthorizationCodeGranting.swift b/Frameworks/Account/Feedly/OAuthAuthorizationCodeGranting.swift index 4e055aa3d..433687c32 100644 --- a/Frameworks/Account/Feedly/OAuthAuthorizationCodeGranting.swift +++ b/Frameworks/Account/Feedly/OAuthAuthorizationCodeGranting.swift @@ -11,7 +11,7 @@ import RSWeb /// Client-specific information for requesting an authorization code grant. /// Accounts are responsible for the scope. -public struct OAuthAuthorizationClient { +public struct OAuthAuthorizationClient: Equatable { public var id: String public var redirectUri: String public var state: String? @@ -142,7 +142,7 @@ public protocol OAuthAccessTokenResponse { } /// The access and refresh tokens from a successful authorization grant. -public struct OAuthAuthorizationGrant { +public struct OAuthAuthorizationGrant: Equatable { public var accessToken: Credentials public var refreshToken: Credentials? } @@ -153,18 +153,21 @@ public protocol OAuthAuthorizationCodeGrantRequesting { associatedtype AccessTokenResponse: OAuthAccessTokenResponse /// Provides the URL request that allows users to consent to the client having access to their information. Typically loaded by a web view. - /// - Parameter request: The - static func authorizationCodeUrlRequest(for request: OAuthAuthorizationRequest) -> URLRequest + /// - Parameter request: The information about the client requesting authorization to be granted access tokens. + /// - Parameter baseUrlComponents: The scheme and host of the url except for the path. + static func authorizationCodeUrlRequest(for request: OAuthAuthorizationRequest, baseUrlComponents: URLComponents) -> URLRequest /// Performs the request for the access token given an authorization code. - /// - Parameter authorizationRequest: The authorization code and other information the authorization server requires to grant the client access tokes on the user's behalf. + /// - Parameter authorizationRequest: The authorization code and other information the authorization server requires to grant the client access tokens on the user's behalf. /// - Parameter completionHandler: On success, the access token response appropriate for concrete type's service. On failure, possibly a `URLError` or `OAuthAuthorizationErrorResponse` value. func requestAccessToken(_ authorizationRequest: OAuthAccessTokenRequest, completionHandler: @escaping (Result) -> ()) } protocol OAuthAuthorizationGranting: AccountDelegate { + static var oauthAuthorizationClient: OAuthAuthorizationClient { get } + static func oauthAuthorizationCodeGrantRequest(for client: OAuthAuthorizationClient) -> URLRequest static func requestOAuthAccessToken(with response: OAuthAuthorizationResponse, client: OAuthAuthorizationClient, transport: Transport, completionHandler: @escaping (Result) -> ()) diff --git a/Frameworks/Account/Feedly/Operations/FeedlyRefreshAccessTokenOperation.swift b/Frameworks/Account/Feedly/Operations/FeedlyRefreshAccessTokenOperation.swift new file mode 100644 index 000000000..2ac2f3a0d --- /dev/null +++ b/Frameworks/Account/Feedly/Operations/FeedlyRefreshAccessTokenOperation.swift @@ -0,0 +1,80 @@ +// +// FeedlyRefreshAccessTokenOperation.swift +// Account +// +// Created by Kiel Gillard on 4/11/19. +// Copyright © 2019 Ranchero Software, LLC. All rights reserved. +// + +import Foundation +import os.log +import RSWeb + +final class FeedlyRefreshAccessTokenOperation: FeedlyOperation { + let service: OAuthAccessTokenRefreshing + let oauthClient: OAuthAuthorizationClient + let account: Account + let log: OSLog + + init(account: Account, service: OAuthAccessTokenRefreshing, oauthClient: OAuthAuthorizationClient, log: OSLog) { + self.oauthClient = oauthClient + self.service = service + self.account = account + self.log = log + } + + override func main() { + guard !isCancelled else { + didFinish() + return + } + + let refreshToken: Credentials + + do { + guard let credentials = try account.retrieveCredentials(type: .oauthRefreshToken) else { + os_log(.debug, log: log, "Could not find a refresh token in the keychain. Check the refresh token is added to the Keychain, remove the account and add it again.") + throw TransportError.httpError(status: 403) + } + + refreshToken = credentials + + } catch { + didFinish(error) + return + } + + os_log(.debug, log: log, "Refreshing access token.") + + // Ignore cancellation after the request is resumed otherwise we may continue storing a potentially invalid token! + service.refreshAccessToken(with: refreshToken.secret, client: oauthClient) { result in + self.didRefreshAccessToken(result) + } + } + + private func didRefreshAccessToken(_ result: Result) { + assert(Thread.isMainThread) + + switch result { + case .success(let grant): + do { + os_log(.debug, log: log, "Storing refresh token.") + // Store the refresh token first because it sends this token to the account delegate. + if let token = grant.refreshToken { + try account.storeCredentials(token) + } + + os_log(.debug, log: log, "Storing access token.") + // Now store the access token because we want the account delegate to use it. + try account.storeCredentials(grant.accessToken) + + didFinish() + } catch { + didFinish(error) + } + + case .failure(let error): + didFinish(error) + } + } +} diff --git a/Mac/Preferences/Accounts/AccountsFeedlyWebWindowController.swift b/Mac/Preferences/Accounts/AccountsFeedlyWebWindowController.swift index f24d44fd6..7529a8a9a 100644 --- a/Mac/Preferences/Accounts/AccountsFeedlyWebWindowController.swift +++ b/Mac/Preferences/Accounts/AccountsFeedlyWebWindowController.swift @@ -29,9 +29,8 @@ class AccountsFeedlyWebWindowController: NSWindowController, WKNavigationDelegat } // MARK: Requesting an Access Token - - private let client = OAuthAuthorizationClient.feedlySandboxClient - + let client = Account.oauthAuthorizationClient(for: .feedly) + private func beginAuthorization() { let request = Account.oauthAuthorizationCodeGrantRequest(for: .feedly, client: client) webView.load(request) @@ -99,21 +98,3 @@ class AccountsFeedlyWebWindowController: NSWindowController, WKNavigationDelegat } } } - -private extension OAuthAuthorizationClient { - - /// Models public sandbox API values found at: - /// https://groups.google.com/forum/#!topic/feedly-cloud/WwQWMgDmOuw - static var feedlySandboxClient: OAuthAuthorizationClient { - return OAuthAuthorizationClient(id: "sandbox", - redirectUri: "http://localhost", - state: nil, - secret: "ReVGXA6WekanCxbf") - } - - /// Models private NetNewsWire client secrets. - /// https://developer.feedly.com/v3/auth/#authenticating-a-user-and-obtaining-an-auth-code - static var netNewsWireClient: OAuthAuthorizationClient { - fatalError("This app is not registered as a client with Feedly. Follow the URL in the code comments for this property.") - } -} diff --git a/xcconfig/common/NetNewsWire_ios_target_common.xcconfig b/xcconfig/common/NetNewsWire_ios_target_common.xcconfig index 5f26c08ca..dffc3a7a0 100644 --- a/xcconfig/common/NetNewsWire_ios_target_common.xcconfig +++ b/xcconfig/common/NetNewsWire_ios_target_common.xcconfig @@ -1,7 +1,7 @@ // High Level Settings common to both the iOS application and any extensions we bundle with it MARKETING_VERSION = 5.0 -CURRENT_PROJECT_VERSION = 5 +CURRENT_PROJECT_VERSION = 6 ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = YES ASSETCATALOG_COMPILER_APPICON_NAME = AppIcon