Repository: auth0/auth0-spa-js (ts)

Issue:
Invalid State error with Nuxt, custom callback page and url-encoded characters
<!--
**Please do not report security vulnerabilities here**. The [Responsible Disclosure Program](https://auth0.com/whitehat) details the procedure for disclosing security issues.

**Thank you in advance for helping us to improve this library!** Please read through the template below and answer all relevant questions. Your additional work here is greatly appreciated and will help us respond as quickly as possible. For general support or usage questions, use the [Auth0 Community](https://community.auth0.com/) or [Auth0 Support](https://support.auth0.com/). Finally, to avoid duplicates, please search existing Issues before submitting one here.

By submitting an Issue to this repository, you agree to the terms within the [Auth0 Code of Conduct](https://github.com/auth0/open-source-template/blob/master/CODE-OF-CONDUCT.md).
-->

### Describe the problem

I tried to implement a custom callback URL to allow for adjusted error handling in a Nuxt.js SPA.
Long story short: Nuxt.js seams to normalize the URL and to replace `%3D` by `=` in `location.href`.

Using the Vue.js integration works when using `createAuth0` as `handleRedirectCallback` is called _before_ that normalization.
When calling `handleRedirectCallback` from inside a Vue.js component that normalization has happened and the replacement causes the state from the transaction to not match that from the URL.

I believe this to be an error in auth0-spa-js due to the way it reads the URL.
The `Invalid State` error should happen whenever `=` characters are encoded in the state and those characters are replaced by anything before `handleRedirectCallback` is called.

**What I found out about the auth0 implementation:**

The `Invalid State` error is thrown in https://github.com/auth0/auth0-spa-js/blob/master/src/Auth0Client.ts#L513.
Key and Value of the URL are parsed by using `.split('=')` in https://github.com/auth0/auth0-spa-js/blob/master/src/utils.ts#L24.

💡 Reading the value by `new URL(location.href).searchParams.get('state')` gives the full value including trailing `'=='`.

`=` chars in the value should be very common, as the values are just base64 encoded random strings ( https://github.com/auth0/auth0-spa-js/blob/master/src/Auth0Client.ts#L293 ).

### What was the expected behavior?

The auth0 SDK should not fail if any middleware replaces query parameters by their URL-Decoded representation. This replacement might be done for human readability.

<!-- Tell us about the behavior you expected to see -->

### Reproduction

- Try to Login via the Auth0 SDK
- Replace `%3D` with `=` in the URL before calling `handleRedirectCallback`
- call `handleRedirectCallback`

<!--
> Detail the steps taken to reproduce this error, and whether this issue can be reproduced consistently or if it is intermittent.
> **Note**: If clear, reproducable steps or the smallest sample app demonstrating misbehavior cannot be provided, we may not be able to follow up on this bug report.

- Step 1..
- Step 2..
- ...
-->

**Can the behavior be reproduced using the [SPA SDK Playground](https://github.com/auth0/auth0-spa-js/blob/master/DEVELOPMENT.md#the-sdk-playground)?**

<!--
If so, provide steps:

> Where applicable, please include:
>
> - The smallest possible sample app that reproduces the undesirable behavior
> - Log files (redact/remove sensitive information)
> - Application settings (redact/remove sensitive information)
> - Screenshots
-->

### Environment

<!--
> Please provide the following:
-->

- **Version of `auth0-spa-js` used:** 1.22.5 (but behaviour shouldn't be changed as the respective code stayed the same) 
- **Which browsers have you tested in?**  Chrome
- **Which framework are you using, if applicable (Angular, React, etc):** Vue.js with Nuxt 3
- **Other modules/plugins/libraries that might be involved:** @auth0/auth0-vue


Pull request:
Use URLSearchParams when parsing callback querystring
### Changes

Ensures we use URLSearchParams when parsing the QueryString while handling the redirect callback.
This was done differently in v1 because of IE support, but as we dropped IE support in v2 we can replace this with URLSearchParams.
 
### References
Fixes #1060 

### Checklist

- [x] I have read the [Auth0 general contribution guidelines](https://github.com/auth0/open-source-template/blob/master/GENERAL-CONTRIBUTING.md)
- [x] I have read the [Auth0 Code of Conduct](https://github.com/auth0/open-source-template/blob/master/CODE-OF-CONDUCT.md)
- [x] All code quality tools/guidelines have been run/followed


Patch:
diff --git a/src/Auth0Client.ts b/src/Auth0Client.ts
index 7389fc7..41d21ea 100644
--- a/src/Auth0Client.ts
+++ b/src/Auth0Client.ts
@@ -3,7 +3,7 @@ import Lock from 'browser-tabs-lock';
 import {
   createQueryParams,
   runPopup,
-  parseQueryResult,
+  parseAuthenticationResult,
   encode,
   createRandomString,
   runIframe,
@@ -486,7 +486,7 @@ export class Auth0Client {
       throw new Error('There are no query params available for parsing.');
     }
 
-    const { state, code, error, error_description } = parseQueryResult(
+    const { state, code, error, error_description } = parseAuthenticationResult(
       queryStringFragments.join('')
     );
 
diff --git a/src/utils.ts b/src/utils.ts
index b1dbf58..1c055f0 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -12,24 +12,21 @@ import {
   PopupCancelledError
 } from './errors';
 
-export const parseQueryResult = (queryString: string): AuthenticationResult => {
+export const parseAuthenticationResult = (
+  queryString: string
+): AuthenticationResult => {
   if (queryString.indexOf('#') > -1) {
-    queryString = queryString.substr(0, queryString.indexOf('#'));
+    queryString = queryString.substring(0, queryString.indexOf('#'));
   }
 
-  const queryParams = queryString.split('&');
-  const parsedQuery: Record<string, any> = {};
+  const searchParams = new URLSearchParams(queryString);
 
-  queryParams.forEach(qp => {
-    const [key, val] = qp.split('=');
-    parsedQuery[key] = decodeURIComponent(val);
-  });
-
-  if (parsedQuery.expires_in) {
-    parsedQuery.expires_in = parseInt(parsedQuery.expires_in);
-  }
-
-  return parsedQuery as AuthenticationResult;
+  return {
+    state: searchParams.get('state')!,
+    code: searchParams.get('code') || undefined,
+    error: searchParams.get('error') || undefined,
+    error_description: searchParams.get('error_description') || undefined
+  };
 };
 
 export const runIframe = (


Test patch:
diff --git a/__tests__/Auth0Client/getTokenSilently.test.ts b/__tests__/Auth0Client/getTokenSilently.test.ts
index 17b017f..2c46b1c 100644
--- a/__tests__/Auth0Client/getTokenSilently.test.ts
+++ b/__tests__/Auth0Client/getTokenSilently.test.ts
@@ -1685,7 +1685,7 @@ describe('Auth0Client', () => {
           redirect_uri: TEST_REDIRECT_URI,
           client_id: TEST_CLIENT_ID,
           grant_type: 'authorization_code',
-          custom_param: 'hello+world',
+          custom_param: 'hello world',
           another_custom_param: 'bar',
           code_verifier: TEST_CODE_VERIFIER
         },
diff --git a/__tests__/Auth0Client/helpers.ts b/__tests__/Auth0Client/helpers.ts
index 794282c..6d94f0d 100644
--- a/__tests__/Auth0Client/helpers.ts
+++ b/__tests__/Auth0Client/helpers.ts
@@ -42,7 +42,12 @@ export const assertPostFn = (mockFetch: jest.Mock) => {
     expect(url).toEqual(actualUrl);
 
     expect(body).toEqual(
-      json ? JSON.parse(call.body) : utils.parseQueryResult(call.body)
+      json
+        ? JSON.parse(call.body)
+        : Array.from(new URLSearchParams(call.body).entries()).reduce(
+            (acc, curr) => ({ ...acc, [curr[0]]: curr[1] }),
+            {}
+          )
     );
 
     if (headers) {
diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts
index 37a5dae..29f43b5 100644
--- a/__tests__/index.test.ts
+++ b/__tests__/index.test.ts
@@ -50,7 +50,7 @@ const setup = async (
   utils.sha256.mockReturnValue(Promise.resolve(TEST_ARRAY_BUFFER));
   utils.bufferToBase64UrlEncoded.mockReturnValue(TEST_BASE64_ENCODED_STRING);
 
-  utils.parseQueryResult.mockReturnValue({
+  utils.parseAuthenticationResult.mockReturnValue({
     state: TEST_ENCODED_STATE,
     code: TEST_CODE
   });
diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts
index 1ef5219..5c8cef4 100644
--- a/__tests__/utils.test.ts
+++ b/__tests__/utils.test.ts
@@ -1,5 +1,5 @@
 import {
-  parseQueryResult,
+  parseAuthenticationResult,
   createQueryParams,
   bufferToBase64UrlEncoded,
   createRandomString,
@@ -23,29 +23,42 @@ afterEach(() => {
 });
 
 describe('utils', () => {
-  describe('parseQueryResult', () => {
+  describe('parseAuthenticationResult', () => {
     it('parses the query string', () => {
       expect(
-        parseQueryResult('value=test&otherValue=another-test')
+        parseAuthenticationResult('code=test&state=another-test')
       ).toMatchObject({
-        value: 'test',
-        otherValue: 'another-test'
+        code: 'test',
+        state: 'another-test'
       });
     });
+
+    it('parses the query string when containing raw =', () => {
+      expect(
+        parseAuthenticationResult('code=test&state=another-test==')
+      ).toMatchObject({
+        code: 'test',
+        state: 'another-test=='
+      });
+    });
+
+    it('parses the query string when containing url encoded =', () => {
+      expect(
+        parseAuthenticationResult('code=test&state=another-test%3D%3D')
+      ).toMatchObject({
+        code: 'test',
+        state: 'another-test=='
+      });
+    });
+
     it('strips off hash values', () => {
       expect(
-        parseQueryResult('code=some-code&state=some-state#__')
+        parseAuthenticationResult('code=some-code&state=some-state#__')
       ).toMatchObject({
         code: 'some-code',
         state: 'some-state'
       });
     });
-    it('converts `expires_in` to int', () => {
-      expect(parseQueryResult('value=test&expires_in=10')).toMatchObject({
-        value: 'test',
-        expires_in: 10
-      });
-    });
   });
   describe('createQueryParams', () => {
     it('creates query string from object', () => {