wp-net / WordPressPCL

This is a portable library for consuimg the WordPress REST-API in (almost) any C# application
MIT License
335 stars 131 forks source link

Possibility to support any general JWT Auth plugin and api improvements related to Auth #276

Closed navjot50 closed 2 years ago

navjot50 commented 2 years ago

I think that the library can support any JWT Auth plugin as long as the plugin is exposing token generation via some REST endpoint. I would propose that we add an additional general method in Auth sub client which takes token generation details from the user and then query to get json response. Newtonsoft.Json library can parse any arbitrary json and type it as JObject. The token generation details can include subpath for the jwt endpoint, request body as json and the property name of the generated token in the response json. The property name will help to get the token from the parsed json response.

Also, in the current api, the AuthMethod enum can be little confusing. The difference between JWT and JWTAuth is not clear by the api itself. I would expect the AuthMethod to have values like Basic and Bearer. An additional enum can be created for the supported auth plugins. Something like JWTPlugin with more descriptive values like JWTByEnriqueChavez and JWTByUsefulTeam. This new enum can then passed as argument to the existing RequestJWTokenAsync method for Auth client. So, the final api for Auth client may look like:

RequestJWTokenAsync(string username, string password, JWTPlugin jwtAuthPlugin) RequestJWTokenAsync(JWTokenGenerationDetails jwtTokenGenerationDetails)

A similar method for ValidateJWTokenAsync can also be added.

where JWTokenGenerationDetails can be a C# record type like public record JWTokenGenerationDetails(string tokenRestSubPath, string requestBodyAsJson, string tokenPropertyPath).

This could be a good compromise if any user is using any plugin different than the supported ones. Also, it can allow the flexibility if any of the supported plugin authors change the json structure which may break the library and give an interim fix for the users. Release 2.0 will be a good time for introducing breaking changes for enums.

I would be happy to submit PR for this.

ThomasPe commented 2 years ago

I'm definitely open to the suggestions, I agree that the current enum naming can be confusing despite the summary stating which plugin each value is for.

Are there currently any other JWT plugins out there that can be used with WordPress?

navjot50 commented 2 years ago

Yeah, there are a few. Here are links:

https://wordpress.org/plugins/simple-jwt-login/ https://wordpress.org/plugins/api-bearer-auth/

I guess I can try to implement my ideas and submit it as a PR. Then, you can review and if it doesn't feel like an improvement we can simply discard the PR.

ThomasPe commented 2 years ago

I just want to make sure we keep functionality of this library to real usecases and also don't do any unnecessary implementation work, but since there are such additional plugins you can go ahead with that. Thanks!

ThomasPe commented 2 years ago

This should be done now with release2.0, right @navjot50 ?

navjot50 commented 2 years ago

Partially yes as we updated the Auth api with release 2.0. For supporting any Auth plugin can lead to some discussions but I guess you can close the issue for now.

ThomasPe commented 2 years ago

Sounds good, V2 package was just published to nuget 🥳

navjot50 commented 2 years ago

Cheers 🍻