Skip to content

Commit

Permalink
Unify async-delegate continuations and fix autofill-to-modal data race (
Browse files Browse the repository at this point in the history
#29)

Instead of maintaining a bunch of internal state and logic to figure out
whether a registration, modal authentication, or autofill authentication
is currently being processed, this collapses the logic significantly to
have a single continuation that manages both modal paths. Now at the
start of all public calls, `reset()` is triggered which should reliably
reset (and clear out) all of the continuation stuff and

Autofill is a little finicky still, but upon further testing on
different branches, it looks like some of the assumptions made in the
initial design (with a delegate class for clients) were incorrect and it
should be possible to move that to async as well. That'll be addressed
separately; for the moment the existing API will continue to work,

Fixes #25
  • Loading branch information
Firehed authored Jul 28, 2024
1 parent 3154457 commit 18f7e89
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 47 deletions.
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

0 comments on commit 18f7e89

Please sign in to comment.