search

denoland / deno_kv_oauth

High-level OAuth 2.0 powered by Deno KV.
https://jsr.io/@deno/kv-oauth
MIT License
246 stars 24 forks source link

Proposal: Separate providers #99

Closed j3lte closed 1 year ago

j3lte commented 1 year ago

I have been adding a few providers for this library. I am basically taking a few popular providers that are defined in NextAuth and porting (including testing if it works) it to Deno KV Auth.

Adding multiple providers becomes tedious when it is in 1 single file. I would propose:

If you want, I can make a PR with my suggested changes

iuioiua commented 1 year ago
  • Creating a separate folder for the providers, making it easier to maintain them. Basically mimicking a similar structure like NextAuth

I think having the same family of functions adjacent to each other in the same file makes managing them more manageable. Comparing and aligning them with each other seems straightforward. However, I'm not strongly opinionated and see some benefit to the opposing structure. I'd happily consider a PR so that others may weigh in.

  • Maybe make the demo.ts more generic, where you basically can import the OAuthProvider for easy testing. Right now I am changing demo.ts to locally test it and then revert the changes before I commit to the repo

Sure! I've been also wanting to do this. I was thinking of controlling the selected provider via an environment variable or CLI argument. But I'm open to other means. Perhaps, take a look at the new createOAuth2Client() tests for guidance on how clients can be dynamically selected. Can you please do this in a separate PR?

iuioiua commented 1 year ago

Sure! I've been also wanting to do this. I was thinking of controlling the selected provider via an environment variable or CLI argument. But I'm open to other means. Perhaps, take a look at the new createOAuth2Client() tests for guidance on how clients can be dynamically selected. Can you please do this in a separate PR?

We should also print a line on the web page showing which provider is being used. Then, screenshots of the web page with the provider and access token can prove that a new provider client has been tested and is working.

iuioiua commented 1 year ago

Both points have been completed in #108.