rethinkdb / horizon-docs

Other
24 stars 36 forks source link

Permissions only mentions `.find` once but it isn't clear if using `.findAll` covers `.find`? #118

Open dalanmiller opened 7 years ago

dalanmiller commented 7 years ago

There are some cases where you'd want to only allow .find versus .findAll in your queries. But in the example here:

To allow only a specific read query, just finish the template off with fetch() or watch():

[groups.default.rules.list_messages_any] template = "collection('public_messages').fetch()"

This restricts the allowed queries as follows:

// This is still ok: horizon('public_messages').fetch()

// These queries no longer match the template: horizon('public_messages').watch() horizon('public_messages').findAll({type: "announcement"}).fetch() horizon('public_messages').order("year").fetch() horizon('public_messages').order("year").above({year: 2015}).fetch()

.find() isn't included in the queries that no longer match the template. And here:

A basic rule with a query template looks like this:

[groups.default.rules.list_messages] template = "collection('public_messages')"

The list_messages rule allows read operations on the public_messages collection, such as:

horizon('public_messages').fetch() horizon('public_messages').watch() horizon('public_messages').findAll({type: "announcement"}).fetch() horizon('public_messages').order("year").fetch() horizon('public_messages').order("year").above({year: 2015}).fetch()

.find() isn't included in the queries that are implicitly allowed. So it's ultimately unclear whether or not .find() is allowed or not. Was this just an accidental omission? Or was .find() not included in the permissions templates and one should use .findAll instead?

Come across this from this question on SO.

@mlucy @Tryneus

dalanmiller commented 7 years ago

Just tested this, seems like we just need to add .find to the implicit list.

danielmewes commented 7 years ago

The list wasn't intended to be exhaustive, it was just a bunch of example queries. But since it has caused confusion, we should probably add a find query to it.