-
Notifications
You must be signed in to change notification settings - Fork 163
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
Changes from 61 commits
Commits
Show all changes
63 commits
Select commit
Hold shift + click to select a range
1f0adfa
Reanem OAuthClient to AuthentincationDataManager
mohannad-hassan af28e6e
Create AppAuthCaller to wrap AppAuth calls
mohannad-hassan 17886fb
Refactor to use the new AppAuthCaller
mohannad-hassan 6a2ee35
Create AppAuthOAuthClientTests in OAuthClient
mohannad-hassan a3f8cca
Wrap OIDAuthState within AuthenticationState
mohannad-hassan 3f1af6b
Persist the tokens after logging in
mohannad-hassan a3a700d
Provide a way to restoreState for AuthentincationDataManager
mohannad-hassan b2614b7
Authenitcate rquests in AuthentincationDataManager
mohannad-hassan 986408a
Rename AuthenticationState to AuthenticationData
mohannad-hassan f601c03
Make AuthentincationDataManager expose an authentication state
mohannad-hassan 1637fc1
Persist updated state
mohannad-hassan b9b5598
Refresh authentication data manager on launch startup
mohannad-hassan 008a93e
Rename AppAuthOAuthClient to AuthentincationDataManagerImpl
mohannad-hassan eaf968a
Organize errors and logs
mohannad-hassan 458d7bb
checking in package resolution for AppAuth-iOS
mohannad-hassan 9b71d61
Linting
mohannad-hassan b8aebcb
Remove configurations
mohannad-hassan a54ceed
Fix linting issues in LaunchStartup
mohannad-hassan 9a4fd81
Documentation
mohannad-hassan c0f4c25
Rename some internal types for brevity
mohannad-hassan a5fa1a8
Rename OAuthClient package to AuthenticationClient
mohannad-hassan bc85910
Rename AuthentincationDataManager to AuthenticationClient
mohannad-hassan 2fa50e6
Some cleanup in AuthenticationClientTests
mohannad-hassan 90bd841
Revert changes in Features/AppStructureFeature -- Pending integration…
mohannad-hassan 8049a8b
Fix a compilation issue
mohannad-hassan 3cdc724
Convert AuthenticationData to be a protocol
mohannad-hassan f07289d
Provide configurations to AuthenticationClient on initialization
mohannad-hassan 8f80afa
Some linting issues
mohannad-hassan 4f7000a
Fix typo in Persistence's name
mohannad-hassan 2731407
Fix compilation issues in AuthenticationClientTests
mohannad-hassan 87d8060
Create OAuthService and AppAuthOAuthService
mohannad-hassan ef596f9
Refactor AuthentincationClientImpl to use the new structure of OAuth …
mohannad-hassan 09a97f1
Remove OAuthCaller and AuthenticationData
mohannad-hassan 5b96158
Refactor AuthenticationClient to assume configurations always set
mohannad-hassan a316eb1
Revise errors throwb by AppAuthOAuthService
mohannad-hassan ba45925
Capture some errors in AuthenticationClient
mohannad-hassan 6bb436f
Convert AuthentincationClientImpl to an actor to avoid possible data …
mohannad-hassan bc1e8f9
Provide coverage for exceptional scenarios
mohannad-hassan 2e2ebe2
PRovide some documentation to OAuthService
mohannad-hassan d5a4ecb
Move OAuthService to a Core package
mohannad-hassan d878504
Handle some exceptional scenarios for login in AuthentincationClientImpl
mohannad-hassan 4cfe501
Linting
mohannad-hassan 644cef1
Update AuthenticationClientConfiguration
mohannad-hassan 7edc942
Update data if data already exists in Persistence
mohannad-hassan f813565
Linting
mohannad-hassan 06786f1
Address some PR comments
mohannad-hassan 4535c99
Refactor KeychainPersistance to use an abstraction for keychain access
mohannad-hassan 7cec053
Extract KeychainAccess and Keychain KeychainAccessFake to SystemDepen…
mohannad-hassan f159c45
Refactor Persistence to make it keyed
mohannad-hassan a2f99f8
Extract SecurePersistence in a separate Core package
mohannad-hassan 989f5b4
Linting
mohannad-hassan 353999e
Linting SecurePersistence
mohannad-hassan 4497234
Extract AppAuthOAuthService to a separate Core package
mohannad-hassan 0ccf658
Refactor mocks in AuthenticationClientTests preparing for extraction
mohannad-hassan 08ce882
Extract OAuthServiceFake into a separate Core package
mohannad-hassan b171b23
Linting
mohannad-hassan 1277091
More linting
mohannad-hassan b029a44
Make AppAuthStateData an internal declaration
mohannad-hassan a79386f
Some cleanup
mohannad-hassan 3d52487
Rename AppAuthOAuthService to OAuthServiceAppAuthImpl
mohannad-hassan d8a1bf6
Linting
mohannad-hassan 950cdd0
Rename SecurityAccessFake to KeychainAccessFake
mohannad-hassan ebce80b
Move OAuth app configurations from OAuthService to OAuthServiceAppAut…
mohannad-hassan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
// | ||
// OAuthService.swift | ||
// QuranEngine | ||
// | ||
// Created by Mohannad Hassan on 08/01/2025. | ||
// | ||
|
||
import Foundation | ||
import UIKit | ||
|
||
public struct OAuthServiceConfiguration { | ||
public let clientID: String | ||
public let redirectURL: URL | ||
/// The client requests the `offline` and `openid` scopes by default. | ||
public let scopes: [String] | ||
public let authorizationIssuerURL: URL | ||
|
||
public init(clientID: String, redirectURL: URL, scopes: [String], authorizationIssuerURL: URL) { | ||
self.clientID = clientID | ||
self.redirectURL = redirectURL | ||
self.scopes = scopes | ||
self.authorizationIssuerURL = authorizationIssuerURL | ||
} | ||
} | ||
|
||
public enum OAuthServiceError: Error { | ||
case failedToRefreshTokens(Error?) | ||
|
||
case stateDataDecodingError(Error?) | ||
|
||
case failedToDiscoverService(Error?) | ||
|
||
case failedToAuthenticate(Error?) | ||
} | ||
|
||
/// Encapsulates the OAuth state data. Should only be managed and mutated by `OAuthService.` | ||
public protocol OAuthStateData { | ||
var isAuthorized: Bool { get } | ||
} | ||
|
||
/// An abstraction for handling the OAuth flow steps. | ||
/// | ||
/// The service is assumed not to have any internal state. It's the responsibility of the client of this service | ||
/// to hold and persist the state data. Each call to the service returns an updated `OAuthStateData` | ||
/// that reflects the latest state. | ||
public protocol OAuthService { | ||
/// Attempts to discover the authentication services and redirects the user to the authentication service. | ||
func login(on viewController: UIViewController) async throws -> OAuthStateData | ||
|
||
func getAccessToken(using data: OAuthStateData) async throws -> (String, OAuthStateData) | ||
|
||
func refreshAccessTokenIfNeeded(data: OAuthStateData) async throws -> OAuthStateData | ||
} | ||
|
||
/// Encodes and decodes the `OAuthStateData`. A convneience to hide the conforming `OAuthStateData` type | ||
/// while preparing the state for persistence. | ||
public protocol OAuthStateDataEncoder { | ||
func encode(_ data: OAuthStateData) throws -> Data | ||
|
||
func decode(_ data: Data) throws -> OAuthStateData | ||
} |
158 changes: 158 additions & 0 deletions
158
Core/OAuthServiceAppAuthImpl/OAuthServiceAppAuthImpl.swift
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,158 @@ | ||
// | ||
// OAuthServiceAppAuthImpl.swift | ||
// QuranEngine | ||
// | ||
// Created by Mohannad Hassan on 08/01/2025. | ||
// | ||
|
||
import AppAuth | ||
import OAuthService | ||
import UIKit | ||
import VLogging | ||
|
||
struct AppAuthStateData: OAuthStateData { | ||
let state: OIDAuthState | ||
|
||
public var isAuthorized: Bool { state.isAuthorized } | ||
} | ||
|
||
public struct OAuthStateEncoderAppAuthImpl: OAuthStateDataEncoder { | ||
public init() { } | ||
|
||
public func encode(_ data: any OAuthStateData) throws -> Data { | ||
guard let data = data as? AppAuthStateData else { | ||
fatalError() | ||
} | ||
let encoded = try NSKeyedArchiver.archivedData( | ||
withRootObject: data.state, | ||
requiringSecureCoding: true | ||
) | ||
return encoded | ||
} | ||
|
||
public func decode(_ data: Data) throws -> any OAuthStateData { | ||
guard let state = try NSKeyedUnarchiver.unarchivedObject(ofClass: OIDAuthState.self, from: data) else { | ||
throw OAuthServiceError.stateDataDecodingError(nil) | ||
} | ||
return AppAuthStateData(state: state) | ||
} | ||
} | ||
|
||
public final class OAuthServiceAppAuthImpl: OAuthService { | ||
// MARK: Lifecycle | ||
|
||
public init(appConfigurations: OAuthServiceConfiguration) { | ||
self.appConfigurations = appConfigurations | ||
} | ||
|
||
// MARK: Public | ||
|
||
public func login(on viewController: UIViewController) async throws -> any OAuthStateData { | ||
let serviceConfiguration = try await discoverConfiguration(forIssuer: appConfigurations.authorizationIssuerURL) | ||
let state = try await login( | ||
withConfiguration: serviceConfiguration, | ||
appConfiguration: appConfigurations, | ||
on: viewController | ||
) | ||
return AppAuthStateData(state: state) | ||
} | ||
|
||
public func getAccessToken(using data: any OAuthStateData) async throws -> (String, any OAuthStateData) { | ||
guard let data = data as? AppAuthStateData else { | ||
// This should be a fatal error. | ||
fatalError() | ||
} | ||
return try await withCheckedThrowingContinuation { continuation in | ||
data.state.performAction { accessToken, clientID, error in | ||
guard error == nil else { | ||
logger.error("Failed to refresh tokens: \(error!)") | ||
continuation.resume(throwing: OAuthServiceError.failedToRefreshTokens(error)) | ||
return | ||
} | ||
guard let accessToken else { | ||
logger.error("Failed to refresh tokens: No access token returned. An unexpected situation.") | ||
continuation.resume(throwing: OAuthServiceError.failedToRefreshTokens(nil)) | ||
return | ||
} | ||
let updatedData = AppAuthStateData(state: data.state) | ||
continuation.resume(returning: (accessToken, updatedData)) | ||
} | ||
} | ||
} | ||
|
||
public func refreshAccessTokenIfNeeded(data: any OAuthStateData) async throws -> any OAuthStateData { | ||
try await getAccessToken(using: data).1 | ||
} | ||
|
||
// MARK: Private | ||
|
||
private let appConfigurations: OAuthServiceConfiguration | ||
|
||
// Needed mainly for retention. | ||
private var authFlow: (any OIDExternalUserAgentSession)? | ||
|
||
// MARK: - Authenication Flow | ||
|
||
private func discoverConfiguration(forIssuer issuer: URL) async throws -> OIDServiceConfiguration { | ||
logger.info("Discovering configuration for OAuth") | ||
return try await withCheckedThrowingContinuation { continuation in | ||
OIDAuthorizationService | ||
.discoverConfiguration(forIssuer: issuer) { configuration, error in | ||
guard error == nil else { | ||
logger.error("Error fetching OAuth configuration: \(error!)") | ||
continuation.resume(throwing: OAuthServiceError.failedToDiscoverService(error)) | ||
return | ||
} | ||
guard let configuration else { | ||
// This should not happen | ||
logger.error("Error fetching OAuth configuration: no configuration was loaded. An unexpected situtation.") | ||
continuation.resume(throwing: OAuthServiceError.failedToDiscoverService(nil)) | ||
return | ||
} | ||
logger.info("OAuth configuration fetched successfully") | ||
continuation.resume(returning: configuration) | ||
} | ||
} | ||
} | ||
|
||
private func login( | ||
withConfiguration configuration: OIDServiceConfiguration, | ||
appConfiguration: OAuthServiceConfiguration, | ||
on viewController: UIViewController | ||
) async throws -> OIDAuthState { | ||
let scopes = [OIDScopeOpenID, OIDScopeProfile] + appConfiguration.scopes | ||
let request = OIDAuthorizationRequest( | ||
configuration: configuration, | ||
clientId: appConfiguration.clientID, | ||
clientSecret: nil, | ||
scopes: scopes, | ||
redirectURL: appConfiguration.redirectURL, | ||
responseType: OIDResponseTypeCode, | ||
additionalParameters: [:] | ||
) | ||
|
||
logger.info("Starting OAuth flow") | ||
return try await withCheckedThrowingContinuation { continuation in | ||
DispatchQueue.main.async { | ||
self.authFlow = OIDAuthState.authState( | ||
byPresenting: request, | ||
presenting: viewController | ||
) { [weak self] state, error in | ||
self?.authFlow = nil | ||
guard error == nil else { | ||
logger.error("Error authenticating: \(error!)") | ||
continuation.resume(throwing: OAuthServiceError.failedToAuthenticate(error)) | ||
return | ||
} | ||
guard let state else { | ||
logger.error("Error authenticating: no state returned. An unexpected situtation.") | ||
continuation.resume(throwing: OAuthServiceError.failedToAuthenticate(nil)) | ||
return | ||
} | ||
logger.info("OAuth flow completed successfully") | ||
continuation.resume(returning: state) | ||
} | ||
} | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
// | ||
// OAuthServiceFake.swift | ||
// QuranEngine | ||
// | ||
// Created by Mohannad Hassan on 22/01/2025. | ||
// | ||
|
||
import OAuthService | ||
import UIKit | ||
|
||
public struct OAuthStateEncoderFake: OAuthStateDataEncoder { | ||
public init() {} | ||
|
||
public func encode(_ data: any OAuthStateData) throws -> Data { | ||
guard let data = data as? OAuthStateDataFake else { | ||
fatalError() | ||
} | ||
return try JSONEncoder().encode(data) | ||
} | ||
|
||
public func decode(_ data: Data) throws -> any OAuthStateData { | ||
try JSONDecoder().decode(OAuthStateDataFake.self, from: data) | ||
} | ||
} | ||
|
||
public final class OAuthServiceFake: OAuthService { | ||
public enum AccessTokenBehavior { | ||
case success(String) | ||
case successWithNewData(String, any OAuthStateData) | ||
case failure(Error) | ||
|
||
func getToken() throws -> String { | ||
switch self { | ||
case .success(let token), .successWithNewData(let token, _): | ||
return token | ||
case .failure(let error): | ||
throw error | ||
} | ||
} | ||
|
||
func getStateData() throws -> (any OAuthStateData)? { | ||
switch self { | ||
case .success: | ||
return nil | ||
case .successWithNewData(_, let data): | ||
return data | ||
case .failure(let error): | ||
throw error | ||
} | ||
} | ||
} | ||
|
||
public init() {} | ||
|
||
public var loginResult: Result<OAuthStateData, Error>? | ||
|
||
public func login(on viewController: UIViewController) async throws -> any OAuthStateData { | ||
try loginResult!.get() | ||
} | ||
|
||
public var accessTokenRefreshBehavior: AccessTokenBehavior? | ||
|
||
public func getAccessToken(using data: any OAuthStateData) async throws -> (String, any OAuthStateData) { | ||
guard let behavior = accessTokenRefreshBehavior else { | ||
fatalError() | ||
} | ||
return (try behavior.getToken(), try behavior.getStateData() ?? data) | ||
} | ||
|
||
public func refreshAccessTokenIfNeeded(data: any OAuthStateData) async throws -> any OAuthStateData { | ||
try await getAccessToken(using: data).1 | ||
} | ||
} | ||
|
||
public final class OAuthStateDataFake: Equatable, Codable, OAuthStateData { | ||
enum Codingkey: String, CodingKey { | ||
case accessToken | ||
} | ||
|
||
public var accessToken: String? { | ||
didSet { | ||
guard oldValue != nil else { return } | ||
} | ||
} | ||
|
||
public init() { } | ||
|
||
public required init(from decoder: any Decoder) throws { | ||
let container = try decoder.container(keyedBy: Codingkey.self) | ||
accessToken = try container.decode(String.self, forKey: .accessToken) | ||
} | ||
|
||
public func encode(to encoder: any Encoder) throws { | ||
var container = encoder.container(keyedBy: Codingkey.self) | ||
try container.encode(accessToken, forKey: .accessToken) | ||
} | ||
|
||
public var isAuthorized: Bool { | ||
accessToken != nil | ||
} | ||
|
||
public static func == (lhs: OAuthStateDataFake, rhs: OAuthStateDataFake) -> Bool { | ||
lhs.accessToken == rhs.accessToken | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this needs to be part of the API, does it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's practically a Parameters Object.