794634 - Handle unverified emails from persona login

Status: RESOLVED FIXED

(Marketplace Graveyard :: Consumer Pages, defect, P1)

Description

[Kumar McMillan [:kumar]]

When a user first logs in to the Marketplace on a phone they might be using an unverified email. Marketplace should parse this email and mark the account as *unverified*. At some future date, the user may log in with the same email but it will be verified.

This feature is not yet implemented in Persona but will be soon. There are some details to work out so check with Dan Mills on those details.

Here is an *example* of what would happen to illustrate (but it's not yet final). A user [e-mail] will log in and Persona will return an assertion to Marketplace with an email like [e-mail]. Marketplace will parse the real email and use that instead of the alias. Later on, we might see [e-mail] so we'll need to merge accounts or treat them as one.

The reason we want to mark accounts as unverified is so that we can restrict certain actions. We might want to restrict PIN resets to verified accounts, for example.

Comment 1

[Kumar McMillan [:kumar]]

Dan, is there a spec for this yet we can work towards?

Comment 2

[Ben Adida [:benadida]]

Kumar: details TBD, but high-level:

- you'll get an assertion

- you'll hit the verifier and get a data structure that looks much like the current one, except with the field "unverified-email" instead of "email".

Comment 3

[Wil Clouser [:clouserw]]

JR is checking this out.

Comment 4

[JR Conlin [:jrconlin,:jconlin]]

A few additional questions:

1. How will these unverified addresses be submitted to unverified.persona.org? (Can I create "[e-mail]"@unverified.persona.org to upload my free "Exploit my Phone!" app?

2. Validators other than persona.org will not be able to validate or provide these, right?

3. Will the "unverified-email" be the "unescaped" email address (E.g. [e-mail])

4. Should unverified email address expire?

5. Will there be other unvalidated email providers.

I'd strongly recommend that unverified assertions containing a previously verified address be rejected as invalid.

Comment 5

[Wil Clouser [:clouserw]]

We're going to revisit this next week after Ben has a chance to finish the spec.  We did some notes here: https://etherpad.mozilla.org/bTjvWrF4wr

Comment 6

[Wil Clouser [:clouserw]]

No spec as of yet.  Ben thinks there will be very minor changes to the marketplace.

Comment 7

[JR Conlin [:jrconlin,:jconlin]]

Is there a spec finalized for this yet? (Or at least a draft i can start working against?)

Comment 8

[Wil Clouser [:clouserw]]

Just talked with Ben.  No spec yet, anticipated delivery date is Nov 6th.

Comment 9

[Austin King [:ozten]]

Changes by area:

# Marketplace (Relying Party)
Instead of

    navigator.id.request();

use

    navigator.id.request({issuer: 'fxos.login.persona.org', allowUnverified: true});

# Remote Verification:

Instead of a POST to https://verifier.login.persona.org/verify with parameters assertion and audience, use

a POST to https://verifier.fxos.login.persona.org/verify with parameters allowUnverified, issuer, assertion, and audience.

Note: allowUnverified is optional and MUST match the value used in the nav.id.request call for this user flow. It is a boolean and defaults to false.

Note: issuer is optional in the general case but required for the b2g UA. It should be set to fxos.login.persona.org. The general case being Android Marketplace users, or the web.

If allowUnverified=true is used, the JSON encoded response body will have either email or unverified-email, to signal if this was an unverified Identity Assertion.

If allowUnverified=false or is not included in the request, only the email property is possible for successful verifications.

If issuer is used, the UA will ignore primary IdPs, so [e-mail] will be authenticated against fxos.login.persona.org even if example.com is a BrowserID Primary IdP.

# Local Verification:
A local verifier should follow the beta1 id-spec[1] with the following additions:

7. If the Relying Party is using the optional unverifiedEmail argument to navigator.id.request, then they SHOULD expect the principal to be either an unverified-email or email.
7.1 For a specific Identity, if the RP had previously accepted a verified Backed Identity Assertion, then it MUST NOT accept an unverified Assertion (Bug#805608)
7.2 Using the optional unverifiedEmail SHOULD accept an unverified-email as a success case and SHOULD continue to treat the identity as untrusted in the rest of 
 their business logic.

Your call if you want to do remote or local verification. We'll support either.

It's best to put the issuer (fxos.login.persona.org) and the verifier hostname into config as some bikeshedding may occur.

[1] Assertion Verification of https://github.com/mozilla/id-specs/blob/beta1/browserid/index.md

Comment 10

[Wil Clouser [:clouserw]]

Thanks Austin.  JR - can you have a look and see if that jives?  We can do some vidyo or email with Austin if there are questions.

Comment 11

[Kumar McMillan [:kumar]]

Thanks ozten. This sounds good. In the case of Android/desktop though, can we still verify assertions against https://verifier.fxos.login.persona.org/verify ? Or do we need to make a special case for these non-B2G requests and use https://verifier.login.persona.org/verify ?

Comment 12

[Austin King [:ozten]]

Yes, in the case of Android/desktop you can we still verify assertions against https://verifier.fxos.login.persona.org/verify

You will only include audience and assertion as parameters to it.

Comment 13

[JR Conlin [:jrconlin,:jconlin]]

(In reply to Kumar McMillan [:kumar] from comment #11)

Looks fairly straight forward and there's enough detail to work with. 
I'll ping ozten if there's any issues with getting this running.

Comment 14

[Jason Smith [:jsmith]]

Not on-device - removing nom.

Comment 15

[JR Conlin [:jrconlin,:jconlin]]

patch ready but pending https://github.com/mozilla/django-browserid/pull/96

Comment 16

[Wil Clouser [:clouserw]]

(In reply to JR Conlin [:jrconlin,:jconlin] from comment #15)
> patch ready but pending https://github.com/mozilla/django-browserid/pull/96

That patch was pulled in.  What's the status of this bug?

Comment 17

[Kumar McMillan [:kumar]]

JR says:

https://github.com/jrconlin/zamboni/commit/212c88f44b70e8b184b0ae3b0eee6ee0ac90450c
"Ignore the reformatting, assertion and quoting spoo and you get you'll get the meat of the patch."

It needs a rebase and a merge but otherwise should be ready to go.

Comment 18

[Wil Clouser [:clouserw]]

Putting in this milestone, but could slip since we're midweek.

Comment 19

[Christopher Van Wiemeersch [:cvan]]

https://github.com/mozilla/zamboni/commit/b65fad4

Comment 20

[Austin King [:ozten]]

I apologize that Comment#9 is out of date.

All references to "issuer" should be changed to "forceIssuer".

So the request parameter is forceIssuer. The verifier query string parameter is also forceIssuer.

Comment 21

[Kumar McMillan [:kumar]]

Whoops, cvan can you fix this? To test it, try forcing the wrong issuer to make sure the assertion fails then put it back.

Comment 22

[Christopher Van Wiemeersch [:cvan]]

https://github.com/mozilla/zamboni/commit/837aa96