Guilherme Gonçalves [:ggp] Reporter
	Description • 10 years ago

According to :spenrose [1], this is not the best way to achieve what we want in the System app, which is mostly to monitor logout events. I've also found that mozFxAccountsUnsolChromeEvent has a slightly odd (and undocumented?) behavior [2].

Note, however, that there may be a case to be made against using mozId.watch() on the System app [3], so we need to figure out what to do here.

1- https://bugzilla.mozilla.org/show_bug.cgi?id=1000323#c9
2- https://bugzilla.mozilla.org/show_bug.cgi?id=1000323#c8
3- https://bugzilla.mozilla.org/show_bug.cgi?id=938901#c24

	Fernando Jiménez Moreno [:ferjm]
	Comment 1 • 10 years ago

I already expressed why I think we should not use mozId.watch() in the system app and use the chrome event instead at https://bugzilla.mozilla.org/show_bug.cgi?id=938901#c24

What I believe we should do here is to check why mozFxAccountsUnsolChromeEvent is giving an 'onlogout' event in this case and change it for a 'oncancel' one if needed.

	Sam Penrose
	Comment 2 • 10 years ago

Reducing memory usage is important. On the other hand, FMD is an FxA client, and there is value both for FMD and for FxA in having it use a client API, especially one like BrowserID that has been heavily tested in production. On the first hand :-), BrowserID is not a perfect fit for FxA. I have discussed this offline with both Jed and Ian (our engineering manager, whom I am Cc:ing). Jed's reaction was similar to mine: it's a tough call. He leaned slightly towards the IAC API. Ian feels strongly that FMD should use the client API, at least if is not "part of the system". (I think being part of the system process should not necessarily means using system internals.) All three of us feel strongly that FMD should not mix APIs. Here is the decision tree I think we have:

  1) If the extra memory usage is a ship/no-ship decision, then we have to use IAC. I do not believe this is the case, but Fernando may know otherwise.
  2) Next, if Guilherme does not want to remove BrowserID and get IAC working, we stay with BrowserID.
  3) Next, if Guilherme does want to convert to IAC, we should make sure that everyone is comfortable with the loss of appropriate separation, and also of the loss of our first client. It means that as we open FxA up to privileged apps, we will not benefit from the experience of having used the client API with FMD.

Guilherme, what do you think?

	Fernando Jiménez Moreno [:ferjm]
	Comment 3 • 10 years ago

Sorry, I have not explained myself properly about this, so let me rephrase and extend my previous comment.

First of all, I don't think making the FMD client use the IAC API instead of the mozId API is an option. I never said that and I did not even mentioned the IAC API in my previous comment.

We are talking about the consumption of a chrome event vs the usage of the mozId API *within the System app*.

I understand that there might be some confusion given that we trigger some IAC API events when we listen for this very same chrome event that FMD is also listening at [2], but the IAC API should be outside of this discussion. The IAC API was done to allow other apps to do FxA management actions (account lifecycle basically) and the FMD client does not need and should not do that. And in any case, the root of this issue is *within* the System app and *not outside* of the System app, which is where the IAC API is available.

With that said, let me review the current status and the reason for this bug. Guilherme can correct me if I am wrong.

0. FMD has two pieces in Gaia: (1) the FMD App and (2) a launcher in the System App.

1. The FMD App is a FxA RP and so it is using the FxA RP API which is navigator.mozId [1]. I believe this is the correct way to do it and we should keep doing this. No FxA IAC API is needed here.

2. The FMD launcher that lives in the System App [2] among other things is listening for a chrome event coming from the FxA implementation to disable FMD if there is no FxA logged in the device. Note that this is not part of the FMD App but part of the System App. I also believe this is the correct way to do it. We don't need the whole mozId mechanism in the System App to listen for a logout event *because we already have that for free in the System app*. That's what I mentioned in [3] and I still defend.

3. The chrome event that the FMD launcher is listening in the System App is erroneously reporting "onlogout" when the user cancels the FxA login flow. *And this is the reason of this issue according to [4]* and what we need to fix.

4. What I think we should do, instead of stop listening to the chrome event in favor of using the whole mozId machinery in the System app, is to find out why we are notifying a "onlogout" within this chrome event when the FxA login flow is closed by the user and notify with "oncancel" instead, which is the correct description of what's happening and will not make FMD to disable itself at [2].

I hope it is more clear this way :).

[1] https://github.com/guilherme-pg/gaia/blob/bug1000323/apps/findmydevice/js/findmydevice.js#L29
[2] https://github.com/guilherme-pg/gaia/blob/bug1000323/apps/system/js/findmydevice_launcher.js#L24
[3] https://bugzilla.mozilla.org/show_bug.cgi?id=938901#c24
[4] https://bugzilla.mozilla.org/show_bug.cgi?id=1000323#c8

	Guilherme Gonçalves [:ggp] Reporter
	Comment 4 • 10 years ago

Fernando, you summarized the situation perfectly, and I agree with you that the most pressing issue here is the "onlogout" chrome event being fired instead of "oncancel".

It does feel a little strange to me that FMD is interacting with FxA using two different methods, despite the fact that one of them lives outside the FMD app. However, as far as I know, the System app is indeed considered part of the system (as it isn't easily upgraded/modified on its own), so I don't really object to tapping into internal chrome events there if it means reduced memory pressure.

(The fact that we have such a component inside the system app means it will be hard/impossible for third-party developers to work on a replacement app for FMD, which is sad but obviously outside scope of this bug).

TL;DR I agree with Fernando.

	Sam Penrose
	Comment 5 • 10 years ago

Fernando thanks very much for your patient explanation. My apologies for not understanding in the first place. I accept your reasoning.

One small detail: because FxA does NOT require a password to sign out, I believe that FMD should not automatically be disabled when it does receive onlogout, per the FMD user story [1]

If my reasoning is correct then:
  1) Maybe FMD does not need to detect these events
  2) In order to disable FMD after a signout, the user will need to sign in, then disable-with-password, then sign out.

If my reasoning is not correct (which would mean that an attacker can disable FMD without knowing the FxA password), then this bug should be assigned to myself and the component should be FxA. Let me know, and thanks again for your comments.

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=941239

	Fernando Jiménez Moreno [:ferjm]
	Comment 6 • 10 years ago

I guess FMD still wants to keep listening for the onlogout event to be able to disable itself when there is no logged in account in the device.

After thinking about this twice, I believe the issue is in the refresh authentication API. We currently assume that a RP asking for a refreshed authentication expects us to log the account out if the introduced password is incorrect. However, that might not always be the case as FMD proves. I think we can fix this bug by improving the refresh authentication API allowing the RP to prevent the default logout if the authentication fails. So we could have something like:

navigator.mozId.request({ refreshAuthentication: { time: 0, preventDefault: true } });

(to be bikeshedded)

What do you think?

	Guilherme Gonçalves [:ggp] Reporter
	Comment 7 • 10 years ago

(In reply to Sam Penrose from comment #5)
> One small detail: because FxA does NOT require a password to sign out, I
> believe that FMD should not automatically be disabled when it does receive
> onlogout, per the FMD user story [1]
> 
> If my reasoning is correct then:
>   1) Maybe FMD does not need to detect these events
>   2) In order to disable FMD after a signout, the user will need to sign in,
> then disable-with-password, then sign out.

Ouch, I believe you are correct. Thanks for catching that. One issue with this solution is that it requires additional bookkeping to make sure that the account that is signing in is the same that registered for FMD. Otherwise the following scenario is possible:

1) Alice logs in to FxA and enables FMD on her phone;
2) Bob steals Alice's phone and logs out FxA on the phone (no password required for this);
3) Bob logs in to _his_ account on the phone;
4) Bob disables FMD.

In case we do adopt your proposed solution, what is an identifier for Alice's account that FMD could store to prevent this scenario? Maybe the email account as provided in the onlogin callback?

As an alternative, would it be possible to require re-authentication when logging out of FxA if FMD is enabled?

	Sam Penrose
	Comment 8 • 10 years ago

(In reply to Fernando Jiménez Moreno [:ferjm] from comment #6)
> We currently assume that a RP asking for a refreshed
> authentication expects us to log the account out if the introduced password
> is incorrect.

The User Story for forceAuth specifically stated that failing an RP-induced test would NOT log the user out:

  https://bugzilla.mozilla.org/show_bug.cgi?id=949093#c0

This makes sense if you think about it for non-FMD RPs: if my kid tries to buy something in MP, and FxA stops him, we don't want to kill my session. So I think we already have the functionality you propose here.

	Sam Penrose
	Comment 9 • 10 years ago

Erin: I think we have a product/UX problem. Please let me know how you would like to move forward. Details below the quoted material.

(In reply to Guilherme Gonçalves [:ggp] from comment #7)
> (In reply to Sam Penrose from comment #5)
> > One small detail: because FxA does NOT require a password to sign out, I
> > believe that FMD should not automatically be disabled when it does receive
> > onlogout, per the FMD user story [1]
> > 
> > If my reasoning is correct then:
> >   1) Maybe FMD does not need to detect these events
> >   2) In order to disable FMD after a signout, the user will need to sign in,
> > then disable-with-password, then sign out.
> 
> Ouch, I believe you are correct. Thanks for catching that. One issue with
> this solution is that it requires additional bookkeping to make sure that
> the account that is signing in is the same that registered for FMD.
> Otherwise the following scenario is possible:
> 
> 1) Alice logs in to FxA and enables FMD on her phone;
> 2) Bob steals Alice's phone and logs out FxA on the phone (no password
> required for this);
> 3) Bob logs in to _his_ account on the phone;
> 4) Bob disables FMD.
> 
> In case we do adopt your proposed solution, what is an identifier for
> Alice's account that FMD could store to prevent this scenario? Maybe the
> email account as provided in the onlogin callback?
> 
> As an alternative, would it be possible to require re-authentication when
> logging out of FxA if FMD is enabled?

Yes, you have found a simple attack that will work :-(. I think the right path forward is to ask FMD product and UX to think carefully about ALL of these scenarios (there are not that many). I want to vote for your first choice (email caching), but in my mind the UX gets weird -- wait, I'm logged in as Bob, but to disable FMD I have to log out, get the phone to Alice, and have her pass a challenge which will log her in, then have her log out? No. I don't see immediate problems with #2, but that doesn't mean anything.

Erin, can you designate a leader for this product exploration? Feel free to begin by asking us to explain the problem more clearly :-).

	Erin Lancaster [:elan]
	Comment 10 • 10 years ago

This should be in Vishy's camp;let's elaborate in today's meeting. Problem summary can go, here (line5): https://etherpad.mozilla.org/FMD-May-15-2014

	vkrishnamoorthy
	Comment 11 • 10 years ago

I remember  discussing this at one of our meetings and the outcome was: when a user logs out of FxA, 
- it will be a secure logout (requiring as password) 
- User will be notified that services such as FMD will be disabled 
- FMD is disabled and settings modified to reflect this state

	Fernando Jiménez Moreno [:ferjm]
	Comment 12 • 10 years ago

(In reply to Sam Penrose from comment #8)
> (In reply to Fernando Jiménez Moreno [:ferjm] from comment #6)
> > We currently assume that a RP asking for a refreshed
> > authentication expects us to log the account out if the introduced password
> > is incorrect.
> 
> The User Story for forceAuth specifically stated that failing an RP-induced
> test would NOT log the user out:
> 
>   https://bugzilla.mozilla.org/show_bug.cgi?id=949093#c0
> 
> This makes sense if you think about it for non-FMD RPs: if my kid tries to
> buy something in MP, and FxA stops him, we don't want to kill my session. So
> I think we already have the functionality you propose here.

D'oh! >< That's correct.

	Erin Lancaster [:elan]
	Comment 13 • 10 years ago

Circling back on this ticket. Can we basically close this as a dupe of Bug 1014816?

	Sam Penrose
	Comment 14 • 10 years ago

Yes, we can resolve it duplicate.