Issue 2497 refactor to idp #3982

ozten · 2013-10-16T17:24:43Z

Background for pull request: Issue #2497 (comment)

Creating a pull request to try to get some frontend help.

Thanks to @shane-tomlinson for previously reviewing and getting it this far.

Known Issues:

siteName and other options aren't passed in from Desktop code, I'm investigating in gecko

Needs Frontend <3

rpinfo changes in dev broke this patch, minimal hardcoded changes were made to get it working again
- Need help doing this correctly

seanmonstar · 2013-10-16T18:02:06Z

fabulous! gets reviewing goggles

seanmonstar · 2013-10-16T18:02:35Z

lib/static/views.js

-      res.json({ 'public-key': publicKey.toSimpleObject() });
+      res.json({
+        'public-key': publicKey.toSimpleObject(),
+          'provisioning': '/provision',


indent is off

shane-tomlinson · 2013-10-17T15:08:56Z

So... The devil hiding in the details, password resets and transition states. Are they going to be handled?

ozten · 2013-10-17T17:13:50Z

The devil hiding in the details, password resets and transition states. Are they going to be handled?

Can we land this and start getting UX feedback, before implementing those features?

ozten · 2013-10-17T21:55:00Z

Trying to get awsbox working, I've merged from dev, but got bitrot for the second time.

I was able to see that logging.js moved and fix that.

The other issue is that the dialog just comes up blank.

To unblock myself, I've copied over scripts/deploy.js and changed the dependency to awsbox@0.6.2... but we'll have to figure out what changed about the dialog code that has broken the initial screen display for the new /auth screen. Any ideas?

jaredhirsch · 2013-10-18T18:24:59Z

@ozten I'll do some git twiddling on Monday, see if I can cleanly apply your changes on top of current dev.

jaredhirsch · 2013-10-21T18:16:40Z

ok, looking now.

jaredhirsch · 2013-10-21T20:07:07Z

@ozten nice work so far!

I was able to cleanly cherry-pick all the changes onto current dev (except for the commits merging dev into this branch). My branch is at https://github.com/6a68/browserid/tree/issue-2497-idp-rebased-not-merged.

One tiny logging path change was needed to get tests running, that's on the HEAD of that branch, a7fd566.

The local RP seems fine to me on my branch, I can log in and out.

ozten · 2013-10-21T20:08:49Z

@6a68 is rocking the merge from dev to update this branch, you rule!

Please try to reproduce the blank dialog I was seeing.

You can download a Persona enabled Desktop build:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/aking@mozilla.com-62351505bca9/

Set a couple preferences (about:config):

dom.identity.enabled = true
dom.identity.persona_fallback = http://127.0.0.1:10002
toolkit.identity.debug = true

Restart Firefox.

dom.identity.persona_fallback defaults to our new ephemeral server https://desktoppersona.personatest.org/ but you can change this to any environment.

Steps to reproduce:

Launch Firefox with a clean profile
Go to http://127.0.0.1:10001
Click Get an assertion
Enter foo@bar.com

Actual: empty dialog

Expected: prompt to set a password

jaredhirsch · 2013-10-21T21:11:58Z

@ozten happy to help :-)

I've got the special FF build, I toggled the settings you mentioned (actually tried using the desktoppersona fallback as well as localhost), but I'm not sure what to do next.

When I try to log into RPs, I click the persona button but nothing happens. Tried 123done, mozillians, nothing. Also not seeing errors in the JS console--maybe there's a desktop-specific console for the native code that I'm unaware of?

I'll poke a bit more at my branch and yours, see if I can at least get an empty dialog using your branch.

ozten · 2013-10-21T21:45:53Z

So in nightly if you go to about:config and search for identity does it look like this

Clicking get Assertion should look like this

jaredhirsch · 2013-10-21T22:37:04Z

It looks like the blank dialog error was caused by using hashes, but the recently-landed #3971 I think means we should be using querystring instead.

@ozten said this got him past the blank dialog problem in a local branch, I've updated my issue-2497-idp-rebased-not-merged branch with the fix (commit (9e8ed0f)[https://github.com/6a68/browserid/commit/9e8ed0f]).

jaredhirsch · 2013-10-21T22:38:50Z

@ozten i don't have the toolkit.identity.enabled pref at all, but I have the other three matching your screenshot.

The second image is broken for me.

ozten · 2013-10-21T22:45:05Z

Thanks Jared!

Using your patch, I see Error: Relay frame could not be found.

Thanks for continuing to dig for breaking changes from merging.

jaredhirsch · 2013-10-21T23:09:38Z

I have my current branch running on https://rebasedidp.personatest.org. Will pick this up again in the morning. 🍻

jaredhirsch · 2013-10-22T23:55:48Z

@ozten looks like this line probably shouldn't have that IP hard-coded. I'm not totally clear on where we can get the proper domain info in this context, though. any ideas?

jaredhirsch · 2013-10-23T00:11:00Z

heh, I caused the Relay Error: we look for the hash, not the querystring, when we setup the channel. Removing that, or changing that code to look for ?NATIVE as window.location.search, returns us to the mysterious empty dialog situation we had before. Continuing to dig.

* Create RpInfo before starting the modules. Hard coding the origin and rpName for now * Update IdpAuthentication module to pass RpInfo to the appropriate modules. * Start the check_registration module instead of calling user.waitForUserValidation since it already takes care of error messages.

ozten · 2013-11-07T15:01:44Z

@fmarier or @shane-tomlinson I've removed temporary hacks, console.log, etc. We should be good to merge as discussed.

General note: The Goldilocks API breaks the Native implementation, so use http://192.168.186.138:10001/watch.html instead of http://192.168.186.138:10001/

ozten · 2013-11-07T16:13:58Z

G R E E N

fmarier · 2013-11-07T16:42:21Z

resources/static/dialog/js/modules/idp_authentication.js

+        mediator.subscribe("new_user", newUserComplete(self));
+
+        var rpInfo = bid.Models.RpInfo.create({
+          origin: "http://127.0.0.1:10001",


Did you forget to take these out?

fmarier · 2013-11-10T13:04:01Z

r+ from me. makes Travis happy. tested by Shane and Austin.

Issue 2497 refactor to idp

ozten · 2013-11-10T13:05:09Z

All the man needed was a Guiness at the airport!

Thanks!

This allows the verifier to work-around a Persona bug that has been fixed (mozilla/persona#3982) but not yet deployed. Fixes mozilla#19

ozten mentioned this pull request Oct 16, 2013

login.persona.org should be a true IdP #2497

Closed

seanmonstar reviewed Oct 16, 2013
View reviewed changes

ozten and others added 18 commits November 7, 2013 11:58

Make wsapi/address_info provie auth and prov info

e806ea4

Created a Discovery web service for Desktop

76b7ae3

A couple gross hacks to get around bugs.

00e66d0

Minor cleanup

7dc2536

Tighten up authentication for new, registering users

7095759

jshint and travis - haters gonna hate

52b67e0

well known now returns auth and prov urls in well known

777dfef

Use cachify for idp_authentication resources

3f43f1d

cleanup - pass in email to authenticate module

c7a3f11

Switch from jquery to micrajax for /provision

959d6f1

Merged dev, which broke new user registration

e602d3f

Refactor to module

7d9173d

Temporary fix for rpinfo changes. Improve getErrorDialog

032bfd4

Fixes based on stomlinson and seanmonstar feedback

8a562be

Adding comments and renaming variable per francois

fe8f7ff

Just kidding, renamed a property of results

af245c1

Remove temporary hacks

e6bf257

fmarier reviewed Nov 7, 2013
View reviewed changes

Added test for disabled domain. Removed hardcoded rpInfo per fmarier

8b18afe

fmarier added a commit that referenced this pull request Nov 10, 2013

Merge pull request #3982 from mozilla/issue-2497-refactor-to-idp

173be4f

Issue 2497 refactor to idp

fmarier merged commit 173be4f into dev Nov 10, 2013

fmarier mentioned this pull request Nov 29, 2013

remove support for poorly behaved fallback mozilla/browserid-local-verify#5

Closed

fmarier mentioned this pull request Apr 9, 2014

Expose the allowURLOmission config option mozilla/browserid-verifier#32

Merged

jaredhirsch deleted the issue-2497-refactor-to-idp branch April 20, 2014 03:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Issue 2497 refactor to idp #3982

Issue 2497 refactor to idp #3982

ozten commented Oct 16, 2013

seanmonstar commented Oct 16, 2013

seanmonstar Oct 16, 2013

shane-tomlinson commented Oct 17, 2013

ozten commented Oct 17, 2013

ozten commented Oct 17, 2013

jaredhirsch commented Oct 18, 2013

jaredhirsch commented Oct 21, 2013

jaredhirsch commented Oct 21, 2013

ozten commented Oct 21, 2013

jaredhirsch commented Oct 21, 2013

ozten commented Oct 21, 2013

jaredhirsch commented Oct 21, 2013

jaredhirsch commented Oct 21, 2013

ozten commented Oct 21, 2013

jaredhirsch commented Oct 21, 2013

jaredhirsch commented Oct 22, 2013

jaredhirsch commented Oct 23, 2013

ozten commented Nov 7, 2013

ozten commented Nov 7, 2013

fmarier Nov 7, 2013

fmarier commented Nov 10, 2013

ozten commented Nov 10, 2013

Issue 2497 refactor to idp #3982

Issue 2497 refactor to idp #3982

Conversation

ozten commented Oct 16, 2013

seanmonstar commented Oct 16, 2013

seanmonstar Oct 16, 2013

Choose a reason for hiding this comment

shane-tomlinson commented Oct 17, 2013

ozten commented Oct 17, 2013

ozten commented Oct 17, 2013

jaredhirsch commented Oct 18, 2013

jaredhirsch commented Oct 21, 2013

jaredhirsch commented Oct 21, 2013

ozten commented Oct 21, 2013

jaredhirsch commented Oct 21, 2013

ozten commented Oct 21, 2013

jaredhirsch commented Oct 21, 2013

jaredhirsch commented Oct 21, 2013

ozten commented Oct 21, 2013

jaredhirsch commented Oct 21, 2013

jaredhirsch commented Oct 22, 2013

jaredhirsch commented Oct 23, 2013

ozten commented Nov 7, 2013

ozten commented Nov 7, 2013

fmarier Nov 7, 2013

Choose a reason for hiding this comment

fmarier commented Nov 10, 2013

ozten commented Nov 10, 2013