feathersjs-ecosystem/feathers-authentication-hooks

Bring back ownerOrRestrict hook

Closed this issue · 6 comments

Came in via this PR but @daffl accidentally clobbered it when we renamed this repo and released as feathers-authentication-hooks on npm.

Need to look at this PR. #2

@daffl since you moved the version back to a pre-1.0.0 version are we going to run into issues? I think there are people using v1.x of feathers-legacy-authentication-hooks which point to this repo.

We should probably deprecate or delete https://www.npmjs.com/package/feathers-legacy-authentication-hooks

daffl commented

I don't think so. It's been published to npm under a different name and I removed all the old tags. It is also not backwards compatible with the legacy hooks since auth 1.0 obsolete hooks (like verifyToken and populateUser) have been removed.

ok. Then should we at least deprecate the legacy hooks package in npm?

daffl commented

Possibly? Also, I totally missed the title. If the ownerOrRestrict hook is still relevant we should totally bring it back.

daffl commented

What is the difference to restrictToOwner though?

It's conditional. Could be an owner or restrict to a certain permissions. It's actually better to probably use the iff or unless hooks from feathers-hooks-common to control flow instead.

For a get operation it may be ok to do iff/unless, but for a find operation, I am not so sure.
The find option should actually use a merged condition { owner or restrict condition }, making it a single db call, or it could become too expensive.