Field permissions

[mattlub]

UPDATE: ready for review

relates #134

what i've done in this PR:

1) add src/constants/apiPermissions.js which is empty constant file

2) move permissions functions to helpers file helpers/permissions.js

• includes new function getUnauthorizedFields
• includes all other functions used by permissions.js and fieldPermissions.js middleware

3) adds specific field permissioning middleware

• it checks if user owns resource
• then finds any unauthorized fields (using getUnauthorizedFields, which just uses req.user.isResourceOwner)
• I updated this from before (last week), as before it was shortcutting and just checking if user owned resource by checking user.isResourceOwner, because the permissions middleware sets it (but only if it checks it). So this would not work for the case where an admin satisfied the minRole condition, and so resource owner is not checked by permissions, but then there were fields with [ SUPER, OWNER ] permissions in fieldPermissions. The resulting middlewares are admittedly not very pretty and probably could be improved greatly by taking a step back. But I will not be doing this unless I return to Naz :)

4) integration tests for fieldPermissions using dummy test routes mimicking API route usage

[mattlub]

UPDATE: failing on Travis but not locally, I don't know why

[mattlub]

@m4v15 YESSSS. I fixed the f*cker.

I'm on linux now, and the tests were failing for me but I could debug. The error was that the expiry date had to be set on the access token, so I've filled out the test tokens with all the attributes. God knows why the tests were passing on our machines before (should we be worried about this?) but all good now.

[mattlub]

:shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit: :shipit:

[m4v15]

Will go through it all tomorrow morning and then hopefully it can get merged - on a side note we now have debug mode on travis if we want it lol

[m4v15]

@mattlub So it should be relatively clear but what I've done is change the checkUserOwnsResource function to just return true or false (or a promise that resolves to that) instead of this function doing the actual rejection. This allows us to skip the .catch(() => {}) in fieldPermissions.js which feels messy to me. Actually more importantly (I think) it also means that the function is now just a helper function that does what the name implies - i.e. check the owner, not reject necessarily. Once it is used, we can then decide whether to reject or not.

I've changed the middlewares to make the check and then reject or not depending on the middleware (reject for permissions and just set whether owner or not for fieldpermissions)

I think this is much clearer.

Also I've changed the fieldPermissions to only check for the owner if it is an actual resource (which it won't be for POST) - I'm not 100% sure they way I've done this is best (conditional promise chaining is a bit weird) but I think it's ok (I resolve a Promise and then use an if statement in a .then)

N.B. this is why I thought the PR was broke this morning - sorry, it wasn't really I guess, but the conditional chaining and the .catch(() => {}) meant error handling was getting wierd

[m4v15]

@mattlub unless any issues, assign @des-des

[mattlub]

@m4v15 looks fine

[des-des]

Woop, ill have a look at this today

[m4v15]

@mattlub nice work on this, sorry I started to get this done but then other shit got in the way.

[des-des]

:tada: :sparkles: good work @mattlub @m4v15