Skip to content
This repository has been archived by the owner. It is now read-only.

Compatability with new hosted verifier #292

Closed
rfk opened this issue Nov 12, 2013 · 21 comments
Closed

Compatability with new hosted verifier #292

rfk opened this issue Nov 12, 2013 · 21 comments
Assignees

Comments

@rfk
Copy link
Member

@rfk rfk commented Nov 12, 2013

Looks like we're going to generate assertions in a format that won't be valid according to the default persona.org verified, per #275. We'll have to host our own verifier that trusts FxA as a secondary, and that echos back the uid along with the email.

(edit: changing title to better reflect current plans, see discussion below)

@ckarlof
Copy link
Contributor

@ckarlof ckarlof commented Nov 12, 2013

@ckarlof
Copy link
Contributor

@ckarlof ckarlof commented Nov 12, 2013

@kparlante has indicated that we should do adequate logging here to enable metrics on what Mozilla services users are using with their FxA.

@rfk
Copy link
Member Author

@rfk rfk commented Nov 13, 2013

we should do adequate logging here to enable metrics on what Mozilla services users are using with their FxA.

Does this boil down to aggregate logging of the "audience" field from the assertions? Or do we need something more fine-grained?

@rfk
Copy link
Member Author

@rfk rfk commented Nov 13, 2013

This should be a simple matter of hosting a fork of the browserid verifier with appropriate config changes. The only trick will be reporting both the uid and email, for which I propose we just include then in the top-level JSON response from the verifier. The flow would look like:

> POST https://verifier.accounts.firefox.com/verify HTTP/1.1
> {
>   "assertion":  "blahblahblah",
>   "audience":  "https://marketplace.firefox.com"
> }
.
< HTTP/1.1 200 OK
< {
<   "email": "me@mycooldomain.com",
<   "uid": "1234-5678-9101112",
<   "audience": "https://marketplace.firefox.com",
<   "expires": 12345678,
<   "issuer":  "https://accounts.firefox.com",
< }

(I'd prefer to echo back the entire "principal" blob as a sub-object, but that's not how the current verifier does things)

I think these details should be transparent to the client code in Firefox, but /cc @jedp to sanity-check whether this lines up with his expectations from #275.

@ckarlof who are going to be the first users of this verifier? I'm guessing Marketplace and WheresMyFox; anyone else I should reach out to at this stage?

@kparlante
Copy link

@kparlante kparlante commented Nov 13, 2013

@rfk Yes, aggregate logging of "audience" field (by day). Its also probably a good idea to log success/failure.

@ckarlof
Copy link
Contributor

@ckarlof ckarlof commented Nov 14, 2013

Please document this when appropriate: https://wiki.mozilla.org/Identity/FirefoxAccounts#Verifier

@lloyd
Copy link
Contributor

@lloyd lloyd commented Nov 14, 2013

/cc @callahad @fmarier @SamPenrose

I think it's a real shame to fork the verifier for this. Extended IdP properties are something that we need to support in persona anyway, and we already have forceIssuer inside the verifier, which is morally equivalent to the changes we would need here.

The thing I hate is this really makes it sloppy for 3rd party implementors who want to use firefox accounts when the UA supports it, but on the web just use persona.

Would anyone strongly object if I gave you a hosted verifier and API that solves this problem? This unlocks payswarm dudes to innovate and stuff on top of persona as well.

Sometimes the spoon is stronger than the fork.

@rfk
Copy link
Member Author

@rfk rfk commented Nov 14, 2013

we already have forceIssuer inside the verifier

Awesome, I didn't realize this, but probably should have given it's already needed for marketplace.

Would anyone strongly object if I gave you a hosted verifier and API that solves this problem?

I would strongly support this! If it's in-scope for the general-purpose hosted verifier, and in-line with deadlines for FxOS work, let's make it happen.

@ckarlof
Copy link
Contributor

@ckarlof ckarlof commented Nov 14, 2013

I'm fine with it as long as @kparlante can do her metrics on the existing verifier to measure how many FxA users are logging in to each relying service.

@lloyd
Copy link
Contributor

@lloyd lloyd commented Nov 14, 2013

wfm. I will propose to appropriate lists in 14ish hours. full list of requirements as I see them:
0. rest api (like today)

  1. accept assertions from a different issuer (that is not the issuer as determined by typical browserid well-known discovery)
  2. return "extended" (signed) properties to caller (those embedded in the certificate)
  3. ability to extract fxa specific metrics.

guten?

@ckarlof
Copy link
Contributor

@ckarlof ckarlof commented Nov 14, 2013

guten?

looks good

@kparlante
Copy link

@kparlante kparlante commented Nov 15, 2013

The verifier logs the data we need, the work will be to get that data to the elasticsearch db so that we can consume it. The work @shane-tomlinson did on refactoring the logging should make this not so hard, I'd think.
/cc @gene1wood

@ckarlof
Copy link
Contributor

@ckarlof ckarlof commented Nov 15, 2013

We can close this as long as we open appropriate issues on the BID side. Who owns that? @lloyd for the verifier changes? @kparlante for the log aggregation changes?

@fmarier
Copy link

@fmarier fmarier commented Nov 15, 2013

I agree it makes sense to share the verifier codebase between Persona and Fx Accounts to make sure they don't drift apart and start using incompatible versions of BrowserID. However it would be good to be able to separate them later if we need to.

So how about this API:

  • verifier.login.persona.org/verify has the same behaviour as now, only trusting login.persona.org as a secondary
  • verifier.accounts.firefox.com/verify (or whatever the URL scheme is for FxA) trusts both login.persona.org and accounts.firefox.com as secondaries

RPs that need to accept FxA assertions just point their code to a different verifier and everything works as expected. All other RPs don't see anything different and continue to treat FxA assertions as invalid.

Would that satisfy the needs of Fx Accounts?

@ckarlof
Copy link
Contributor

@ckarlof ckarlof commented Nov 16, 2013

@fmarier huh, I interpreted from @lloyd's proposal there would be one hosted verifier. Are you proposing two?

@fmarier
Copy link

@fmarier fmarier commented Nov 16, 2013

@ckarlof No, my suggestion was just one verifier instance but listening on two different URLs.

@ckarlof
Copy link
Contributor

@ckarlof ckarlof commented Nov 16, 2013

Interesting.

@warner, look.

This proposal allows for a smooth transition for Marketplace, but it also effectively allows "login to FxA attached services" via vanilla Persona. This is something we've discussed, but it's not clear we want to pull the pin on it yet. For example, this would allow "your IdP silent access to your class A sync data" as @warner likes to say.

Some alternatives:

  • verifier.accounts.firefox.com/verify only verifies FxA backed assertions (denies primary backed, i.e., federation disabled)
  • verifier.accounts.firefox.com/verify only verifies FxA or login.persona.org backed assertions (denies primary backed, i.e., federation disabled)

I'm sure the idea of those alternatives will raise some blood pressures. I think the second one would work for Marketplace because it currently disables federation.

@fmarier
Copy link

@fmarier fmarier commented Nov 16, 2013

I prefer the first alternative. verifier.accounts.firefox.com should either allow all Persona assertions (native IdPs and fallback) or none at all. The whole point of Persona is federated logins, if we're going to disable federation, we may as well not use Persona at all.

As to whether or not the FxA verifier should accept Persona assertions, that's something that could easily be changed later.

@lloyd
Copy link
Contributor

@lloyd lloyd commented Nov 20, 2013

It sounds like we've got some minor technical details, with major implications to figure out.

Can we have an in person meeting the 2nd-6th of december and come back to this thread with a concrete proposal?

Meanwhile, I've started work on the major technical details - a single verification library that satisfies all of the requirements imposed by both persona and fxa. I'll share on list(s) this week.

@rfk
Copy link
Member Author

@rfk rfk commented Nov 25, 2013

Link to the mailing-list post for my own bookkeeping purposes: https://mail.mozilla.org/pipermail/dev-fxacct/2013-November/000231.html

@pdehaan
Copy link
Contributor

@pdehaan pdehaan commented Feb 4, 2014

Closing per @rfk in triage since these bugs are captured elsewhere.

@pdehaan pdehaan closed this Feb 4, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants