feat(security): add PKCE support

[jacobsandersen]

This PR intends to add PKCE support to this library

To do so, it adds a new function getAuthUrlPkce() which first calls the original getAuthUrl(), then it calls a new function generatePkceParameters() (see below) which generates random parameters usable for PKCE. It appends the PKCE parameters to the auth URL and then returns the URL as well as the code verifier to use later during code exchange.

The generatePkceParameters() function creates a random string for the code verifier using the web crypto API, then hashes it with SHA256 to create the challenge. It returns both values. Since it uses SHA256, this will only support the S256 code_challenge_method.

As mentioned before, the code_challenge and code_challenge_method parameters are then added to the standard auth url which should satisfy auth servers requiring PKCE. The caller is provided the codeVerifier value to use after receiving an authorization code.

To use the codeVerifier, I've modified the getToken() function to add an optional parameter codeVerifier which defaults to undefined for backwards compatibility. If the user wants to use PKCE, they can modify their getToken call from getToken(code) to getToken(code, codeVerifier). The authorization server will hash the codeVerifier on its side with SHA-256 and compare to the previously provided code challenge. If all is well, the auth server will return a token as usual with PKCE satisfied.

I've added some tests, but I can't get any tests to run on my side. Not sure why - there is some sort of misconfiguration. I'll write back if I can get it working. Otherwise, before merging, please make sure the tests are good!!

If accepted, this will close #47.

Thank you!

[jacobsandersen]

Oh, also, I added base64url-js verbatim from: https://github.com/supabase-community/base64url-js

This is the intended distribution method for that bit of code. However, there may be a more desirable way to do this. Let me know what you think.

[aciccarello]

@jacobsandersen Great to see this PR! Regarding the base64 library, I wonder if TextEncoder could be used instead. I see base64url-js says it only works in browsers but the MDN Docs say it's been supported in browsers and node for a while.

[jacobsandersen]

@jacobsandersen Great to see this PR! Regarding the base64 library, I wonder if TextEncoder could be used instead. I see base64url-js says it only works in browsers but the MDN Docs say it's been supported in browsers and node for a while.

Good call. I'll take a look and see if I can use that instead.

[jacobsandersen]

note: I noticed I had inadvertently changed some other parts of the code (probably by trying to run tests, which runs lint), which was not intentional. I reverted any changes that are not specific to adding PKCE and force pushed the first commit before I keep working on other items.

[jacobsandersen]

@jacobsandersen Great to see this PR! Regarding the base64 library, I wonder if TextEncoder could be used instead. I see base64url-js says it only works in browsers but the MDN Docs say it's been supported in browsers and node for a while.

TextEncoder only encodes to UTF8. However, upon checking btoa is supported in both browsers and node which is nice. Using that, I was able to create a pretty bog standard base64url encoder which can replace the base64url-js library entirely.

[grantcodes]

Thanks so much for the PR! The code looks good to me, and definitely an important feature which this library should support :)

I'll see if I can fix the issues with the tests and get a new release up soon

[aciccarello]

I was able to test this after making the node:utils change. I did hit some issues with IndieKit missing a cookie for auth that needs to be addressed but I'm wondering if that is an issue with indiekit.

[grantcodes]

This is looking really good thanks! Just had a few notes:

• We'll need to update the documentation to include the pkce info
• I've fixed issues with the tests, so can you pull in latest main branch to get them to work?

[jacobsandersen]

Rebased to main

[jacobsandersen]

Tests are working! Thanks @grantcodes

I've made the getAuthUrl method use PKCE by default. Users can opt out by using getAuthUrl(false). Users need not specify true to opt in, as it is the default.

If using PKCE (default), the code_challenge and code_challenge_method are automatically attached to the auth URL and the code_verifier is saved in the options. Upon calling getToken(code), the code_verifier is used automatically.

I don't know if you actually want to keep the ability to not use PKCE, since it has been part of the spec since Sept 2020. But, erring on the side of compatibility, I've kept that in. If you want to remove it, we can do that.

[aciccarello]

Noticed an issue while testing this out in Omnibear. The API looks good though 👍

[jacobsandersen]

@aciccarello: I updated the code to call the setter now - can you try it again and let me know? Thanks!

[aciccarello]

Confirmed this works in Omnibear!

[jacobsandersen]

Hi @grantcodes! Anything remaining to be done here?