Fix memory leak issue when using fetch in oidc authentication (#2640)
* 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
This commit is contained in:
1 parent
ca4aad0a2b
commit
faf4ecfac6
3 files changed
+18
-5
No files matched your search
@@ -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();
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user