From 8229eecc3a072f32466e93b54d39db263012d5d8 Mon Sep 17 00:00:00 2001 From: Justin Mazzocchi <2831158+jzzocc@users.noreply.github.com> Date: Tue, 8 Sep 2020 18:02:55 -0700 Subject: [PATCH] Refactoring --- .../MastodonAPI/MastodonAPIClient.swift | 11 +- .../Services/AllIdentitiesService.swift | 26 ++-- .../Services/AuthenticationService.swift | 139 ++++++++++-------- .../Services/IdentityService.swift | 4 +- .../Utilities/WebAuthSession.swift | 8 +- .../MockAppEnvironment.swift | 10 +- .../AuthenticationServiceTests.swift | 14 +- .../ViewModels/AddIdentityViewModel.swift | 38 ++--- .../Extensions/Publisher+Extensions.swift | 6 +- 9 files changed, 131 insertions(+), 125 deletions(-) diff --git a/MastodonAPI/Sources/MastodonAPI/MastodonAPIClient.swift b/MastodonAPI/Sources/MastodonAPI/MastodonAPIClient.swift index 4ea62df..9e64ecd 100644 --- a/MastodonAPI/Sources/MastodonAPI/MastodonAPIClient.swift +++ b/MastodonAPI/Sources/MastodonAPI/MastodonAPIClient.swift @@ -6,10 +6,11 @@ import HTTP import Mastodon public final class MastodonAPIClient: HTTPClient { - public var instanceURL: URL? + public var instanceURL: URL public var accessToken: String? - public required init(session: Session) { + public required init(session: Session, instanceURL: URL) { + self.instanceURL = instanceURL super.init(session: session, decoder: MastodonDecoder()) } @@ -20,11 +21,7 @@ public final class MastodonAPIClient: HTTPClient { extension MastodonAPIClient { public func request(_ endpoint: E) -> AnyPublisher { - guard let instanceURL = instanceURL else { - return Fail(error: URLError(.badURL)).eraseToAnyPublisher() - } - - return super.request( + super.request( MastodonAPITarget(baseURL: instanceURL, endpoint: endpoint, accessToken: accessToken), decodeErrorsAs: APIError.self) } diff --git a/ServiceLayer/Sources/ServiceLayer/Services/AllIdentitiesService.swift b/ServiceLayer/Sources/ServiceLayer/Services/AllIdentitiesService.swift index 3ce5020..0dd8025 100644 --- a/ServiceLayer/Sources/ServiceLayer/Services/AllIdentitiesService.swift +++ b/ServiceLayer/Sources/ServiceLayer/Services/AllIdentitiesService.swift @@ -36,29 +36,25 @@ public extension AllIdentitiesService { database.createIdentity(id: id, url: instanceURL) } - func authorizeIdentity(id: UUID, instanceURL: URL) -> AnyPublisher { - let secrets = Secrets(identityID: id, keychain: environment.keychain) - let authenticationService = AuthenticationService(environment: environment) + func authorizeAndCreateIdentity(id: UUID, url: URL) -> AnyPublisher { + AuthenticationService(url: url, environment: environment) + .authenticate() + .tryMap { + let secrets = Secrets(identityID: id, keychain: environment.keychain) - return authenticationService.authorizeApp(instanceURL: instanceURL) - .tryMap { appAuthorization -> (URL, AppAuthorization) in - try secrets.setInstanceURL(instanceURL) - try secrets.setClientID(appAuthorization.clientId) - try secrets.setClientSecret(appAuthorization.clientSecret) - - return (instanceURL, appAuthorization) + try secrets.setInstanceURL(url) + try secrets.setClientID($0.clientId) + try secrets.setClientSecret($0.clientSecret) + try secrets.setAccessToken($1.accessToken) } - .flatMap(authenticationService.authenticate(instanceURL:appAuthorization:)) - .tryMap { try secrets.setAccessToken($0.accessToken) } + .flatMap { database.createIdentity(id: id, url: url) } .ignoreOutput() .eraseToAnyPublisher() } func deleteIdentity(_ identity: Identity) -> AnyPublisher { let secrets = Secrets(identityID: identity.id, keychain: environment.keychain) - let mastodonAPIClient = MastodonAPIClient(session: environment.session) - - mastodonAPIClient.instanceURL = identity.url + let mastodonAPIClient = MastodonAPIClient(session: environment.session, instanceURL: identity.url) return database.deleteIdentity(id: identity.id) .collect() diff --git a/ServiceLayer/Sources/ServiceLayer/Services/AuthenticationService.swift b/ServiceLayer/Sources/ServiceLayer/Services/AuthenticationService.swift index a350524..8837956 100644 --- a/ServiceLayer/Sources/ServiceLayer/Services/AuthenticationService.swift +++ b/ServiceLayer/Sources/ServiceLayer/Services/AuthenticationService.swift @@ -5,69 +5,33 @@ import Foundation import Mastodon import MastodonAPI -public struct AuthenticationService { +public enum AuthenticationError: Error { + case canceled +} + +struct AuthenticationService { private let mastodonAPIClient: MastodonAPIClient private let webAuthSessionType: WebAuthSession.Type private let webAuthSessionContextProvider = WebAuthSessionContextProvider() - public init(environment: AppEnvironment) { - mastodonAPIClient = MastodonAPIClient(session: environment.session) + init(url: URL, environment: AppEnvironment) { + mastodonAPIClient = MastodonAPIClient(session: environment.session, instanceURL: url) webAuthSessionType = environment.webAuthSessionType } } -public extension AuthenticationService { - func authorizeApp(instanceURL: URL) -> AnyPublisher { - let endpoint = AppAuthorizationEndpoint.apps( - clientName: OAuth.clientName, - redirectURI: OAuth.callbackURL.absoluteString, - scopes: OAuth.scopes, - website: OAuth.website) - let target = MastodonAPITarget(baseURL: instanceURL, endpoint: endpoint, accessToken: nil) +extension AuthenticationService { + func authenticate() -> AnyPublisher<(AppAuthorization, AccessToken), Error> { + let appAuthorization = mastodonAPIClient.request( + AppAuthorizationEndpoint.apps( + clientName: OAuth.clientName, + redirectURI: OAuth.callbackURL.absoluteString, + scopes: OAuth.scopes, + website: OAuth.website)) + .share() - return mastodonAPIClient.request(target) - } - - func authenticate(instanceURL: URL, appAuthorization: AppAuthorization) -> AnyPublisher { - guard let authorizationURL = authorizationURL( - instanceURL: instanceURL, - clientID: appAuthorization.clientId) else { - return Fail(error: URLError(.badURL)).eraseToAnyPublisher() - } - - return webAuthSessionType.publisher( - url: authorizationURL, - callbackURLScheme: OAuth.callbackURLScheme, - presentationContextProvider: webAuthSessionContextProvider) - .tryCatch { error -> AnyPublisher in - if (error as? WebAuthSessionError)?.code == .canceledLogin { - return Just(nil).setFailureType(to: Error.self).eraseToAnyPublisher() - } - - throw error - } - .compactMap { $0 } - .tryMap { url -> String in - guard let queryItems = URLComponents(url: url, resolvingAgainstBaseURL: true)?.queryItems, - let code = queryItems.first(where: { - $0.name == OAuth.codeCallbackQueryItemName - })?.value - else { throw OAuthError.codeNotFound } - - return code - } - .flatMap { code -> AnyPublisher in - let endpoint = AccessTokenEndpoint.oauthToken( - clientID: appAuthorization.clientId, - clientSecret: appAuthorization.clientSecret, - code: code, - grantType: OAuth.grantType, - scopes: OAuth.scopes, - redirectURI: OAuth.callbackURL.absoluteString) - let target = MastodonAPITarget(baseURL: instanceURL, endpoint: endpoint, accessToken: nil) - - return mastodonAPIClient.request(target) - } + return appAuthorization + .zip(appAuthorization.flatMap(authenticate(appAuthorization:))) .eraseToAnyPublisher() } } @@ -87,19 +51,66 @@ private extension AuthenticationService { case codeNotFound } - func authorizationURL(instanceURL: URL, clientID: String) -> URL? { - guard var authorizationURLComponents = URLComponents(url: instanceURL, resolvingAgainstBaseURL: true) else { - return nil - } + static func extractCode(oauthCallbackURL: URL) throws -> String { + guard let queryItems = URLComponents( + url: oauthCallbackURL, + resolvingAgainstBaseURL: true)?.queryItems, + let code = queryItems.first(where: { + $0.name == OAuth.codeCallbackQueryItemName + })?.value + else { throw OAuthError.codeNotFound } + + return code + } + + func authorizationURL(appAuthorization: AppAuthorization) throws -> URL { + guard var authorizationURLComponents = URLComponents( + url: mastodonAPIClient.instanceURL, + resolvingAgainstBaseURL: true) + else { throw URLError(.badURL) } authorizationURLComponents.path = "/oauth/authorize" authorizationURLComponents.queryItems = [ - "client_id": clientID, - "scope": OAuth.scopes, - "response_type": "code", - "redirect_uri": OAuth.callbackURL.absoluteString - ].map { URLQueryItem(name: $0, value: $1) } + .init(name: "client_id", value: appAuthorization.clientId), + .init(name: "scope", value: OAuth.scopes), + .init(name: "response_type", value: "code"), + .init(name: "redirect_uri", value: OAuth.callbackURL.absoluteString) + ] - return authorizationURLComponents.url + guard let authorizationURL = authorizationURLComponents.url else { + throw URLError(.badURL) + } + + return authorizationURL + } + + func authenticate(appAuthorization: AppAuthorization) -> AnyPublisher { + Just(appAuthorization) + .tryMap(authorizationURL(appAuthorization:)) + .flatMap { + webAuthSessionType.publisher( + url: $0, + callbackURLScheme: OAuth.callbackURLScheme, + presentationContextProvider: webAuthSessionContextProvider) + } + .mapError { error -> Error in + if (error as? WebAuthSessionError)?.code == .canceledLogin { + return AuthenticationError.canceled as Error + } + + return error + } + .tryMap(Self.extractCode(oauthCallbackURL:)) + .flatMap { + mastodonAPIClient.request( + AccessTokenEndpoint.oauthToken( + clientID: appAuthorization.clientId, + clientSecret: appAuthorization.clientSecret, + code: $0, + grantType: OAuth.grantType, + scopes: OAuth.scopes, + redirectURI: OAuth.callbackURL.absoluteString)) + } + .eraseToAnyPublisher() } } diff --git a/ServiceLayer/Sources/ServiceLayer/Services/IdentityService.swift b/ServiceLayer/Sources/ServiceLayer/Services/IdentityService.swift index f0f35cf..f905a9b 100644 --- a/ServiceLayer/Sources/ServiceLayer/Services/IdentityService.swift +++ b/ServiceLayer/Sources/ServiceLayer/Services/IdentityService.swift @@ -24,8 +24,8 @@ public struct IdentityService { secrets = Secrets( identityID: id, keychain: environment.keychain) - mastodonAPIClient = MastodonAPIClient(session: environment.session) - mastodonAPIClient.instanceURL = try secrets.getInstanceURL() + mastodonAPIClient = MastodonAPIClient(session: environment.session, + instanceURL: try secrets.getInstanceURL()) mastodonAPIClient.accessToken = try? secrets.getAccessToken() contentDatabase = try ContentDatabase(identityID: id, diff --git a/ServiceLayer/Sources/ServiceLayer/Utilities/WebAuthSession.swift b/ServiceLayer/Sources/ServiceLayer/Utilities/WebAuthSession.swift index 5db626a..89e4ff0 100644 --- a/ServiceLayer/Sources/ServiceLayer/Utilities/WebAuthSession.swift +++ b/ServiceLayer/Sources/ServiceLayer/Utilities/WebAuthSession.swift @@ -16,8 +16,8 @@ extension WebAuthSession { static func publisher( url: URL, callbackURLScheme: String?, - presentationContextProvider: WebAuthPresentationContextProviding) -> AnyPublisher { - Future { promise in + presentationContextProvider: WebAuthPresentationContextProviding) -> AnyPublisher { + Future { promise in let webAuthSession = Self( url: url, callbackURLScheme: callbackURLScheme) { oauthCallbackURL, error in @@ -25,6 +25,10 @@ extension WebAuthSession { return promise(.failure(error)) } + guard let oauthCallbackURL = oauthCallbackURL else { + return promise(.failure(URLError(.unknown))) + } + return promise(.success(oauthCallbackURL)) } diff --git a/ServiceLayer/Sources/ServiceLayerMocks/MockAppEnvironment.swift b/ServiceLayer/Sources/ServiceLayerMocks/MockAppEnvironment.swift index a1995f2..a49e3cd 100644 --- a/ServiceLayer/Sources/ServiceLayerMocks/MockAppEnvironment.swift +++ b/ServiceLayer/Sources/ServiceLayerMocks/MockAppEnvironment.swift @@ -18,11 +18,11 @@ public extension AppEnvironment { fixtureDatabase: IdentityDatabase? = nil) -> Self { AppEnvironment( session: Session(configuration: .stubbing), - webAuthSessionType: SuccessfulMockWebAuthSession.self, - keychain: MockKeychain.self, - userDefaults: MockUserDefaults(), - userNotificationClient: .mock, - inMemoryContent: true, + webAuthSessionType: webAuthSessionType, + keychain: keychain, + userDefaults: userDefaults, + userNotificationClient: userNotificationClient, + inMemoryContent: inMemoryContent, fixtureDatabase: fixtureDatabase) } } diff --git a/ServiceLayer/Tests/ServiceLayerTests/AuthenticationServiceTests.swift b/ServiceLayer/Tests/ServiceLayerTests/AuthenticationServiceTests.swift index bf5824b..1fe8a54 100644 --- a/ServiceLayer/Tests/ServiceLayerTests/AuthenticationServiceTests.swift +++ b/ServiceLayer/Tests/ServiceLayerTests/AuthenticationServiceTests.swift @@ -8,20 +8,12 @@ import XCTest class AuthenticationServiceTests: XCTestCase { func testAuthentication() throws { - let sut = AuthenticationService(environment: .mock()) - let instanceURL = URL(string: "https://mastodon.social")! - let appAuthorizationRecorder = sut.authorizeApp(instanceURL: instanceURL).record() - let appAuthorization = try wait(for: appAuthorizationRecorder.next(), timeout: 1) + let sut = AuthenticationService(url: URL(string: "https://mastodon.social")!, environment: .mock()) + let authenticationRecorder = sut.authenticate().record() + let (appAuthorization, accessToken) = try wait(for: authenticationRecorder.next(), timeout: 1) XCTAssertEqual(appAuthorization.clientId, "AUTHORIZATION_CLIENT_ID_STUB_VALUE") XCTAssertEqual(appAuthorization.clientSecret, "AUTHORIZATION_CLIENT_SECRET_STUB_VALUE") - - let accessTokenRecorder = sut.authenticate( - instanceURL: instanceURL, - appAuthorization: appAuthorization) - .record() - let accessToken = try wait(for: accessTokenRecorder.next(), timeout: 1) - XCTAssertEqual(accessToken.accessToken, "ACCESS_TOKEN_STUB_VALUE") } } diff --git a/ViewModels/Sources/ViewModels/AddIdentityViewModel.swift b/ViewModels/Sources/ViewModels/AddIdentityViewModel.swift index 56f9915..83ea701 100644 --- a/ViewModels/Sources/ViewModels/AddIdentityViewModel.swift +++ b/ViewModels/Sources/ViewModels/AddIdentityViewModel.swift @@ -11,12 +11,12 @@ public final class AddIdentityViewModel: ObservableObject { public let addedIdentityID: AnyPublisher private let allIdentitiesService: AllIdentitiesService - private let addedIdentityIDInput = PassthroughSubject() + private let addedIdentityIDSubject = PassthroughSubject() private var cancellables = Set() init(allIdentitiesService: AllIdentitiesService) { self.allIdentitiesService = allIdentitiesService - addedIdentityID = addedIdentityIDInput.eraseToAnyPublisher() + addedIdentityID = addedIdentityIDSubject.eraseToAnyPublisher() } } @@ -33,22 +33,26 @@ public extension AddIdentityViewModel { return } - allIdentitiesService.authorizeIdentity(id: identityID, instanceURL: instanceURL) - .collect() - .map { _ in (identityID, instanceURL) } - .flatMap(allIdentitiesService.createIdentity(id:instanceURL:)) - .mapError { - return $0 - } + allIdentitiesService.authorizeAndCreateIdentity(id: identityID, url: instanceURL) .receive(on: DispatchQueue.main) - .assignErrorsToAlertItem(to: \.alertItem, on: self) - .handleEvents( - receiveSubscription: { [weak self] _ in self?.loading = true }, - receiveCompletion: { [weak self] _ in self?.loading = false }) - .sink { [weak self] in - guard let self = self, case .finished = $0 else { return } + .catch { [weak self] error -> Empty in + if case AuthenticationError.canceled = error { + // no-op + } else { + self?.alertItem = AlertItem(error: error) + } - self.addedIdentityIDInput.send(identityID) + return Empty() + } + .handleEvents(receiveSubscription: { [weak self] _ in self?.loading = true }) + .sink { [weak self] in + guard let self = self else { return } + + self.loading = false + + if case .finished = $0 { + self.addedIdentityIDSubject.send(identityID) + } } receiveValue: { _ in } .store(in: &cancellables) } @@ -71,7 +75,7 @@ public extension AddIdentityViewModel { .sink { [weak self] in guard let self = self, case .finished = $0 else { return } - self.addedIdentityIDInput.send(identityID) + self.addedIdentityIDSubject.send(identityID) } receiveValue: { _ in } .store(in: &cancellables) } diff --git a/ViewModels/Sources/ViewModels/Extensions/Publisher+Extensions.swift b/ViewModels/Sources/ViewModels/Extensions/Publisher+Extensions.swift index 7b01be4..7663240 100644 --- a/ViewModels/Sources/ViewModels/Extensions/Publisher+Extensions.swift +++ b/ViewModels/Sources/ViewModels/Extensions/Publisher+Extensions.swift @@ -8,8 +8,10 @@ extension Publisher { to keyPath: ReferenceWritableKeyPath, on object: Root) -> AnyPublisher { self.catch { [weak object] error -> Empty in - DispatchQueue.main.async { - object?[keyPath: keyPath] = AlertItem(error: error) + if let object = object { + DispatchQueue.main.async { + object[keyPath: keyPath] = AlertItem(error: error) + } } return Empty()