FirebaseExtended / bolt

Bolt Compiler (Firebase Security and Modeling)
Apache License 2.0
897 stars 107 forks source link

Rule test: passing custom payload to token generator #88

Open mzratio opened 8 years ago

mzratio commented 8 years ago

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

mckoss commented 8 years ago

This is an excellent feature request. For most tests, we want to generate a uid for each user that is "opaque" - generally it's not a best practice to compare it to a constant value EXCEPT for the case where you are generating custom tokens.

I would just modify your suggestion to require that you explicitly set the uid for the "queue_manager" user rather than set the uid to the name of the passed-in user. Also, I'd rather not repeat custom auth token fields each time it is used in an as() statement. I would suggest either the first as() statement allows for additional properties (and uid) to be set while adding additional properties to subsequent as() statements would be an error).

Alternatively, we could add a add a new method to define custom auth tokens.

defineToken('queue_manager', {uid: 'queue_manager'})
mzratio commented 8 years ago

Mike, I do agree with you that it is not a best practice to compare the uid against a constant value. I indeed generate custom tokens on my server.

I have a fork of your repository where I tried out an implementation of my requirements (that works great for what I need). I'll play around with your suggestions (I particularly like the idea of defineToken()) in the next couple of days and let you know what I find out.

Thanks, Michele

mckoss commented 8 years ago

Thanks, Michele.

I'd be happy to accept a PR along these lines if you implement this!