prevent asymmetric facebook session management with flag

[mitchymitch]

I was running into the following issue:

I was acquiring my facebook accessToken and then linking with ParseFacebookUtils.linkInBackground(...). Then, if I had already linked with that Facebook id, I was getting a 208 code. In that case, I then have to use ParseFacebookUtils.logInInBackground(...) to retrieve that old linked ParseUser. The problem is, logInInBackground will log me out of Facebook, even though I'm supplying my own accessToken.

Logic tells me, that if I'm supplying my own accessToken, its erroneous for it to assume its managing the lifecycle of my accessToken.

My Solution: I added a flag to track the cases (only 2) where the accessToken is acquired/expired externally to stop it from thinking it has to log you out.

Another Solution: Do we ever need to explicitly logout of Facebook? This could allow us to remove this flag.

[flovilmart]

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?

[mitchymitch]

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

[mitchymitch]

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

[flovilmart]

@rogerhu what are your thoughts? @mitchymitch is a colleague of mine.

[codecov]

Codecov Report

Merging #28 into master will increase coverage by 0.38%.
The diff coverage is 83.33%.

@@             Coverage Diff              @@
##             master      #28      +/-   ##
============================================
+ Coverage      57.3%   57.69%   +0.38%     
- Complexity       35       36       +1     
============================================
  Files             2        2              
  Lines           178      182       +4     
  Branches         18       19       +1     
============================================
+ Hits            102      105       +3     
- Misses           69       70       +1     
  Partials          7        7

Impacted Files	Coverage Δ	Complexity Δ
...ry/src/main/java/com/parse/FacebookController.java	78.87% <100%> (+0.3%)	15 <0> (+1)	⬆️
...ry/src/main/java/com/parse/ParseFacebookUtils.java	44.14% <75%> (+0.62%)	21 <1> (ø)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1b52507...460faed. Read the comment docs.

[rogerhu]

@flovilmart I'm inclined to say that setAuthData() shouldn't have the side effect of logging out... may add a deprecated warning if someone passes a null to indicate that this behavior has changed.

[flovilmart]

This will yield a lot a warnings. My main concern is that the current behavior is the same as on iOS, so I’m unsure about the Fox and the potential side effects on existing installations. Otherwise, that’s your call @rogerhu.

[rogerhu]

Does the iOS logout too if authData is null?

[rogerhu]

you'll need to rebase anyways given the set of changes... my question is still how iOS handles authData null..

[jyoon17]

@rogerhu Yes, it does. See this link.

I think the name setAuthData confuses us. Logging out is not a side effect of setAuthData. Since it is called when ParseUser.logOut is called, it is quite an intended behavior.

[jyoon17]

@mitchymitch How did you get the user you provided to ParseFacebookUtils.linkInBackground? I am wondering why you need to call ParseFacebookUtils.logInInBackground to get the user you provided to linkInBackground. It's not important but I am just curious.

[mitchymitch]

@jyoon17 with ParseUser.enableAutomaticUser();

[mitchymitch]

Since it is called when ParseUser.logOut is called, it is quite an intended behavior.

@jyoon17 you're right - in a way. I don't think its intended behaviour though to log out of Facebook if I call linkInBackground(user) & then logInInBackground(token), since using logInInBackground(fbtoken) assumes YOU are the one responsible for the FB lifecycle. I'm curious how else people are using logInInBackground, since it seems pretty useless unless I'm not reading the code properly.

[rogerhu]

let's keep this as 2.3.3