Skip to content

Various bugfixes for 1.0 #380

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

Merged
merged 3 commits into from
Oct 26, 2016
Merged

Various bugfixes for 1.0 #380

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4701474
Various bugfixes for 1.0
Oct 26, 2016
decdf60
Review feedback and fix tests
Oct 26, 2016
d3b526c
Fix a few mistakes
Oct 26, 2016
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
27 changes: 19 additions & 8 deletions auth/src/main/AndroidManifest.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,46 +13,57 @@
<activity
android:name="com.firebase.ui.auth.ui.email.ConfirmRecoverPasswordActivity"
android:label="@string/title_confirm_recover_password_activity"
android:exported="false"
android:theme="@style/FirebaseUI.Dialog" />
<activity
android:name="com.firebase.ui.auth.ui.email.EmailHintContainerActivity"
android:name=".ui.email.EmailHintContainerActivity"
android:label="@string/default_toolbar_title"
android:exported="false"
android:theme="@style/FirebaseUI.Translucent" />
<activity
android:name="com.firebase.ui.auth.ui.email.RecoverPasswordActivity"
android:name=".ui.email.RecoverPasswordActivity"
android:label="@string/title_recover_password_activity"
android:exported="false"
android:theme="@style/FirebaseUI" />
<activity
android:name="com.firebase.ui.auth.ui.email.RegisterEmailActivity"
android:name=".ui.email.RegisterEmailActivity"
android:label="@string/title_register_email_activity"
android:exported="false"
android:theme="@style/FirebaseUI" />
<activity
android:name="com.firebase.ui.auth.ui.email.SignInNoPasswordActivity"
android:name=".ui.email.SignInNoPasswordActivity"
android:label="@string/title_sign_in_no_password_activity"
android:exported="false"
android:theme="@style/FirebaseUI" />
<activity
android:name="com.firebase.ui.auth.ui.email.SignInActivity"
android:name=".ui.email.SignInActivity"
android:label="@string/title_sign_in_activity"
android:exported="false"
android:theme="@style/FirebaseUI" />
<activity
android:name="com.firebase.ui.auth.ui.account_link.WelcomeBackIdpPrompt"
android:name=".ui.account_link.WelcomeBackIdpPrompt"
android:label="@string/title_welcome_back_idp_prompt"
android:exported="false"
android:theme="@style/FirebaseUI" />
<activity
android:name="com.firebase.ui.auth.ui.account_link.WelcomeBackPasswordPrompt"
android:name=".ui.account_link.WelcomeBackPasswordPrompt"
android:label="@string/title_welcome_back_password_prompt"
android:exported="false"
android:theme="@style/FirebaseUI" />
<activity
android:name=".ui.idp.AuthMethodPickerActivity"
android:label="@string/default_toolbar_title"
android:exported="false"
android:theme="@style/FirebaseUI" />
<activity
android:name="com.firebase.ui.auth.ui.idp.IdpSignInContainerActivity"
android:name=".ui.idp.IdpSignInContainerActivity"
android:label="@string/default_toolbar_title"
android:exported="false"
android:theme="@style/FirebaseUI.Translucent" />
<activity
android:name=".ui.ChooseAccountActivity"
android:label="@string/default_toolbar_title"
android:exported="false"
android:theme="@style/FirebaseUI.Translucent" />

<activity
Expand Down
8 changes: 6 additions & 2 deletions auth/src/main/java/com/firebase/ui/auth/ui/account_link/WelcomeBackPasswordPrompt.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import android.widget.EditText;
import android.widget.TextView;

import com.firebase.ui.auth.AuthUI;
import com.firebase.ui.auth.IdpResponse;
import com.firebase.ui.auth.R;
import com.firebase.ui.auth.ui.ActivityHelper;
Expand Down Expand Up @@ -105,7 +106,7 @@ public void onClick(View view) {
}
}

private void next(String email, final String password) {
private void next(final String email, final String password) {
final FirebaseAuth firebaseAuth = mActivityHelper.getFirebaseAuth();

// Check for null or empty password
Expand Down Expand Up @@ -146,14 +147,17 @@ public void onSuccess(AuthResult authResult) {
mActivityHelper,
authResult.getUser(),
password,
null /* provider */);
new IdpResponse(
AuthUI.EMAIL_PROVIDER,
email));
}
});
}
})
.addOnFailureListener(new OnFailureListener() {
@Override
public void onFailure(@NonNull Exception e) {
mActivityHelper.dismissDialog();
String error = e.getLocalizedMessage();
mPasswordLayout.setError(error);
}
Expand Down
17 changes: 11 additions & 6 deletions auth/src/main/java/com/firebase/ui/auth/ui/email/RegisterEmailActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
import android.widget.ImageView;
import android.widget.TextView;

import com.firebase.ui.auth.AuthUI;
import com.firebase.ui.auth.IdpResponse;
import com.firebase.ui.auth.R;
import com.firebase.ui.auth.ui.ActivityHelper;
import com.firebase.ui.auth.ui.AppCompatBase;
Expand Down Expand Up @@ -131,7 +133,7 @@ public void onClick(View view) {
});
}

private void registerUser(String email, final String name, final String password) {
private void registerUser(final String email, final String name, final String password) {
final FirebaseAuth firebaseAuth = mActivityHelper.getFirebaseAuth();
// create the user
firebaseAuth.createUserWithEmailAndPassword(email, password)
Expand All @@ -155,11 +157,14 @@ public void onComplete(@NonNull Task<Void> task) {
// the account creation succeeded and we want to save
// the credential to SmartLock (if enabled).
SmartLock.getInstance(RegisterEmailActivity.this, TAG)
.saveCredentialsOrFinish(RegisterEmailActivity.this,
mActivityHelper,
firebaseUser,
password,
null /* provider */);
.saveCredentialsOrFinish(
RegisterEmailActivity.this,
mActivityHelper,
firebaseUser,
password,
new IdpResponse(
AuthUI.EMAIL_PROVIDER,
email));
}
});
}
Expand Down
8 changes: 6 additions & 2 deletions auth/src/main/java/com/firebase/ui/auth/ui/email/SignInActivity.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@
import android.widget.ImageView;
import android.widget.TextView;

import com.firebase.ui.auth.AuthUI;
import com.firebase.ui.auth.IdpResponse;
import com.firebase.ui.auth.R;
import com.firebase.ui.auth.ui.ActivityHelper;
import com.firebase.ui.auth.ui.AppCompatBase;
Expand Down Expand Up @@ -89,7 +91,7 @@ protected void onCreate(Bundle savedInstanceState) {
recoveryButton.setOnClickListener(this);
}

private void signIn(String email, final String password) {
private void signIn(final String email, final String password) {
mActivityHelper.getFirebaseAuth()
.signInWithEmailAndPassword(email, password)
.addOnFailureListener(
Expand All @@ -103,7 +105,9 @@ public void onSuccess(AuthResult authResult) {
mActivityHelper,
authResult.getUser(),
password,
null /* provider */);
new IdpResponse(
AuthUI.EMAIL_PROVIDER,
email));
}
})
.addOnFailureListener(new OnFailureListener() {
Expand Down
2 changes: 1 addition & 1 deletion auth/src/main/java/com/firebase/ui/auth/ui/idp/CredentialSignInHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ public void onComplete(@NonNull Task<AuthResult> task) {
mActivityHelper,
firebaseUser,
null /* password */,
mResponse.getProviderType());
mResponse);
} else {
if (task.getException() instanceof FirebaseAuthUserCollisionException) {
final String email = mResponse.getEmail();
Expand Down
28 changes: 19 additions & 9 deletions auth/src/main/java/com/firebase/ui/auth/util/SmartLock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,10 @@
import android.util.Log;

import com.firebase.ui.auth.BuildConfig;
import com.firebase.ui.auth.IdpResponse;
import com.firebase.ui.auth.ui.ActivityHelper;
import com.firebase.ui.auth.ui.AppCompatBase;
import com.firebase.ui.auth.ui.ExtraConstants;
import com.google.android.gms.auth.api.Auth;
import com.google.android.gms.auth.api.credentials.Credential;
import com.google.android.gms.auth.api.credentials.IdentityProviders;
Expand Down Expand Up @@ -61,6 +63,7 @@ public class SmartLock extends Fragment
private String mPassword;
private String mProvider;
private String mProfilePictureUri;
private IdpResponse mResponse;
private GoogleApiClient mCredentialsApiClient;

@Override
Expand Down Expand Up @@ -147,8 +150,9 @@ public void onConnectionFailed(@NonNull ConnectionResult connectionResult) {

@Override
public void onResult(@NonNull Status status) {
Intent resultIntent = new Intent().putExtra(ExtraConstants.EXTRA_IDP_RESPONSE, mResponse);
if (status.isSuccess()) {
finish(RESULT_OK);
finish(RESULT_OK, resultIntent);
} else {
if (status.hasResolution()) {
// Try to resolve the save request. This will prompt the user if
Expand All @@ -164,10 +168,10 @@ public void onResult(@NonNull Status status) {
} catch (IntentSender.SendIntentException e) {
// Could not resolve the request
Log.e(TAG, "STATUS: Failed to send resolution.", e);
finish(RESULT_CANCELED);
finish(RESULT_CANCELED, resultIntent);
}
} else {
finish(RESULT_CANCELED);
finish(RESULT_CANCELED, resultIntent);
}
}
}
Expand Down Expand Up @@ -201,7 +205,11 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) {
}

private void finish(int resultCode) {
mActivity.finish(RESULT_OK, mActivity.getIntent());
mActivity.finish(resultCode, mActivity.getIntent());
}

private void finish(int resultCode, Intent data) {
mActivity.finish(resultCode, data);
Copy link
Collaborator

Choose a reason for hiding this comment

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

You'll need to do some more refactoring for this to work. The reason I chose not to pass down the resultCode is because SmartLock used to be an Activity called by other activities (such as RegisterEmailActivity). Those activities didn't care about what the result code was and always returned Activity.RESULT_OK. Say you start the email flow through AuthMethodPickerActivity and end up at RegisterEmailActivity which calls this smart lock code. If saving the credential fails,RegisterEmailActivity will be finished with Activity.RESULT_CANCELED. However, the user is signed in and this line in AuthMethodPickerActivity will prevent the activity from finishing even though the user is singed in. I don't think this is expected behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, but since the resultCode was being ignored finish(RESULT_CANCELLED) like on line 167 would result in finish(RESULT_OK) being called. Maybe it would make sense to change line 167 and line 174 to call finish(RESULT_OK) directly instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are a few other lines I missed that would need to change to RESULT_OK as well

Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this is probably going to be a point of confusion in the future, I think we should just remove the option to set a result at all and always pass down RESULT_OK. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good to me, done!

Copy link
Collaborator

Choose a reason for hiding this comment

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

Awesome!

}

/**
Expand All @@ -217,32 +225,34 @@ public void saveCredentialsOrFinish(AppCompatBase activity,
ActivityHelper helper,
FirebaseUser firebaseUser,
@Nullable String password,
@Nullable String provider) {
@NonNull IdpResponse response) {
mActivity = activity;
mActivityHelper = helper;
mName = firebaseUser.getDisplayName();
mEmail = firebaseUser.getEmail();
mPassword = password;
mProvider = provider;
mResponse = response;
mProvider = response.getProviderType();
mProfilePictureUri = firebaseUser.getPhotoUrl() != null ? firebaseUser.getPhotoUrl()
.toString() : null;

// If SmartLock is disabled, finish the Activity
Intent returnIntent = new Intent().putExtra(ExtraConstants.EXTRA_IDP_RESPONSE, response);
if (!helper.getFlowParams().smartLockEnabled) {
finish(RESULT_CANCELED);
finish(RESULT_OK, returnIntent);
return;
}

// If Play Services is not available, finish the Activity
if (!PlayServicesHelper.getInstance(activity).isPlayServicesAvailable()) {
finish(RESULT_CANCELED);
finish(RESULT_OK, returnIntent);
return;
}

if (!FirebaseAuthWrapperFactory
.getFirebaseAuthWrapper(helper.getFlowParams().appName)
.isPlayServicesAvailable(activity)) {
finish(RESULT_CANCELED);
finish(RESULT_OK, returnIntent);
return;
}

Expand Down