Skip to content
This repository was archived by the owner on Feb 14, 2020. It is now read-only.

prevent asymmetric facebook session management with flag #28

Open
mitchymitch wants to merge 4 commits into
base: master
Choose a base branch
from
Open

prevent asymmetric facebook session management with flag #28

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0bd24ce
prevent asymmetric facebook session management with flag
mitchymitch Jul 27, 2017
a5304dc
fixed tests
mitchymitch Jul 27, 2017
03810e8
Update to Support 26 and buildTools 3
Oct 24, 2017
460faed
Update gradle to 3.0 stable
Oct 25, 2017
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
4 changes: 2 additions & 2 deletions library/src/main/java/com/parse/FacebookController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,9 +139,9 @@ public Map<String, String> getAuthData(AccessToken accessToken) {
return authData;
}

public void setAuthData(Map<String, String> authData)
public void setAuthData(Map<String, String> authData, boolean isFacebookLoginScopeExternallyManaged)
throws java.text.ParseException {
if (authData == null) {
if (authData == null && !isFacebookLoginScopeExternallyManaged) {
Copy link

@flovilmart flovilmart Jul 27, 2017

Choose a reason for hiding this comment

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

Perhaps we should not logout from FB when calling with authData null. What’s the behavior on the iOS repo? We try to have them consistent if possible. Also who calls that with null? When linking fails right?

Copy link
Author

@mitchymitch mitchymitch Jul 27, 2017

Choose a reason for hiding this comment

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

I'll check regarding the iOS repo, but authData==null occurs on a logout flow (which is triggered from a login flow).

Copy link
Author

@mitchymitch mitchymitch Jul 27, 2017

Choose a reason for hiding this comment

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

But yes if we could just remove this logout all together, then I wouldn't need my flag at all.

facebookSdkDelegate.getLoginManager().logOut();
return;
}
Expand Down
5 changes: 4 additions & 1 deletion library/src/main/java/com/parse/ParseFacebookUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ public final class ParseFacebookUtils {

private static final Object lock = new Object();
/* package for tests */ static boolean isInitialized;
/* package for tests */ static boolean isFacebookLoginScopeExternallyManaged = false;
/* package for tests */ static FacebookController controller;
/* package for tests */ static ParseUserDelegate userDelegate = new ParseUserDelegateImpl();

Expand Down Expand Up @@ -111,7 +112,7 @@ public static void initialize(Context context, int callbackRequestCodeOffset) {
@Override
public boolean onRestore(Map<String, String> authData) {
try {
getController().setAuthData(authData);
getController().setAuthData(authData, isFacebookLoginScopeExternallyManaged);
return true;
} catch (Exception e) {
return false;
Expand Down Expand Up @@ -167,6 +168,7 @@ public static boolean onActivityResult(int requestCode, int resultCode, Intent d
* @return A task that will be resolved when logging in is complete.
*/
public static Task<ParseUser> logInInBackground(AccessToken accessToken) {
isFacebookLoginScopeExternallyManaged = true;
checkInitialization();
return userDelegate.logInWithInBackground(AUTH_TYPE, getController().getAuthData(accessToken));
}
Expand All @@ -180,6 +182,7 @@ public static Task<ParseUser> logInInBackground(AccessToken accessToken) {
* @return A task that will be resolved when logging in is complete.
*/
public static Task<ParseUser> logInInBackground(AccessToken accessToken, LogInCallback callback) {
isFacebookLoginScopeExternallyManaged = true;
return callbackOnMainThreadAsync(logInInBackground(accessToken), callback, true);
}

Expand Down
26 changes: 20 additions & 6 deletions library/src/test/java/com/parse/FacebookControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,20 +102,34 @@ public void testGetAuthData() {
//region testSetAuthData

@Test
public void testSetAuthDataWithNull() throws java.text.ParseException {
public void testSetAuthDataWithNullNotExternallyManaged() throws java.text.ParseException {
FacebookController.FacebookSdkDelegate facebookSdk =
mock(FacebookController.FacebookSdkDelegate.class);
LoginManager loginManager = mock(LoginManager.class);
when(facebookSdk.getLoginManager()).thenReturn(loginManager);
FacebookController controller = new FacebookController(facebookSdk);

controller.setAuthData(null);
controller.setAuthData(null, true);
verify(facebookSdk).getLoginManager();
verifyNoMoreInteractions(facebookSdk);
verify(loginManager).logOut();
verifyNoMoreInteractions(loginManager);
}

@Test
public void testSetAuthDataWithNullExternallyManaged() throws java.text.ParseException {
FacebookController.FacebookSdkDelegate facebookSdk =
mock(FacebookController.FacebookSdkDelegate.class);
LoginManager loginManager = mock(LoginManager.class);
when(facebookSdk.getLoginManager()).thenReturn(loginManager);
FacebookController controller = new FacebookController(facebookSdk);

controller.setAuthData(null, false);
verify(facebookSdk).getLoginManager();
verifyNoMoreInteractions(facebookSdk);
verifyNoMoreInteractions(loginManager);
}

@Test
public void testSetAuthData() throws ParseException {
Locale.setDefault(new Locale("ar")); // Mimic the device's locale
Expand All @@ -130,7 +144,7 @@ public void testSetAuthData() throws ParseException {
authData.put("id", "test_id");
authData.put("access_token", "test_token");
authData.put("expiration_date", "2015-07-03T07:00:00.000Z");
controller.setAuthData(authData);
controller.setAuthData(authData, true);
ArgumentCaptor<AccessToken> accessTokenCapture = ArgumentCaptor.forClass(AccessToken.class);
verify(facebookSdk).setCurrentAccessToken(accessTokenCapture.capture());
AccessToken accessToken = accessTokenCapture.getValue();
Expand All @@ -153,13 +167,13 @@ public void testSetAuthDataWithDifferentAccessToken() throws ParseException {
authData.put("id", "new_id");
authData.put("access_token", "test_token");
authData.put("expiration_date", "2015-07-03T07:00:00.000Z");
controller.setAuthData(authData);
controller.setAuthData(authData, true);
verify(facebookSdk, times(1)).setCurrentAccessToken(any(AccessToken.class));

authData.put("id", "new_id");
authData.put("access_token", "new_token");
authData.put("expiration_date", "2015-07-03T07:00:00.000Z");
controller.setAuthData(authData);
controller.setAuthData(authData, true);
verify(facebookSdk, times(2)).setCurrentAccessToken(any(AccessToken.class));
}

Expand All @@ -175,7 +189,7 @@ public void testSetAuthDataWithSameAccessToken() throws ParseException {
authData.put("id", "test_id");
authData.put("access_token", "test_token");
authData.put("expiration_date", "2015-07-03T07:00:00.000Z");
controller.setAuthData(authData);
controller.setAuthData(authData, true);
verify(facebookSdk, never()).setCurrentAccessToken(any(AccessToken.class));
}

Expand Down
7 changes: 4 additions & 3 deletions library/src/test/java/com/parse/ParseFacebookUtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyBoolean;
import static org.mockito.Matchers.anyInt;
import static org.mockito.Matchers.anyListOf;
import static org.mockito.Matchers.anyMapOf;
Expand Down Expand Up @@ -102,7 +103,7 @@ public void testRestoreAuthentication() throws java.text.ParseException {
Map<String, String> authData = new HashMap<>();

assertTrue(callback.onRestore(authData));
verify(controller).setAuthData(authData);
verify(controller).setAuthData(authData, true);
}

@Test
Expand All @@ -115,10 +116,10 @@ public void testRestoreAuthenticationFailure() throws java.text.ParseException {
Map<String, String> authData = new HashMap<>();
doThrow(new RuntimeException())
.when(controller)
.setAuthData(anyMapOf(String.class, String.class));
.setAuthData(anyMapOf(String.class, String.class), anyBoolean());

assertFalse(callback.onRestore(authData));
verify(controller).setAuthData(authData);
verify(controller).setAuthData(authData, false);
}

//endregion
Expand Down