Skip to content

Reorder Firebase auth to create email account before linking phone credential #14

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
daatsuka wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Reorder Firebase auth to create email account before linking phone credential #14

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
94 changes: 53 additions & 41 deletions app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -38,6 +36,7 @@ public class OTPActivity extends AppCompatActivity {
public static PhoneAuthCredential autoCredential = null;

String name, email, number, password, verificationId;
boolean autoVerify;

ProgressDialog dialog;

Expand Down Expand Up @@ -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);
});
Comment on lines 70 to 87
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Edge case: process death during auto-verify flow leaves verificationId empty.

When autoVerify=true is set (from SignUpActivity.onVerificationCompleted), the verificationId is passed as an empty string "". If the app process is killed and restored:

  • autoVerify remains true (from Intent)
  • autoCredential becomes null (static field lost)
  • User enters OTP manually, but PhoneAuthProvider.getCredential("", otp) at line 82 will fail

This is a rare edge case. Consider adding validation:

Proposed fix 
             } else {
                 String otp = getOtp();
                 if (otp.length() < 6) {
                     Toast.makeText(this, getString(R.string.enter_valid_otp), Toast.LENGTH_SHORT).show();
                     return;
                 }
+                if (verificationId == null || verificationId.isEmpty()) {
+                    Toast.makeText(this, getString(R.string.verification_expired), Toast.LENGTH_SHORT).show();
+                    return;
+                }
                 credential = PhoneAuthProvider.getCredential(verificationId, otp);
             }
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java`
around lines 70 - 87, The click handler may call PhoneAuthProvider.getCredential
with an empty verificationId when autoVerify is true but autoCredential was lost
(process death); update the btnLogin OnClickListener to validate that when
autoVerify is true and autoCredential is null the verificationId is non-empty
before calling PhoneAuthProvider.getCredential(verificationId, otp), and if
verificationId is empty either flip autoVerify to false and proceed with the
manual flow (or prompt the user to resend/request a new code) or show an error
Toast; ensure getOtp() and verifyCredential(credential) are only used after this
validation to avoid passing an empty verificationId.

}

Expand All @@ -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);
Expand Down Expand Up @@ -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;
}

Expand All @@ -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();
}
Comment on lines +203 to 208
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 25, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Async delete() call has no completion handling.

auth.getCurrentUser().delete() returns a Task but the result is ignored. If deletion fails (e.g., network issue), the orphaned email account remains, potentially blocking future signup attempts with the same email.

Consider adding minimal error handling or at least logging:

Proposed improvement 
                                     } else {
                                         // OTP invalid or other verification failure — roll back email account
-                                        auth.getCurrentUser().delete();
+                                        auth.getCurrentUser().delete()
+                                                .addOnFailureListener(deleteErr -> {
+                                                    // Log or handle - user may need to use a different email
+                                                    android.util.Log.w("OTPActivity", "Failed to delete orphaned account", deleteErr);
+                                                });
                                         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();
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// 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();
}
// OTP invalid or other verification failure — roll back email account
auth.getCurrentUser().delete()
.addOnFailureListener(deleteErr -> {
// Log or handle - user may need to use a different email
android.util.Log.w("OTPActivity", "Failed to delete orphaned account", deleteErr);
});
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();
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@app/src/main/java/com/example/updateapp/views/activites/OTPActivity.java`
around lines 203 - 208, The call to auth.getCurrentUser().delete() in
OTPActivity is asynchronous and its Task result is ignored; wrap that delete()
call with completion handling (e.g., addOnCompleteListener /
addOnSuccessListener and addOnFailureListener) so you can detect deletion
failures, log the error (or report to process logger) and optionally show a
Toast if deletion failed, while still dismissing the dialog and handling the OTP
failure flow; ensure you reference auth.getCurrentUser().delete() and attach
listeners to handle success and failure instead of firing-and-forgetting.

}
});
Expand Down
Loading