From faf4ecfac642f5f7fe91bd5a3a4b801f7ad22f1a Mon Sep 17 00:00:00 2001
From: Lisbet Alvarez <107949262+lisbet-alvarez@users.noreply.github.com>
Date: Fri, 3 Jan 2025 14:06:27 -0800
Subject: [PATCH] Fix memory leak issue when using fetch in oidc authentication
 (#2640)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* test: 💍 test 1

* chore: 🤖 remove waitForPromise use

* chore: 🤖 remove all other waitforpromise usage

* test: 💍 turn down polling

* test: 💍 use .json

* refactor: 💡 add waitForPromise back in & consume response body

* test: 💍 some testing

* test: 💍 more testing

* refactor: 💡 clean-up a bit

* refactor: 💡 use head method request & clean up comments

* refactor: 💡 use get method & consume response body

* test: 💍 fix token validation test cases
---
 addons/auth/addon/authenticators/base.js           |  7 +++++++
 addons/auth/addon/authenticators/oidc.js           | 10 ++++++++--
 addons/auth/tests/unit/authenticators/base-test.js |  6 +++---
 3 files changed, 18 insertions(+), 5 deletions(-)

diff --git a/addons/auth/addon/authenticators/base.js b/addons/auth/addon/authenticators/base.js
index a3cdafabd..a48aaaeac 100644
--- a/addons/auth/addon/authenticators/base.js
+++ b/addons/auth/addon/authenticators/base.js
@@ -58,6 +58,7 @@ export default class BaseAuthenticator extends SimpleAuthBaseAuthenticator {
    */
   async validateToken(token, tokenID) {
     const tokenValidationURL = this.buildTokenValidationEndpointURL(tokenID);
+
     // Note: waitForPromise is needed to provide the necessary integration with @ember/test-helpers
     // visit https://www.npmjs.com/package/@ember/test-waiters for more info.
     const response = await waitForPromise(
@@ -66,6 +67,12 @@ export default class BaseAuthenticator extends SimpleAuthBaseAuthenticator {
         headers: { Authorization: `Bearer ${token}` },
       }),
     );
+    // Note: Always consume response body in order to avoid memory leaks
+    // visit https://undici.nodejs.org/#/?id=garbage-collection for more info.
+    // We do not use the undici package but the link informs us that garbage
+    // collection is undefined when response body is not consumed.
+    await response.json();
+
     // 401 and 404 responses mean the token is invalid, whereas other types of
     // error responses do not tell us about the validity of the token.
     if (response.status === 401 || response.status === 404) return reject();
diff --git a/addons/auth/addon/authenticators/oidc.js b/addons/auth/addon/authenticators/oidc.js
index dc1063d5e..6b38c6c05 100644
--- a/addons/auth/addon/authenticators/oidc.js
+++ b/addons/auth/addon/authenticators/oidc.js
@@ -9,7 +9,7 @@ import { reject } from 'rsvp';
 import { waitForPromise } from '@ember/test-waiters';
 
 /**
- * The OIDC base authenticator encapsulates the multistep OIDC flow.
+ * The OIDC base authenticator encapsulates the multi-step OIDC flow.
  *
  * 1. Start authentication flow:  this step is actually a combination of two
  *    sub steps:
@@ -97,13 +97,19 @@ export default class OIDCAuthenticator extends BaseAuthenticator {
     });
     // Fetch the endpoint and get the response JSON
     const response = await waitForPromise(fetch(url, { method: 'post', body }));
+
+    // Note: Always consume response body in order to avoid memory leaks
+    // visit https://undici.nodejs.org/#/?id=garbage-collection for more info.
+    // We do not use the undici package but the link informs us that garbage
+    // collection is undefined when response body is not consumed.
+    const json = await response.json();
+
     if (response.status === 202) {
       // The token isn't ready yet, keep trying.
       return false;
     } else if (response.status < 400) {
       // Response was successful, meaning a token was obtained.
       // Authenticate with the session service using the response JSON.
-      const json = await response.json();
       await this.session.authenticate('authenticator:oidc', json);
       return true;
     } else {
diff --git a/addons/auth/tests/unit/authenticators/base-test.js b/addons/auth/tests/unit/authenticators/base-test.js
index 6bcf988f6..3a7f2f38c 100644
--- a/addons/auth/tests/unit/authenticators/base-test.js
+++ b/addons/auth/tests/unit/authenticators/base-test.js
@@ -52,7 +52,7 @@ module('Unit | Authenticator | base', function (hooks) {
     const authenticator = this.owner.lookup('authenticator:base');
     server.get(authenticator.buildTokenValidationEndpointURL(id), () => {
       assert.ok(true, 'token validation was requested');
-      return [200];
+      return [200, {}, '{}'];
     });
     await authenticator.restore(mockData);
   });
@@ -64,7 +64,7 @@ module('Unit | Authenticator | base', function (hooks) {
     const authenticator = this.owner.lookup('authenticator:base');
     server.get(authenticator.buildTokenValidationEndpointURL(id), () => {
       assert.ok(true, 'token validation was requested');
-      return [401];
+      return [401, {}, '{}'];
     });
     try {
       await authenticator.restore(mockData);
@@ -80,7 +80,7 @@ module('Unit | Authenticator | base', function (hooks) {
     const authenticator = this.owner.lookup('authenticator:base');
     server.get(authenticator.buildTokenValidationEndpointURL(id), () => {
       assert.ok(true, 'token validation was requested');
-      return [404];
+      return [404, {}, '{}'];
     });
     try {
       await authenticator.restore(mockData);