From dbb403feb23731b0c10dc06b0b41d60bc14fd7f3 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Tue, 16 Feb 2021 11:06:08 +1300 Subject: [PATCH 01/11] Remove use of express in InteractiveBrowserCredential --- common/config/rush/pnpm-lock.yaml | 17 +++++++- sdk/identity/identity/package.json | 3 +- .../interactiveBrowserCredential.ts | 41 +++++++++++++------ 3 files changed, 47 insertions(+), 14 deletions(-) diff --git a/common/config/rush/pnpm-lock.yaml b/common/config/rush/pnpm-lock.yaml index e7e0c5f77643..eb7ef253547d 100644 --- a/common/config/rush/pnpm-lock.yaml +++ b/common/config/rush/pnpm-lock.yaml @@ -1145,6 +1145,12 @@ packages: dev: false resolution: integrity: sha512-dIPoZ3g5gcx9zZEszaxLSVTvMReD3xxyyDnQUjA6IYDG9Ba2AV0otMPs+77sG9ojB4Qr2N2Vk5RnKeuA0X/0bg== + /@types/stoppable/1.1.0: + dependencies: + '@types/node': 10.17.51 + dev: false + resolution: + integrity: sha512-BRR23Q9CJduH7AM6mk4JRttd8XyFkb4qIPZu4mdLF+VoP+wcjIxIWIKiBbN78NBbEuynrAyMPtzOHnIp2B/JPQ== /@types/tough-cookie/4.0.0: dev: false resolution: @@ -6778,6 +6784,13 @@ packages: node: '>= 0.6' resolution: integrity: sha1-Fhx9rBd2Wf2YEfQ3cfqZOBR4Yow= + /stoppable/1.1.0: + dev: false + engines: + node: '>=4' + npm: '>=6' + resolution: + integrity: sha512-KXDYZ9dszj6bzvnEMRYvxgeTHU74QBFL54XKtP3nyMuJ81CFYtABZ3bAzL2EdFUaEwJOBOgENyFj3R7oTzDyyw== /stream-browserify/2.0.2: dependencies: inherits: 2.0.4 @@ -9354,6 +9367,7 @@ packages: '@types/node': 8.10.66 '@types/qs': 6.9.5 '@types/sinon': 9.0.10 + '@types/stoppable': 1.1.0 '@types/uuid': 8.3.0 assert: 1.5.0 axios: 0.21.1 @@ -9385,6 +9399,7 @@ packages: rollup-plugin-terser: 5.3.1_rollup@1.32.1 rollup-plugin-visualizer: 4.2.0_rollup@1.32.1 sinon: 9.2.4 + stoppable: 1.1.0 tslib: 2.1.0 typedoc: 0.15.2 typescript: 4.1.2 @@ -9395,7 +9410,7 @@ packages: optionalDependencies: keytar: 7.3.0 resolution: - integrity: sha512-tFR61LCHIZDfYb97aYITRuRpTDM7+7O4bQt+Rxuj07YPyrHQH11lmrFsSq2RwaEJUG6W9qfAkvo2ONiPK5k3fw== + integrity: sha512-n9Sn03tFbNDFSt+Z5Ofvd6weyUrUxgVV0KiCfg6ztX6NlRk7dRPHE7oW0zFs5dpYzF0oVqANkrCL+8j3Z3l5dA== tarball: file:projects/identity.tgz version: 0.0.0 file:projects/keyvault-admin.tgz: diff --git a/sdk/identity/identity/package.json b/sdk/identity/identity/package.json index 9b21af3677c2..07eaf7b08ae0 100644 --- a/sdk/identity/identity/package.json +++ b/sdk/identity/identity/package.json @@ -87,9 +87,10 @@ "@azure/msal-browser": "2.9.0", "@opentelemetry/api": "^0.10.2", "@types/express": "^4.16.0", + "@types/stoppable": "^1.1.0", + "stoppable": "^1.1.0", "axios": "^0.21.1", "events": "^3.0.0", - "express": "^4.16.3", "jws": "^4.0.0", "msal": "^1.0.2", "open": "^7.0.0", diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts index 93576fd19bc1..de61a1f4c044 100644 --- a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts +++ b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts @@ -11,9 +11,10 @@ import { Socket } from "net"; import { AuthenticationRequired, MsalClient } from "../client/msalClient"; import { AuthorizationCodeRequest } from "@azure/msal-node"; -import express from "express"; import open from "open"; import http from "http"; +import stoppable from "stoppable"; + import { checkTenantId } from "../util/checkTenantId"; const logger = credentialLogger("InteractiveBrowserCredential"); @@ -26,6 +27,7 @@ const logger = credentialLogger("InteractiveBrowserCredential"); export class InteractiveBrowserCredential implements TokenCredential { private redirectUri: string; private port: number; + private host: string; private msalClient: MsalClient; constructor(options?: InteractiveBrowserCredentialOptions) { @@ -53,6 +55,8 @@ export class InteractiveBrowserCredential implements TokenCredential { this.port = 80; } + this.host = url.host; + let authorityHost; if (options && options.authorityHost) { if (options.authorityHost.endsWith("/")) { @@ -126,24 +130,32 @@ export class InteractiveBrowserCredential implements TokenCredential { if (socketToDestroy) { socketToDestroy.destroy(); } + + if (server) { + server.close(); + server.stop(); + } } // Create Express App and Routes - const app = express(); - app.get("/", async (req, res) => { + let self = this; + + const requestListener = async function (req: http.IncomingMessage, res: http.ServerResponse) { + const url = new URL(req.url!, self.redirectUri); const tokenRequest: AuthorizationCodeRequest = { - code: req.query.code as string, - redirectUri: this.redirectUri, + code: url.searchParams.get("code")!, + redirectUri: self.redirectUri, scopes: scopeArray }; try { - const authResponse = await this.msalClient.acquireTokenByCode(tokenRequest); + const authResponse = await self.msalClient.acquireTokenByCode(tokenRequest); const successMessage = `Authentication Complete. You can close the browser and return to the application.`; if (authResponse && authResponse.expiresOn) { const expiresOnTimestamp = authResponse?.expiresOn.valueOf(); - res.status(200).send(successMessage); + res.writeHead(200); + res.end(successMessage); logger.getToken.info(formatSuccess(scopeArray)); resolve({ @@ -158,22 +170,27 @@ export class InteractiveBrowserCredential implements TokenCredential { ); } } catch (error) { + const url = new URL(req.url!, self.redirectUri); + url.searchParams.get("error") const errorMessage = formatError( scopeArray, - `${req.query["error"]}. ${req.query["error_description"]}` + `${url.searchParams.get("error")}. ${url.searchParams.get("error_description")}` ); - res.status(500).send(errorMessage); + res.writeHead(500) + res.end(errorMessage); logger.getToken.info(errorMessage); reject(new Error(errorMessage)); } finally { cleanup(); } - }); + }; + const app = http.createServer(requestListener); - listen = app.listen(this.port, () => + listen = app.listen(this.port, this.host, () => logger.info(`Msal Node Auth Code Sample app listening on port ${this.port}!`) ); - listen.on("connection", (socket) => (socketToDestroy = socket)); + app.on("connection", (socket) => (socketToDestroy = socket)); + let server = stoppable(app); try { await this.openAuthCodeUrl(scopeArray); From 97efc0af3a9031c185595a5e8db605a12c991752 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Tue, 16 Feb 2021 11:06:23 +1300 Subject: [PATCH 02/11] Remove use of express in InteractiveBrowserCredential --- .../src/credentials/interactiveBrowserCredential.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts index de61a1f4c044..d7e92a3aba5a 100644 --- a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts +++ b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts @@ -141,7 +141,7 @@ export class InteractiveBrowserCredential implements TokenCredential { let self = this; - const requestListener = async function (req: http.IncomingMessage, res: http.ServerResponse) { + const requestListener = async function(req: http.IncomingMessage, res: http.ServerResponse) { const url = new URL(req.url!, self.redirectUri); const tokenRequest: AuthorizationCodeRequest = { code: url.searchParams.get("code")!, @@ -171,12 +171,12 @@ export class InteractiveBrowserCredential implements TokenCredential { } } catch (error) { const url = new URL(req.url!, self.redirectUri); - url.searchParams.get("error") + url.searchParams.get("error"); const errorMessage = formatError( scopeArray, `${url.searchParams.get("error")}. ${url.searchParams.get("error_description")}` ); - res.writeHead(500) + res.writeHead(500); res.end(errorMessage); logger.getToken.info(errorMessage); reject(new Error(errorMessage)); From e53a76c46997f183d34254739d90e5cf85be3394 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Tue, 16 Feb 2021 11:47:50 +1300 Subject: [PATCH 03/11] Fix lint issues --- .../interactiveBrowserCredential.ts | 45 +++++++++---------- 1 file changed, 20 insertions(+), 25 deletions(-) diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts index d7e92a3aba5a..aa619af12758 100644 --- a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts +++ b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts @@ -120,37 +120,20 @@ export class InteractiveBrowserCredential implements TokenCredential { // eslint-disable-next-line return new Promise(async (resolve, reject) => { // eslint-disable-next-line - let listen: http.Server | undefined; let socketToDestroy: Socket | undefined; - function cleanup(): void { - if (listen) { - listen.close(); - } - if (socketToDestroy) { - socketToDestroy.destroy(); - } - - if (server) { - server.close(); - server.stop(); - } - } - // Create Express App and Routes - let self = this; - - const requestListener = async function(req: http.IncomingMessage, res: http.ServerResponse) { - const url = new URL(req.url!, self.redirectUri); + const requestListener = async (req: http.IncomingMessage, res: http.ServerResponse) => { + const url = new URL(req.url!, this.redirectUri); const tokenRequest: AuthorizationCodeRequest = { code: url.searchParams.get("code")!, - redirectUri: self.redirectUri, + redirectUri: this.redirectUri, scopes: scopeArray }; try { - const authResponse = await self.msalClient.acquireTokenByCode(tokenRequest); + const authResponse = await this.msalClient.acquireTokenByCode(tokenRequest); const successMessage = `Authentication Complete. You can close the browser and return to the application.`; if (authResponse && authResponse.expiresOn) { const expiresOnTimestamp = authResponse?.expiresOn.valueOf(); @@ -170,8 +153,6 @@ export class InteractiveBrowserCredential implements TokenCredential { ); } } catch (error) { - const url = new URL(req.url!, self.redirectUri); - url.searchParams.get("error"); const errorMessage = formatError( scopeArray, `${url.searchParams.get("error")}. ${url.searchParams.get("error_description")}` @@ -186,11 +167,11 @@ export class InteractiveBrowserCredential implements TokenCredential { }; const app = http.createServer(requestListener); - listen = app.listen(this.port, this.host, () => + const listen = app.listen(this.port, this.host, () => logger.info(`Msal Node Auth Code Sample app listening on port ${this.port}!`) ); app.on("connection", (socket) => (socketToDestroy = socket)); - let server = stoppable(app); + const server = stoppable(app); try { await this.openAuthCodeUrl(scopeArray); @@ -198,6 +179,20 @@ export class InteractiveBrowserCredential implements TokenCredential { cleanup(); throw e; } + + function cleanup(): void { + if (listen) { + listen.close(); + } + if (socketToDestroy) { + socketToDestroy.destroy(); + } + + if (server) { + server.close(); + server.stop(); + } + } }); } } From 2f280f9867be9f7d267a0a9fb1eba1c6e3bdea85 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Wed, 17 Feb 2021 13:03:37 +1300 Subject: [PATCH 04/11] Use hostname instead of host --- .../src/credentials/interactiveBrowserCredential.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts index aa619af12758..2b767f16a23b 100644 --- a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts +++ b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts @@ -27,7 +27,7 @@ const logger = credentialLogger("InteractiveBrowserCredential"); export class InteractiveBrowserCredential implements TokenCredential { private redirectUri: string; private port: number; - private host: string; + private hostname: string; private msalClient: MsalClient; constructor(options?: InteractiveBrowserCredentialOptions) { @@ -55,7 +55,7 @@ export class InteractiveBrowserCredential implements TokenCredential { this.port = 80; } - this.host = url.host; + this.hostname = url.hostname; let authorityHost; if (options && options.authorityHost) { @@ -167,7 +167,7 @@ export class InteractiveBrowserCredential implements TokenCredential { }; const app = http.createServer(requestListener); - const listen = app.listen(this.port, this.host, () => + const listen = app.listen(this.port, this.hostname, () => logger.info(`Msal Node Auth Code Sample app listening on port ${this.port}!`) ); app.on("connection", (socket) => (socketToDestroy = socket)); From 80d7104427abe8d51ed4a2147854370646b6b611 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Wed, 17 Feb 2021 13:52:15 +1300 Subject: [PATCH 05/11] Add changelog entry --- sdk/identity/identity/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md index 668004011c21..7c871b89281f 100644 --- a/sdk/identity/identity/CHANGELOG.md +++ b/sdk/identity/identity/CHANGELOG.md @@ -4,6 +4,7 @@ ## 1.2.4-beta.2 (Unreleased) +- Replaced the use of the 'express' module with a Node-native http server, shrinking the resulting identity module considerably ## 1.2.4-beta.1 (2021-02-12) From cc7ef9216d987fd9e8986e6faac5d90d4f2a4f9c Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Fri, 19 Feb 2021 11:15:10 +1300 Subject: [PATCH 06/11] Address feedback --- sdk/identity/identity/package.json | 1 - .../interactiveBrowserCredential.ts | 38 ++++++++++++++----- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/sdk/identity/identity/package.json b/sdk/identity/identity/package.json index 07eaf7b08ae0..dc9df37b3abc 100644 --- a/sdk/identity/identity/package.json +++ b/sdk/identity/identity/package.json @@ -86,7 +86,6 @@ "@azure/msal-node": "1.0.0-beta.6", "@azure/msal-browser": "2.9.0", "@opentelemetry/api": "^0.10.2", - "@types/express": "^4.16.0", "@types/stoppable": "^1.1.0", "stoppable": "^1.1.0", "axios": "^0.21.1", diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts index 2b767f16a23b..6c9678908957 100644 --- a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts +++ b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts @@ -122,18 +122,23 @@ export class InteractiveBrowserCredential implements TokenCredential { // eslint-disable-next-line let socketToDestroy: Socket | undefined; - // Create Express App and Routes - - const requestListener = async (req: http.IncomingMessage, res: http.ServerResponse) => { - const url = new URL(req.url!, this.redirectUri); + const requestListener = (req: http.IncomingMessage, res: http.ServerResponse) => { + if (!req.url) { + reject( + new Error( + `Interactive Browser Authentication Error "Did not receive token with a valid expiration"` + ) + ); + return; + } + const url = new URL(req.url, this.redirectUri); const tokenRequest: AuthorizationCodeRequest = { code: url.searchParams.get("code")!, redirectUri: this.redirectUri, scopes: scopeArray }; - try { - const authResponse = await this.msalClient.acquireTokenByCode(tokenRequest); + this.msalClient.acquireTokenByCode(tokenRequest).then((authResponse) => { const successMessage = `Authentication Complete. You can close the browser and return to the application.`; if (authResponse && authResponse.expiresOn) { const expiresOnTimestamp = authResponse?.expiresOn.valueOf(); @@ -146,13 +151,22 @@ export class InteractiveBrowserCredential implements TokenCredential { token: authResponse.accessToken }); } else { + const errorMessage = formatError( + scopeArray, + `${url.searchParams.get("error")}. ${url.searchParams.get("error_description")}` + ); + res.writeHead(500); + res.end(errorMessage); + logger.getToken.info(errorMessage); + reject( new Error( `Interactive Browser Authentication Error "Did not receive token with a valid expiration"` ) ); } - } catch (error) { + cleanup(); + }).catch(() => { const errorMessage = formatError( scopeArray, `${url.searchParams.get("error")}. ${url.searchParams.get("error_description")}` @@ -160,10 +174,14 @@ export class InteractiveBrowserCredential implements TokenCredential { res.writeHead(500); res.end(errorMessage); logger.getToken.info(errorMessage); - reject(new Error(errorMessage)); - } finally { + + reject( + new Error( + `Interactive Browser Authentication Error "Did not receive token with a valid expiration"` + ) + ); cleanup(); - } + }); }; const app = http.createServer(requestListener); From 0f2688cf09d736cc6eeaf1adac1e61495574f5a3 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Fri, 19 Feb 2021 11:15:32 +1300 Subject: [PATCH 07/11] Address feedback --- .../interactiveBrowserCredential.ts | 65 ++++++++++--------- 1 file changed, 34 insertions(+), 31 deletions(-) diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts index 6c9678908957..5e475c5f5bcb 100644 --- a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts +++ b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts @@ -138,19 +138,38 @@ export class InteractiveBrowserCredential implements TokenCredential { scopes: scopeArray }; - this.msalClient.acquireTokenByCode(tokenRequest).then((authResponse) => { - const successMessage = `Authentication Complete. You can close the browser and return to the application.`; - if (authResponse && authResponse.expiresOn) { - const expiresOnTimestamp = authResponse?.expiresOn.valueOf(); - res.writeHead(200); - res.end(successMessage); - logger.getToken.info(formatSuccess(scopeArray)); - - resolve({ - expiresOnTimestamp, - token: authResponse.accessToken - }); - } else { + this.msalClient + .acquireTokenByCode(tokenRequest) + .then((authResponse) => { + const successMessage = `Authentication Complete. You can close the browser and return to the application.`; + if (authResponse && authResponse.expiresOn) { + const expiresOnTimestamp = authResponse?.expiresOn.valueOf(); + res.writeHead(200); + res.end(successMessage); + logger.getToken.info(formatSuccess(scopeArray)); + + resolve({ + expiresOnTimestamp, + token: authResponse.accessToken + }); + } else { + const errorMessage = formatError( + scopeArray, + `${url.searchParams.get("error")}. ${url.searchParams.get("error_description")}` + ); + res.writeHead(500); + res.end(errorMessage); + logger.getToken.info(errorMessage); + + reject( + new Error( + `Interactive Browser Authentication Error "Did not receive token with a valid expiration"` + ) + ); + } + cleanup(); + }) + .catch(() => { const errorMessage = formatError( scopeArray, `${url.searchParams.get("error")}. ${url.searchParams.get("error_description")}` @@ -164,24 +183,8 @@ export class InteractiveBrowserCredential implements TokenCredential { `Interactive Browser Authentication Error "Did not receive token with a valid expiration"` ) ); - } - cleanup(); - }).catch(() => { - const errorMessage = formatError( - scopeArray, - `${url.searchParams.get("error")}. ${url.searchParams.get("error_description")}` - ); - res.writeHead(500); - res.end(errorMessage); - logger.getToken.info(errorMessage); - - reject( - new Error( - `Interactive Browser Authentication Error "Did not receive token with a valid expiration"` - ) - ); - cleanup(); - }); + cleanup(); + }); }; const app = http.createServer(requestListener); From 707c3f434098a90b348cf2c7100e9d4465ac3629 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Mon, 22 Feb 2021 09:20:10 +1300 Subject: [PATCH 08/11] lint --- .../identity/src/credentials/interactiveBrowserCredential.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts index a8f0c8317cf1..45fca1f81b36 100644 --- a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts +++ b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts @@ -168,6 +168,7 @@ export class InteractiveBrowserCredential implements TokenCredential { ); } cleanup(); + return; }) .catch(() => { const errorMessage = formatError( From 5eead8768c82a88bb4c119eaab2f2cf116b489ea Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Wed, 24 Feb 2021 09:18:12 +1300 Subject: [PATCH 09/11] address feedback --- sdk/identity/identity/CHANGELOG.md | 3 -- sdk/identity/identity/package.json | 2 +- .../interactiveBrowserCredential.ts | 35 +++++++++++-------- 3 files changed, 22 insertions(+), 18 deletions(-) diff --git a/sdk/identity/identity/CHANGELOG.md b/sdk/identity/identity/CHANGELOG.md index c1b5be4b1376..bd44947c9a48 100644 --- a/sdk/identity/identity/CHANGELOG.md +++ b/sdk/identity/identity/CHANGELOG.md @@ -4,11 +4,8 @@ ## 1.2.4-beta.2 (Unreleased) -<<<<<<< HEAD - Replaced the use of the 'express' module with a Node-native http server, shrinking the resulting identity module considerably -======= - Breaking change: `DefaultAzureCredential` now only uses `InteractiveBrowserCredential` in the browser since it's the only credential intended to be used to authenticate within browsers. ->>>>>>> 1827d69a042da834f813c9f78b8be0b3bd8aa6f1 ## 1.2.4-beta.1 (2021-02-12) diff --git a/sdk/identity/identity/package.json b/sdk/identity/identity/package.json index dc9df37b3abc..baedc591e207 100644 --- a/sdk/identity/identity/package.json +++ b/sdk/identity/identity/package.json @@ -87,13 +87,13 @@ "@azure/msal-browser": "2.9.0", "@opentelemetry/api": "^0.10.2", "@types/stoppable": "^1.1.0", - "stoppable": "^1.1.0", "axios": "^0.21.1", "events": "^3.0.0", "jws": "^4.0.0", "msal": "^1.0.2", "open": "^7.0.0", "qs": "^6.7.0", + "stoppable": "^1.1.0", "tslib": "^2.0.0", "uuid": "^8.3.0" }, diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts index 45fca1f81b36..dff32092f10a 100644 --- a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts +++ b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts @@ -116,11 +116,9 @@ export class InteractiveBrowserCredential implements TokenCredential { await open(response); } - private async acquireTokenFromBrowser(scopeArray: string[]): Promise { - // eslint-disable-next-line + private acquireTokenFromBrowser(scopeArray: string[]): Promise { return new Promise(async (resolve, reject) => { - // eslint-disable-next-line - let socketToDestroy: Socket | undefined; + let socketToDestroy: Socket[] = []; const requestListener = (req: http.IncomingMessage, res: http.ServerResponse) => { if (!req.url) { @@ -131,7 +129,17 @@ export class InteractiveBrowserCredential implements TokenCredential { ); return; } - const url = new URL(req.url, this.redirectUri); + let url: URL; + try { + url = new URL(req.url, this.redirectUri); + } catch (e) { + reject( + new Error( + `Interactive Browser Authentication Error "Did not receive token with a valid expiration"` + ) + ); + return; + } const tokenRequest: AuthorizationCodeRequest = { code: url.searchParams.get("code")!, redirectUri: this.redirectUri, @@ -192,22 +200,21 @@ export class InteractiveBrowserCredential implements TokenCredential { const listen = app.listen(this.port, this.hostname, () => logger.info(`InteractiveBrowerCredential listening on port ${this.port}!`) ); - app.on("connection", (socket) => (socketToDestroy = socket)); + app.on("connection", (socket) => (socketToDestroy.push(socket))); const server = stoppable(app); - try { - await this.openAuthCodeUrl(scopeArray); - } catch (e) { - cleanup(); - throw e; - } + this.openAuthCodeUrl(scopeArray).catch(e => { + cleanup(); + reject(e); + }); function cleanup(): void { if (listen) { listen.close(); } - if (socketToDestroy) { - socketToDestroy.destroy(); + + for (let socket of socketToDestroy) { + socket.destroy(); } if (server) { From 7109655215790971b75500bfccc8b6d9c4c464bf Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Wed, 24 Feb 2021 09:18:26 +1300 Subject: [PATCH 10/11] address feedback --- .../src/credentials/interactiveBrowserCredential.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts index dff32092f10a..409a0a38da89 100644 --- a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts +++ b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts @@ -200,12 +200,12 @@ export class InteractiveBrowserCredential implements TokenCredential { const listen = app.listen(this.port, this.hostname, () => logger.info(`InteractiveBrowerCredential listening on port ${this.port}!`) ); - app.on("connection", (socket) => (socketToDestroy.push(socket))); + app.on("connection", (socket) => socketToDestroy.push(socket)); const server = stoppable(app); - this.openAuthCodeUrl(scopeArray).catch(e => { - cleanup(); - reject(e); + this.openAuthCodeUrl(scopeArray).catch((e) => { + cleanup(); + reject(e); }); function cleanup(): void { From 327b96d601e199fe4e1d245878b04b52626468b7 Mon Sep 17 00:00:00 2001 From: Jonathan Turner Date: Wed, 24 Feb 2021 09:22:50 +1300 Subject: [PATCH 11/11] lint --- .../src/credentials/interactiveBrowserCredential.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts index 409a0a38da89..cdb5f2050a18 100644 --- a/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts +++ b/sdk/identity/identity/src/credentials/interactiveBrowserCredential.ts @@ -117,8 +117,8 @@ export class InteractiveBrowserCredential implements TokenCredential { } private acquireTokenFromBrowser(scopeArray: string[]): Promise { - return new Promise(async (resolve, reject) => { - let socketToDestroy: Socket[] = []; + return new Promise((resolve, reject) => { + const socketToDestroy: Socket[] = []; const requestListener = (req: http.IncomingMessage, res: http.ServerResponse) => { if (!req.url) { @@ -213,7 +213,7 @@ export class InteractiveBrowserCredential implements TokenCredential { listen.close(); } - for (let socket of socketToDestroy) { + for (const socket of socketToDestroy) { socket.destroy(); }