Implemented Thomas's Feedback

AzureAD · Feb 7, 2022 · 30e6adb · 30e6adb
1 parent 8e0c6d9
commit 30e6adb
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 14 deletions.
diff --git a/lib/msal-common/src/authority/Authority.ts b/lib/msal-common/src/authority/Authority.ts
@@ -19,7 +19,7 @@ import { CloudInstanceDiscoveryResponse, isCloudInstanceDiscoveryResponse } from
 import { CloudDiscoveryMetadata } from "./CloudDiscoveryMetadata";
 import { RegionDiscovery } from "./RegionDiscovery";
 import { RegionDiscoveryMetadata } from "./RegionDiscoveryMetadata";
-
+import { ImdsOptions } from "./ImdsOptions";
 /**
  * The authority class validates the authority URIs used by the user, and retrieves the OpenID Configuration Data from the
  * endpoint. It will store the pertinent config data in this object for use during token calls.
@@ -45,15 +45,15 @@ export class Authority {
     // Proxy url string
     private proxyUrl: string;
 
-    constructor(authority: string, networkInterface: INetworkModule, cacheManager: ICacheManager, authorityOptions: AuthorityOptions, proxyUrl: string) {
+    constructor(authority: string, networkInterface: INetworkModule, cacheManager: ICacheManager, authorityOptions: AuthorityOptions, proxyUrl?: string) {
         this.canonicalAuthority = authority;
         this._canonicalAuthority.validateAsUri();
         this.networkInterface = networkInterface;
         this.cacheManager = cacheManager;
         this.authorityOptions = authorityOptions;
         this.regionDiscovery = new RegionDiscovery(networkInterface);
         this.regionDiscoveryMetadata = { region_used: undefined, region_source: undefined, region_outcome: undefined };
-        this.proxyUrl = proxyUrl;
+        this.proxyUrl = proxyUrl || Constants.EMPTY_STRING;
     }
 
     // See above for AuthorityType
@@ -338,9 +338,9 @@ export class Authority {
      * Gets OAuth endpoints from the given OpenID configuration endpoint.
      */
     private async getEndpointMetadataFromNetwork(): Promise<OpenIdConfigResponse | null> {
-        const options = {};
-        if (this.proxyUrl.length) {
-            options["proxyUrl"] = this.proxyUrl;
+        const options: ImdsOptions = {};
+        if (this.proxyUrl) {
+            options.proxyUrl = this.proxyUrl;
         }
 
         try {
@@ -410,9 +410,9 @@ export class Authority {
      */
     private async getCloudDiscoveryMetadataFromNetwork(): Promise<CloudDiscoveryMetadata | null> {
         const instanceDiscoveryEndpoint = `${Constants.AAD_INSTANCE_DISCOVERY_ENDPT}${this.canonicalAuthority}oauth2/v2.0/authorize`;
-        const options = {};
-        if (this.proxyUrl.length) {
-            options["proxyUrl"] = this.proxyUrl;
+        const options: ImdsOptions = {};
+        if (this.proxyUrl) {
+            options.proxyUrl = this.proxyUrl;
         }
 
         let match = null;

diff --git a/lib/msal-common/src/authority/ImdsOptions.ts b/lib/msal-common/src/authority/ImdsOptions.ts
@@ -0,0 +1,6 @@
+export type ImdsOptions = {
+    headers?: {
+        Metadata: string,
+    },
+    proxyUrl?: string,
+};
diff --git a/lib/msal-common/src/authority/RegionDiscovery.ts b/lib/msal-common/src/authority/RegionDiscovery.ts
@@ -8,12 +8,17 @@ import { NetworkResponse } from "../network/NetworkManager";
 import { IMDSBadResponse } from "../response/IMDSBadResponse";
 import { Constants, RegionDiscoverySources, ResponseCodes } from "../utils/Constants";
 import { RegionDiscoveryMetadata } from "./RegionDiscoveryMetadata";
+import { ImdsOptions } from "./ImdsOptions";
 
 export class RegionDiscovery {
     // Network interface to make requests with.
     protected networkInterface: INetworkModule;
     // Options for the IMDS endpoint request
-    protected static IMDS_OPTIONS = {headers: {"Metadata": "true"}};
+    protected static IMDS_OPTIONS: ImdsOptions = {
+        headers: {
+            Metadata: "true",
+        },
+    };
 
     constructor(networkInterface: INetworkModule) {
         this.networkInterface = networkInterface;
@@ -31,8 +36,8 @@ export class RegionDiscovery {
         // Check if a region was detected from the environment, if not, attempt to get the region from IMDS 
         if (!autodetectedRegionName) {
             const options = RegionDiscovery.IMDS_OPTIONS;
-            if (proxyUrl.length) {
-                options["proxyUrl"] = proxyUrl;
+            if (proxyUrl) {
+                options.proxyUrl = proxyUrl;
             }
 
             try {
@@ -78,7 +83,7 @@ export class RegionDiscovery {
      * @param imdsEndpointUrl
      * @returns Promise<NetworkResponse<string>>
      */
-    private async getRegionFromIMDS(version: string, options: object): Promise<NetworkResponse<string>> {
+    private async getRegionFromIMDS(version: string, options: ImdsOptions): Promise<NetworkResponse<string>> {
         return this.networkInterface.sendGetRequestAsync<string>(`${Constants.IMDS_ENDPOINT}?api-version=${version}&format=text`, options, Constants.IMDS_TIMEOUT);
     }
 
@@ -87,7 +92,7 @@ export class RegionDiscovery {
      *  
      * @returns Promise<string | null>
      */
-    private async getCurrentVersion(options: object): Promise<string | null> {
+    private async getCurrentVersion(options: ImdsOptions): Promise<string | null> {
         try {
             const response = await this.networkInterface.sendGetRequestAsync<IMDSBadResponse>(`${Constants.IMDS_ENDPOINT}?format=json`, options);