961253 - Restrict the set of issuers trusted by tokenserver

Status: VERIFIED FIXED

(Cloud Services Graveyard :: Server: Token, defect)

Description

[Ryan Kelly [:rfkelly]]

The initial plan for FxA+sync is to have tokenserver accept *only* assertions issued by the authoritative FxA auth server.  We should add a configuration option "trusted_issuers" that can be used to restrict this.  Probably need to push support for that down into PyBrowserID, but we could hack it in the tokenserver directly in the first instance.

Comment 1

[Ryan Kelly [:rfkelly]]

This is part of the new verifier API, so preliminary support will be through Bug 965135.  /cc @ckarlof to confirm that the only trustedIssuer for production should be "api.accounts.firefox.com

Comment 2

[Chris Karlof [:ckarlof]]

(In reply to Ryan Kelly [:rfkelly] from comment #1)
> /cc @ckarlof to confirm that the only trustedIssuer for
> production should be "api.accounts.firefox.com

I think you thought you were in github here. :) Use a need info flag in the future for this please. And yes, the only production issuer will be api.accounts.firefox.com.

Comment 3

[Ryan Kelly [:rfkelly]]

Attachment: tokenserver-allowed-issuers.diff

This patch adds an "allowed_issuers" option to the verifier.  It can be set to a list in order to restrict the set of issuers from which we'll accept assertions.

Note that the semantics here are different from the trusted_issuers option; that specifieds which issuers are trusted to assert identities on any domain, while allowed_issuers controls which issuers you will accept assertions from, for their own domain or otherwise.

We will have to add "api.accounts.firefox.com" to both trusted_issuers and allowed_issuers to get the semantics we want.  I don't think we can let one fill in defaults for the other without created wierd semantics in the general case.

Comment 4

[Toby Elliott [:telliott]]

Comment on attachment 8369275 [details] [diff] [review]
tokenserver-allowed-issuers.diff

Review of attachment 8369275 [details] [diff] [review]:
-----------------------------------------------------------------

::: tokenserver/verifiers.py
@@ +46,4 @@
>          if isinstance(trusted_issuers, basestring):
>              trusted_issuers = trusted_issuers.split()
>          self.trusted_issuers = trusted_issuers
> +        if isinstance(allowed_issuers, basestring): 

There's a space at the end of this line that is colored strangely in the review markup. I suspect it's nothing, but just in case it's a weird nondisplaying character, I'm highlighting it.

Comment 5

[Ryan Kelly [:rfkelly]]

Fixed stray trailing space and committed: https://github.com/mozilla-services/tokenserver/commit/9216aa99eb50a6fcf9805916f319650547c8e7d2

Comment 6

[James Bonacci [:jbonacci]]

Verified in code.