Convert methods to async await. Delete FeedlyRefreshAccessTokenOperation (we’re moving away from MainThreadOperation).
This commit is contained in:
Brent Simmons
parent
42ce0d2f64
commit
f9a3d8e2c1
|
|
@ -62,6 +62,7 @@ extension FeedlyAccountDelegate: OAuthAuthorizationGranting {
|
|||
}
|
||||
|
||||
extension FeedlyAccountDelegate: OAuthAccessTokenRefreshing {
|
||||
|
||||
func refreshAccessToken(with refreshToken: String, client: OAuthAuthorizationClient) async throws -> OAuthAuthorizationGrant {
|
||||
|
||||
let request = OAuthRefreshAccessTokenRequest(refreshToken: refreshToken, scope: nil, client: client)
|
||||
|
|
|
|||
|
|
@ -610,7 +610,7 @@ final class FeedlyAccountDelegate: AccountDelegate {
|
|||
|
||||
static func validateCredentials(transport: Transport, credentials: Credentials, endpoint: URL?, secretsProvider: SecretsProvider) async throws -> Credentials? {
|
||||
|
||||
assertionFailure("An `account` instance should enqueue an \(FeedlyRefreshAccessTokenOperation.self) instead.")
|
||||
assertionFailure("An `account` instance should refresh the access token first instead.")
|
||||
return credentials
|
||||
}
|
||||
|
||||
|
|
@ -642,34 +642,36 @@ final class FeedlyAccountDelegate: AccountDelegate {
|
|||
|
||||
extension FeedlyAccountDelegate: FeedlyAPICallerDelegate {
|
||||
|
||||
@MainActor func reauthorizeFeedlyAPICaller(_ caller: FeedlyAPICaller, completionHandler: @escaping (Bool) -> ()) {
|
||||
@MainActor func reauthorizeFeedlyAPICaller(_ caller: FeedlyAPICaller) async -> Bool {
|
||||
|
||||
guard let account = initializedAccount else {
|
||||
completionHandler(false)
|
||||
return
|
||||
return false
|
||||
}
|
||||
|
||||
/// Captures a failure to refresh a token, assuming that it was refreshed unless told otherwise.
|
||||
final class RefreshAccessTokenOperationDelegate: FeedlyOperationDelegate {
|
||||
|
||||
private(set) var didReauthorize = true
|
||||
|
||||
func feedlyOperation(_ operation: FeedlyOperation, didFailWith error: Error) {
|
||||
didReauthorize = false
|
||||
}
|
||||
|
||||
do {
|
||||
try await refreshAccessToken(account: account)
|
||||
return true
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
|
||||
let refreshAccessToken = FeedlyRefreshAccessTokenOperation(account: account, service: self, oauthClient: oauthAuthorizationClient, log: log)
|
||||
refreshAccessToken.downloadProgress = refreshProgress
|
||||
|
||||
/// This must be strongly referenced by the completionBlock of the `FeedlyRefreshAccessTokenOperation`.
|
||||
let refreshAccessTokenDelegate = RefreshAccessTokenOperationDelegate()
|
||||
refreshAccessToken.delegate = refreshAccessTokenDelegate
|
||||
|
||||
refreshAccessToken.completionBlock = { operation in
|
||||
assert(Thread.isMainThread)
|
||||
completionHandler(refreshAccessTokenDelegate.didReauthorize && !operation.isCanceled)
|
||||
}
|
||||
|
||||
private func refreshAccessToken(account: Account) async throws {
|
||||
|
||||
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)
|
||||
}
|
||||
|
||||
MainThreadOperationQueue.shared.add(refreshAccessToken)
|
||||
|
||||
os_log(.debug, log: log, "Refreshing access token.")
|
||||
let grant = try await refreshAccessToken(with: credentials.secret, client: oauthAuthorizationClient)
|
||||
|
||||
os_log(.debug, log: log, "Storing refresh token.")
|
||||
if let refreshToken = grant.refreshToken {
|
||||
try account.storeCredentials(refreshToken)
|
||||
}
|
||||
|
||||
os_log(.debug, log: log, "Storing access token.")
|
||||
try account.storeCredentials(grant.accessToken)
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -12,9 +12,10 @@ import Secrets
|
|||
import Feedly
|
||||
|
||||
protocol FeedlyAPICallerDelegate: AnyObject {
|
||||
|
||||
/// Implemented by the `FeedlyAccountDelegate` reauthorize the client with a fresh OAuth token so the client can retry the unauthorized request.
|
||||
/// Pass `true` to the completion handler if the failing request should be retried with a fresh token or `false` if the unauthorized request should complete with the original failure error.
|
||||
@MainActor func reauthorizeFeedlyAPICaller(_ caller: FeedlyAPICaller, completionHandler: @escaping (Bool) -> ())
|
||||
@MainActor func reauthorizeFeedlyAPICaller(_ caller: FeedlyAPICaller) async -> Bool
|
||||
}
|
||||
|
||||
@MainActor final class FeedlyAPICaller {
|
||||
|
|
@ -102,47 +103,46 @@ protocol FeedlyAPICallerDelegate: AnyObject {
|
|||
}
|
||||
|
||||
private func send<R: Decodable & Sendable>(request: URLRequest, resultType: R.Type, dateDecoding: JSONDecoder.DateDecodingStrategy = .iso8601, keyDecoding: JSONDecoder.KeyDecodingStrategy = .useDefaultKeys, completion: @escaping (Result<(HTTPURLResponse, R?), Error>) -> Void) {
|
||||
|
||||
transport.send(request: request, resultType: resultType, dateDecoding: dateDecoding, keyDecoding: keyDecoding) { [weak self] result in
|
||||
|
||||
MainActor.assumeIsolated {
|
||||
|
||||
|
||||
Task { @MainActor [weak self] in
|
||||
|
||||
switch result {
|
||||
case .success:
|
||||
completion(result)
|
||||
case .failure(let error):
|
||||
switch error {
|
||||
case TransportError.httpError(let statusCode) where statusCode == 401:
|
||||
|
||||
|
||||
assert(self == nil ? true : self?.delegate != nil, "Check the delegate is set to \(FeedlyAccountDelegate.self).")
|
||||
|
||||
|
||||
guard let self = self, let delegate = self.delegate else {
|
||||
completion(result)
|
||||
return
|
||||
}
|
||||
|
||||
|
||||
/// Capture the credentials before the reauthorization to check for a change.
|
||||
let credentialsBefore = self.credentials
|
||||
|
||||
delegate.reauthorizeFeedlyAPICaller(self) { [weak self] isReauthorizedAndShouldRetry in
|
||||
assert(Thread.isMainThread)
|
||||
|
||||
guard isReauthorizedAndShouldRetry, let self = self else {
|
||||
completion(result)
|
||||
return
|
||||
}
|
||||
|
||||
// Check for a change. Not only would it help debugging, but it'll also catch an infinitely recursive attempt to refresh.
|
||||
guard let accessToken = self.credentials?.secret, accessToken != credentialsBefore?.secret else {
|
||||
assertionFailure("Could not update the request with a new OAuth token. Did \(String(describing: self.delegate)) set them on \(self)?")
|
||||
completion(result)
|
||||
return
|
||||
}
|
||||
|
||||
var reauthorizedRequest = request
|
||||
reauthorizedRequest.setValue("OAuth \(accessToken)", forHTTPHeaderField: HTTPRequestHeader.authorization)
|
||||
|
||||
self.send(request: reauthorizedRequest, resultType: resultType, dateDecoding: dateDecoding, keyDecoding: keyDecoding, completion: completion)
|
||||
|
||||
let isReauthorizedAndShouldRetry = await delegate.reauthorizeFeedlyAPICaller(self)
|
||||
guard isReauthorizedAndShouldRetry else {
|
||||
completion(result)
|
||||
return
|
||||
}
|
||||
|
||||
// Check for a change. Not only would it help debugging, but it'll also catch an infinitely recursive attempt to refresh.
|
||||
guard let accessToken = self.credentials?.secret, accessToken != credentialsBefore?.secret else {
|
||||
assertionFailure("Could not update the request with a new OAuth token. Did \(String(describing: self.delegate)) set them on \(self)?")
|
||||
completion(result)
|
||||
return
|
||||
}
|
||||
|
||||
var reauthorizedRequest = request
|
||||
reauthorizedRequest.setValue("OAuth \(accessToken)", forHTTPHeaderField: HTTPRequestHeader.authorization)
|
||||
|
||||
self.send(request: reauthorizedRequest, resultType: resultType, dateDecoding: dateDecoding, keyDecoding: keyDecoding, completion: completion)
|
||||
|
||||
default:
|
||||
completion(result)
|
||||
}
|
||||
|
|
@ -150,7 +150,7 @@ protocol FeedlyAPICallerDelegate: AnyObject {
|
|||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
func importOPML(_ opmlData: Data) async throws {
|
||||
|
||||
guard !isSuspended else { throw TransportError.suspended }
|
||||
|
|
|
|||
|
|
@ -1,60 +0,0 @@
|
|||
//
|
||||
// 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 Web
|
||||
import Secrets
|
||||
import Feedly
|
||||
|
||||
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 run() {
|
||||
|
||||
Task { @MainActor in
|
||||
|
||||
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)
|
||||
}
|
||||
|
||||
// Ignore cancellation after the request is resumed otherwise we may continue storing a potentially invalid token!
|
||||
os_log(.debug, log: log, "Refreshing access token.")
|
||||
let grant = try await service.refreshAccessToken(with: credentials.secret, client: oauthClient)
|
||||
|
||||
// Store the refresh token first because it sends this token to the account delegate.
|
||||
os_log(.debug, log: log, "Storing refresh token.")
|
||||
if let refreshToken = grant.refreshToken {
|
||||
try account.storeCredentials(refreshToken)
|
||||
}
|
||||
|
||||
// Now store the access token because we want the account delegate to use it.
|
||||
os_log(.debug, log: log, "Storing access token.")
|
||||
try account.storeCredentials(grant.accessToken)
|
||||
|
||||
didFinish()
|
||||
|
||||
} catch {
|
||||
didFinish(with: error)
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -1,218 +0,0 @@
|
|||
//
|
||||
// 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 Web
|
||||
import Secrets
|
||||
|
||||
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<OAuthAuthorizationGrant, Error>?
|
||||
var refreshAccessTokenExpectation: XCTestExpectation?
|
||||
var parameterTester: ((String, OAuthAuthorizationClient) -> ())?
|
||||
|
||||
func refreshAccessToken(with refreshToken: String, client: OAuthAuthorizationClient, completion: @escaping (Result<OAuthAuthorizationGrant, Error>) -> ()) {
|
||||
|
||||
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 {
|
||||
completion(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 = { _ in
|
||||
completionExpectation.fulfill()
|
||||
}
|
||||
|
||||
MainThreadOperationQueue.shared.add(refresh)
|
||||
|
||||
MainThreadOperationQueue.shared.cancelOperations([refresh])
|
||||
|
||||
waitForExpectations(timeout: 1)
|
||||
|
||||
XCTAssertTrue(refresh.isCanceled)
|
||||
}
|
||||
|
||||
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 = { _ in
|
||||
completionExpectation.fulfill()
|
||||
}
|
||||
|
||||
MainThreadOperationQueue.shared.add(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 = { _ in
|
||||
completionExpectation.fulfill()
|
||||
}
|
||||
|
||||
MainThreadOperationQueue.shared.add(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 = { _ in
|
||||
completionExpectation.fulfill()
|
||||
}
|
||||
|
||||
MainThreadOperationQueue.shared.add(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).")
|
||||
}
|
||||
}
|
||||
}
|
||||
Loading…
Reference in New Issue