digidem / react-mapfilter

Visualizing, exploring, filtering and printing geographic data and geotagged photos and video
https://lab.digital-democracy.org/mapfilter
29 stars 11 forks source link

Feature/auth0 #24

Closed jlev closed 8 years ago

jlev commented 8 years ago

@gmaclennan, here's an initial working implementation of authentication using Auth0. It requires a config entry with a valid clientID and domain. Existing authentication by prompt continues to work if the config value is 'true' true.

Allowed email addresses are checked against a whitelist in Dropbox, and anything from the domain digital-democracy.org (or spacedog.xyz, for testing) is automatically allowed. Rules to enforce this are included in the rules directory, and can be synced to Auth0 via github webhook, although I've found this feature a little buggy.

The login text will be translated into the current locale, although I haven't yet merged in changes to pull that from the navigator property. I can also add a language selector dropdown, if that would be easier for users to adjust.

gmaclennan commented 8 years ago

This looks great @jlev. I don't think we should handle whitelists and email addresses ourselves however - it adds complexity and additional configuration to maintain. Why not leave this to Github? i.e. use Auth0 to login with a Github user account, and control access through Github? If we configure Auth0 to only allow Github logins, then we will get back a valid Github API token under the profile we get back from Auth0 lock I think:

"identities": [
    {
      "access_token": "xxxxxxxxxxxxxxxxxxxxxx",
      "provider": "github",
      "user_id": 999999,
      "connection": "github",
      "isSocial": true
    }
  ],

I think we should configure Mapfilter to take user/repo from the url hash, something similar to how geojson.io does it

For specific deployments under custom domains we would read that from the config rather than url hash - or simply a redirect to the URL hash for that config.

One question is whether we assume a standard repo structure and just point to the repo, or we point to a specific file in the repo for the data, and separately one for the config / translations.

This would allow Mapfilter to be used to read any geojson file on Github. I am thinking that for storage of monitoring data we should probably work with a single geojson feature collection rather than single files for each point? This would simplify things from Mapfilter's side, at least whilst we are not doing any writes. We could write a webhook to concat individual files into a feature collection.

jlev commented 8 years ago

Updated to use github access_token directly from auth0. If user is not allowed access to config'ed dataUrl, or other collection error thrown from backbone-sync, they are immediately logged out.

gmaclennan commented 8 years ago

Do you think we should respond differently to different types of error? e.g. what if this is being used over a poor connection and one of the GET requests returns an error? We don't necessarily want that to cause a logout.

jlev commented 8 years ago

Added code to parse JSON response from GitHub, and only trigger logout on auth errors. I determine that by checking if message === "Bad credentials", because we don't have access to the headers at that point. This will be brittle if they change the error message, but should be good enough for simple authentication.

Ready to merge?

gmaclennan commented 8 years ago

Looks good, can you add a // TODO: before I merge so we remember to update this with a less brittle implementation, maybe trying to get the error code or something?

jlev commented 8 years ago

NM, figured out how to check status code.

gmaclennan commented 8 years ago

This is definitely the status code we'll get? not 404? https://developer.github.com/v3/troubleshooting/

jlev commented 8 years ago

I was getting 401's with a dummy access token. But we should also be able to handle anything in 4xx range.

gmaclennan commented 8 years ago

Merging for now, let's open an issue for any remaining issues to resolve around errors

jlev commented 8 years ago

Ok, we'll also need to update config to use digidem.auth0.com, and add a github application clientId and clientSecret for the popup to work. I've done this for testing, but can't transfer it to the Digidem organization without being an admin there.

gmaclennan commented 8 years ago

Ok, so I create an app in Github for Digidem/mapfilter, then paste the clientID into config.json Where do I put the client secret and id in Auth0?

jlev commented 8 years ago

Auth0 Dashboard -> Connections -> Social GitHub, click the [ ! ], fill in the Settings and click save

On Jan 15, 2016, at 11:45 AM, Gregor MacLennan notifications@github.com wrote:

Ok, so I create an app in Github for Digidem/mapfilter, then paste the clientID into config.json Where do I put the client secret and id in Auth0?

— Reply to this email directly or view it on GitHub https://github.com/digidem/mapfilter/pull/24#issuecomment-172064892.

gmaclennan commented 8 years ago

Homepage URL and Authorization callback URL in Github?

jlev commented 8 years ago

Homepage URL doesn’t matter. Callback url should be https://digidem.auth0.com/login/callback

per https://auth0.com/docs/connections/social/github

On Jan 15, 2016, at 11:48 AM, Gregor MacLennan notifications@github.com wrote:

Homepage URL and Authorization callback URL in Github?

— Reply to this email directly or view it on GitHub https://github.com/digidem/mapfilter/pull/24#issuecomment-172065677.

gmaclennan commented 8 years ago

Should now be working with https://github.com/digidem/mapfilter/commit/fa059e828d176c4da1f93175bb9a33d3da42ea2c but I'm not sure how to test? If I go to https://lab.digital-democracy.org/mapfilter/ it's as before.

jlev commented 8 years ago

Only loads auth0 if the config.json key is set like { clientId: 'asdf', domain: 'digidem.auth0.com' }

If config.auth is just true, it will continue to use the prompt functionality. This allows continued use of non-auth deployments, like on lab.digital-democracy.org

gmaclennan commented 8 years ago

Ok. I think we should move dataUrl and tileUrl out of config and into the url hash, then remove domain-specific config.json (all uses should use the same Bing Key and ClientID and auth domain).

Not sure the best way to handle the demo data?