Skip to content

feat(auth): add content slot to MFA enrolment screen - #1415

Draft
demolaf wants to merge 4 commits into
auth/phone-content-slotsfrom
auth/mfa-enrolment-content-slot
Draft

demolaf wants to merge 4 commits into
auth/phone-content-slotsfrom
auth/mfa-enrolment-content-slot

Conversation

@demolaf

@demolaf demolaf commented Oct 21, 2026 •

Copy link
Copy Markdown
Member

Adds public MFAEnrolmentScreen, so apps can draw the MFA enrolment screen themselves while the library keeps starting enrolment, sending codes, reauthenticating and reporting errors. Stacked on #1412.

Changes

  • MFAEnrolmentScreen owns the enrolment state and passes its content an MFAEnrollmentContentState: the current step, Bindings for input, closures for actions, the TOTP secret and QR URL, plus isValid, isLoading and a localized errorMessage.
  • DefaultMFAEnrolmentContent holds the stock markup, and MFAEnrolmentView now renders it through the screen. Reauthentication stays inside the screen.
  • Actions ignore repeated taps while one is in flight, and isLoading also covers the AuthService signing in.
  • Get Started stays disabled until the selected factor is allowed (it used to start SMS enrolment regardless), and sessions that expire or finish return to factor selection.
  • Unit tests cover the step mapping, isValid rules and action guards.

API Usage

AuthPickerView { authenticatedApp }
  .pickerDestination { screen in
    switch screen {
    case .mfaEnrollment:
      MFAEnrolmentScreen { state in
        switch state.step {
        case .enterPhoneNumber:
          TextField("Phone number", text: state.phoneNumber)
          TextField("Device name", text: state.displayName)
          Button("Send code", action: state.sendCode)
            .disabled(!state.isValid || state.isLoading)
        default:
          DefaultMFAEnrolmentContent(state: state)
        }
      }
    default:
      DefaultAuthPickerDestination(screen: screen)
    }
  }

Preview


Maintainer note: Fixes internal CPRN-520

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the multi-factor authentication (MFA) enrollment UI by splitting it into a reusable state container MFAEnrolmentScreen, a default UI component DefaultMFAEnrolmentContent, and updating MFAEnrolmentView to use these new components. The review feedback suggests using SwiftUI's native @Environment(\.openURL) instead of UIApplication.shared.open for better platform support and testability. Additionally, it recommends avoiding recreating CIContext on every invocation of generateQRCode to prevent performance degradation during view updates.

Comment on lines +31 to +33
@Environment(AuthService.self) private var authService
@FocusState private var focus: FocusableField?
private let state: MFAEnrollmentContentState

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Use SwiftUI's native openURL environment action instead of accessing UIApplication.shared directly. This is more idiomatic in SwiftUI, supports platform-agnostic URL opening, and improves testability.

Suggested change
@Environment(AuthService.self) private var authService
@FocusState private var focus: FocusableField?
private let state: MFAEnrollmentContentState
@Environment(AuthService.self) private var authService
@Environment(\.openURL) private var openURL
@FocusState private var focus: FocusableField?
private let state: MFAEnrollmentContentState

Comment on lines +341 to +343
Button(action: {
UIApplication.shared.open(qrURL)
}) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Use the openURL environment action instead of UIApplication.shared.open.

Suggested change
Button(action: {
UIApplication.shared.open(qrURL)
}) {
Button(action: {
openURL(qrURL)
}) {

Comment on lines +457 to +476
private func generateQRCode(from string: String) -> UIImage? {
let data = Data(string.utf8)

guard let filter = CIFilter(name: "CIQRCodeGenerator") else { return nil }
filter.setValue(data, forKey: "inputMessage")
filter.setValue("H", forKey: "inputCorrectionLevel")

guard let ciImage = filter.outputImage else { return nil }

// Scale up the QR code for better quality
let transform = CGAffineTransform(scaleX: 10, y: 10)
let scaledImage = ciImage.transformed(by: transform)

let context = CIContext()
guard let cgImage = context.createCGImage(scaledImage, from: scaledImage.extent) else {
return nil
}

return UIImage(cgImage: cgImage)
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

Creating a CIContext is a very expensive operation. Since generateQRCode is called directly during the view's body rendering, recreating the CIContext on every view update will cause significant performance degradation. Reusing a single CIContext instance via a local static let avoids this overhead.

Suggested change
private func generateQRCode(from string: String) -> UIImage? {
let data = Data(string.utf8)
guard let filter = CIFilter(name: "CIQRCodeGenerator") else { return nil }
filter.setValue(data, forKey: "inputMessage")
filter.setValue("H", forKey: "inputCorrectionLevel")
guard let ciImage = filter.outputImage else { return nil }
// Scale up the QR code for better quality
let transform = CGAffineTransform(scaleX: 10, y: 10)
let scaledImage = ciImage.transformed(by: transform)
let context = CIContext()
guard let cgImage = context.createCGImage(scaledImage, from: scaledImage.extent) else {
return nil
}
return UIImage(cgImage: cgImage)
}
private func generateQRCode(from string: String) -> UIImage? {
let data = Data(string.utf8)
guard let filter = CIFilter(name: "CIQRCodeGenerator") else { return nil }
filter.setValue(data, forKey: "inputMessage")
filter.setValue("H", forKey: "inputCorrectionLevel")
guard let ciImage = filter.outputImage else { return nil }
// Scale up the QR code for better quality
let transform = CGAffineTransform(scaleX: 10, y: 10)
let scaledImage = ciImage.transformed(by: transform)
static let context = CIContext()
guard let cgImage = context.createCGImage(scaledImage, from: scaledImage.extent) else {
return nil
}
return UIImage(cgImage: cgImage)
}

@demolaf
demolaf force-pushed the auth/phone-content-slots branch from cc01c83 to ca1c05e Compare October 22, 2026 11:45
@demolaf
demolaf force-pushed the auth/mfa-enrolment-content-slot branch 2 times, most recently from 0a6209e to 34071fe Compare October 22, 2026 13:34
@demolaf
demolaf force-pushed the auth/phone-content-slots branch from ca1c05e to 5336ee4 Compare October 22, 2026 13:34
@demolaf
demolaf force-pushed the auth/mfa-enrolment-content-slot branch from 34071fe to e1e5941 Compare October 22, 2026 13:48
@demolaf
demolaf force-pushed the auth/phone-content-slots branch 2 times, most recently from 75d9824 to ba7ecda Compare October 22, 2026 14:09
Add public MFAEnrolmentScreen, which owns the enrolment state, reauthentication and error reporting and passes its content an MFAEnrollmentContentState. DefaultMFAEnrolmentContent holds the stock markup, and MFAEnrolmentView now renders it through the screen.
Map finished enrolment sessions back to factor selection, require the selected factor to be allowed before starting, keep copy feedback from being cut short, clear focus between steps, and cover the step and isValid rules with unit tests.
Map sessions past expiresAt back to factor selection, only send codes from the SMS steps, disable Resend Code while loading, and note that custom content sets its own navigation title.
Move the startEnrollment, sendCode and enroll guards into Step helpers with unit tests, and replace the unused focus enum with a single phone number focus flag.
@demolaf
demolaf force-pushed the auth/mfa-enrolment-content-slot branch from e1e5941 to bdc298d Compare October 22, 2026 14:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant