867223 - [meta] SSO for webmaker.org apps

Status: RESOLVED INVALID

(Webmaker Graveyard :: webmaker.org, defect)

Description

[Kieran Sedgwick (:sedge)]

In order to leverage the full benefits of an SSO system, a shared sessionCookie must be used by all Webmaker services and tools.  For this to work, they must all be hosted on the same root domain, and read/write the same cookie for session management.

Comment 1

[Mike Beltzner [:beltzner, not reading bugmail]]

That should be OK, as long as we make sure to open the published "make" in a separate window and are happy to not have the sessionCookie work there.

Comment 2

[Chris DeCairos (:cade)]

If the MakeAPI is going to use this for authenticated requests, the cookie must provide at least:

a) the user's email address
b) the users account type (admin/non-admin)

Comment 3

[Kieran Sedgwick (:sedge)]

We could tack the entire user object on there.  It looks like this:

   _id: {
      type: String,
      required: true
    },
    email: {
      type: String,
      required: true,
      unique: true,
      validate: validate('isEmail')
    },
    name: {
      type: String,
      required: true,
      unique: true,
      validate: validate('isDomain')
    },
    createdAt: {
      type: Date,
      required: true,
      "default": Date.now
    },
    updatedAt: {
      type: Date,
      required: true,
      "default": Date.now
    },
    deletedAt:{
      type: Date,
      required: false
    },
    isAdmin: {
      type: Boolean,
      "default": false
    },
    isSuspended: {
      type: Boolean,
      "default": false
    },
    sendNotifications: {
      type: Boolean,
      "default": true
    },
    sendEngagements: {
      type: Boolean,
      "default": true
    }

... and can be accessed with:

user.attribute = value;

Comment 4

[Simon C. Wex]

New data should be added to that cookie kicking and screaming. Let's do _id and isAdmin for now. 

The more we add to it, the more we have to continually support. -- Let's keep that surface area as small as possible.

Comment 5

[David Humphrey (:humph)]

+1 to Simon's points.

Comment 6

[Kieran Sedgwick (:sedge)]

Nice point :wex.

Since _id could be variable (it may not stay the email), lets put email and isAdmin in instead.

Comment 7

[Ross Bruniges [:boozeniges]]

Would also add displayName/subdomain to this

Comment 8

[Kieran Sedgwick (:sedge)]

Lets not forget that the LoginAPIs can be accessed directly if the Login session data is stored in a cookie accessible by all Webmaker services.  This means that if they need more information, all they need is the user's email and they can gain access to the entire user object anyway.

Comment 9

[Simon C. Wex]

:sedge, why email instead? It's going to be used for looking up records etc, this should be done with the `_id`. `email` should be used for contact and we should only add it if there's a requirement to do so. -- I did mean kicking and screaming.

Comment 10

[Simon C. Wex]

:boozeniges those are fine fields, but again, let's wait until there's a request to add it.

Comment 11

[Kieran Sedgwick (:sedge)]

:wex - it could go either way, but the makeAPI explicitly needs the email.

Comment 12

[Chris DeCairos (:cade)]

(In reply to Simon C. Wex from comment #9)
> :sedge, why email instead? It's going to be used for looking up records etc,
> this should be done with the `_id`. `email` should be used for contact and
> we should only add it if there's a requirement to do so. -- I did mean
> kicking and screaming.

(In reply to Kieran Sedgwick (:sedge) from comment #11)
> :wex - it could go either way, but the makeAPI explicitly needs the email.

Yes, I need to know what email is associate with the Webmaker account so that I can enforce tagging requirements in the MakeAPI.

Comment 13

[Simon C. Wex]

:cade you should be using the user id instead of email for the tag prefixes. It so happens that often _id == email

:sedge ^ that was the original design, you aren't doing anything funny with _id, are you?

Comment 14

[Kieran Sedgwick (:sedge)]

:wex - nope, just acknowledging that email will always be email, while _id could end up being something else in the future.  If you think _id is safe to use for that, _id it is!

Comment 15

[Simon C. Wex]

:cade, :sedge

It is possible that we may not have a value for `email`, we will always have an _id though.

Comment 16

[Kieran Sedgwick (:sedge)]

:cade - Here's where we're at so far: https://github.com/ksedge/login.webmaker.org/tree/b867218

Comment 17

[Kieran Sedgwick (:sedge)]

At this point we're attempting to fully understand why the session-sharing between apps isn't stable.  Apps can assume we WILL get it there, and the other teams can assume "req.session.auth"'s existence to confirm a logged in user.  

Req.session.auth will contain an attribute called "_id", containing the user's email for further API calls to the login server if needed.

Comment 18

[Pomax]

it's actually stable now, but we might have redundant code at this point. I filed an issue with express to find out whether that is the case or not, but in terms of stable session management, the pull requests are pending review atm.

Comment 19

[Kieran Sedgwick (:sedge)]

I'm going to refactor the login integration stuff into a module that encapsulates the standard calls an app might need to make to the login server. See bug 869576

Comment 20

[Jon Buckley]

We've decided to use https://github.com/jbuck/persona-sso for our SSO needs. It's currently being hosted at http://personasso.s3-website-us-east-1.amazonaws.com/include.html and works exactly like the current Persona API, except you need to use navigator.idSSO.* and use http://personasso.s3-website-us-east-1.amazonaws.com as your audience for verifying the assertion.

Comment 21

[Pomax]

invalidated, we landed local-cookies