-
-
Notifications
You must be signed in to change notification settings - Fork 51
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Black list #62
base: master
Are you sure you want to change the base?
Black list #62
Conversation
Add feature blacklist Update whitelist for authorize all request
Thanks for taking the time to create a PR! Personally, I do not think we should make the functionality even more complex than it already is. There are dedicated plugins that go into that direction; e.g., https://wordpress.org/plugins/disable-rest-api-and-require-jwt-oauth-authentication/ As mentioned in #60 (comment) in my opinion the whole whitelist/blacklist approach is fundamentally flawed as it duplicates the regular permission layer of the REST API. |
As mentioned in #60 (comment) white/blacklist/permission_callback is necessary when you use Wordpress as a Headless CMS (no is_user_logged_in). Also even in a more general context you wouldn't be able to decide which endpoints are public or Authorized by JWT only (which is the aim of that plugin I think). |
I support this idea. |
FYI: I also needed a blacklist feature and implemented it before I saw this pull request. A small "case study" why I still think a blacklist is a good idea:
The approach is simple: If you specify a blacklist by the filter, only these endpoints are checked. If not, default JWT auth behaviour is applied. https://github.com/niklasdahlheimer/jwt-auth/tree/feature/blacklist |
Hey @niklasdahlheimer, not sure if you've had a look at this discussion - #60 - but there were some good points about why this plugin shouldn't have a whitelist / blacklist feature, the main point being that REST routes should determine who can access the route via the permissions callback leaving this plugin just to authenticate users. i.e. if there's a route that should be protected for logged-in users only, the permissions callback of the route should take care of that and users can potentially authenticate via any method. |
Hey @dominic-ks , I get the point, that route permissions should be handled by their permission callbacks and would be happy to do it this way BUT, if I see this correctly, the current version of JWT intercepts/blocks all calls on any routes if they are not whitelisted. So, right now I have this options:
which both is not what I want :). So, yes, if you
in JWT Auth, I could use the permission callbacks for my custom routes to only allow authenticated users and let JWT handle the authentication. Or do I miss something? |
You are right, and that PR is proposing to remove the whitelist all together. I think we're all pretty much agreed on that now, there are just a couple of things to do to make sure existing users are aware of the upcoming changes. |
Add blacklist feature
Update whitelist for authorize all request