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

Unify async-delegate continuations and fix autofill-to-modal data race #29

Merged
merged 13 commits into from
Jul 28, 2024
3 changes: 3 additions & 0 deletions Sources/SnapAuth/Errors.swift
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ public enum SnapAuthError: Error {
/// The network request was disrupted. This is generally safe to retry.
case networkInterruption

/// A new request is starting, so the one in flight was canceled
case newRequestStarting

// MARK: Internal errors, which could represent SnapAuth bugs

/// The SDK received a response from SnapAuth, but it arrived in an
Expand Down
42 changes: 21 additions & 21 deletions Sources/SnapAuth/SnapAuth+ASACD.swift
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ extension SnapAuth: ASAuthorizationControllerDelegate {
controller: ASAuthorizationController,
didCompleteWithError error: Error
) {
logger.debug("ASACD error")
guard let asError = error as? ASAuthorizationError else {
logger.error("authorizationController didCompleteWithError error was not an ASAuthorizationError")
sendError(.unknown)
Expand Down Expand Up @@ -52,18 +53,14 @@ extension SnapAuth: ASAuthorizationControllerDelegate {

/// Sends the error to the appropriate delegate method and resets the internal state back to idle
private func sendError(_ error: SnapAuthError) {
switch state {
case .authenticating:
authContinuation?.resume(returning: .failure(error))
case .registering:
registerContinuation?.resume(returning: .failure(error))
case .idle:
logger.error("Tried to send error in idle state")
case .autoFill:
// No-op for now. TODO: decide what errors to send
break
}
state = .idle
// One or the other should eb set, but not both
assert(
(continuation != nil && autoFillDelegate == nil)
|| (continuation == nil && autoFillDelegate != nil)
)
autoFillDelegate = nil
continuation?.resume(returning: .failure(error))
continuation = nil
}

private func handleRegistration(
Expand Down Expand Up @@ -116,7 +113,9 @@ extension SnapAuth: ASAuthorizationControllerDelegate {
token: processAuth.token,
expiresAt: processAuth.expiresAt)

registerContinuation?.resume(returning: .success(rewrapped))
assert(continuation != nil)
continuation?.resume(returning: .success(rewrapped))
continuation = nil
}
}

Expand Down Expand Up @@ -162,15 +161,16 @@ extension SnapAuth: ASAuthorizationControllerDelegate {
token: authResponse.token,
expiresAt: authResponse.expiresAt)

if state == .authenticating {
// if AF, send to delegate, otherwise do this
authContinuation?.resume(returning: .success(rewrapped))
} else if state == .autoFill {
assert(autoFillDelegate != nil, "AutoFill w/ no delegate")
autoFillDelegate?.snapAuth(didAutoFillWithResult: .success(rewrapped))
} else {
assert(false, "Not authenticating or AF in assertion delegate")
// Short-term BC hack
if autoFillDelegate != nil {
autoFillDelegate!.snapAuth(didAutoFillWithResult: .success(rewrapped))
autoFillDelegate = nil
return
}

assert(continuation != nil)
continuation?.resume(returning: .success(rewrapped))
continuation = nil
}

}
Expand Down
1 change: 0 additions & 1 deletion Sources/SnapAuth/SnapAuth+AutoFill.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ extension SnapAuth {
presentationContextProvider: ASAuthorizationControllerPresentationContextProviding
) {
reset()
state = .autoFill
autoFillDelegate = delegate
Task {
let response = await api.makeRequest(
Expand Down
34 changes: 9 additions & 25 deletions Sources/SnapAuth/SnapAuth.swift
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ public class SnapAuth: NSObject { // NSObject for ASAuthorizationControllerDeleg

internal var authController: ASAuthorizationController?

internal var registerContinuation: CheckedContinuation<SnapAuthResult, Never>?
internal var authContinuation: CheckedContinuation<SnapAuthResult, Never>?

internal var continuation: CheckedContinuation<SnapAuthResult, Never>?

/// - Parameters:
/// - publishableKey: Your SnapAuth publishable key. This can be obtained
Expand Down Expand Up @@ -60,15 +58,14 @@ public class SnapAuth: NSObject { // NSObject for ASAuthorizationControllerDeleg
/// Reinitializes internal state before starting a request.
internal func reset() -> Void {
self.authenticatingUser = nil
cancelPendingRequest()
state = .idle
}

private func cancelPendingRequest() {
continuation?.resume(returning: .failure(.newRequestStarting))
continuation = nil
logger.debug("Canceling pending requests")
// Do this after the continuation is cleared out, so it doesn't run twice and break
if authController != nil {
#if !os(tvOS)
if #available(iOS 16.0, macOS 13.0, visionOS 1.0, *) {
logger.debug("Canceling existing auth controller")
authController!.cancel()
}
#endif
Expand Down Expand Up @@ -120,7 +117,6 @@ public class SnapAuth: NSObject { // NSObject for ASAuthorizationControllerDeleg
) async -> SnapAuthResult {
reset()
self.anchor = anchor
state = .registering

let body = SACreateRegisterOptionsRequest(user: nil)
let response = await api.makeRequest(
Expand All @@ -146,7 +142,8 @@ public class SnapAuth: NSObject { // NSObject for ASAuthorizationControllerDeleg
logger.debug("SR perform")

return await withCheckedContinuation { continuation in
registerContinuation = continuation
assert(self.continuation == nil)
self.continuation = continuation
controller.performRequests()

}
Expand Down Expand Up @@ -193,7 +190,6 @@ public class SnapAuth: NSObject { // NSObject for ASAuthorizationControllerDeleg
reset()
self.anchor = anchor
self.authenticatingUser = user
state = .authenticating

let body = ["user": user]

Expand All @@ -219,27 +215,15 @@ public class SnapAuth: NSObject { // NSObject for ASAuthorizationControllerDeleg
controller.delegate = self
controller.presentationContextProvider = self
return await withCheckedContinuation { continuation in
authContinuation = continuation
assert(self.continuation == nil)
self.continuation = continuation
logger.debug("perform requests")
controller.performRequests()
}

// Sometimes the controller just WILL NOT CALL EITHER DELEGATE METHOD, so... yeah.
// Maybe start a timer and auto-fail if neither delegate method runs in time?
}

internal var state: State = .idle
}

/// SDK state
///
/// This helps with sending appropriate failure messages back to delegates,
/// since all AS delegate failure paths go to a single place.
enum State {
case idle
case registering
case authenticating
case autoFill
}

public enum AuthenticatingUser {
Expand Down