nuxt-auth-utils icon indicating copy to clipboard operation
nuxt-auth-utils copied to clipboard

feat: added simple PKCE and state checks utils, used PKCE and state checks in auth0

Open Azurency opened this issue 1 year ago • 9 comments

I don't if that's something that something in the scope of a "Minimalist Authentication module" but I added simple PKCE and state checks in the auth0 provider for extra security.

The crypto methods and check logic could easily be extracted and reused in other oauth provider. I think that Google and Spotify accept the PKCE flow and Google and Twitch both accept a state check.

Azurency avatar Nov 10 '23 21:11 Azurency

The other providers could definitly benefit from a universal util that allows state checks and/or a pkce check. If we can get this into like a importable util, i'd happily add it to the Discord and LinkedIn provider too for extra security.

justserdar avatar Nov 11 '23 16:11 justserdar

You should probably use uncrypto instead of crypto here, as crypto might not be available in all server runtimes 😁

sifferhans avatar Nov 12 '23 12:11 sifferhans

I would indeed leverage https://github.com/unjs/uncrypto

Also here we use randomUUID to create a random string: https://github.com/nuxt/nuxters/blob/d77b0e92092c39710aeb36d604d85b886bdc87b7/server/routes/connect/%5Bprovider%5D.get.ts#L12

atinux avatar Nov 12 '23 22:11 atinux

Tanks for the feedback 👍, I updated the checks to use uncrypto.

I also moved the logic in a separate util file so that other providers can simply call

const authParam = await checks.create(event, config.checks)

to get extra query param to be passed in the authorization request and

const checkResult = await checks.use(event, config.checks)

to verify the checks and get back the code_verifier if applicable.

Azurency avatar Nov 13 '23 17:11 Azurency

I would like to build upon this PR and add a generic OIDC provider. I already started but these utils would be super helpful for supporting pkce flow.

maximilianmikus avatar Nov 29 '23 10:11 maximilianmikus

Hey @Azurency, the current implementation in the security utils file is not RFC 7636 compliant. There are some issues with the generation of the verifier and the challenge, the verifier is saved server side, which should be the challenge (the other way around if used as confidential client) and the hash validation doesn't take place. I would like to create a PR to adjust your implementation to the spec, but I have some questions:

  • What is the target of the current handler implementation for PKCE? There is currently no identity provider in this repo that would need a verification of the PKCE challenge or verifier on our Nuxt server side, as we are not issuing tokens to the user. The validation of the challenge takes place at the Idp, so auth0 in you example. The helper functions, as soon as adjusted to the spec are still helpful for usage in the providers though.
  • What is the target of the state check? The referenced auth0 documentation refers to SPA flow, but as we are using Nuxt SSR, we have a confidential client scenario. For simple CSRF protection, requiring a fixed CSRF header value would be enough, as this would force all client to make preflight requests. PKCE also helps mitigate some scenarios that can be abused due to a missing state, but I would assume that the state check would also just be used as a helper function inside of the respective providers, as it is also used for providing context to the application.

itpropro avatar Dec 16 '23 04:12 itpropro

@itpropro There is currently no identity provider in this repo that would need a verification of the PKCE challenge

As @Azurency states in the PR description: The crypto methods and check logic could easily be extracted and reused in other oauth provider. I think that Google and Spotify accept the PKCE flow and Google and Twitch both accept a state check. Also https://github.com/Atinux/nuxt-auth-utils/pull/25 relies on this

@itpropro I think it would be great if you could improve the implementation.

maximilianmikus avatar Jan 24 '24 09:01 maximilianmikus

@itpropro There is currently no identity provider in this repo that would need a verification of the PKCE challenge

As @Azurency states in the PR description: The crypto methods and check logic could easily be extracted and reused in other oauth provider. I think that Google and Spotify accept the PKCE flow and Google and Twitch both accept a state check. Also #25 relies on this

@itpropro I think it would be great if you could improve the implementation.

Hey, I think I wasn't clear with my statement. I didn't mean that there is no authentication provider that would use PKCE, but they would validate the verifier on their end, so that there is no need to verify at the Nuxt server side. I was just talking about the right place and level of PKCE in the implementation as it's about having a "PKCE client" no a PKCE server implementation. I think it makes sense to have some kind of pipeline where the requests go through and the supported/configured features are applied (like PKCE, Nonce, State, etc.).

itpropro avatar Mar 26 '24 22:03 itpropro

Thanks @itpropro I was exactly getting confused by the same issue all along while reading code changes and discussion.

Yes, PKCE is meant to be used by SPAs since they cannot have secret keys. All the security comes by providing correct redirect_uri to provider.

What I would suggest is adding those utils on client side, there by allowing this module to work with SPAs and nuxt generate. Then we can have a separate set of clients which work client side. I think it should be a separate discussion. Work done in this PR is extemely valuable but should be ported to client side.

amandesai01 avatar May 22 '24 10:05 amandesai01