Skip to content

Conversation

spetrescu84
Copy link
Contributor

Proposed changes

Updated E2E tests for Email OTP MFA for EC Implementation. The test test_signInUsingPasswordWithMFANoDefaultAuthMethod_completeSuccessfully was removed as it was incorrect since the user has to specify a method now.

Type of change

  • Feature work
  • Bug fix
  • Documentation
  • Engineering change
  • Test
  • Logging/Telemetry

Risk

  • High – Errors could cause MAJOR regression of many scenarios. (Example: new large features or high level infrastructure changes)
  • Medium – Errors could cause regression of 1 or more scenarios. (Example: somewhat complex bug fixes, small new features)
  • Small – No issues are expected. (Example: Very small bug fixes, string changes, or configuration settings changes)

Additional information

@spetrescu84 spetrescu84 self-assigned this Aug 29, 2025
@Copilot Copilot AI review requested due to automatic review settings August 29, 2025 08:24
@spetrescu84 spetrescu84 requested review from a team as code owners August 29, 2025 08:24
@spetrescu84 spetrescu84 requested a review from Veena11 August 29, 2025 08:24
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates the iOS SDK's Email OTP MFA implementation for Enhanced Connect (EC) by changing the API to require explicit authentication method selection rather than allowing automatic defaults. The key changes streamline the MFA flow by removing the separate auth method retrieval step and integrating auth methods directly into the awaiting MFA state.

  • Removed getAuthMethods functionality and related components as users must now specify auth methods explicitly
  • Updated MFA request challenge APIs to require MSALAuthMethod parameter instead of optional
  • Modified sign-in delegates to automatically provide auth methods when transitioning to MFA state

Reviewed Changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
MSAL/src/native_auth/public/state_machine/state/MFAStates.swift Removed getAuthMethods method and made authMethod parameter required in requestChallenge
MSAL/src/native_auth/public/state_machine/delegate/SignInDelegates.swift Updated delegate methods to include auth methods parameter
MSAL/src/native_auth/controllers/sign_in/MSALNativeAuthSignInController.swift Integrated auth method retrieval into token response handling and updated grant types
MSAL/test/integration/native_auth/end_to_end/mfa/MSALNativeAuthSignInWithMFAEndToEndTests.swift Updated tests to use new API pattern and removed obsolete test cases
Multiple test files Updated unit tests to reflect API changes and removed tests for deprecated functionality

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 42 to 43
func test_defaultErrorDescription() {
let sut: [MFAGetAuthMethodsError] = [
let sut: [MFARequestChallengeError] = [
Copy link

Copilot AI Aug 29, 2025

Choose a reason for hiding this comment

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

The variable type annotation was changed from [MFAGetAuthMethodsError] to [MFARequestChallengeError] but this appears to be a correction rather than an error, as the test method name and context suggest it should be testing MFARequestChallengeError. However, verify that this change aligns with the intended test behavior.

Copilot uses AI. Check for mistakes.

// MARK: private methods


struct SingInUsernameAndPasswordResult {
Copy link

Copilot AI Aug 29, 2025

Choose a reason for hiding this comment

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

The struct name has a typo: 'SingInUsernameAndPasswordResult' should be 'SignInUsernameAndPasswordResult'.

Copilot uses AI. Check for mistakes.

@spetrescu84 spetrescu84 changed the base branch from dev to feature/email-otp-mfa August 29, 2025 08:51
@nilo-ms nilo-ms added the native-auth Code related to native authentication label Aug 29, 2025
@nilo-ms nilo-ms merged commit a75e0a4 into feature/email-otp-mfa Sep 3, 2025
9 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

native-auth Code related to native authentication

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants