usefulteam / jwt-auth

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

UI / option page in admin area #56

Closed contactjavas closed 2 years ago

contactjavas commented 2 years ago

Hi Guys, is it a good idea to have a UI in the admin area to allow people to whitelist some endpoints / maybe modify the default whitelisted endpoints?

Sure, we have some filters. But seems like it's quite often people come with issues like "x or y or z" plugins don't work (due to blocked request). And then we suggest using a filter to solve that, and then they ask where to put the codes.

Not sure if an options page in the admin area is necessary, but what do you think?

pesseba commented 2 years ago

I already think this too... It could be an options page inside Settings menu item. Could be just a text area with the endpoints separated by lines.

pesseba commented 2 years ago

Other idea is a configuration to active and deactivate the whitelist at all. Some users doesn't need whitelist anyway...

sun commented 2 years ago

Can someone explain why does the whitelist exist in this plugin in the first place? Why are we modifying access to routes that we do not own?

Unless I'm missing something big, I don't think the jwt-auth plugin should be in the business of doing this. Other WordPress REST API endpoints have their own security and access permission schemes and it's their job to protect sensitive resources.

The jwt-auth plugin only supplies a method to authenticate. It should not affect who is authorized to do what.

pesseba commented 2 years ago

Hi @sun I think this is just an override behaviour to protect all access easily, but it causes many problems with other plugins. Maybe it must be implemented in an inverse way, with a blocklist, if the intent is to protect. So, with a page options this feature could be disabled by default, and people who wants to use this can enable an allowedlist or a blocklist.

sun commented 2 years ago

I think that should be a separate plugin. Maybe there are some already and we should just link to them.

pesseba commented 2 years ago

I found this plugins to perform the same behaviour: https://br.wordpress.org/plugins/disable-rest-api-and-require-jwt-oauth-authentication/

sun commented 2 years ago

Shall we remove the whitelist as part of the new major version 3.0 (introducing refresh tokens) as another breaking change?

sun commented 2 years ago

Created https://github.com/usefulteam/jwt-auth/pull/60 to remove the whitelist.

contactjavas commented 2 years ago

Hi @sun , thanks for your awesome efforts on the plugins :) We're grateful for your work!

I think, whitelist/ignore is an important feature here. The whitelisting came because there was some problems in Tmeister plugin. But I'm not sure which one, you might find some in their issue list. This features seems helpful for a lot of people.

Beside of that, when we create a mobile app, some endpoints must be open (not restricted by jwt auth). For instance: registration, check account availability, etc. So the whitelist/ignore feature is very handy to use here. Or, do I miss something?

Also, whitelisting/ignoring endpoints is a common implementation right? If you look at slim-jwt-auth (JWT Auth for Slim Framework), they also have whitelist/ignore feature.

What do you think @sun , @pesseba , @dominic-ks ?

dominic-ks commented 2 years ago

Hi @sun , thanks for your awesome efforts on the plugins :) We're grateful for your work!

I think, whitelist/ignore is an important feature here. The whitelisting came because there was some problems in Tmeister plugin. But I'm not sure which one, you might find some in their issue list. This features seems helpful for a lot of people.

Beside of that, when we create a mobile app, some endpoints must be open (not restricted by jwt auth). For instance: registration, check account availability, etc. So the whitelist/ignore feature is very handy to use here. Or, do I miss something?

Also, whitelisting/ignoring endpoints is a common implementation right? If you look at slim-jwt-auth (JWT Auth for Slim Framework), they also have whitelist/ignore feature.

What do you think @sun , @pesseba , @dominic-ks ?

I agree that the whitelist is an important feature. For me adding this plugin to a site and locking down all routes by default is a good thing - it means that I know that routes are only allowed if I allow them.

Sorry I hadn't been very active here, but I did have a look through a PR the other day that looked like it was offering options to disable the whitelist and use a blacklist approach instead. I think that's fine and gives people flexibility, adding an admin UI for this is also fine, though I do find it confusing as to the users who want a plugin like this one that aren't able to use the filters. That's not intended as a criticism, I just assume that those using this plugin would be developers and so would be able to understand.

I also don't think changing the whitelist approach as a "breaking change" is a good idea either. Anyone who updates automatically and / or doesn't spot that it's happening is suddenly going to have authentication removed from their API...

pesseba commented 2 years ago

Hi @contactjavas, I understand whitelist as an extended feature for some cases... I strongly believe this should not be enable by default, because it generate conflicts with other plugins, also if you call an Rest API endpoint internally to get data with WP_REST_Request. Below, I list some directions that I see: 1) Remove whitelist feature: Plugin lost a feature, some users can migrate to another plugin or use other to complement this one. This will not break any site at all. 2) Create an options page: It could help enable/disable whitelist and create a blocklist and help to manage this without development. Keep the feature and deal with these issues by interface. 3) Keep whitelist as a filter only: If this is a filter, the users/developers should decide to use or not this feature. So, it must be disable by default.

contactjavas commented 2 years ago

Hi @sun @pesseba @dominic-ks , thanks a lot for your opinions & suggestions! I'll leave the direction to whoever active maintainer in this repo :). Of course when you guys are willing and have the time :) .

Let's just ignore my opinion😄 I don't have much experience in the JWT field + I don't seem to have time to maintain this repo. I still use this plugin though, in a Flutter app + WP.