mindbender icon indicating copy to clipboard operation
mindbender copied to clipboard

Oauth support

Open raphaelhoffmann opened this issue 10 years ago • 6 comments

@netj Added basic support for authentication and authorization. If you have time, you might want to look if you want to integrate this into Mindbender.

raphaelhoffmann avatar Sep 20 '15 03:09 raphaelhoffmann

See the following instructions for how to use this module.

https://github.com/HazyResearch/mindbender/blob/oauth-support/auth/README.md

raphaelhoffmann avatar Sep 20 '15 03:09 raphaelhoffmann

First of all, thanks for sharing this high-quality code. I also want auth support in mindbender, but some parts of this PR feel like an overkill. Here are my questions:

  1. Does authorization only check URL /? Shouldn't it be a middleware that gatekeeps all paths (except the one for authentication)?
  2. Why do we need to rely on Google IDs? Can't we support simple user-password pairs first, then also OAuth or OpenID? One implicit but important design point of mindbender has been the ability to use it offline (or off the grid). Relying only on OAuth compromises that, and moreover it seems to be complicating the setup too much.
  3. Why is mongodb used for managing authorizations? Is it used for anything else? If not, can't we use a simple JSON file that holds a list of known users (with password hashes or OAuth ids)? Again, adding another moving part seems too much for just managing an ACL. I'm not sure if there's a use case with >10k users, but even then the file approach would probably work unless the ACL constantly changes.

netj avatar Sep 21 '15 00:09 netj

You're right that this could be improved in several ways. It's the quickest thing we could come up with, and I thought I'd separate it out from the other changes we're making in case you are interested in picking it up and enhancing it. There are definitely a few things you might want to change.

  1. Yes. It should check other paths as well, and could be called as some sort of middleware to avoid calls to ensureAuthenticated in submodules.
  2. Right, a simple username/passwords strategy and other OAuth providers would be interesting as well. At this moment, we only need OAuth with Google, so we built the MVP :).
  3. JSON files instead of mongo is fine too. I would also add that the authentication works quite well, but there might be a more elegant approach to authorization (I couldn't find a good example for that).

raphaelhoffmann avatar Sep 21 '15 01:09 raphaelhoffmann

Is it difficult to simply have a switch in the config file for oauth?

Chris

On Sun, Sep 20, 2015 at 6:07 PM Raphael Hoffmann [email protected] wrote:

You're right that this could be improved in several ways. It's the quickest thing we could come up with, and I thought I'd separate it out from the other changes we're making in case you are interested in picking it up and enhancing it. There are definitely a few things you might want to change.

  1. Yes. It should check other paths as well, and could be called as some sort of middleware to avoid calls to ensureAuthenticated in submodules.
  2. Right, a simple username/passwords strategy and other OAuth providers would be interesting as well. At this moment, we only need OAuth with Google, so we built the MVP :).
  3. JSON files instead of mongo is fine too. I would also add that the authentication works quite well, but there might be a more elegant approach to authorization (I couldn't find a good example for that).

— Reply to this email directly or view it on GitHub https://github.com/HazyResearch/mindbender/pull/48#issuecomment-141852168 .

chrismre avatar Sep 21 '15 01:09 chrismre

Just to second @netj and @chrismre,

  1. We shouldn't hard code the auth params in the compiled auth-api.coffee. It should be configurable out of the big final binary. Env vars would be an option, but maybe this is a good reason to introduce a config file.
  2. Mongo is probably too heavy. Can we use an embedded DB instead (SQLite or any embedded doc/KV store)?

alldefector avatar Oct 02 '15 05:10 alldefector

Confirmed: anonymous users can bypass this middleware and successfully query the ES proxy (even if the request has not csrftoken cookie). I'm not familiar with nodejs enough to fix it.

alldefector avatar Oct 04 '15 18:10 alldefector