MSAL-Node Samples - console.log + error Improvements (#7447)

This PR resolves several instances of the error found in CodeQL: `Information exposure through a stack trace`. All instances of `console.log` have been deleted for acquireToken calls that make a network request. Additionally, for all instances of `console.log` **NOT** for acquireToken calls that make a network request: 1. `console.log` has been converted to `console.error` 2. All instances of `err` have been changed to `error` for consistency (There's one instance of a `console.log` change to a msal-browser sample as well)
AzureAD · Dec 21, 2024 · c837638 · c837638
1 parent 40ab70a
commit c837638
Show file tree

Hide file tree

Showing 44 changed files with 269 additions and 411 deletions.
diff --git a/extensions/samples/msal-node-extensions/index.js b/extensions/samples/msal-node-extensions/index.js
@@ -52,7 +52,7 @@ PersistenceCreator
         // get url to sign user in and consent to scopes needed for application
         pca.getAuthCodeUrl(authCodeUrlParameters).then((response) => {
             res.redirect(response);
-        }).catch((error) => console.log(JSON.stringify(error)));
+        });
     });
 
     app.get('/redirect', (req, res) => {
@@ -66,7 +66,7 @@ PersistenceCreator
             console.log("\nResponse: \n", response);
             res.sendStatus(200);
         }).catch((error) => {
-            console.log(error);
+            console.error(error.errorMessage);
             res.status(500).send(error);
         });
     });

diff --git a/samples/msal-browser-samples/HybridSample/routes/auth.js b/samples/msal-browser-samples/HybridSample/routes/auth.js
@@ -31,11 +31,8 @@ router.get('/login', (req, res) => {
     // Generate auth code url and redirect the user
     msalInstance.getAuthCodeUrl(authCodeUrlParameters)
         .then((response) => {
-            console.log("getAuthCodeURL RESPONSE");
-            console.log(response);
             res.redirect(response);
-        })
-        .catch((error) => console.log(JSON.stringify(error)));
+        });
 });
 
 // Route to capture auth code that will be posted by AAD
@@ -105,7 +102,6 @@ router.post('/server-redirect', (req, res) => {
         })
         .catch((error) => {
             console.timeEnd(timeLabel);
-            console.log(error);
             res.status(500).send(error);
         });
 });

diff --git a/samples/msal-node-samples/ElectronSystemBrowserTestApp/src/AuthProvider.ts b/samples/msal-node-samples/ElectronSystemBrowserTestApp/src/AuthProvider.ts
@@ -60,7 +60,7 @@ export default class AuthProvider {
                 .removeAccount(this.account);
             this.account = null;
         } catch (error) {
-            console.log(error);
+            console.error(error);
         }
     }
 

diff --git a/samples/msal-node-samples/ElectronSystemBrowserTestApp/src/CachePlugin.ts b/samples/msal-node-samples/ElectronSystemBrowserTestApp/src/CachePlugin.ts
@@ -10,8 +10,8 @@ export const cachePlugin = (CACHE_LOCATION: string): ICachePlugin => {
     const beforeCacheAccess = async (cacheContext: TokenCacheContext) => {
         return new Promise<void>(async (resolve, reject) => {
             if (fs.existsSync(CACHE_LOCATION)) {
-                fs.readFile(CACHE_LOCATION, "utf-8", (err, data) => {
-                    if (err) {
+                fs.readFile(CACHE_LOCATION, "utf-8", (error, data) => {
+                    if (error) {
                         reject();
                     } else {
                         cacheContext.tokenCache.deserialize(data);
@@ -22,8 +22,8 @@ export const cachePlugin = (CACHE_LOCATION: string): ICachePlugin => {
                 fs.writeFile(
                     CACHE_LOCATION,
                     cacheContext.tokenCache.serialize(),
-                    (err) => {
-                        if (err) {
+                    (error) => {
+                        if (error) {
                             reject();
                         }
                     }
@@ -37,9 +37,9 @@ export const cachePlugin = (CACHE_LOCATION: string): ICachePlugin => {
             fs.writeFile(
                 CACHE_LOCATION,
                 cacheContext.tokenCache.serialize(),
-                (err) => {
-                    if (err) {
-                        console.log(err);
+                (error) => {
+                    if (error) {
+                        console.error(error);
                     }
                 }
             );

diff --git a/samples/msal-node-samples/ElectronSystemBrowserTestApp/src/Main.ts b/samples/msal-node-samples/ElectronSystemBrowserTestApp/src/Main.ts
@@ -163,7 +163,7 @@ export default class Main {
                 Main.publish(IpcMessages.SHOW_WELCOME_MESSAGE, account);
             }
         } catch (error) {
-            console.log(error);
+            console.error(error);
         }
     }
 

diff --git a/samples/msal-node-samples/ElectronTestApp/src/AuthProvider.ts b/samples/msal-node-samples/ElectronTestApp/src/AuthProvider.ts
@@ -124,8 +124,8 @@ export default class AuthProvider {
                 tokenRequest
             );
         } catch (error) {
-            console.log(
-                "Silent token acquisition failed, acquiring token using pop up"
+            console.error(
+                "Silent token acquisition failed, acquiring token interactively"
             );
             const authCodeRequest = {
                 ...this.authCodeUrlParams,

diff --git a/samples/msal-node-samples/ElectronTestApp/src/CachePlugin.ts b/samples/msal-node-samples/ElectronTestApp/src/CachePlugin.ts
@@ -9,8 +9,8 @@ export const cachePlugin = (CACHE_LOCATION: string): ICachePlugin => {
     const beforeCacheAccess = async (cacheContext) => {
         return new Promise<void>(async (resolve, reject) => {
             if (fs.existsSync(CACHE_LOCATION)) {
-                fs.readFile(CACHE_LOCATION, "utf-8", (err, data) => {
-                    if (err) {
+                fs.readFile(CACHE_LOCATION, "utf-8", (error, data) => {
+                    if (error) {
                         reject();
                     } else {
                         cacheContext.tokenCache.deserialize(data);
@@ -21,8 +21,8 @@ export const cachePlugin = (CACHE_LOCATION: string): ICachePlugin => {
                 fs.writeFile(
                     CACHE_LOCATION,
                     cacheContext.tokenCache.serialize(),
-                    (err) => {
-                        if (err) {
+                    (error) => {
+                        if (error) {
                             reject();
                         }
                     }
@@ -36,9 +36,9 @@ export const cachePlugin = (CACHE_LOCATION: string): ICachePlugin => {
             fs.writeFile(
                 CACHE_LOCATION,
                 cacheContext.tokenCache.serialize(),
-                (err) => {
-                    if (err) {
-                        console.log(err);
+                (error) => {
+                    if (error) {
+                        console.error(error);
                     }
                 }
             );

diff --git a/samples/msal-node-samples/auth-code-cli-app/src/graph.js b/samples/msal-node-samples/auth-code-cli-app/src/graph.js
@@ -13,7 +13,6 @@ async function callMicrosoftGraph(accessToken) {
         const response = await axios.get(graphMeEndpoint, options);
         return response.data;
     } catch (error) {
-        console.log(error)
         return error;
     }
 };

diff --git a/samples/msal-node-samples/auth-code-cli-app/src/index.js b/samples/msal-node-samples/auth-code-cli-app/src/index.js
@@ -52,11 +52,9 @@ acquireToken().then((response) => {
     callMicrosoftGraph(response.accessToken).then((graphResponse) => {
         console.log(graphResponse);
         process.exit(0);
-    }).catch((e) => {
-        console.log(e);
+    }).catch((error) => {
         process.exit(1);
     });
-}).catch(e => {
-    console.error(e);
+}).catch((error) => {
     process.exit(1);
 });
diff --git a/samples/msal-node-samples/auth-code-cli-brokered-app/src/graph.js b/samples/msal-node-samples/auth-code-cli-brokered-app/src/graph.js
@@ -12,7 +12,6 @@ async function callMicrosoftGraph(accessToken) {
         const response = await fetch(graphMeEndpoint, options);
         return response.json();
     } catch (error) {
-        console.log(error)
         return error;
     }
 };

diff --git a/samples/msal-node-samples/auth-code-distributed-cache/src/AuthProvider.ts b/samples/msal-node-samples/auth-code-distributed-cache/src/AuthProvider.ts
@@ -178,50 +178,38 @@ export class AuthProvider {
             .authority!.split("/")
             .pop()!;
 
-        try {
-            let [cloudDiscoveryMetadata, authorityMetadata] = await Promise.all(
-                [
-                    cacheClient.get(
-                        `${clientId}.${tenantId}.discovery-metadata`
-                    ),
-                    cacheClient.get(
-                        `${clientId}.${tenantId}.authority-metadata`
-                    ),
-                ]
-            );
-
-            if (!cloudDiscoveryMetadata || !authorityMetadata) {
-                [cloudDiscoveryMetadata, authorityMetadata] = await Promise.all(
-                    [
-                        AuthProvider.fetchCloudDiscoveryMetadata(tenantId),
-                        AuthProvider.fetchOIDCMetadata(tenantId),
-                    ]
+        let [cloudDiscoveryMetadata, authorityMetadata] = await Promise.all([
+            cacheClient.get(`${clientId}.${tenantId}.discovery-metadata`),
+            cacheClient.get(`${clientId}.${tenantId}.authority-metadata`),
+        ]);
+
+        if (!cloudDiscoveryMetadata || !authorityMetadata) {
+            [cloudDiscoveryMetadata, authorityMetadata] = await Promise.all([
+                AuthProvider.fetchCloudDiscoveryMetadata(tenantId),
+                AuthProvider.fetchOIDCMetadata(tenantId),
+            ]);
+
+            if (cloudDiscoveryMetadata && authorityMetadata) {
+                await cacheClient.set(
+                    `${clientId}.${tenantId}.discovery-metadata`,
+                    JSON.stringify(cloudDiscoveryMetadata)
+                );
+                await cacheClient.set(
+                    `${clientId}.${tenantId}.authority-metadata`,
+                    JSON.stringify(authorityMetadata)
                 );
-
-                if (cloudDiscoveryMetadata && authorityMetadata) {
-                    await cacheClient.set(
-                        `${clientId}.${tenantId}.discovery-metadata`,
-                        JSON.stringify(cloudDiscoveryMetadata)
-                    );
-                    await cacheClient.set(
-                        `${clientId}.${tenantId}.authority-metadata`,
-                        JSON.stringify(authorityMetadata)
-                    );
-                }
             }
-
-            msalConfigWithMetadata.auth.cloudDiscoveryMetadata =
-                typeof cloudDiscoveryMetadata === "string"
-                    ? cloudDiscoveryMetadata
-                    : JSON.stringify(cloudDiscoveryMetadata);
-            msalConfigWithMetadata.auth.authorityMetadata =
-                typeof authorityMetadata === "string"
-                    ? authorityMetadata
-                    : JSON.stringify(authorityMetadata);
-        } catch (error) {
-            console.log(error);
         }
 
+        msalConfigWithMetadata.auth.cloudDiscoveryMetadata =
+            typeof cloudDiscoveryMetadata === "string"
+                ? cloudDiscoveryMetadata
+                : JSON.stringify(cloudDiscoveryMetadata);
+        msalConfigWithMetadata.auth.authorityMetadata =
+            typeof authorityMetadata === "string"
+                ? authorityMetadata
+                : JSON.stringify(authorityMetadata);
+
         return msalConfigWithMetadata;
     }
 
@@ -231,30 +219,15 @@ export class AuthProvider {
         const endpoint =
             "https://login.microsoftonline.com/common/discovery/instance";
 
-        try {
-            const response = await AxiosHelper.callDownstreamApi(
-                endpoint,
-                undefined,
-                {
-                    "api-version": "1.1",
-                    authorization_endpoint: `https://login.microsoftonline.com/${tenantId}/oauth2/v2.0/authorize`,
-                }
-            );
-
-            return response;
-        } catch (error) {
-            console.log(error);
-        }
+        return await AxiosHelper.callDownstreamApi(endpoint, undefined, {
+            "api-version": "1.1",
+            authorization_endpoint: `https://login.microsoftonline.com/${tenantId}/oauth2/v2.0/authorize`,
+        });
     }
 
     private static async fetchOIDCMetadata(tenantId: string): Promise<any> {
         const endpoint = `https://login.microsoftonline.com/${tenantId}/v2.0/.well-known/openid-configuration`;
 
-        try {
-            const response = await AxiosHelper.callDownstreamApi(endpoint);
-            return response;
-        } catch (error) {
-            console.log(error);
-        }
+        return await AxiosHelper.callDownstreamApi(endpoint);
     }
 }
diff --git a/samples/msal-node-samples/auth-code-distributed-cache/src/PartitionManager.ts b/samples/msal-node-samples/auth-code-distributed-cache/src/PartitionManager.ts
@@ -37,7 +37,7 @@ class PartitionManager implements IPartitionManager {
             const session = JSON.parse(sessionData) as SessionCacheData;
             return session.account?.homeAccountId || EMPTY_STRING;
         } catch (error) {
-            console.log(error);
+            console.error(error);
         }
 
         return EMPTY_STRING;

diff --git a/samples/msal-node-samples/auth-code-distributed-cache/src/RedisClientWrapper.ts b/samples/msal-node-samples/auth-code-distributed-cache/src/RedisClientWrapper.ts
@@ -41,7 +41,7 @@ class RedisClientWrapper implements ICacheClient {
         try {
             return (await this.cacheClient.get(key)) || EMPTY_STRING;
         } catch (error) {
-            console.log(error);
+            console.error(error);
         }
 
         return EMPTY_STRING;
@@ -61,7 +61,7 @@ class RedisClientWrapper implements ICacheClient {
                 })) || EMPTY_STRING
             );
         } catch (error) {
-            console.log(error);
+            console.error(error);
         }
 
         return EMPTY_STRING;

diff --git a/samples/msal-node-samples/auth-code-distributed-cache/src/app.ts b/samples/msal-node-samples/auth-code-distributed-cache/src/app.ts
@@ -166,13 +166,13 @@ async function main() {
         });
     });
 
-    app.use((err: any, req: Request, res: Response, next: NextFunction) => {
+    app.use((error: any, req: Request, res: Response, next: NextFunction) => {
         // set locals, only providing error in development
-        res.locals.message = err.message;
-        res.locals.error = req.app.get("env") === "development" ? err : {};
+        res.locals.message = error.message;
+        res.locals.error = req.app.get("env") === "development" ? error : {};
 
         // render the error page
-        res.status(err.status || 500);
+        res.status(error.status || 500);
         res.render("error");
     });
 
@@ -209,9 +209,9 @@ function initializePerformanceObserver(): void {
             fs.appendFile(
                 "benchmarks.json",
                 `${JSON.stringify(results)}\n`,
-                function (err) {
-                    if (err) {
-                        throw err;
+                function (error) {
+                    if (error) {
+                        throw error;
                     }
                 }
             );
@@ -232,7 +232,9 @@ async function initializeRedisClient(): Promise<RedisClientType> {
         },
     });
 
-    redis.on("error", (err: any) => console.log("Redis Client Error", err));
+    redis.on("error", (error: any) =>
+        console.error("Redis Client Error", error)
+    );
 
     await redis.connect();
     return redis as RedisClientType;