Address PR and add UT

bgavrilMS · bgavrilMS · commit 61af76af83d5 · 2024-08-13T10:58:54.000+03:00
diff --git a/src/Microsoft.Identity.Web.Diagnostics/IdHelper.cs b/src/Microsoft.Identity.Web.Diagnostics/IdHelper.cs
@@ -6,7 +6,7 @@
 using System.Reflection;
 using System.Text.RegularExpressions;
 
-namespace Microsoft.Identity.Web
+namespace Microsoft.Identity.Web.Diagnostics
 {
     internal static class IdHelper
     {
@@ -37,13 +37,13 @@ internal static class IdHelper
            });
 
         public static string GetIdWebVersion()
-        {             
+        {
             return s_idWebVersion.Value;
         }
 
         public static string CreateTelemetryInfo()
         {
-            return string.Format(CultureInfo.InvariantCulture, IDWebSku + s_idWebVersion.Value);
+            return string.Format(CultureInfo.InvariantCulture, IDWebSku + GetIdWebVersion());
         }
     }
 }
diff --git a/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.cs b/src/Microsoft.Identity.Web.DownstreamApi/DownstreamApi.cs
@@ -16,6 +16,7 @@
 using Microsoft.Extensions.Options;
 using Microsoft.Identity.Abstractions;
 using Microsoft.Identity.Client;
+using Microsoft.Identity.Web.Diagnostics;
 
 namespace Microsoft.Identity.Web
 {
@@ -306,7 +307,6 @@ private async Task<HttpResponseMessage> CallApiInternalAsync(
         {
             // Downstream API URI
             string apiUrl = effectiveOptions.GetApiUrl();
-            AddCallerSDKTelemetry(effectiveOptions);
 
             // Create an HTTP request message
             using HttpRequestMessage httpRequestMessage = new(
@@ -340,14 +340,16 @@ private async Task<HttpResponseMessage> CallApiInternalAsync(
             return downstreamApiResult;
         }
 
-        internal async Task UpdateRequestAsync(
+        internal /* internal for test */ async Task UpdateRequestAsync(
             HttpRequestMessage httpRequestMessage,
             HttpContent? content,
             DownstreamApiOptions effectiveOptions,
             bool appToken,
             ClaimsPrincipal? user,
             CancellationToken cancellationToken)
         {
+            AddCallerSDKTelemetry(effectiveOptions);
+
             if (content != null)
             {
                 httpRequestMessage.Content = content;
@@ -379,8 +381,7 @@ internal async Task UpdateRequestAsync(
             effectiveOptions.CustomizeHttpRequestMessage?.Invoke(httpRequestMessage);
         }
 
-
-        private static readonly Dictionary<string, string> s_callerSDKDetails = new()
+        internal /* for test */ static Dictionary<string, string> CallerSDKDetails { get; } = new()
           {
               { "caller-sdk-id", "1" },  // 1 = Downstream API SDK ID
               { "caller-sdk-ver", IdHelper.GetIdWebVersion() }
@@ -390,14 +391,14 @@ private static void AddCallerSDKTelemetry(DownstreamApiOptions effectiveOptions)
         {
             if (effectiveOptions.AcquireTokenOptions.ExtraQueryParameters == null)
             {
-                effectiveOptions.AcquireTokenOptions.ExtraQueryParameters = s_callerSDKDetails;
+                effectiveOptions.AcquireTokenOptions.ExtraQueryParameters = CallerSDKDetails;
             }
             else
             {
                 effectiveOptions.AcquireTokenOptions.ExtraQueryParameters["caller-sdk-id"] =
-                    s_callerSDKDetails["caller-sdk-id"];
+                    CallerSDKDetails["caller-sdk-id"];
                 effectiveOptions.AcquireTokenOptions.ExtraQueryParameters["caller-sdk-ver"] =
-                    s_callerSDKDetails["caller-sdk-ver"];
+                    CallerSDKDetails["caller-sdk-ver"];
             }
         }
     }
diff --git a/src/Microsoft.Identity.Web.DownstreamApi/Microsoft.Identity.Web.DownstreamApi.csproj b/src/Microsoft.Identity.Web.DownstreamApi/Microsoft.Identity.Web.DownstreamApi.csproj
@@ -7,9 +7,6 @@
     <EnableConfigurationBindingGenerator>true</EnableConfigurationBindingGenerator>
     <PackageReadmeFile>README.md</PackageReadmeFile>
   </PropertyGroup>
-	<ItemGroup>
-		<Compile Include="..\Microsoft.Identity.Web.TokenAcquisition\IdHelper.cs" Link="IdHelper.cs" />
-	</ItemGroup>
   <ItemGroup>
     <None Include="..\..\README.md">
       <Pack>True</Pack>
diff --git a/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs b/src/Microsoft.Identity.Web.OWIN/AppBuilderExtension.cs
@@ -17,6 +17,7 @@
 using Microsoft.Owin.Security.Jwt;
 using Microsoft.Owin.Security.OAuth;
 using Microsoft.Owin.Security.OpenIdConnect;
+using Microsoft.Identity.Web.Diagnostics;
 using Owin;
 
 namespace Microsoft.Identity.Web
diff --git a/src/Microsoft.Identity.Web.TokenAcquisition/MsalAspNetCoreHttpClientFactory.cs b/src/Microsoft.Identity.Web.TokenAcquisition/MsalAspNetCoreHttpClientFactory.cs
@@ -3,6 +3,7 @@
 
 using System.Net.Http;
 using Microsoft.Identity.Client;
+using Microsoft.Identity.Web.Diagnostics;
 
 namespace Microsoft.Identity.Web
 {
diff --git a/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilderExtensions.cs b/src/Microsoft.Identity.Web/WebAppExtensions/MicrosoftIdentityWebAppAuthenticationBuilderExtensions.cs
@@ -16,6 +16,7 @@
 using Microsoft.IdentityModel.Protocols.OpenIdConnect;
 using System.Linq;
 using System.Diagnostics.CodeAnalysis;
+using Microsoft.Identity.Web.Diagnostics;
 
 namespace Microsoft.Identity.Web
 {
diff --git a/tests/Microsoft.Identity.Web.Test/DownstreamWebApiSupport/DownstreamApiTests.cs b/tests/Microsoft.Identity.Web.Test/DownstreamWebApiSupport/DownstreamApiTests.cs
@@ -48,14 +48,51 @@ public async Task UpdateRequestAsync_WithContent_AddsContentToRequest()
             // Arrange
             var httpRequestMessage = new HttpRequestMessage(HttpMethod.Post, "https://example.com");
             var content = new StringContent("test content");
+            var options = new DownstreamApiOptions();
 
             // Act
-            await _input.UpdateRequestAsync(httpRequestMessage, content, new DownstreamApiOptions(), false, null, CancellationToken.None);
+            await _input.UpdateRequestAsync(httpRequestMessage, content, options, false, null, CancellationToken.None);
 
             // Assert
             Assert.Equal(content, httpRequestMessage.Content);
             Assert.Equal("application/json", httpRequestMessage.Headers.Accept.Single().MediaType);
             Assert.Equal("text/plain", httpRequestMessage.Content?.Headers.ContentType?.MediaType);
+            Assert.Equal(options.AcquireTokenOptions.ExtraQueryParameters, DownstreamApi.CallerSDKDetails);
+        }
+
+
+        [Fact]
+        public async Task UpdateRequestAsync_AddsToExtraQP()
+        {
+            // Arrange
+            var httpRequestMessage = new HttpRequestMessage(HttpMethod.Post, "https://example.com");
+            var content = new StringContent("test content");
+            var options = new DownstreamApiOptions() {  
+                AcquireTokenOptions = new AcquireTokenOptions() { 
+                    ExtraQueryParameters = new Dictionary<string, string>() 
+                    { 
+                        { "n1", "v1" },
+                        { "n2", "v2" },
+                        { "caller-sdk-id", "bogus" } // value will be overwritten by the SDK
+                    }
+                } };
+
+            // Act
+            await _input.UpdateRequestAsync(httpRequestMessage, content, options, false, null, CancellationToken.None);
+
+            // Assert
+            Assert.Equal(content, httpRequestMessage.Content);
+            Assert.Equal("application/json", httpRequestMessage.Headers.Accept.Single().MediaType);
+            Assert.Equal("text/plain", httpRequestMessage.Content?.Headers.ContentType?.MediaType);
+            Assert.Equal("v1", options.AcquireTokenOptions.ExtraQueryParameters["n1"]);
+            Assert.Equal("v2", options.AcquireTokenOptions.ExtraQueryParameters["n2"]);
+            Assert.Equal(
+                DownstreamApi.CallerSDKDetails["caller-sdk-id"], 
+                options.AcquireTokenOptions.ExtraQueryParameters["caller-sdk-id"] );
+            Assert.Equal(
+                DownstreamApi.CallerSDKDetails["caller-sdk-ver"],
+                options.AcquireTokenOptions.ExtraQueryParameters["caller-sdk-ver"]);
+
         }
 
         [Theory]
@@ -82,6 +119,7 @@ public async Task UpdateRequestAsync_WithScopes_AddsAuthorizationHeaderToRequest
             Assert.Equal("ey", httpRequestMessage.Headers.Authorization?.Parameter);
             Assert.Equal("Bearer", httpRequestMessage.Headers.Authorization?.Scheme);
             Assert.Equal("application/json", httpRequestMessage.Headers.Accept.Single().MediaType);
+            Assert.Equal(options.AcquireTokenOptions.ExtraQueryParameters, DownstreamApi.CallerSDKDetails);
         }
 
         [Fact]

Original file line number	Diff line number	Diff line change
`@@ -6,7 +6,7 @@`
`6`	`6`	`using System.Reflection;`
`7`	`7`	`using System.Text.RegularExpressions;`
`8`	`8`
`9`		`-namespace Microsoft.Identity.Web`
	`9`	`+namespace Microsoft.Identity.Web.Diagnostics`
`10`	`10`	`{`
`11`	`11`	`internal static class IdHelper`
`12`	`12`	`{`
`@@ -37,13 +37,13 @@ internal static class IdHelper`
`37`	`37`	`});`
`38`	`38`
`39`	`39`	`public static string GetIdWebVersion()`
`40`		`- {`
	`40`	`+ {`
`41`	`41`	`return s_idWebVersion.Value;`
`42`	`42`	`}`
`43`	`43`
`44`	`44`	`public static string CreateTelemetryInfo()`
`45`	`45`	`{`
`46`		`- return string.Format(CultureInfo.InvariantCulture, IDWebSku + s_idWebVersion.Value);`
	`46`	`+ return string.Format(CultureInfo.InvariantCulture, IDWebSku + GetIdWebVersion());`
`47`	`47`	`}`
`48`	`48`	`}`
`49`	`49`	`}`
Original file line number	Diff line number	Diff line change
`@@ -16,6 +16,7 @@`
`16`	`16`	`using Microsoft.Extensions.Options;`
`17`	`17`	`using Microsoft.Identity.Abstractions;`
`18`	`18`	`using Microsoft.Identity.Client;`
	`19`	`+using Microsoft.Identity.Web.Diagnostics;`
`19`	`20`
`20`	`21`	`namespace Microsoft.Identity.Web`
`21`	`22`	`{`
`@@ -306,7 +307,6 @@ private async Task<HttpResponseMessage> CallApiInternalAsync(`
`306`	`307`	`{`
`307`	`308`	`// Downstream API URI`
`308`	`309`	`string apiUrl = effectiveOptions.GetApiUrl();`
`309`		`- AddCallerSDKTelemetry(effectiveOptions);`
`310`	`310`
`311`	`311`	`// Create an HTTP request message`
`312`	`312`	`using HttpRequestMessage httpRequestMessage = new(`
`@@ -340,14 +340,16 @@ private async Task<HttpResponseMessage> CallApiInternalAsync(`
`340`	`340`	`return downstreamApiResult;`
`341`	`341`	`}`
`342`	`342`
`343`		`- internal async Task UpdateRequestAsync(`
	`343`	`+ internal /* internal for test */ async Task UpdateRequestAsync(`
`344`	`344`	`HttpRequestMessage httpRequestMessage,`
`345`	`345`	`HttpContent? content,`
`346`	`346`	`DownstreamApiOptions effectiveOptions,`
`347`	`347`	`bool appToken,`
`348`	`348`	`ClaimsPrincipal? user,`
`349`	`349`	`CancellationToken cancellationToken)`
`350`	`350`	`{`
	`351`	`+ AddCallerSDKTelemetry(effectiveOptions);`
	`352`	`+`
`351`	`353`	`if (content != null)`
`352`	`354`	`{`
`353`	`355`	`httpRequestMessage.Content = content;`
`@@ -379,8 +381,7 @@ internal async Task UpdateRequestAsync(`
`379`	`381`	`effectiveOptions.CustomizeHttpRequestMessage?.Invoke(httpRequestMessage);`
`380`	`382`	`}`
`381`	`383`
`382`		`-`
`383`		`- private static readonly Dictionary<string, string> s_callerSDKDetails = new()`
	`384`	`+ internal /* for test */ static Dictionary<string, string> CallerSDKDetails { get; } = new()`
`384`	`385`	`{`
`385`	`386`	`{ "caller-sdk-id", "1" }, // 1 = Downstream API SDK ID`
`386`	`387`	`{ "caller-sdk-ver", IdHelper.GetIdWebVersion() }`
`@@ -390,14 +391,14 @@ private static void AddCallerSDKTelemetry(DownstreamApiOptions effectiveOptions)`
`390`	`391`	`{`
`391`	`392`	`if (effectiveOptions.AcquireTokenOptions.ExtraQueryParameters == null)`
`392`	`393`	`{`
`393`		`- effectiveOptions.AcquireTokenOptions.ExtraQueryParameters = s_callerSDKDetails;`
	`394`	`+ effectiveOptions.AcquireTokenOptions.ExtraQueryParameters = CallerSDKDetails;`
`394`	`395`	`}`
`395`	`396`	`else`
`396`	`397`	`{`
`397`	`398`	`effectiveOptions.AcquireTokenOptions.ExtraQueryParameters["caller-sdk-id"] =`
`398`		`- s_callerSDKDetails["caller-sdk-id"];`
	`399`	`+ CallerSDKDetails["caller-sdk-id"];`
`399`	`400`	`effectiveOptions.AcquireTokenOptions.ExtraQueryParameters["caller-sdk-ver"] =`
`400`		`- s_callerSDKDetails["caller-sdk-ver"];`
	`401`	`+ CallerSDKDetails["caller-sdk-ver"];`
`401`	`402`	`}`
`402`	`403`	`}`
`403`	`404`	`}`
Original file line number	Diff line number	Diff line change
`@@ -3,6 +3,7 @@`
`3`	`3`
`4`	`4`	`using System.Net.Http;`
`5`	`5`	`using Microsoft.Identity.Client;`
	`6`	`+using Microsoft.Identity.Web.Diagnostics;`
`6`	`7`
`7`	`8`	`namespace Microsoft.Identity.Web`
`8`	`9`	`{`