Skip to content

Logout of Twitter #645

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.

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 1 commit into from
Mar 22, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
12 changes: 12 additions & 0 deletions auth/src/main/java/com/firebase/ui/auth/AuthUI.java
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import android.support.v4.app.FragmentActivity;

import com.facebook.login.LoginManager;
import com.firebase.ui.auth.provider.TwitterProvider;
import com.firebase.ui.auth.ui.FlowParameters;
import com.firebase.ui.auth.ui.idp.AuthMethodPickerActivity;
import com.firebase.ui.auth.util.CredentialTaskApi;
Expand All @@ -49,6 +50,7 @@
import com.google.firebase.auth.FirebaseUser;
import com.google.firebase.auth.GoogleAuthProvider;
import com.google.firebase.auth.TwitterAuthProvider;
import com.twitter.sdk.android.Twitter;

import java.util.ArrayList;
import java.util.Arrays;
Expand All @@ -58,6 +60,8 @@
import java.util.List;
import java.util.Set;

import io.fabric.sdk.android.Fabric;

/**
* The entry point to the AuthUI authentication flow, and related utility methods. If your
* application uses the default {@link FirebaseApp} instance, an AuthUI instance can be retrieved
Expand Down Expand Up @@ -192,6 +196,10 @@ public Void then(@NonNull Task<GoogleApiClient> task) throws Exception {
// Facebook sign out
LoginManager.getInstance().logOut();

// Twitter sign out
if (!Fabric.isInitialized()) TwitterProvider.initialize(activity);
Twitter.logOut();
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we also want to call:
Twitter.getSessionManager().clearActiveSession();

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@samtstern I thought so too, but Twitter.logOut() already does that for us:

// In Twitter.java
public static void logOut() {
    checkInitialized();
    getInstance().core.logOut(); // core == TwitterCore
}

// In TwitterCore.java
public void logOut() {
    checkInitialized();
    final SessionManager<TwitterSession> sessionManager = getSessionManager();
    if (sessionManager != null) {
        sessionManager.clearActiveSession();
    }
}

public static SessionManager<TwitterSession> getSessionManager() {
    checkInitialized();
    return getInstance().core.getSessionManager(); // Same reference as logOut in TwitterCore.java
}

Actually, now that I'm paying more attention, it looks like that's all Twitter#logOut() does.

The docs for TwitterCore#logOut() say it logs the user out, so I think that's all we have to do:

/**
 * Logs out the user, clearing user session. This will not make a network request to invalidate
 * the session.
 *
 * @throws java.lang.IllegalStateException if {@link io.fabric.sdk.android.Fabric}
 *          or {@link TwitterCore} has not been initialized.
 */

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for digging, works for me!

The Twitter docs don't have anything at all on signOut ... oversight I guess.


// Wait for all tasks to complete
return Tasks.whenAll(disableCredentialsTask, googleSignOutTask);
}
Expand Down Expand Up @@ -239,6 +247,10 @@ public Task<Void> signOut(@NonNull FragmentActivity activity) {
// Facebook sign out
LoginManager.getInstance().logOut();

// Twitter sign out
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@samtstern Signing out of Twitter and Facebook should probably a method (something like signOutProviders), but that will cause more thrashing in #609. Would you like me to do the refactor anyway and then undo it in #609 or are we not going through with #609?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep sign out all as one method for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

SGTM!

if (!Fabric.isInitialized()) TwitterProvider.initialize(activity);
Twitter.logOut();

// Wait for all tasks to complete
return Tasks.whenAll(disableCredentialsTask, signOutTask);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,8 @@ public class TwitterProvider extends Callback<TwitterSession> implements IdpProv
private IdpCallback mCallbackObject;
private TwitterAuthClient mTwitterAuthClient;

public TwitterProvider(Context appContext) {
TwitterAuthConfig authConfig = new TwitterAuthConfig(
appContext.getString(R.string.twitter_consumer_key),
appContext.getString(R.string.twitter_consumer_secret));
Fabric.with(appContext.getApplicationContext(), new Twitter(authConfig));
public TwitterProvider(Context context) {
initialize(context);
mTwitterAuthClient = new TwitterAuthClient();
}

Expand All @@ -43,6 +40,13 @@ public static AuthCredential createAuthCredential(IdpResponse response) {
return TwitterAuthProvider.getCredential(response.getIdpToken(), response.getIdpSecret());
}

public static void initialize(Context context) {
TwitterAuthConfig authConfig = new TwitterAuthConfig(
context.getString(R.string.twitter_consumer_key),
context.getString(R.string.twitter_consumer_secret));
Fabric.with(context.getApplicationContext(), new Twitter(authConfig));
}

@Override
public String getName(Context context) {
return context.getString(R.string.idp_name_twitter);
Expand Down