bpedroza / js-pkce

A package that makes using the OAuth2 PKCE flow easier
MIT License
31 stars 12 forks source link

Unique code_challenge per request #40

Open dyron opened 1 year ago

dyron commented 1 year ago

Our requirements are that every request must use a unique code_challenge within a server to server connection. So in the mean time sessionstorage or localstorage arn't available.

Are there any chance setting a code verifier per user/request? The storage keys are fixed, too.

bpedroza commented 1 year ago

I think the chance that the code challenge would not be unique is very near zero.

For storage, the constructor option can be any object that implements the Storage interface so you can do that any way you want.

dyron commented 1 year ago

Our generated authorization urls always contains the same code_challenge, since the pkce_state and pkce_code_verifier are static while using node-localstorage as storage.

pkce.pkceChallengeFromVerifier 6_o2FDwkNqOsM6IuvagWlI2Z9ZjXKCd_YnceduPeJx8
/identity/vxauth/authorize?response_type=code&client_id=XXX&state=16a08554894038b92b5eb3bfa3b71b98952201c9f8103357feb732ccb55bd2cb4ffb2c37e8b3cef903c6793df95fbd2e28daefa4464aaf599d2e167659b09718&scope=*&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2FreceiveLogin&code_challenge=6_o2FDwkNqOsM6IuvagWlI2Z9ZjXKCd_YnceduPeJx8&code_challenge_method=S256
GET / 304 14.222 ms - -
pkce.pkceChallengeFromVerifier 6_o2FDwkNqOsM6IuvagWlI2Z9ZjXKCd_YnceduPeJx8
/identity/vxauth/authorize?response_type=code&client_id=XXX&state=16a08554894038b92b5eb3bfa3b71b98952201c9f8103357feb732ccb55bd2cb4ffb2c37e8b3cef903c6793df95fbd2e28daefa4464aaf599d2e167659b09718&scope=*&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2FreceiveLogin&code_challenge=6_o2FDwkNqOsM6IuvagWlI2Z9ZjXKCd_YnceduPeJx8&code_challenge_method=S256
GET / 304 1.411 ms - -
pkce.pkceChallengeFromVerifier 6_o2FDwkNqOsM6IuvagWlI2Z9ZjXKCd_YnceduPeJx8
/identity/vxauth/authorize?response_type=code&client_id=XXX&state=16a08554894038b92b5eb3bfa3b71b98952201c9f8103357feb732ccb55bd2cb4ffb2c37e8b3cef903c6793df95fbd2e28daefa4464aaf599d2e167659b09718&scope=*&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2FreceiveLogin&code_challenge=6_o2FDwkNqOsM6IuvagWlI2Z9ZjXKCd_YnceduPeJx8&code_challenge_method=S256
GET / 304 1.115 ms - -
pkce.pkceChallengeFromVerifier 6_o2FDwkNqOsM6IuvagWlI2Z9ZjXKCd_YnceduPeJx8
/identity/vxauth/authorize?response_type=code&client_id=XXX&state=16a08554894038b92b5eb3bfa3b71b98952201c9f8103357feb732ccb55bd2cb4ffb2c37e8b3cef903c6793df95fbd2e28daefa4464aaf599d2e167659b09718&scope=*&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2FreceiveLogin&code_challenge=6_o2FDwkNqOsM6IuvagWlI2Z9ZjXKCd_YnceduPeJx8&code_challenge_method=S256

Even passing a different state results in the same code_challenge

pkce.getState 44f9e2b5e59c0af540a93862a2e77b280fedbb64d35b258659e8cd59f7864aed1955cc0c384662a27dd7589ee5bf67a3c31b21bed908c07a1c673d1fa42ff965
pkce.pkceChallengeFromVerifier 47DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU
/identity/vxauth/authorize?response_type=code&client_id=XXX&state=44f9e2b5e59c0af540a93862a2e77b280fedbb64d35b258659e8cd59f7864aed1955cc0c384662a27dd7589ee5bf67a3c31b21bed908c07a1c673d1fa42ff965&scope=*&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2FreceiveLogin&code_challenge=47DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU&code_challenge_method=S256
GET / 304 4.947 ms - -
pkce.getState 82d677ec2ecda976ef87da47141a0f31af48904c29ae976acbe8f764ce917e397d3b25607e9db0ea7904cc98131762798044de8af1dd49892d51c4c4da4651b7
pkce.pkceChallengeFromVerifier 47DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU
/identity/vxauth/authorize?response_type=code&client_id=XXX&state=82d677ec2ecda976ef87da47141a0f31af48904c29ae976acbe8f764ce917e397d3b25607e9db0ea7904cc98131762798044de8af1dd49892d51c4c4da4651b7&scope=*&redirect_uri=http%3A%2F%2Flocalhost%3A3000%2FreceiveLogin&code_challenge=47DEQpj8HBSa-_TImW-5JCeuQeRkm5NMpJWZG3hSuFU&code_challenge_method=S256

Is this caused by the shared storage?

bpedroza commented 1 year ago

Ah. I see what you mean.

I think the package can, and probably should, clear this data from storage after it's done using it. I will consider making this change.

You can just remove the storage key before making another request for now.