firebaseextended / bolt Goto Github PK

Given that Bolt is a non-procedural language, the current procedural looking functions are not ideal.

Note that ES6 defines a more concise form of function declaration, arrow functions:

function f(x) { return exp; }  // function

is the same as

f = (x) => { return exp; } // function

or even more concisely

f = (x) => exp;

I would personally prefer the even more generic looking:

f(x) = exp;

Methods, though, still have the un-needed { return ... }:

read() { return exp; }

The logical extension (not in ES6, AFIK) is to allow method definitions like:

read() = exp;

or, for zero-argument functions:

read = exp;

I have a couple of requirement that I believe are not supported by Bolt's testing module yet.
This is an example of rules I’d like to be able to test:

path /path/to/my/queue/tasks {
    write() = isQueueManager() || isAdmin() && hasPermission("queue:add");
    read() = isQueueManager() || isAdmin();
}

isQueueManager() = auth.uid === “queue_manager";
isAdmin() = auth.tid === "admin";
hasPermission(permission) = auth.permissions[permission] === true;

Basically I need:

1. to be able to control the uid from my tests to be able to verify that the rule auth.id == “queue_manager” is evaluated successfully. Currently the function ensureUser() in simulator.ts uses generatedUidAuthToken() in firebase-rest.js to generate a token with a self-generated uid by the function. There is no option to pass the uid to be embedded in the generated token from my test.
2. a way to pass a custom payload to the token generator that allows me to set custom properties like tenantId and permission.

The kind of tests I’d like to write would look like the example below:

    test(“/path/to/my/queue/tasks", function (rules) {
        rules
            .at("/path/to/my/queue/tasks")

            .as(“queue_manager")
            .write({test: "data"})
            .succeeds(“queue_manager can write tasks")
            .read()
            .succeeds(“queue_manager can read tasks")

            .as("admin_user", {tid: "admin"})
            .write({test: "data"})
            .fails("admin cannot write tasks without right scope")

            .as("admin_user", {tid: "admin", permission: {“queue:add": true}})
            .write({test: "data"})
            .succeeds("admin can write tasks with right scope")

            .as("admin_user", {tid: "admin"})
            .read()
            .succeeds("admin can read tasks")

            .as("other_user")
            .write({test: "data"})
            .fails("user without tenant cannot write tasks")
            .read()
            .fails("user without tenant cannot read tasks")

            .as("other_user", {tid: "other_tenant"})
            .write({test: "data"})
            .fails("other tenant's user cannot write tasks")
            .read()
            .fails("other tenant's user cannot read tasks")
    });

Is this kind of requirements something you're planning to support soon?

Thanks a lot,
Michele

It would be awesome to support one level of nested type validation instead of having to create a new single use types and without having to cram all the validation rules at the top of the type.

type Chat {
  id: String;
  timestamp: Number;
  user: String {
    validate() = this == auth.uid;
  }
}

// Cramming all the validation rules for the type in a single rule
// can be hard to read after a while
type Chat {
  validate() = this.user == auth.uid && condition2 && condition3;
}

// Seems odd to have to new type that gets used only once
type ChatUser {
  validate() = this == auth.uid;
}

// This works ok, but then it removes "user" from the
// top level hasChildren validation on the Chat
// and separates it from the type definition
/chats/$chat/user is String {
  validate() = this == auth.uid;
}

The code quality and readability would be improved by incorporating static typing of the sources. Plan to migrate the code to TypeScript.

If you reference an unknown variable, Bolt will leave it in the output unchanged.

Expect:

Warn user about use of undefined variables.

(need to list know globals, like now).

For more complex validations it would be good to generate rules using loops.

type StaticLoop {
  validate() {
    return
      this.value == true &&
      for (let i = 1; i <= 5; i = i + 2) {
        this.child(i + '_' + i).val() < this.child(i+2 + '_' + i+2).val();
      }
      && this['7_7'] < 100
      ;
  }
}

".validate": "newData.value == true && newData.child('1_1').val() < newData.child('3_3').val() && newData.child('3_3').val() < newData.child('5_5').val() && newData.child('5_5').val() < newData.child('7_7').val() && newData.child('7_7').val() < 100"

Instead of

path /x is MyType {}

allow

path /x is MyType;

In order to allow for all Firebase-compatible keys in type statements, we need to be able to "quote" key values in the grammar.

For example:

type T {
  '0,0': Boolean,
}

Should be legal. Propose allowing String: Type in addition to Identifier: Type in the grammar for property definition.

In the 'safe by default' vein, we should not allow unspecified properties to be written to a model that has named properties specified.

Accomplish by adding "$other": {".validate": "false"} is parallel with the explicitly names properties.

We should not allow Firebase methods to be used in Bolt.

When you define a nested collection, you have to define a separate type for it, and refer to it in a separate path statement. This is non-obvious and seems like a language design problem. For example:

type Location {
  name: String,
  // Hack
  sessions: Object,
}

type Session {
  name: String,
  created: Timestamp,
}

path /locations/$id is Location;
path /locations/$id/sessions/$id is Session;

This would be much better as:

type Location {
  name: String,
  sessions: Map<string, Session>;
}

type Session {
  name: String,
  created: Timestamp,
}

path /locations/$id is Location;

or even (using anonymous type):

type Location {
  name: String,
  sessions: Map<string, {
      name: String,
      created: Timestamp,
  }
}

path /locations/$id is Location;

using "[] - notation" (kind of obscure):

type Location {
  name: String,
  sessions: {
      name: String,
      created: Timestamp,
  }[],
}

path /locations/$id is Location;

using "wildchild" notation:

type Location {
  name: String,
  sessions: {
    $id: {
      name: String,
      created: Timestamp,
    }
  }
}

path /locations/$id is Location;

type PositiveInteger extends number {
  validate() { return this >= 0; }
}

type UnixTimestamp extends PositiveInteger {}

type NonEmptyString extends string {
  validate() { return this.length > 0; }
}

type URL extends string {
  validate() { return this.beginsWith('http'); }
}

type Test {
  time: UnixTimestamp
  name: NonEmptyString
  url: URL
}


path / {
  validate() { return this instanceof Test; }
}

Curently this translates to
".validate": "newData.child('time').isNumber() && newData.child('time').val() >= 0 && (newData.child('name').isString() && newData.child('name').child('length').val() > 0) && (newData.child('url').isString() && newData.child('url').child('beginsWith')('http'))" which is incorrect.

Firebase work in a permissive mode by default (everyone allowed to read and write anything) I always set the paths and objects to read and write to false. However doing that inside a *.bolt file will translate to nothing. Let's use the following rules as an example:

path / {
    read() = false;
    write() = false;
}

Nothing get's generated in the resulting json file, the desired effect, however, should be

{
  "rules": {
    ".read": "false",
    ".write": "false"
  }
}

Since bolt only have one expression per function return statement is unnecessary.

function isAuth() {
  auth != null
}

or in more ES6 way

isAuth = () => auth != null

extensible(prop) { return false; } could be simply converted to {"$other": {".validate": false}}

extensible(prop) { return prop == 'p1' || prop == 'p2'; } is converted to {"$other": {".validate":"$other == 'p1' || $other == 'p2'"}}

There could also be simpler (CoffeeScript) syntax extensible(prop) { return prop in ['p1', 'p2']; }

I would prefer to see mixins for types. Comparing to classes, mixins are composable allowing more flexibility.

https://github.com/Microsoft/TypeScript/wiki/Mixins

Currently, if I want to change the type of a property that is declared in some parent type I have to completely redeclare all properties for my child type and not extend the base type.

Example of what I'd like to be able to do...

// Want to change id to a type that does not match the Id regex
type Child extends Entity {
  id: ChildId
}

type ChildId extends String {
  validate() = isChildId(this);
}

function isChildId(value) {
  return value.test(/some other regex/);
}

type Entity {
  createdAt: Number,
  id: Id,
  updatedAt: Number
}

type Id extends String {
  validate() = isId(this);
}

function isId(value) {
  return value.test(/^-[a-zA-Z0-9_]+$/);
}

However, the above code outputs the id for Child to...

 "id": {
          ".validate": "newData.isString() && newData.val().matches(/^-[a-zA-Z0-9_]+$/) && newData.isString() && newData.val().matches(/some other regex/)"
 }

Which isn't useful since the id value can't match both regex tests.

When we are compiling the bolt to json using hypen sign, example.

in bolt:

type Test1 {
validate() = this.team-member.length >= 10;
"team-member": String
}
path /teamMember is Test1 {
read() = true;
write() = true;
}

after complies:

{
"rules": {
"teamMember": {
".validate": "newData.hasChildren(['team-member']) && newData.child('team').val() - member.length >= 10",
"team-member": {
".validate": "newData.isString()"
},
"$other": {
".validate": "false"
},
".read": "true",
".write": "true"
}
}
}

Since we don't have a newRoot variable in Firebase rules, references that access the root (w/o a prior wrapper) should use the newData.parent() trick to reference a relative path. For example:

path /secret/$uid is String {
  read() = isUser($uid);
  validate() = root.users[$uid] != null;
}

path /users/$uid is User {
  read() = true;
  validate() = root.secret[$uid] != null;
}

It's odd to have a "global" symbol represent the prior state of a data element.

I propose instead to offer a built-in-function, prior(), that we return the previous value of a datum (one referenced via this).

'prior(this.prop)' => data.prop

Note how you can then write:

function nonDecreasing(n) { return prior(n) == null || n > prior(n); }
path /x { write() { return nonDecreasing(this.counter);  }

@tomlarkworthy @matjaz @TristonianJones - Comments?

Would be nice to be able to access earlier values in path through key() using something like...

key().parent()

I think this would provide an easy way of performing path checks in type validation.

My specific use case. I have a base Entity with an id field which all other types in my bolt file extend. Currently, in order to check if the id field matches the value in path, I have to add a path to the id for each entity and then compare against the variable in the path.

Here's an example of my current code.

path /messages/$messageId is Message {}

//Check id here
path /messages/$messageId/id {
  validate() = this == $messageId; 
}

type Message extends Entity  {
  message: String
}

path /users/$userId is User {}

//Repeat id check here
path /users/$userId/id {
  validate() = this == $userId;  
}

type User extends Entity  {
  username: String
}

type Entity {
  id: Id
}

type Id extends String {
  validate() = isId(this);
}

function isId(value) {
  return value.test(/^-[a-zA-Z0-9_]+$/);
}

This would be a lot simpler if I could create a PrimaryId type that could check if id matched key().parent()

path /messages/$messageId is Message {}

type Message extends Entity  {
  message: String
}

path /users/$userId is User {}

type User extends Entity  {
  username: String
}

type Entity {
  id: PrimaryId
}

// Only have to check id here
type PrimaryId extends Id {
  validate() = this == key().parent(); 
}

type Id extends String {
  validate() = isId(this);
}

function isId(value) {
  return value.test(/^-[a-zA-Z0-9_]+$/);
}

It would be great to have an overview of how to get tests running for your own rules.

I keep getting stuck at ReferenceError: suite is not defined

based on https://github.com/firebase/bolt/blob/master/docs/language.md#ruledatasnapshot-methods
For function
repoHasUser(repo, user) = prior(root).repos[repo].users[user] != null;
expected output
root.child('jobs').child($repo).child($job).exists()
actual output
root.child('jobs').child($repo).child($job).val() != null
Am I missing something?

I understand from a logical standpoint that Null is a type and thus should have a have a capital "N" vs true/false which are built in values of Boolean type, however I think we need to consider the users who will be writing Bolt scripts.

I don't think many people will recognize Null as a type and they will think of it as an empty value and will probably forget the capital N causing unneeded confusion.

Thoughts?

when compiles

type Test1 {
"first-name": String;
}

path /team-member2 is Test1 {
read() = true;
write() = true;
}

error:
bolt:5:1: Expected comment, end of input, function definition, path statement, type statement or whitespace but "p" found.

How about accepting file names as params, adding command line help, etc in:

/bin/firebase-bolt

(from @matjaz)

Adding key() method to RuleDataSnapshot would add more flexibility to
validation when using $ variables.

For example:

1. {
2. $item: {
3. ".validate": "newData.key().matches(/^foo/)"
4. }
5. }

Please let me know your opinion.

For easier maintenance and encapsulate add custom methods.

type CustomMethods {
  validate() {
    return this.magic(this.value);
  }

  magic(question) {
    return question == this.answer();
  }

  answer() {
    return 42;
  }

}

type IncrementalTimestamp extends Number {
  validate() { return this > data; }
}

type Test {
  ts: IncrementalTimestamp
}

path /test is Test {}

Current output
newData.child('ts').isNumber() && newData.child('ts').val() > data.val()

Expected
newData.child('ts').isNumber() && newData.child('ts').val() > data.child('ts').val()

There is no way to write a test for a multi-location update in the simulator since all writes are to an atomic location and sequential.

Expect a way to test a multi-location write scenario.

A couple ideas of ways to implement this:

• With no change to API - but treat successive writes (between succeeds or fails statements) as a multi-location update.
• Add a .update({"path-1": <value-1>, "path-2": <value-2>); API.
• Add explicit grouping command around multi-location updates:

.beginUpdate()
  .at(<path>)
  .write(<data>)
  .at(<path>)
  .write(<data>)
.endUpdate()
.succeeds(<reason>)

I'm leaning to the last approach as being most consistent with the current testing API.

We'll probably need to push down some breaking change in the future, perhaps we should proactively provide a version annotation in all bolt files.

If we want to use a JS comment syntax something like this would probably be all we need.

/* bolt:1.0 */
...

Obviously dealing with backwards compatibility will still be an issue, but at least this way we're a little ahead of the curve.

Hey, first time trying bolt and some weird stuff happened.

Here's the input:

// Functions

isAdmin() = root.admin[auth.uid] != null;

// Paths

path / {
  read() = false;
  write() = false;
}

path /admin {
  read() = true;
}

path /accounts {
  read() = isAdmin();
  write() = isAdmin();
}

Here's the output:

{
  "rules": {
    "admin": {
      ".read": "true"
    },
    "accounts": {
      ".read": "root.child('admin').child(auth.uid).val() != null",
      ".write": "newData.parent().child('admin').child(auth.uid).val() != null"
    }
  }
}

It looks like isAdmin() means one thing for accounts read(), and something different for accounts write(). Is root a context-sensitive keyword?

Also how come the root level instructions didn't make it in? To my understanding firebase implicitly allows reads and writes unless you explicitly disallow root. Is that true? If it is true, is there some way to reference the root path besides path / like how i tried?

Error loading bolt bundle in browser.

Same bug as in Blaze_Compiler

If I have a rule that says:

!next.some_boolean
blaze outputs:

!newData.child('some_boolean').val()
but when I try to add that to my forge's rules I get an error saying:

! only operates on booleans.

Hey, so I was testing out my rules on the simulator when I came across an error when I tried to write to the root. Turns out the rule /object/$userid/property/ was being enforced on the root, I removed the trailing slash and it removed it from the root. Not sure why that happened but it's a pretty weird bug that needs to be squashed...

Do simplification based on the ast tree rather than the textual representation. I'd like to provide a simplify() function that will convert:

validate: true => remove
write: false => remove
read: false => remove

as well as simple expression simplifications:

true || exp => true
false || exp => exp
true && exp => exp
false && exp => false
!(a == b) => a != b
!(a != b) => a == b
!(a < b) => a >= b (etc)
!!a => a == true

The latter can arise from trivial conditional rules, or when trivial base rules are combined with more complex validation expression in derived Types.

The current version of Bolt (after 0.4) allows read() and write() expressions in type statements. Since these are not explicitly "schema" based (more authorization than schema), this could cause rules to be created that are not portable or easy to maintain.

The counter example is the chat.bolt sample.

This experiment intermixes validate(), read() and write() rules in various type expressions. The advantage being that you don't have to repeat the path structure in two places (once in path statement, and again in the type containment hierarchy).

One proposal is to introduce user-defined methods within types to allow for not repeating "behavior" in path statments - but this still does not resolve the issue with duplicating the two storage hierarchies.

The bulk of testing in blaze is done through a DSL:

1 example per file:
https://github.com/firebase/blaze_compiler/tree/master/test/scenarios
many examples per file:
https://github.com/firebase/blaze_compiler/blob/master/test/anticases/objects.yaml

This is the quickest way to react to user bugs. Given that in bolt, putting source code inside source code is already a problem (e.g. decision to use ' instead of " due to escaping issues). I suggest Bolt should move to a more productive test harness, it will pay for iteself over the long term

The following file is giving an error because of hyphen in "data-created", but it is allowed in rules syntax:

type User {
email: String,
firstName: String,
lastName: String,
data-created: Number,
status: Number
}

isCurrentTime(timestamp) = timestamp == now;

isAnyAuthUser() = auth != null;

isAuthUser(userID) = auth.uid == userID;

path /users {
read() = isAnyAuthUser();
}

path /users/$user is User {
write() = isCurrentTime(this.dateCreated) && isAuthUser($user);
}

type path /x/y/ { read() = true; }

Results in

{
  "rules": {
    ".read": "true"
  }
}

Expect:

{
  "rules": {
    "x": {
      "y": {
        ".read": "true"
      }
    }
  }
}

There are no unit tests for ast.js (just it's use in parser and generators).

path /x is T1 {
   path /y is T2;
}

should be the same as:

path /x is T1;
path /x/y is T2;

I would love for the Bolt CLI to have a basic help page which describes usage if it's designed to be a standalone tool (i.e. not exclusively used with firebase-tools).

Bolt should detect this is an error:

path /x/$a is String;
path /x/$b is Number;

There is also no way to have an internal collection, and a corresponding path/read/write permissions:

type T {
  values: Number[]
}

path /t is T;
path /t/values/$n is Number {
  read() = true;
}

Hi,

Just tried this - when I do a firebase deploy:rules from the command line it deploys to Firebase with an extra level of rules:{ i.e. rules{rules:{}} instead of rules:{}

Weird.

Sean

My bolts file is getting pretty large and it's getting pretty messy with heaps of different entities. It'd be really cool if you could do something like #import "users.bolts" and all your other bolts files into your "rules.bolts".

The following Bolt rules:

path /some/$path {
  validate() = root.items[$path].exists();
}

output the following invalid Firebase rules:

{
  "rules": {
    "some": {
      "$path": {
        ".validate": "newData.parent().parent().child('items').child($path).child('exists')()"
      }
    }
  }
}

I have a type:

type Class {
  className: String,
  school: String | Null,
  students: Boolean[]
}

which results in:

"students": {
          "$key1": {
            ".validate": "newData.isBoolean() && newData.isString()",
            ".read": "auth.uid == $uid",
            ".write": "auth.uid == $uid"
          }
        }

But I end up modifying my validate rule to just be newData.isBoolean() otherwise I can't write.
Am doing something wrong?

We want to keep type statements as abstract schema definitions regardless of writing location or auth state.

We should output an error if a read() or write() statement appears in a type.

Bolt currently will only report one syntax error at a time (the first one found).

Expect:

The parser to advance to the next statement and resume parsing so all the errors in the file can be reported on (and fixed) at once.