Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Authentication – Refresh, persistence & API #681

Merged
merged 63 commits into from
Jan 25, 2025
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
Show all changes
63 commits
Select commit Hold shift + click to select a range
1f0adfa
Reanem OAuthClient to AuthentincationDataManager
mohannad-hassan Dec 26, 2024
af28e6e
Create AppAuthCaller to wrap AppAuth calls
mohannad-hassan Dec 26, 2024
17886fb
Refactor to use the new AppAuthCaller
mohannad-hassan Dec 26, 2024
6a2ee35
Create AppAuthOAuthClientTests in OAuthClient
mohannad-hassan Dec 27, 2024
a3f8cca
Wrap OIDAuthState within AuthenticationState
mohannad-hassan Dec 28, 2024
3f1af6b
Persist the tokens after logging in
mohannad-hassan Dec 28, 2024
a3a700d
Provide a way to restoreState for AuthentincationDataManager
mohannad-hassan Dec 30, 2024
b2614b7
Authenitcate rquests in AuthentincationDataManager
mohannad-hassan Dec 30, 2024
986408a
Rename AuthenticationState to AuthenticationData
mohannad-hassan Dec 30, 2024
f601c03
Make AuthentincationDataManager expose an authentication state
mohannad-hassan Dec 30, 2024
1637fc1
Persist updated state
mohannad-hassan Dec 30, 2024
b9b5598
Refresh authentication data manager on launch startup
mohannad-hassan Dec 30, 2024
008a93e
Rename AppAuthOAuthClient to AuthentincationDataManagerImpl
mohannad-hassan Dec 30, 2024
eaf968a
Organize errors and logs
mohannad-hassan Dec 31, 2024
458d7bb
checking in package resolution for AppAuth-iOS
mohannad-hassan Dec 31, 2024
9b71d61
Linting
mohannad-hassan Dec 31, 2024
b8aebcb
Remove configurations
mohannad-hassan Dec 31, 2024
a54ceed
Fix linting issues in LaunchStartup
mohannad-hassan Jan 1, 2025
9a4fd81
Documentation
mohannad-hassan Jan 1, 2025
c0f4c25
Rename some internal types for brevity
mohannad-hassan Jan 1, 2025
a5fa1a8
Rename OAuthClient package to AuthenticationClient
mohannad-hassan Jan 1, 2025
bc85910
Rename AuthentincationDataManager to AuthenticationClient
mohannad-hassan Jan 4, 2025
2fa50e6
Some cleanup in AuthenticationClientTests
mohannad-hassan Jan 4, 2025
90bd841
Revert changes in Features/AppStructureFeature -- Pending integration…
mohannad-hassan Jan 4, 2025
8049a8b
Fix a compilation issue
mohannad-hassan Jan 4, 2025
3cdc724
Convert AuthenticationData to be a protocol
mohannad-hassan Jan 5, 2025
f07289d
Provide configurations to AuthenticationClient on initialization
mohannad-hassan Jan 5, 2025
8f80afa
Some linting issues
mohannad-hassan Jan 5, 2025
4f7000a
Fix typo in Persistence's name
mohannad-hassan Jan 5, 2025
2731407
Fix compilation issues in AuthenticationClientTests
mohannad-hassan Jan 5, 2025
87d8060
Create OAuthService and AppAuthOAuthService
mohannad-hassan Jan 7, 2025
ef596f9
Refactor AuthentincationClientImpl to use the new structure of OAuth …
mohannad-hassan Jan 8, 2025
09a97f1
Remove OAuthCaller and AuthenticationData
mohannad-hassan Jan 8, 2025
5b96158
Refactor AuthenticationClient to assume configurations always set
mohannad-hassan Jan 8, 2025
a316eb1
Revise errors throwb by AppAuthOAuthService
mohannad-hassan Jan 8, 2025
ba45925
Capture some errors in AuthenticationClient
mohannad-hassan Jan 8, 2025
6bb436f
Convert AuthentincationClientImpl to an actor to avoid possible data …
mohannad-hassan Jan 9, 2025
bc1e8f9
Provide coverage for exceptional scenarios
mohannad-hassan Jan 10, 2025
2e2ebe2
PRovide some documentation to OAuthService
mohannad-hassan Jan 10, 2025
d5a4ecb
Move OAuthService to a Core package
mohannad-hassan Jan 10, 2025
d878504
Handle some exceptional scenarios for login in AuthentincationClientImpl
mohannad-hassan Jan 10, 2025
4cfe501
Linting
mohannad-hassan Jan 10, 2025
644cef1
Update AuthenticationClientConfiguration
mohannad-hassan Jan 10, 2025
7edc942
Update data if data already exists in Persistence
mohannad-hassan Jan 17, 2025
f813565
Linting
mohannad-hassan Jan 17, 2025
06786f1
Address some PR comments
mohannad-hassan Jan 20, 2025
4535c99
Refactor KeychainPersistance to use an abstraction for keychain access
mohannad-hassan Jan 21, 2025
7cec053
Extract KeychainAccess and Keychain KeychainAccessFake to SystemDepen…
mohannad-hassan Jan 21, 2025
f159c45
Refactor Persistence to make it keyed
mohannad-hassan Jan 21, 2025
a2f99f8
Extract SecurePersistence in a separate Core package
mohannad-hassan Jan 21, 2025
989f5b4
Linting
mohannad-hassan Jan 21, 2025
353999e
Linting SecurePersistence
mohannad-hassan Jan 21, 2025
4497234
Extract AppAuthOAuthService to a separate Core package
mohannad-hassan Jan 21, 2025
0ccf658
Refactor mocks in AuthenticationClientTests preparing for extraction
mohannad-hassan Jan 22, 2025
08ce882
Extract OAuthServiceFake into a separate Core package
mohannad-hassan Jan 22, 2025
b171b23
Linting
mohannad-hassan Jan 22, 2025
1277091
More linting
mohannad-hassan Jan 22, 2025
b029a44
Make AppAuthStateData an internal declaration
mohannad-hassan Jan 22, 2025
a79386f
Some cleanup
mohannad-hassan Jan 23, 2025
3d52487
Rename AppAuthOAuthService to OAuthServiceAppAuthImpl
mohannad-hassan Jan 23, 2025
d8a1bf6
Linting
mohannad-hassan Jan 23, 2025
950cdd0
Rename SecurityAccessFake to KeychainAccessFake
mohannad-hassan Jan 25, 2025
ebce80b
Move OAuth app configurations from OAuthService to OAuthServiceAppAut…
mohannad-hassan Jan 25, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 121 additions & 0 deletions Data/AuthenticationClient/Sources/AuthenticationData.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
//
// AuthenticationData.swift
// QuranEngine
//
// Created by Mohannad Hassan on 27/12/2024.
//

import AppAuth
import Combine
import Foundation
import VLogging

enum AuthenticationStateError: Error {
/// Throws when the refresh token operation fails. Assume that the user is not authenticated anymore.
case failedToRefreshTokens(Error?)

/// Failed to decode the persisted state back.
case decodingError(Error?)
}

/// A wrapper for the authentication state's data.
///
/// The abstraction is mainly for testing purposes. The API has been designed to be in conjunction
/// with the `AppAuth's OIDAuthState` class.
class AuthenticationData: NSObject, Codable {
/// Invokes subscribers when the state changes. Usually happens during refreshing tokens.
var stateChangedPublisher: AnyPublisher<Void, Never> { fatalError() }

var isAuthorized: Bool {
fatalError()
}

/// Returns fresh access token to be used for API requests.
///
/// - throws: `AuthenticationStateError.failedToRefreshTokens` if the
/// refresh operation fails for any reason.
func getFreshTokens() async throws -> String {
fatalError()
}

override init() { }

required init(from decoder: any Decoder) throws {
fatalError()
}
}
mohannad-hassan marked this conversation as resolved.
Show resolved Hide resolved

class AppAuthAuthenticationData: AuthenticationData {
private enum CodingKeys: String, CodingKey {
case state
}

private let stateChangedSubject: PassthroughSubject<Void, Never> = .init()
override var stateChangedPublisher: AnyPublisher<Void, Never> {
stateChangedSubject.eraseToAnyPublisher()
}

private var state: OIDAuthState {
didSet {
stateChangedSubject.send()
}
}

override var isAuthorized: Bool {
state.isAuthorized
}

init(state: OIDAuthState) {
self.state = state
super.init()
state.stateChangeDelegate = self
}

required init(from decoder: any Decoder) throws {
do {
let container = try decoder.container(keyedBy: CodingKeys.self)
let data = try container.decode(Data.self, forKey: .state)
guard let state = try NSKeyedUnarchiver.unarchivedObject(ofClass: OIDAuthState.self, from: data) else {
logger.error("Failed to decode OIDAuthState: Failed to unarchive data")
throw AuthenticationStateError.decodingError(nil)
}
self.state = state
} catch {
logger.error("Failed to decode OIDAuthState: \(error)")
throw AuthenticationStateError.decodingError(error)
}
super.init()
state.stateChangeDelegate = self
}

override func encode(to encoder: any Encoder) throws {
var container: KeyedEncodingContainer<CodingKeys> = encoder.container(keyedBy: CodingKeys.self)
let data = try NSKeyedArchiver.archivedData(withRootObject: state, requiringSecureCoding: true)
try container.encode(data, forKey: .state)
}

override func getFreshTokens() async throws -> String {
return try await withCheckedThrowingContinuation { continuation in
state.performAction { accessToken, clientID, error in
guard error == nil else {
logger.error("Failed to refresh tokens: \(error!)")
continuation.resume(throwing: AuthenticationStateError.failedToRefreshTokens(error))
return
}
guard let accessToken else {
logger.error("Failed to refresh tokens: No access token returned. An unexpected situation.")
continuation.resume(throwing: AuthenticationStateError.failedToRefreshTokens(nil))
return
}
continuation.resume(returning: accessToken)
}
}
}
}

extension AppAuthAuthenticationData: OIDAuthStateChangeDelegate {
func didChange(_ state: OIDAuthState) {
logger.info("OIDAuthState changed - isAuthorized: \(state.isAuthorized)")
stateChangedSubject.send()
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//
// OAuthClient.swift
// AuthentincationDataManager.swift
mohannad-hassan marked this conversation as resolved.
Show resolved Hide resolved
// QuranEngine
//
// Created by Mohannad Hassan on 19/12/2024.
Expand All @@ -10,8 +10,23 @@ import UIKit

public enum OAuthClientError: Error {
case oauthClientHasNotBeenSet
case errorFetchingConfiguration(Error?)
case errorAuthenticating(Error?)

/// Thrown when an operation, that needs authentication, is attempted wheile the client
mohannad-hassan marked this conversation as resolved.
Show resolved Hide resolved
/// hasn't been authenticated or if the client's access has been revoked.
case clientIsNotAuthenticated
}

public enum AuthenticationState: Equatable {
/// Authentication is not available. Any action dependent on authentication
/// (such as logging in or invoking user APIs) should not be attempted..
case notAvailable

/// No user is currently authenticated, or access has been revoked or is expired.
/// Logging in is availble and is required for further APIs.
case notAuthenticated

case authenticated
}

public struct OAuthAppConfiguration {
Expand All @@ -32,13 +47,21 @@ public struct OAuthAppConfiguration {

/// Handles the OAuth flow to Quran.com
///
/// Note that the connection relies on dicvoering the configuration from the issuer service.
public protocol OAuthClient {
/// Expected to be configuered with the host app's OAuth configuration before further operations are attempted.
public protocol AuthentincationDataManager {
/// Sets the app configuration to be used for authentication.
func set(appConfiguration: OAuthAppConfiguration)
mohamede1945 marked this conversation as resolved.
Show resolved Hide resolved

/// Performs the login flow to Quran.com
///
/// - Parameter viewController: The view controller to be used as base for presenting the login flow.
/// - Returns: Nothing is returned for now. The client may return the profile infromation in the future.
func login(on viewController: UIViewController) async throws

/// Returns `true` if the client is authenticated.
func restoreState() async throws -> Bool
mohamede1945 marked this conversation as resolved.
Show resolved Hide resolved

func authenticate(request: URLRequest) async throws -> URLRequest

var authenticationState: AuthenticationState { get }
}
120 changes: 120 additions & 0 deletions Data/AuthenticationClient/Sources/AuthentincationDataManagerImpl.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
//
// AuthentincationDataManagerImpl.swift
// QuranEngine
//
// Created by Mohannad Hassan on 23/12/2024.
//

import AppAuth
import Combine
import Foundation
import UIKit
import VLogging

public final class AuthentincationDataManagerImpl: AuthentincationDataManager {
// MARK: Lifecycle

init(caller: OAuthCaller, persistance: Persistance) {
self.caller = caller
self.persistance = persistance
}

// MARK: Public

public var authenticationState: AuthenticationState {
guard appConfiguration != nil else {
return .notAvailable
}
return state?.isAuthorized == true ? .authenticated : .notAuthenticated
}

public func set(appConfiguration: OAuthAppConfiguration) {
self.appConfiguration = appConfiguration
}

public func login(on viewController: UIViewController) async throws {
do {
try persistance.clear()
logger.info("Cleared previous authentication state before login")
} catch {
// If persisting the new state works, this error should be of little concern.
logger.warning("Failed to clear previous authentication state before login: \(error)")
}

guard let configuration = appConfiguration else {
logger.error("login invoked without OAuth client configurations being set")
throw OAuthClientError.oauthClientHasNotBeenSet
}

let state = try await caller.login(using: configuration, on: viewController)
self.state = state
logger.info("login succeeded with state. isAuthorized: \(state.isAuthorized)")
persist(state: state)
}

public func restoreState() async throws -> Bool {
guard appConfiguration != nil else {
logger.error("restoreState invoked without OAuth client configurations being set")
throw OAuthClientError.oauthClientHasNotBeenSet
}
guard let state = try persistance.retrieve() else {
logger.info("No previous authentication state found")
return false
}
// TODO: Called for the side effects!
_ = try await state.getFreshTokens()
self.state = state
logger.info("Restored previous authentication state. isAuthorized: \(state.isAuthorized)")
return state.isAuthorized
}

public func authenticate(request: URLRequest) async throws -> URLRequest {
guard let configuration = appConfiguration else {
logger.error("authenticate invoked without OAuth client configurations being set")
throw OAuthClientError.oauthClientHasNotBeenSet
}
guard authenticationState == .authenticated, let state else {
logger.error("authenticate invoked without client being authenticated")
throw OAuthClientError.clientIsNotAuthenticated
}
let token = try await state.getFreshTokens()
var request = request
request.setValue(token, forHTTPHeaderField: "x-auth-token")
request.setValue(configuration.clientID, forHTTPHeaderField: "x-client-id")
return request
}

// MARK: Private

private let caller: OAuthCaller
private let persistance: Persistance

private var cancellables = Set<AnyCancellable>()

private var appConfiguration: OAuthAppConfiguration?

private var state: AuthenticationData? {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property is not thread-safe and needs protection. I recommend converting this class into an actor to ensure thread safety.

didSet {
guard let state else { return }
state.stateChangedPublisher.sink { [weak self] _ in
self?.persist(state: state)
}.store(in: &cancellables)
mohannad-hassan marked this conversation as resolved.
Show resolved Hide resolved
}
}

private func persist(state: AuthenticationData) {
do {
try persistance.persist(state: state)
} catch {
// If this happens, the state will not nullified so to keep the current session usable
// for the user. As for now, no workaround is in hand.
logger.error("Failed to persist authentication state. No workaround in hand.: \(error)")
}
}
}

extension AuthentincationDataManagerImpl {
public convenience init() {
self.init(caller: AppAuthCaller(), persistance: KeychainPersistance())
}
}
Loading