mikemintz / rethinkdb-websocket-server

Node.js WebSocket server that proxies to RethinkDB. Supports query validation.
MIT License
156 stars 22 forks source link

Meteor-style allow/deny functions #1

Open mhuebert opened 9 years ago

mhuebert commented 9 years ago

Great work on this! I'm excited to try it out.

I haven't yet grokked the queryWhiteList bit - I think I need to read the code a couple more times. I was wondering if you had any thoughts about the feasibility of an auth API similar to the allow/deny functions of Meteor? I can't tell yet how much effort that would take. I also like Firebase's approach.

In both cases there is a clean, easy-to-read mapping of data paths to functions which allow or deny writes and reads, given user/session data as well as snapshots of the current state of database when relevant.

mikemintz commented 9 years ago

@mhuebert that's a really good question.

For very simple insert/update/delete queries, meteor-style allow/deny functions would be quite feasible to implement. By "very simple", I mean inserting one row with static terms, updating one row by primary key and static terms, or deleting one row with by primary key. Once we deal with dynamic terms like r.now() or update/delete by filter instead of primary key, we'll have to reimplement rethinkdb logic into the query validation. Perhaps reqlite can help here. And even if we do so, the dynamic terms or filters could have changed from the time we validate the query to the time the rethinkdb cluster receives it. So we'd probably also have to rewrite the query to turn dynamic terms into static terms, and filters into lists of primary keys.

I couldn't find anything about read queries in your allow/deny functions link. Do you know how meteor locks those down?

You should check out meteor-rethinkdb, which integrates rethinkdb into meteor. Now that meteor is officially supporting react, that may be a better solution for doing things the meteor way. I think meteor-rethinkdb still has some issues due to missing functionality in rethinkdb, but hopefully those will get resolved, and most of those issues affect this project as well.

The Firebase approach looks a bit more intimidating, since it seems to introduce a whole DSL to describing permissions, as opposed to meteor's vanilla allow/deny callback functions.

One of my main goals in designing this was to make the query whitelist very easy to create and reason about. When developing the front-end, I can copy/paste rejected queries from the server log directly into my whitelist, and generalize any dynamic terms and write authorization logic. Then once I deploy it in production, I know that it will be impossible for any unanticipated queries to be accepted. Contrast this to meteor or firebase, where I have to reason about my schema separately and make sure I fully understand the mapping between the range of possible queries and the rules that I've created.

I do think there's a lot of room for improvement to make the query whitelist easier to use. Like if we can make the syntax more similar to rethinkdb query syntax, or bake user/authentication logic in rather than leaving that up to the developer.

mhuebert commented 9 years ago

Really interesting. The workflow you describe of logging requests & then adding failed queries to the whitelist (w/ possible modifications to generalize) sounds like it could be part of a really intuitive solution. One could do this from a web UI & store the whitelist in the DB.

So a dev could get started quickly with simple declarative rules to cover base cases, & then progressively add advanced queries without worrying about 'leakage'.

Firebase's approach has some fantastic advantages but rules are not easy to write for complex cases and it's not always easy to intuit what will pass or fail.

I'm not at my computer so I have to wait to comment on the rest.

On Saturday, July 4, 2015, Mike Mintz notifications@github.com wrote:

@mhuebert https://github.com/mhuebert that's a really good question.

For very simple insert/update/delete queries, meteor-style allow/deny functions would be quite feasible to implement. By "very simple", I mean inserting one row with static terms, updating one row by primary key and static terms, or deleting one row with by primary key. Once we deal with dynamic terms like r.now() or update/delete by filter instead of primary key, we'll have to reimplement rethinkdb logic into the query validation. Perhaps reqlite https://github.com/neumino/reqlite can help here. And even if we do so, the dynamic terms or filters could have changed from the time we validate the query to the time the rethinkdb cluster receives it. So we'd probably also have to rewrite the query to turn dynamic terms into static terms, and filters into lists of primary keys.

I couldn't find anything about read queries in your allow/deny functions http://docs.meteor.com/#/full/allow link. Do you know how meteor locks those down?

You should check out meteor-rethinkdb https://github.com/Slava/meteor-rethinkdb, which integrates rethinkdb into meteor. Now that meteor is officially supporting react http://info.meteor.com/blog/meteor-the-missing-infrastructure-for-building-great-react-apps, that may be a better solution for doing things the meteor way. I think meteor-rethinkdb still has some issues due to missing functionality in rethinkdb, but hopefully those will get resolved, and most of those issues affect this project as well.

The Firebase approach looks a bit more intimidating, since it seems to introduce a whole DSL to describing permissions, as opposed to meteor's vanilla allow/deny callback functions.

One of my main goals in designing this was to make the query whitelist very easy to create and reason about. When developing the front-end, I can copy/paste rejected queries from the server log directly into my whitelist, and generalize any dynamic terms and write authorization logic. Then once I deploy it in production, I know that it will be impossible for any unanticipated queries to be accepted. Contrast this to meteor or firebase, where I have to reason about my schema separately and make sure I fully understand the mapping between the range of possible queries and the rules that I've created.

I do think there's a lot of room for improvement to make the query whitelist easier to use. Like if we can make the syntax more similar to rethinkdb query syntax, or bake user/authentication logic in rather than leaving that up to the developer.

— Reply to this email directly or view it on GitHub https://github.com/mikemintz/rethinkdb-websocket-server/issues/1#issuecomment-118547017 .

mhuebert commented 9 years ago

Meteor restricts view access in the publish flow: http://docs.meteor.com/#/full/meteor_publish

Clients do not in fact subscribe directly to collections/tables. Instead, developers publish "record sets" to which clients may subscribe. When a client attempts to subscribe to a record set, a function is called in which the developer can either deny the request or return a cursor (or multiple cursors) to specify the documents. This function is supplied with the current user, so cursor(s) may be provided that are specific to the current user.

mikemintz commented 9 years ago

One could do this from a web UI & store the whitelist in the DB.

That's a really interesting thought! This would make it easy for a third-party service to host RethinkDB-powered web backends that frontend engineers could use without having to worry at all about the server (like firebase). The web UI could make the generalization and validation design process easy with some cookie-cutter templates. Storing JS functions in the database might be weird, but I'm guessing it would just work with eval().

For my use case, I personally prefer keeping the whitelist in my own JS file, so I can version control, use ES6, use linting tools, and share libraries with the rest of my code base. But having a more streamlined solution like you propose could be a really cool extension.

Clients do not in fact subscribe directly to collections/tables.

For read queries, are there benefits to the meteor publish/subscribe method over the query whitelist here? They seem pretty similar to me, with the developer explicitly providing details of the allowed query on the server side. Does meteor allow multiple client-generated queries (with server-side filtering/mapping/etc) to use the same server-side publish code?

Since RethinkDB supports joins, I imagine it would be hard to coerce queries into meteor-style collections.