From da3d98a6d414c4cf9c60eb77da4ce178fc12357f Mon Sep 17 00:00:00 2001 From: daatsuka Date: Wed, 25 Mar 2026 16:40:21 +0900 Subject: [PATCH] resolve issue #5: $0.4 Bounty : Login fails with "Invalid Email" after success --- .../views/activites/OTPActivity.java | 94 +++--- .../updateapp/OTPActivityLogicTest.java | 279 ++++++++++++++++++ .../com/example/updateapp/UserModelTest.java | 120 ++++++++ 3 files changed, 452 insertions(+), 41 deletions(-) create mode 100644 app/src/test/java/com/example/updateapp/OTPActivityLogicTest.java create mode 100644 app/src/test/java/com/example/updateapp/UserModelTest.java diff --git a/app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java b/app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java index 918ed05..14ea4d7 100644 --- a/app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java +++ b/app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java @@ -21,8 +21,6 @@ import com.example.updateapp.databinding.ActivityOtpactivityBinding; import com.example.updateapp.models.UserModel; import com.example.updateapp.utils.LocaleHelper; -import com.google.firebase.auth.AuthCredential; -import com.google.firebase.auth.EmailAuthProvider; import com.google.firebase.auth.FirebaseAuth; import com.google.firebase.auth.FirebaseAuthUserCollisionException; import com.google.firebase.auth.PhoneAuthCredential; @@ -38,6 +36,7 @@ public class OTPActivity extends AppCompatActivity { public static PhoneAuthCredential autoCredential = null; String name, email, number, password, verificationId; + boolean autoVerify; ProgressDialog dialog; @@ -69,25 +68,22 @@ protected void onCreate(Bundle savedInstanceState) { }); binding.btnLogin.setOnClickListener(v -> { - String otp = getOtp(); + PhoneAuthCredential credential; - if (otp.length() < 6) { - Toast.makeText(this, getString(R.string.enter_valid_otp), Toast.LENGTH_SHORT).show(); - return; + if (autoVerify && autoCredential != null) { + credential = autoCredential; + autoCredential = null; // Prevent stale reuse across signup sessions + } else { + String otp = getOtp(); + if (otp.length() < 6) { + Toast.makeText(this, getString(R.string.enter_valid_otp), Toast.LENGTH_SHORT).show(); + return; + } + credential = PhoneAuthProvider.getCredential(verificationId, otp); } dialog.show(); - - if (autoCredential != null) { - - verifyCredential(autoCredential); - } else { - - PhoneAuthCredential credential = - PhoneAuthProvider.getCredential(verificationId, otp); - - verifyCredential(credential); - } + verifyCredential(credential); }); } @@ -97,6 +93,7 @@ private void getDataFromIntent() { number = getIntent().getStringExtra("number"); password = getIntent().getStringExtra("password"); verificationId = getIntent().getStringExtra("verificationId"); + autoVerify = getIntent().getBooleanExtra("autoVerify", false); if (number == null) number = ""; binding.tvUserNumber.setText(number); @@ -146,16 +143,39 @@ private String getOtp() { binding.etOtp6.getText().toString().trim(); } - private void verifyCredential(PhoneAuthCredential credential) { + private void verifyCredential(PhoneAuthCredential phoneCredential) { if (!dialog.isShowing()) dialog.show(); - auth.signInWithCredential(credential) - .addOnCompleteListener(task -> { + // If no email/password provided, fall back to phone-only auth + if (email == null || email.isEmpty() || password == null || password.isEmpty()) { + auth.signInWithCredential(phoneCredential) + .addOnCompleteListener(task -> { + if (!task.isSuccessful()) { + dialog.dismiss(); + String m = task.getException() != null ? task.getException().getMessage() : getString(R.string.otp_verification_failed); + Toast.makeText(OTPActivity.this, getString(R.string.otp_error, m), Toast.LENGTH_LONG).show(); + return; + } + if (auth.getCurrentUser() == null) { + dialog.dismiss(); + Toast.makeText(OTPActivity.this, getString(R.string.auth_error_user_not_found), Toast.LENGTH_LONG).show(); + return; + } + saveUserToFirestoreAndContinue(auth.getCurrentUser().getUid(), name, email, number, password); + }); + return; + } - if (!task.isSuccessful()) { + // Create email/password user FIRST so signInWithEmailAndPassword is guaranteed to work. + // The previous approach (signInWithCredential(phone) → linkWithCredential(email)) created + // a phone-primary account where email login could fail with "Invalid Email". + auth.createUserWithEmailAndPassword(email, password) + .addOnCompleteListener(createTask -> { + if (!createTask.isSuccessful()) { dialog.dismiss(); - String m = task.getException() != null ? task.getException().getMessage() : getString(R.string.otp_verification_failed); - Toast.makeText(OTPActivity.this, getString(R.string.otp_error, m), Toast.LENGTH_LONG).show(); + Exception e = createTask.getException(); + String msg = e != null ? e.getMessage() : getString(R.string.generic_error); + Toast.makeText(OTPActivity.this, msg, Toast.LENGTH_LONG).show(); return; } @@ -167,32 +187,24 @@ private void verifyCredential(PhoneAuthCredential credential) { final String uid = auth.getCurrentUser().getUid(); - if (email == null || email.isEmpty() || password == null || password.isEmpty()) { - saveUserToFirestoreAndContinue(uid, name, email, number, password); - return; - } - - AuthCredential emailCredential = EmailAuthProvider.getCredential(email, password); - - auth.getCurrentUser().linkWithCredential(emailCredential) + // Link phone credential to verify the OTP and attach the phone number + auth.getCurrentUser().linkWithCredential(phoneCredential) .addOnCompleteListener(linkTask -> { - if (linkTask.isSuccessful()) { saveUserToFirestoreAndContinue(uid, name, email, number, password); } else { - dialog.dismiss(); - Exception e = linkTask.getException(); - String msg = e != null ? e.getMessage() : "Unknown linking error"; if (e instanceof FirebaseAuthUserCollisionException) { - Toast.makeText(OTPActivity.this, - "This email is already registered with a different account. Please login with that email or use another email.", - Toast.LENGTH_LONG).show(); + // Phone number already linked to another account. + // Email/password account is still valid — proceed. + saveUserToFirestoreAndContinue(uid, name, email, number, password); } else { - Toast.makeText(OTPActivity.this, - "Failed to link email: " + msg, - Toast.LENGTH_LONG).show(); + // OTP invalid or other verification failure — roll back email account + auth.getCurrentUser().delete(); + dialog.dismiss(); + String msg = e != null ? e.getMessage() : getString(R.string.otp_verification_failed); + Toast.makeText(OTPActivity.this, getString(R.string.otp_error, msg), Toast.LENGTH_LONG).show(); } } }); diff --git a/app/src/test/java/com/example/updateapp/OTPActivityLogicTest.java b/app/src/test/java/com/example/updateapp/OTPActivityLogicTest.java new file mode 100644 index 0000000..e2d4c76 --- /dev/null +++ b/app/src/test/java/com/example/updateapp/OTPActivityLogicTest.java @@ -0,0 +1,279 @@ +package com.example.updateapp; + +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * Unit tests for the pure-logic conditions in OTPActivity's auth flow. + * + * These test the boolean decision logic that was changed in the "Login fails + * with Invalid Email after signup and OTP verification" fix, without requiring + * Android framework or Firebase dependencies. + * + * Conditions under test: + * 1. Credential selection: autoVerify && autoCredential != null + * 2. Phone-only fallback: email == null || email.isEmpty() || password == null || password.isEmpty() + * 3. OTP length validation: otp.length() < 6 + * 4. autoCredential stale-reuse prevention (nulling after use) + * 5. Null-coalescing in saveUserToFirestoreAndContinue + */ +public class OTPActivityLogicTest { + + // ═══════════════════════════════════════════════════════════════════ + // 1. Credential selection: autoVerify && autoCredential != null + // Determines whether to use the auto-detected credential or manual OTP + // ═══════════════════════════════════════════════════════════════════ + + @Test + public void credentialSelection_autoVerifyTrue_credentialPresent_usesAutoCredential() { + boolean autoVerify = true; + Object autoCredential = new Object(); // non-null represents a PhoneAuthCredential + + assertTrue("Should use autoCredential when autoVerify=true and credential is present", + autoVerify && autoCredential != null); + } + + @Test + public void credentialSelection_autoVerifyFalse_credentialPresent_usesManualOtp() { + boolean autoVerify = false; + Object autoCredential = new Object(); + + assertFalse("Should NOT use autoCredential when autoVerify=false even if credential exists", + autoVerify && autoCredential != null); + } + + @Test + public void credentialSelection_autoVerifyTrue_credentialNull_usesManualOtp() { + boolean autoVerify = true; + Object autoCredential = null; + + assertFalse("Should NOT use autoCredential when credential is null despite autoVerify=true", + autoVerify && autoCredential != null); + } + + @Test + public void credentialSelection_autoVerifyFalse_credentialNull_usesManualOtp() { + boolean autoVerify = false; + Object autoCredential = null; + + assertFalse("Should use manual OTP when both autoVerify=false and credential is null", + autoVerify && autoCredential != null); + } + + // ═══════════════════════════════════════════════════════════════════ + // 2. Phone-only fallback routing + // email == null || email.isEmpty() || password == null || password.isEmpty() + // → true = phone-only auth (signInWithCredential) + // → false = email-first auth (createUserWithEmailAndPassword → link phone) + // ═══════════════════════════════════════════════════════════════════ + + private boolean shouldUsePhoneOnlyAuth(String email, String password) { + return email == null || email.isEmpty() || password == null || password.isEmpty(); + } + + @Test + public void phoneOnlyFallback_bothPresent_usesEmailFirstAuth() { + assertFalse("email+password present → email-first path", + shouldUsePhoneOnlyAuth("user@example.com", "securePass1")); + } + + @Test + public void phoneOnlyFallback_emailNull_usesPhoneAuth() { + assertTrue("email null → phone-only path", + shouldUsePhoneOnlyAuth(null, "pass123")); + } + + @Test + public void phoneOnlyFallback_emailEmpty_usesPhoneAuth() { + assertTrue("email empty → phone-only path", + shouldUsePhoneOnlyAuth("", "pass123")); + } + + @Test + public void phoneOnlyFallback_passwordNull_usesPhoneAuth() { + assertTrue("password null → phone-only path", + shouldUsePhoneOnlyAuth("user@example.com", null)); + } + + @Test + public void phoneOnlyFallback_passwordEmpty_usesPhoneAuth() { + assertTrue("password empty → phone-only path", + shouldUsePhoneOnlyAuth("user@example.com", "")); + } + + @Test + public void phoneOnlyFallback_bothNull_usesPhoneAuth() { + assertTrue("both null → phone-only path", + shouldUsePhoneOnlyAuth(null, null)); + } + + @Test + public void phoneOnlyFallback_bothEmpty_usesPhoneAuth() { + assertTrue("both empty → phone-only path", + shouldUsePhoneOnlyAuth("", "")); + } + + @Test + public void phoneOnlyFallback_emailNullPasswordEmpty_usesPhoneAuth() { + assertTrue("email null, password empty → phone-only path", + shouldUsePhoneOnlyAuth(null, "")); + } + + @Test + public void phoneOnlyFallback_emailEmptyPasswordNull_usesPhoneAuth() { + assertTrue("email empty, password null → phone-only path", + shouldUsePhoneOnlyAuth("", null)); + } + + // ═══════════════════════════════════════════════════════════════════ + // 3. OTP length validation: otp.length() < 6 + // ═══════════════════════════════════════════════════════════════════ + + @Test + public void otpValidation_sixDigits_passes() { + String otp = "123456"; + assertFalse("6-digit OTP should pass validation", otp.length() < 6); + } + + @Test + public void otpValidation_fiveDigits_fails() { + String otp = "12345"; + assertTrue("5-digit OTP should fail validation", otp.length() < 6); + } + + @Test + public void otpValidation_empty_fails() { + String otp = ""; + assertTrue("Empty OTP should fail validation", otp.length() < 6); + } + + @Test + public void otpValidation_oneDigit_fails() { + String otp = "1"; + assertTrue("1-digit OTP should fail validation", otp.length() < 6); + } + + @Test + public void otpValidation_sevenDigits_passes() { + // Boundary: longer-than-expected still passes the < 6 check. + // PhoneAuthProvider.getCredential handles further validation server-side. + String otp = "1234567"; + assertFalse("7-digit OTP passes the <6 length check", otp.length() < 6); + } + + @Test + public void otpValidation_exactlySixCharsNonNumeric_passesLengthCheck() { + // The length check is purely numeric-agnostic; server-side validates content. + String otp = "abcdef"; + assertFalse("6-char non-numeric passes the length check (server validates content)", + otp.length() < 6); + } + + // ═══════════════════════════════════════════════════════════════════ + // 4. autoCredential stale-reuse prevention + // After using autoCredential, it must be set to null. + // ═══════════════════════════════════════════════════════════════════ + + @Test + public void autoCredentialReuse_nulledAfterConsumption() { + // Simulates the pattern: use → null → subsequent check should be false + Object[] holder = { new Object() }; // mutable holder for autoCredential + boolean autoVerify = true; + + // First use: should select autoCredential + assertTrue(autoVerify && holder[0] != null); + holder[0] = null; // mirrors: autoCredential = null; + + // Second attempt: must NOT use autoCredential + assertFalse("autoCredential must not be reused after being consumed", + autoVerify && holder[0] != null); + } + + @Test + public void autoCredentialReuse_separateSessionsGetFreshState() { + Object[] session1Credential = { new Object() }; + Object[] session2Credential = { new Object() }; + boolean autoVerify = true; + + // Session 1 consumes + assertTrue(autoVerify && session1Credential[0] != null); + session1Credential[0] = null; + + // Session 2 still has its own credential + assertTrue("Second session credential is independent and still valid", + autoVerify && session2Credential[0] != null); + } + + // ═══════════════════════════════════════════════════════════════════ + // 5. Null-coalescing in saveUserToFirestoreAndContinue + // null fields are replaced with "" before constructing UserModel + // ═══════════════════════════════════════════════════════════════════ + + private String coalesce(String value) { + return value == null ? "" : value; + } + + @Test + public void nullCoalescing_nullBecomesEmpty() { + assertEquals("", coalesce(null)); + } + + @Test + public void nullCoalescing_emptyStaysEmpty() { + assertEquals("", coalesce("")); + } + + @Test + public void nullCoalescing_valuePreserved() { + assertEquals("hello", coalesce("hello")); + } + + @Test + public void nullCoalescing_allFieldsCoalesced() { + String name = null, email = null, number = null, password = null; + + name = coalesce(name); + email = coalesce(email); + number = coalesce(number); + password = coalesce(password); + + assertEquals("", name); + assertEquals("", email); + assertEquals("", number); + assertEquals("", password); + } + + @Test + public void nullCoalescing_mixedNullAndPresent() { + String name = "Alice"; + String email = null; + String number = "+1234"; + String password = null; + + name = coalesce(name); + email = coalesce(email); + number = coalesce(number); + password = coalesce(password); + + assertEquals("Alice", name); + assertEquals("", email); + assertEquals("+1234", number); + assertEquals("", password); + } + + // ═══════════════════════════════════════════════════════════════════ + // 6. autoVerify default value + // getBooleanExtra("autoVerify", false) defaults to false + // ═══════════════════════════════════════════════════════════════════ + + @Test + public void autoVerifyDefault_isFalse() { + // Mirrors the Intent default: getBooleanExtra("autoVerify", false) + boolean autoVerify = false; // default + Object autoCredential = new Object(); + + assertFalse("Default autoVerify=false must prevent autoCredential usage even if credential exists", + autoVerify && autoCredential != null); + } +} diff --git a/app/src/test/java/com/example/updateapp/UserModelTest.java b/app/src/test/java/com/example/updateapp/UserModelTest.java new file mode 100644 index 0000000..bae2c92 --- /dev/null +++ b/app/src/test/java/com/example/updateapp/UserModelTest.java @@ -0,0 +1,120 @@ +package com.example.updateapp; + +import com.example.updateapp.models.UserModel; + +import org.junit.Test; + +import static org.junit.Assert.*; + +/** + * Unit tests for UserModel — verifies construction, getters/setters, + * and null-safety behaviour that mirrors saveUserToFirestoreAndContinue(). + */ +public class UserModelTest { + + // ── Normal: full construction ────────────────────────────────────── + + @Test + public void constructor_allFields_setsCorrectly() { + UserModel user = new UserModel("Alice", "alice@example.com", "+1234567890", "pass123", "https://img.url/pic.png"); + + assertEquals("Alice", user.getName()); + assertEquals("alice@example.com", user.getEmail()); + assertEquals("+1234567890", user.getNumber()); + assertEquals("pass123", user.getPassword()); + assertEquals("https://img.url/pic.png", user.getProfile()); + } + + @Test + public void defaultConstructor_fieldsAreNull() { + UserModel user = new UserModel(); + + assertNull(user.getName()); + assertNull(user.getEmail()); + assertNull(user.getNumber()); + assertNull(user.getPassword()); + assertNull(user.getProfile()); + } + + // ── Normal: setters ──────────────────────────────────────────────── + + @Test + public void setters_overwriteValues() { + UserModel user = new UserModel("A", "a@b.c", "111", "pw", "url"); + + user.setName("Bob"); + user.setEmail("bob@example.com"); + user.setNumber("+9876543210"); + user.setPassword("newpass"); + user.setProfile("https://new.url/img.png"); + + assertEquals("Bob", user.getName()); + assertEquals("bob@example.com", user.getEmail()); + assertEquals("+9876543210", user.getNumber()); + assertEquals("newpass", user.getPassword()); + assertEquals("https://new.url/img.png", user.getProfile()); + } + + // ── Boundary: null fields (mirrors saveUserToFirestoreAndContinue null-coalescing) ── + + @Test + public void constructor_nullName_acceptsNull() { + UserModel user = new UserModel(null, "e@x.com", "+1", "pw", "url"); + assertNull(user.getName()); + } + + @Test + public void constructor_nullEmail_acceptsNull() { + UserModel user = new UserModel("N", null, "+1", "pw", "url"); + assertNull(user.getEmail()); + } + + @Test + public void constructor_nullNumber_acceptsNull() { + UserModel user = new UserModel("N", "e@x.com", null, "pw", "url"); + assertNull(user.getNumber()); + } + + @Test + public void constructor_nullPassword_acceptsNull() { + UserModel user = new UserModel("N", "e@x.com", "+1", null, "url"); + assertNull(user.getPassword()); + } + + @Test + public void constructor_allNulls_acceptsNull() { + UserModel user = new UserModel(null, null, null, null, null); + assertNull(user.getName()); + assertNull(user.getEmail()); + assertNull(user.getNumber()); + assertNull(user.getPassword()); + assertNull(user.getProfile()); + } + + // ── Boundary: empty strings ──────────────────────────────────────── + + @Test + public void constructor_emptyStrings_storedAsEmpty() { + UserModel user = new UserModel("", "", "", "", ""); + + assertEquals("", user.getName()); + assertEquals("", user.getEmail()); + assertEquals("", user.getNumber()); + assertEquals("", user.getPassword()); + assertEquals("", user.getProfile()); + } + + @Test + public void setName_toNull_allowsNull() { + UserModel user = new UserModel("Name", "e@x.com", "+1", "pw", "url"); + user.setName(null); + assertNull(user.getName()); + } + + @Test + public void setEmail_toEmpty_allowsEmpty() { + UserModel user = new UserModel("Name", "e@x.com", "+1", "pw", "url"); + user.setEmail(""); + assertEquals("", user.getEmail()); + } +}