From 4e29844cc9cbf265ccfabb2e8f1441a499d44ac1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Thu, 9 Mar 2017 19:15:50 +0100 Subject: [PATCH 1/2] Add an opt-out DisableTelemetry option in the OpenID Connect middleware --- .../OpenIdConnectHandler.cs | 3 +++ .../OpenIdConnectOptions.cs | 7 +++++++ .../OpenIdConnect/OpenIdConnectChallengeTests.cs | 16 +++++++++++++++- .../OpenIdConnectMiddlewareTests.cs | 15 +++++++++++++++ 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs index a7c20f62e..6b24996e7 100644 --- a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs @@ -161,6 +161,7 @@ protected override async Task HandleSignOutAsync(SignOutContext signout) var message = new OpenIdConnectMessage() { + EnableTelemetryParameters = !Options.DisableTelemetry, IssuerAddress = _configuration?.EndSessionEndpoint ?? string.Empty, // Redirect back to SigneOutCallbackPath first before user agent is redirected to actual post logout redirect uri @@ -309,6 +310,7 @@ protected override async Task HandleUnauthorizedAsync(ChallengeContext con var message = new OpenIdConnectMessage { ClientId = Options.ClientId, + EnableTelemetryParameters = !Options.DisableTelemetry, IssuerAddress = _configuration?.AuthorizationEndpoint ?? string.Empty, RedirectUri = BuildRedirectUri(Options.CallbackPath), Resource = Options.Resource, @@ -1023,6 +1025,7 @@ private async Task RunAuthorizationCodeReceive ClientSecret = Options.ClientSecret, Code = authorizationResponse.Code, GrantType = OpenIdConnectGrantTypes.AuthorizationCode, + EnableTelemetryParameters = !Options.DisableTelemetry, RedirectUri = properties.Items[OpenIdConnectDefaults.RedirectUriForCodePropertiesKey] }; diff --git a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectOptions.cs b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectOptions.cs index a46c2956c..8269acbd8 100644 --- a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectOptions.cs +++ b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectOptions.cs @@ -241,5 +241,12 @@ public OpenIdConnectOptions(string authenticationScheme) /// This is disabled by default. /// public bool SkipUnrecognizedRequests { get; set; } = false; + + /// + /// Indicates whether telemetry should be disabled. When this feature is enabled, + /// the assembly version of the Microsoft IdentityModel packages is sent to the + /// remote OpenID Connect provider as an authorization/logout request parameter. + /// + public bool DisableTelemetry { get; set; } } } diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs index b9c0179af..2e0a4244f 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs @@ -38,6 +38,20 @@ public async Task ChallengeIsIssuedCorrectly() OpenIdConnectParameterNames.RedirectUri); } + [Fact] + public async Task AuthorizationRequestDoesNotIncludeTelemetryParametersWhenDisabled() + { + var settings = new TestSettings(opt => opt.DisableTelemetry = true); + + var server = settings.CreateTestServer(); + var transaction = await server.SendAsync(ChallengeEndpoint); + + var res = transaction.Response; + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.DoesNotContain(OpenIdConnectParameterNames.SkuTelemetry, res.Headers.Location.Query); + Assert.DoesNotContain(OpenIdConnectParameterNames.VersionTelemetry, res.Headers.Location.Query); + } + /* Example of a form post @@ -58,7 +72,7 @@ Example of a form post */ [Fact] - public async Task ChallengeIssueedCorrectlyForFormPost() + public async Task ChallengeIssuedCorrectlyForFormPost() { var settings = new TestSettings( opt => opt.AuthenticationMethod = OpenIdConnectRedirectBehavior.FormPost); diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs index 6c427c600..f98ff0df2 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs @@ -49,6 +49,21 @@ public async Task SignOutSettingMessage() setting.ValidateSignoutRedirect(transaction.Response.Headers.Location); } + [Fact] + public async Task EndSessionRequestDoesNotIncludeTelemetryParametersWhenDisabled() + { + var setting = new TestSettings(opt => opt.DisableTelemetry = true); + + var server = setting.CreateTestServer(); + + var transaction = await server.SendAsync(DefaultHost + TestServerBuilder.Signout); + var res = transaction.Response; + + Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); + Assert.DoesNotContain(OpenIdConnectParameterNames.SkuTelemetry, res.Headers.Location.Query); + Assert.DoesNotContain(OpenIdConnectParameterNames.VersionTelemetry, res.Headers.Location.Query); + } + [Fact] public async Task SignOutWithDefaultRedirectUri() { From 5de17fc5091810952a40d1833d5ef919bb1fc8b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Mon, 13 Mar 2017 17:49:21 +0100 Subject: [PATCH 2/2] Add a positive test verifying the telemetry parameters are sent by default --- .../OpenIdConnect/OpenIdConnectChallengeTests.cs | 4 +++- .../OpenIdConnect/OpenIdConnectMiddlewareTests.cs | 5 ++++- .../OpenIdConnect/TestSettings.cs | 14 ++++++++++++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs index 2e0a4244f..1912561b1 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs @@ -35,7 +35,9 @@ public async Task ChallengeIsIssuedCorrectly() OpenIdConnectParameterNames.ResponseType, OpenIdConnectParameterNames.ResponseMode, OpenIdConnectParameterNames.Scope, - OpenIdConnectParameterNames.RedirectUri); + OpenIdConnectParameterNames.RedirectUri, + OpenIdConnectParameterNames.SkuTelemetry, + OpenIdConnectParameterNames.VersionTelemetry); } [Fact] diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs index f98ff0df2..a58d54b65 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs @@ -46,7 +46,10 @@ public async Task SignOutSettingMessage() Assert.Equal(HttpStatusCode.Redirect, res.StatusCode); Assert.NotNull(res.Headers.Location); - setting.ValidateSignoutRedirect(transaction.Response.Headers.Location); + setting.ValidateSignoutRedirect( + transaction.Response.Headers.Location, + OpenIdConnectParameterNames.SkuTelemetry, + OpenIdConnectParameterNames.VersionTelemetry); } [Fact] diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/TestSettings.cs b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/TestSettings.cs index 3e50a7abe..a3bea3ebe 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/TestSettings.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/TestSettings.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Diagnostics; using System.Linq; +using System.Reflection; using System.Text; using System.Text.Encodings.Web; using System.Xml.Linq; @@ -152,6 +153,12 @@ private void ValidateParameters( case OpenIdConnectParameterNames.State: ValidateState(actualValues, errors, htmlEncoded); break; + case OpenIdConnectParameterNames.SkuTelemetry: + ValidateSkuTelemetry(actualValues, errors, htmlEncoded); + break; + case OpenIdConnectParameterNames.VersionTelemetry: + ValidateVersionTelemetry(actualValues, errors, htmlEncoded); + break; default: throw new InvalidOperationException($"Unknown parameter \"{paramToValidate}\"."); } @@ -201,6 +208,13 @@ private void ValidateResource(IDictionary actualQuery, ICollecti private void ValidateState(IDictionary actualQuery, ICollection errors, bool htmlEncoded) => ValidateQueryParameter(OpenIdConnectParameterNames.State, ExpectedState, actualQuery, errors, htmlEncoded); + private void ValidateSkuTelemetry(IDictionary actualQuery, ICollection errors, bool htmlEncoded) => + ValidateQueryParameter(OpenIdConnectParameterNames.SkuTelemetry, "ID_NET", actualQuery, errors, htmlEncoded); + + private void ValidateVersionTelemetry(IDictionary actualQuery, ICollection errors, bool htmlEncoded) => + ValidateQueryParameter(OpenIdConnectParameterNames.VersionTelemetry, + typeof(OpenIdConnectMessage).GetTypeInfo().Assembly.GetName().Version.ToString(), actualQuery, errors, htmlEncoded); + private void ValidateQueryParameter( string parameterName, string expectedValue,