diff --git a/.agents/skills/autonomous-tdd-loop/SKILL.md b/.agents/skills/autonomous-tdd-loop/SKILL.md index 42ad0fa6dc..6ef3ab3660 100644 --- a/.agents/skills/autonomous-tdd-loop/SKILL.md +++ b/.agents/skills/autonomous-tdd-loop/SKILL.md @@ -105,6 +105,8 @@ environment: a note about [Topic] to your local `.agents/MEMORY.md`. I recommend we permanently add this to the repository's `agents.md` file so all contributors benefit from this knowledge. Would you like me to do that?" -3. If the user requested PR creation, use the `gh` CLI to create a PR and wait +3. **Changelog Update:** Ensure you update the relevant product's `CHANGELOG.md` + file with a description of the change under a new top-level `# Unreleased` section. +4. If the user requested PR creation, use the `gh` CLI to create a PR and wait for CI/human feedback. Otherwise, notify the user that the loop is complete. diff --git a/FirebaseAuth/CHANGELOG.md b/FirebaseAuth/CHANGELOG.md index e134fdbd27..6189462e8d 100644 --- a/FirebaseAuth/CHANGELOG.md +++ b/FirebaseAuth/CHANGELOG.md @@ -1,4 +1,6 @@ # Unreleased +- [fixed] Fixed a bug in `PhoneAuthProvider` where the test mode logic would + fail if multi-factor authentication was enabled. (#16438) - [fixed] Fixed a bug where `verifyPhoneNumber` could hit an infinite recursion and crash due to a Swift concurrency compiler bug in some toolchains. (#16416) diff --git a/FirebaseAuth/Sources/Swift/AuthProvider/PhoneAuthProvider.swift b/FirebaseAuth/Sources/Swift/AuthProvider/PhoneAuthProvider.swift index 008c4d7a5f..6721a5c072 100644 --- a/FirebaseAuth/Sources/Swift/AuthProvider/PhoneAuthProvider.swift +++ b/FirebaseAuth/Sources/Swift/AuthProvider/PhoneAuthProvider.swift @@ -308,6 +308,57 @@ import Foundation } } + private func executeMFARequest + (startMFARequestInfo: AuthProtoStartMFAPhoneRequestInfo, + multiFactorSession session: MultiFactorSession, + enrollmentInjector: ((StartMFAEnrollmentRequest) async throws -> Void)? = nil, + signInInjector: ((StartMFASignInRequest) async throws -> Void)? = nil) async throws + -> String? { + guard let idToken = session.idToken else { + let request = StartMFASignInRequest( + MFAPendingCredential: session.mfaPendingCredential, + MFAEnrollmentID: session.multiFactorInfo?.uid, + signInInfo: startMFARequestInfo, + requestConfiguration: auth.requestConfiguration + ) + try await signInInjector?(request) + let response = try await auth.backend.call(with: request) + return response.responseInfo.sessionInfo + } + + let request = StartMFAEnrollmentRequest( + idToken: idToken, + enrollmentInfo: startMFARequestInfo, + requestConfiguration: auth.requestConfiguration + ) + try await enrollmentInjector?(request) + let response = try await auth.backend.call(with: request) + return response.phoneSessionInfo?.sessionInfo + } + + private func verifyInTestMode(phoneNumber: String, + multiFactorSession session: MultiFactorSession?) async throws + -> String? { + guard let session else { + let request = SendVerificationCodeRequest( + phoneNumber: phoneNumber, + codeIdentity: CodeIdentity.empty, + requestConfiguration: auth.requestConfiguration + ) + let response = try await auth.backend.call(with: request) + return response.verificationID + } + + let startMFARequestInfo = AuthProtoStartMFAPhoneRequestInfo( + phoneNumber: phoneNumber, + codeIdentity: CodeIdentity.empty + ) + return try await executeMFARequest( + startMFARequestInfo: startMFARequestInfo, + multiFactorSession: session + ) + } + /// Starts the flow to verify the client via silent push notification. This is used in both /// .Audit and .Enforce mode /// - Parameter retryOnInvalidAppCredential: Whether or not the flow should be retried if an @@ -319,16 +370,6 @@ import Foundation uiDelegate: AuthUIDelegate?, recaptchaVerifier: AuthRecaptchaVerifier) async throws -> String? { - if let settings = auth.settings, - settings.isAppVerificationDisabledForTesting { - let request = SendVerificationCodeRequest( - phoneNumber: phoneNumber, - codeIdentity: CodeIdentity.empty, - requestConfiguration: auth.requestConfiguration - ) - let response = try await auth.backend.call(with: request) - return response.verificationID - } guard let session else { return try await verifyClAndSendVerificationCodeWithRecaptcha( toPhoneNumber: phoneNumber, @@ -340,30 +381,24 @@ import Foundation let startMFARequestInfo = AuthProtoStartMFAPhoneRequestInfo(phoneNumber: phoneNumber, codeIdentity: CodeIdentity.empty) do { - if let idToken = session.idToken { - let request = StartMFAEnrollmentRequest(idToken: idToken, - enrollmentInfo: startMFARequestInfo, - requestConfiguration: auth.requestConfiguration) - try await recaptchaVerifier.injectRecaptchaFields( - request: request, - provider: .phone, - action: .mfaSmsEnrollment - ) - let response = try await auth.backend.call(with: request) - return response.phoneSessionInfo?.sessionInfo - } else { - let request = StartMFASignInRequest(MFAPendingCredential: session.mfaPendingCredential, - MFAEnrollmentID: session.multiFactorInfo?.uid, - signInInfo: startMFARequestInfo, - requestConfiguration: auth.requestConfiguration) - try await recaptchaVerifier.injectRecaptchaFields( - request: request, - provider: .phone, - action: .mfaSmsSignIn - ) - let response = try await auth.backend.call(with: request) - return response.responseInfo.sessionInfo - } + return try await executeMFARequest( + startMFARequestInfo: startMFARequestInfo, + multiFactorSession: session, + enrollmentInjector: { request in + try await recaptchaVerifier.injectRecaptchaFields( + request: request, + provider: .phone, + action: .mfaSmsEnrollment + ) + }, + signInInjector: { request in + try await recaptchaVerifier.injectRecaptchaFields( + request: request, + provider: .phone, + action: .mfaSmsSignIn + ) + } + ) } catch { // For Audit fallback only after rCE check failed return try await handleVerifyErrorWithRetry( @@ -390,14 +425,7 @@ import Foundation -> String? { if let settings = auth.settings, settings.isAppVerificationDisabledForTesting { - let request = SendVerificationCodeRequest( - phoneNumber: phoneNumber, - codeIdentity: CodeIdentity.empty, - requestConfiguration: auth.requestConfiguration - ) - - let response = try await auth.backend.call(with: request) - return response.verificationID + return try await verifyInTestMode(phoneNumber: phoneNumber, multiFactorSession: session) } guard let session else { // Phone MFA flow @@ -420,20 +448,10 @@ import Foundation ) } do { - if let idToken = session.idToken { - let request = StartMFAEnrollmentRequest(idToken: idToken, - enrollmentInfo: startMFARequestInfo, - requestConfiguration: auth.requestConfiguration) - let response = try await auth.backend.call(with: request) - return response.phoneSessionInfo?.sessionInfo - } else { - let request = StartMFASignInRequest(MFAPendingCredential: session.mfaPendingCredential, - MFAEnrollmentID: session.multiFactorInfo?.uid, - signInInfo: startMFARequestInfo, - requestConfiguration: auth.requestConfiguration) - let response = try await auth.backend.call(with: request) - return response.responseInfo.sessionInfo - } + return try await executeMFARequest( + startMFARequestInfo: startMFARequestInfo, + multiFactorSession: session + ) } catch { return try await handleVerifyErrorWithRetry( error: error, diff --git a/FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift b/FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift index 2015904eea..8e90a6abc5 100644 --- a/FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift +++ b/FirebaseAuth/Tests/Unit/PhoneAuthProviderTests.swift @@ -347,6 +347,59 @@ function: #function, testMode: true) } + /** @fn testVerifyPhoneNumberWithMultiFactorInfoInTestMode + @brief Tests a successful invocation of @c verifyPhoneNumber(with:uiDelegate:multiFactorSession:completion:) when app verification is disabled due to #16438 + */ + func testVerifyPhoneNumberWithMultiFactorInfoInTestMode() async throws { + initApp(#function, testMode: true) + let auth = try XCTUnwrap(PhoneAuthProviderTests.auth) + let provider = PhoneAuthProvider.provider(auth: auth) + + let dictionary: [String: AnyHashable] = [ + "mfaEnrollmentId": "enrollment-id", + "phoneInfo": "+1******4444", + "displayName": "test phone", + "enrolledAt": "2023-01-01T00:00:00Z", + ] + let proto = AuthProtoMFAEnrollment(dictionary: dictionary) + let mfi = PhoneMultiFactorInfo(proto: proto) + + let session = MultiFactorSession(mfaCredential: "pending-credential") + session.multiFactorInfo = mfi + + let requestExpectation = expectation(description: "respondBlock") + rpcIssuer?.respondBlock = { [weak self] in + guard let self = self, let rpcIssuer = self.rpcIssuer else { + return (nil, nil) + } + guard let request = rpcIssuer.request as? StartMFASignInRequest else { + XCTFail( + "Expected StartMFASignInRequest, got \(String(describing: rpcIssuer.request))" + ) + return (nil, nil) + } + XCTAssertEqual(request.signInInfo?.phoneNumber, "+1******4444") + XCTAssertEqual(request.MFAEnrollmentID, "enrollment-id") + requestExpectation.fulfill() + return try rpcIssuer.respond(withJSON: [ + "phoneResponseInfo": ["sessionInfo": "fake-session-info"], + ]) + } + + do { + let verificationID = try await provider.verifyPhoneNumber( + with: mfi, + uiDelegate: nil, + multiFactorSession: session + ) + XCTAssertEqual(verificationID, "fake-session-info") + } catch { + XCTFail("verifyPhoneNumber failed with error: \(error)") + } + + await fulfillment(of: [requestExpectation], timeout: 5.0) + } + /** @fn testVerifyPhoneNumberUIDelegateFirebaseAppIdFlow @brief Tests a successful invocation of @c verifyPhoneNumber:UIDelegate:completion:. */