Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Bring back ownerOrRestrict hook #11

Closed
ekryski opened this issue Apr 14, 2017 · 6 comments
Closed

Bring back ownerOrRestrict hook #11

ekryski opened this issue Apr 14, 2017 · 6 comments

Comments

@ekryski
Copy link
Member

ekryski commented Apr 14, 2017

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
Copy link
Member

daffl commented Apr 14, 2017

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.

@ekryski
Copy link
Member Author

ekryski commented Apr 15, 2017

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

@daffl
Copy link
Member

daffl commented Apr 15, 2017

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

@daffl
Copy link
Member

daffl commented Apr 15, 2017

What is the difference to restrictToOwner though?

@ekryski
Copy link
Member Author

ekryski commented Jun 1, 2017

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.

@abielikesu
Copy link

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.

@daffl daffl closed this as completed Apr 20, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants