Prefer {uid,email} to {email} throughout

[ncalexan]

In general, we should identify by UID. It's my understanding that not all pages accept uid=UID (like they accept email=EMAIL).

In particular, at

fxa-content-server/app/scripts/models/auth_brokers/fx-desktop.js

Line 78 in 2a78a14

return self.request(self._commands.CAN_LINK_ACCOUNT, { email: email })

, we don't give the client a UID to check against its records, so a user could potentially switch the underlying server.

[vladikoff]

@ncalexan is this required for the first version of fx_fennec_v1? I think we are going to target the next train for this feature

[ncalexan]

On Tue, Oct 6, 2015 at 9:21 AM, Vlad Filippov notifications@github.com
wrote:

@ncalexan https://github.com/ncalexan is this required for the first
version of fx_fennec_v1? I think we are going to target the next train for
this feature

@vladikoff: nope! Just a thing we should remember for new API surfaces,
and that we should move towards in the future.

[vladikoff]

Looking at the code linked and the CAN_LINK_ACCOUNT event, it happens "beforeSignIn" (before actual sign in) for a few reasons commented in the code. At the point of CAN_LINK_ACCOUNT we do not know the user's uid and cannot provide it to the client to check.

@ncalexan what other pages did you have in mind when you said It's my understanding that not all pages accept uid=UID are you worried about changing the primary email of the account?

so a user could potentially switch the underlying server.

a bit of an edge case. However if that happens I assume the session will be different or expired.

side-note: I think we were looking into the uid vs email problem with the primary email change as well.

@shane-tomlinson @rfk thoughts?

[ncalexan]

On Wed, Oct 7, 2015 at 11:58 AM, Vlad Filippov notifications@github.com
wrote:

Looking at the code linked and the CAN_LINK_ACCOUNT event, it happens
"beforeSignIn" (before actual sign in) for a few reasons commented in the
code. At the point of CAN_LINK_ACCOUNT we do not know the user's uid and
cannot provide it to the client to check.

Hmm. OK.

@ncalexan https://github.com/ncalexan what other pages did you have in
mind when you said It's my understanding that not all pages accept uid=UID
are you worried about changing the primary email of the account?

so a user could potentially switch the underlying server.

a bit of an edge case. However if that happens I assume the session will
be different or expired.

Right now we use force_auth&email=..., rather than the more secure
force_auth&uid=.... I read the code and thought that force_auth did
not support uid=....

I agree that this is an edge case, but this ticket is partly for education
and setting technical direction: in future, we should uid exclusively.
(Partly because email addresses may come and go.)

[rfk]

I support the general direction here, of "prefer uid= to email= as a way to identify the user where possible", since it's a more robust and stable identifier.

force_auth&email=..., rather than the more secureforce_auth&uid=...`

In this case, I think we need the email address to show an appropriately-filled-out login screen. We can't necessarily lookup the email given the uid, although IIRC we can check whether the account exists.

UX affordances aside, we could accept uid= in the query params and insist that the user login to that particular account. It's not clear how much value that would provide @ncalexan. It also puts us in the same conundrum as #3057, dead-ending users if they get into a bad state.

So...I'm not sure whether there's anything actionable coming out of this bug at this time.

(It will be relevant when we do more device-handshake stuff, because in that case the browser can give us a usable sessionToken, allowing us to look up the account info given just the uid)

[vladikoff]

So...I'm not sure whether there's anything actionable coming out of this bug at this time.

(It will be relevant when we do more device-handshake stuff, because in that case the browser can give us a usable sessionToken, allowing us to look up the account info given just the uid)

I agree that we should set this up. However removing the fennec milestone from this and removing assignment for now, until we need this later.

[rfk]

There was good discussion here, thanks for raising it @ncalexan. However it doesn't look like there's anything actionable to do right now, but just something to keep in mind in future, so I'm closing it out.