Using iff with restrictToOwner

[beevelop]

Steps to reproduce

const auth = require('feathers-authentication').hooks
const iff = require('feathers-hooks-common').iff
const isNot = require('feathers-hooks-common').isNot
const isCortex = require('../../hooks/cortex').isCortex
const isUser = require('../../hooks/auth').isUser

exports.before = {
  [...]
  patch: [ isUser, iff(isNot(isCortex()), auth.restrictToOwner({ ownerField: 'id' })) ]
  [...]
}

Send a patch request to the service as user ("Cortex" is the equivalent of an admin user).

Actual behavior

TypeError: Cannot read property 'get' of undefined
  File "/[...]/node_modules/feathers-authentication/lib/hooks/restrict-to-owner.js", line 47, col 19, in exports.default
    return _this.get(hook.id, params).then(function (data) {
  File "/[...]/node_modules/feathers-authentication/lib/hooks/restrict-to-owner.js", line 42, col 12, in exports.default
    return new Promise(function (resolve, reject) {

Module versions (especially the part that's not working):

feathers: "2.1.1",
feathers-authentication: "0.7.11",
feathers-hooks: "1.8.1",
feathers-hooks-common: "2.0.3" or "3.0.0-pre"

NodeJS version:

Node v6.9.4

Unfortunately I could not find any reference if this kind of hook combination should properly work this way.

[eddyystop]

@marshall @ekryski

…

On Fri, Mar 10, 2017 at 5:41 AM, Maik Hummel ***@***.***> wrote: Steps to reproduce const auth = require('feathers-authentication').hooksconst iff = require('feathers-hooks-common').iffconst isNot = require('feathers-hooks-common').isNotconst isCortex = require('../../hooks/cortex').isCortexconst isUser = require('../../hooks/auth').isUser exports.before = { [...] patch: [ isUser, iff(isNot(isCortex()), auth.restrictToOwner({ ownerField: 'id' })) ] [...] } Send a patch request to the service as user ("Cortex" is the equivalent of an admin user). Actual behavior TypeError: Cannot read property 'get' of undefined File "/[...]/node_modules/feathers-authentication/lib/hooks/restrict-to-owner.js", line 47, col 19, in exports.default return _this.get(hook.id, params).then(function (data) { File "/[...]/node_modules/feathers-authentication/lib/hooks/restrict-to-owner.js", line 42, col 12, in exports.default return new Promise(function (resolve, reject) { *Module versions* (especially the part that's not working): feathers: "2.1.1",feathers-authentication: "0.7.11",feathers-hooks: "1.8.1",feathers-hooks-common: "2.0.3" *NodeJS version*: Node v6.9.4 Unfortunately I could not find any reference if this kind of hook combination should properly work this way. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#140>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABezn6SrRt7_Y6q89hHm_oKVFOdI3Wb3ks5rkSjCgaJpZM4MZO8V> .

[ekryski]

hmm... is likely an issue with the restrictToOwner hook but they were never designed with this in mind because they were created before feathers-hooks-common came into play. Likely it's because they are doing checks on usage inside the hook. We have new permissions ones coming soon that play nicer and should work better.

[beevelop]

@ekryski Well, that's unhandy. As a temporary work-around (until feathers-permissions, etc.) are ready-to-rock, is there a simple way to replicate iff's behaviour without loosing the context (this)? – restrictToOwner does in fact work seamless on its own, I just want to apply the conditional logic to the relevant hook.

[ekryski]

Actually @eddyystop I did notice this when using hooks. That the this context was no longer set to the service. I wonder if it is this line? https://github.com/feathersjs/feathers-hooks-common/blob/master/src/common/iff.js#L10

@beevelop so I take back my previous comment, those hooks should work with the common hooks.

[eddyystop]

That function needs to be bound `that`. I'm out for a few hours right now.

…

On Fri, Mar 10, 2017 at 12:58 PM, Eric Kryski ***@***.***> wrote: Actually @eddyystop <https://github.com/eddyystop> I did notice this when using hooks. That the this context was no longer set to the service. I wonder if it is this line? https://github.com/feathersjs/ feathers-hooks-common/blob/master/src/common/iff.js#L10 @beevelop <https://github.com/beevelop> so I take back my previous comment, those hooks *should* work with the common hooks. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#140 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABeznw5xhhnvW58up6X3037wc1JOQDBsks5rkY9NgaJpZM4MZO8V> .

[beevelop]

@eddyystop @ekryski Do you mean something like this 👇 in common/iff.js:

iffWithoutElse.else = (...falseHooks) => _iffElse(predicate, trueHooks, falseHooks).call(that, hook);

[eddyystop]

Something like that. Please try it and report back. :) I'm tied up with other Feathers things ATM.

…

On Tue, Mar 21, 2017 at 12:10 PM, Maik Hummel ***@***.***> wrote: @eddyystop <https://github.com/eddyystop> @ekryski <https://github.com/ekryski> Do you mean something like this 👇 in common/iff.js <https://github.com/feathersjs/feathers-hooks-common/blob/master/src/common/iff.js#L10> : iffWithoutElse.else = (...falseHooks) => _iffElse(predicate, trueHooks, falseHooks).call(that, hook); — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#140 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABezn-3xA4AwxDR0q7tSsEda4NBsb0OWks5rn_ZcgaJpZM4MZO8V> .

[ekryski]

I think so. @beevelop I'm also tied up with Feathers docs right now but if you are able to write a test and create a PR I'll ❤️ you forever. 😄

[eddyystop]

I have spent way too long on this without success. The expected context isn't being passed to functions with call. Instead they seemingly get the export of src/index.js. (Babel?)

I've changed the docs to say this !== service in predicates and hooks used within iff, else and iffElse , and that hook.service should be used instead. feathersjs-ecosystem/docs#580

BTW we've run into other issues with Babel. #116

[eddyystop]

Reopen this issue if you want to take a shot at it.