usefulteam / jwt-auth

WordPress JSON Web Token Authentication
https://wordpress.org/plugins/jwt-auth/
124 stars 50 forks source link

Black list #62

Open cedricDevWP opened 2 years ago

cedricDevWP commented 2 years ago

Add blacklist feature

Update whitelist for authorize all request

sun commented 2 years ago

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 https://github.com/usefulteam/jwt-auth/pull/60#issuecomment-1070811847 in my opinion the whole whitelist/blacklist approach is fundamentally flawed as it duplicates the regular permission layer of the REST API.

mikmikmik commented 2 years ago

As mentioned in https://github.com/usefulteam/jwt-auth/pull/60#issuecomment-1073031117 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).

vyarmolenko commented 2 years ago

I support this idea. We recently ran into an issue with a similar request. We need to allow all routes except for one. We must obtain all routes from the WP_REST_Server instance with the current implementation and then filter them. The thing is that we have a 3rd-party plugin that triggers current_user_can before the REST initialization and we have an empty list of routes/namespaces in jwt_auth_whitelist and jwt_auth_default_whitelist filters. This leads to the inability to filter REST routes and results in the target route not being blacklisted.

niklasdahlheimer commented 1 year ago

FYI: I also needed a blacklist feature and implemented it before I saw this pull request. It looks like a blacklist feature is not wanted by the maintainers, but I link my fork here for anyone interested in it.

A small "case study" why I still think a blacklist is a good idea: In my case switching to other plugin or using the default WP auth mechanism was not an option:

  1. I needed a way to authenticate my app against a Wordpress E-Learning website, so only authenticated users have access to certain assets.
  2. I used JWT Auth -> all works perfectly
  3. The website is growing, other PlugIns are needed which are heavily depending on rest routes -> I started whitelisting these endpoints
  4. The endpoints used by other plugins become too many and sometimes are not well documented. So I always have to keep in mind: "If a plugin is not working as expected, maybe JWT is blocking a certain endpoint". This makes it hard to maintain the site. So what a needed was JWT auth to protect ONLY my specific endpoints and ignore ALL others.

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

dominic-ks commented 1 year ago

Hey @niklasdahlheimer, not sure if you've had a look at this discussion - https://github.com/usefulteam/jwt-auth/pull/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.

niklasdahlheimer commented 1 year ago

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:

  1. list all routes of all my plugins in the JWT whitelist
  2. stop using JWT auth

which both is not what I want :).

So, yes, if you

  1. remove the whitelist completely
  2. remove the interception of all endpoints
  3. only handle authenticating users by JWT

in JWT Auth, I could use the permission callbacks for my custom routes to only allow authenticated users and let JWT handle the authentication. But right now the only solution to use JWT for my custom routes AND avoid listing all plugin routes in the JWT whitelist is to use my blacklist-mod

Or do I miss something?

dominic-ks commented 1 year ago
  1. remove the whitelist completely

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.