Austin King [:ozten] Reporter
	Description • 12 years ago

We're adding an issuer  parameter to navigator.id.request in Bug#794634 Comment#9.

It is a string parameter which is the hostname. When present, the UA should not attempt discovery on the primary or persona secondary. Instead it should try to issue assertions from the specified issuer.

Task break down is a little fuzzy, but for now definitely:
1) Ensure the value flows through via postMessage API
2) Limits callers to Marketplace via whitelist

	Austin King [:ozten] Reporter
	Comment 1 • 12 years ago

Ben wants to remove #2.

	Jed Parsons (use needinfo, please) [:jedp, :jparsons] Assignee
	Comment 2 • 12 years ago

Marking this as dependent on Bug 811812 because the separate patches must be applied in order

	Jed Parsons (use needinfo, please) [:jedp, :jparsons] Assignee
	Comment 3 • 12 years ago

To force the issuer, this may be sufficient as the gecko patch.

	Jed Parsons (use needinfo, please) [:jedp, :jparsons] Assignee
	Comment 4 • 12 years ago

To clarify my patch a little: It is just tests to confirm that the parameter is passed along.  I think Bug 804932, "pass arbitrary options." actually implements the feature; this is just to test that it works for this parameter.

	Ben Adida [:benadida]
	Comment 5 • 12 years ago

Comment on attachment 680966 [details] [diff] [review]
pass an 'issuer' parameter from the RP to the shim

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

love the test-focused stuff.

::: toolkit/identity/tests/unit/test_minimalidentity.js
@@ +95,5 @@
> +    run_next_test();
> +   });
> +
> +  MinimalIDService.RP.watch(mockedDoc);
> +  MinimalIDService.RP.request(mockedDoc.id, {issuer: "https://jed.gov"});

wait, you're a government entity now? I better watch my review.

::: toolkit/identity/tests/unit/test_relying_party.js
@@ +186,5 @@
> +
> +  RelyingParty.watch(mockedDoc);
> +
> +  makeObserver("identity-request", function(aSubject, aTopic, aData) {
> +    dump("teh obj is " + JSON.stringify(aSubject.wrappedJSObject) + "\n");

I suppose you should fix that typo: "teh" --> "the"

	Jed Parsons (use needinfo, please) [:jedp, :jparsons] Assignee
	Comment 6 • 12 years ago

(In reply to Ben Adida [:benadida] from comment #5)
> Comment on attachment 680966 [details] [diff] [review]
> pass an 'issuer' parameter from the RP to the shim
> 
> Review of attachment 680966 [details] [diff] [review]:

> I suppose you should fix that typo: "teh" --> "the"

Now we see which spelling I use more often ...

	Jed Parsons (use needinfo, please) [:jedp, :jparsons] Assignee
	Comment 7 • 12 years ago

(In reply to Ben Adida [:benadida] from comment #5)
> Comment on attachment 680966 [details] [diff] [review]
> pass an 'issuer' parameter from the RP to the shim
> 
> Review of attachment 680966 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> love the test-focused stuff.

Yes, the forward-compatible arbitrary params takes care of all this.  Just making sure it doesn't break in the future.

> ::: toolkit/identity/tests/unit/test_minimalidentity.js
> @@ +95,5 @@
> > +    run_next_test();
> > +   });
> > +
> > +  MinimalIDService.RP.watch(mockedDoc);
> > +  MinimalIDService.RP.request(mockedDoc.id, {issuer: "https://jed.gov"});
> 
> wait, you're a government entity now? I better watch my review.

malloc(4 * sizeof(year_t))

> ::: toolkit/identity/tests/unit/test_relying_party.js
> @@ +186,5 @@
> > +
> > +  RelyingParty.watch(mockedDoc);
> > +
> > +  makeObserver("identity-request", function(aSubject, aTopic, aData) {
> > +    dump("teh obj is " + JSON.stringify(aSubject.wrappedJSObject) + "\n");
> 
> I suppose you should fix that typo: "teh" --> "the"

The whole debug statement can go.

	Jed Parsons (use needinfo, please) [:jedp, :jparsons] Assignee
	Comment 8 • 12 years ago

Addresses Ben's comments.

Checkin must wait until Bug 804932 passes review.

	Ryan VanderMeulen [:RyanVM]
	Comment 9 • 12 years ago

This has f+ but no r+?

	Jed Parsons (use needinfo, please) [:jedp, :jparsons] Assignee
	Comment 10 • 12 years ago

I'll resubmit the patch anyway; it's missing username and comment

	Ryan VanderMeulen [:RyanVM]
	Comment 12 • 12 years ago

https://hg.mozilla.org/integration/mozilla-inbound/rev/9a33aa064e10

	Ryan VanderMeulen [:RyanVM]
	Comment 13 • 12 years ago

https://hg.mozilla.org/mozilla-central/rev/9a33aa064e10

	Jed Parsons (use needinfo, please) [:jedp, :jparsons] Assignee
	Comment 14 • 12 years ago

This needs a bb+ for the patch to be uplifted to beta

	Jed Parsons (use needinfo, please) [:jedp, :jparsons] Assignee
	Comment 15 • 12 years ago

Comment on attachment 682779 [details] [diff] [review]
ensure forceIssuer parameter is passed through

[Approval Request Comment]
Bug caused by (feature/regressing bug #): feature; these are unit tests
User impact if declined: native identity will not be fully tested
Testing completed (on m-c, etc.): m-c
Risk to taking this patch (and alternatives if risky): none
String or UUID changes made by this patch: none

	Jed Parsons (use needinfo, please) [:jedp, :jparsons] Assignee
	Comment 16 • 12 years ago

(In reply to Jed Parsons [:jparsons] from comment #14)
> This needs a bb+ for the patch to be uplifted to beta

Sorry, no it doesn't.  Requested beta approval per jsmith's suggestion.

	Ryan VanderMeulen [:RyanVM]
	Comment 17 • 12 years ago

https://hg.mozilla.org/releases/mozilla-beta/rev/f962f111e31e