Skip to content
This repository has been archived by the owner on May 10, 2019. It is now read-only.

Realms #3854

Closed
wants to merge 9 commits into from
Closed

Realms #3854

wants to merge 9 commits into from

Conversation

seanmonstar
Copy link
Contributor

This adds realm support to Persona, specifically using the watch() api.

Realms allow a group of sites to state they are connected as part of a group, and if a user logs into 1 site of the group, or realm, the user should be logged into all the sites in realm.

This can be done by passing an extra parameter to watch():

navigator.id.watch({
  realm: 'http://realm.123done.org'
});

The second part of the puzzle is that the watched realm must publish a well-known file describing everyone that is part of the realm.

GET http://realm.123done.org/.well-known/browserid-realm
{
  realm: ['http://foo.123done.org', 'http://bar.123done.org']
}

If the site's origin is found in the list of the realms found at the well-known file, then Persona will believe it is part of the realm. If a user had logged in to foo.123done.org, and then visited bar.123done.org, and that site also watches the same realm, the user will be automatically logged in upon page load.

fixes #2555

Blockers to merging

  • B2G testing with @jedp
  • QA? perhaps QA is involved once merged to dev?
  • Plan for automated testing, how can we avoid regression? Do we need a new (set) of RPs so we can confirm we don't regress on this feature?
  • security testing with Yvan
  • legal bug with @lloyd @seanmonstar and Jishnu
  • identity-ops deploying new proxy mozilla/identity-ops@e6e2931

This adds realm support to Persona, specifically using the watch() api.

Realms allow a group of sites to state they are connected as part of a
group,
and if a user logs into 1 site of the group, or realm, the user should
be logged
into all the sites in realm.

This can be done by passing an extra parameter to watch():

navigator.id.watch({
realm: 'http://realm.123done.org'
});

The second part of the puzzle is that the watched realm must publish a
well-known file describing everyone that is part of the realm.

GET http://realm.123done.org/.well-known/browserid-realm
{
realm: ['http://foo.123done.org', 'http://bar.123done.org']
}

If the site's origin is found in the list of the realms found at the
well-known file,
then Persona will believe it is part of the realm. If a user had logged
in to
foo.123done.org, and then visited bar.123done.org, and that site also
watches
the same realm, the user will be automatically logged in upon page load.
const allowed_kpi = /^https:\/\/[a-zA-Z0-9\.\-_]+\/wsapi\/interaction_data(\?.*)?$/;

var server = http.createServer(function (req, res) {
var url = req.url;
if (!allowed.test(url) && !allowed_kpi.test(url) ) {
if (!allowed.test(url) && !allowed_realm.test(url) && !allowed_kpi.test(url) ) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will this also require changes to our production proxy?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly, @gene1wood or @jrgm?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shane-tomlinson Will this change result in persona initiating outbound connections out to the internet to new destinations (destinations other than sites' well-known/.browserid files and identity bridge endpoints [yahoo, gmail, etc])?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gene1wood - indeed, the new outbound request will search for realm files at <rp_location>/.well-known/browserid-realm

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proxy server support added here mozilla/identity-ops@e6e2931

When QA wants to see this code in stage they'll need to request that new proxy server AMIs be built with this new code in the process. (cc @jrgm)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent, thanks @gene1wood

to use a realm, pass a query parameter

Example: localhost:10001/?realm=http://localhost:10001
@seanmonstar
Copy link
Contributor Author

@ozten I can, but I dislike it. hacks away

@jrgm
Copy link
Contributor

jrgm commented Sep 9, 2013

You can use something like stunnel[1] to get https working with the local environment.
[1] https://www.stunnel.org/

@ozten
Copy link
Contributor

ozten commented Sep 9, 2013

Word. I also dislike SHIMMED_PRIMARIES ( I think a config file or something would be nicer), but something like it.

@seanmonstar
Copy link
Contributor Author

Yea, I've just now make a SHIMMED_REALMS dealio, and it works. But I had to remove 2 tests that were specific to http requests (status code and content-type), which is unfortunate.

@seanmonstar
Copy link
Contributor Author

New commit requires that the browserid-realm file be served over HTTPS.


// Support for "shimmed realms" for local development.
// CSV values:
// <realm>|<browserid-realm filepath>,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This took me a while to figure out how to do from bash command line:

SHIMMED_REALMS=127.0.0.1\|example/rp/.well-known/browserid-realm npm start

Can you add a comment about how to make it so? This info should probably go into the front end wiki as well.

@shane-tomlinson
Copy link

While testing this, I came across a problem: I can't log out!

STR

  1. Set up dev environment to use a shimmed realm of 127.0.0.1
  2. In tab 1, load up http://127.0.0.1:10001/?realm=127.0.0.1
  3. In tab 2, load up http://localhost:10001/?realm=127.0.0.1
  4. Sign in to the RP in tab 1
  5. Sign in to the RP in tab 2
  6. In tab 2, click "logout". Refresh the page. Still logged in... :(

@seanmonstar
Copy link
Contributor Author

@shane-tomlinson ah, I just found out why. The communication_iframe doesn't call user.logout(), instead it has it's own implementation. Can you of a reason why? I can't, so I'll likely just have it call User.logout().

- fixes navigator.id.logout() to logout of realm
- tests for storage.realmInfo
- code style change for lib/realm
- comments
- realm no longer can include port
- consolidating fixupIssuer and fixupRealm
@seanmonstar
Copy link
Contributor Author

@shane-tomlinson Newest commit tries to address your concerns. Please berate if I forgot any.

@jaredhirsch
Copy link
Member

@seanmonstar anything I can do to push this thing along?

@seanmonstar
Copy link
Contributor Author

@6a68 blockers are in a checklist at top of this PR.

currently, trying to deploy a couple of 123dones that use realms, so that security and @jedp can test against them

request does tunneling with proxies, and our proxies don't support
tunneling. until request provides an option to disable tunnels, we'll
have to use our own http code
@jedp
Copy link
Contributor

jedp commented Sep 21, 2013

There are some interesting issues with b2g. (What else is new?) I think we're going to need some platform code to make this work.

Basically, the removal of items from localStorage fires events that we can catch on normal web sites, but on b2g, data siloing and the discarding of the persona iframe when RPs are not using it (a we-will-kill-you-if-you-do-not-do-this requirement from platform) conspire to make it impossible for other RPs to hear about the logout unless we write some native code to fire onlogout for every RP with the same realm. If that's the only solution, backwards compatibility with existing FirefoxOS devices is not possible.

I've filed this bug for FirefoxOS: https://bugzilla.mozilla.org/show_bug.cgi?id=919157

@shane-tomlinson maybe we can pow-wow quickly about this for a sanity check? Make sure I've not got this completely wrong?

with multiple tabs open on sites that all belong to a realm, when
logging into one, all the others should also be logged in
@lloyd
Copy link
Contributor

lloyd commented Sep 25, 2013

wild question. If we didn't have to worry about logout events, how much simpler would this code be? In that case, we would simply flip the logged-on bit at login time, and users would logout of each realm independently... right?

@callahad
Copy link
Contributor

See my nay-saying re: logout events in #3915 (comment) :)

@jaredhirsch
Copy link
Member

@seanmonstar I think, as far as automated tests go, having 2-3 long-lived subdomains of 123done is perfectly fine. They're distinct from the perspective of the same-origin policy, and that's the simplest solution I can think of.

I'd check that box and get on with the other 5. Do you need help with writing automated tests?

@jaredhirsch
Copy link
Member

Closing because upcoming API changes (see #3961) remove realms support. @seanmonstar please reopen if I'm off base.

jrgm pushed a commit to jrgm/identity-ops that referenced this pull request Dec 5, 2013
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants