From 32dd435c6e02267afddbfc715c7b92439f21d8cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Chalet?= Date: Mon, 13 Mar 2017 18:02:59 +0100 Subject: [PATCH] Add an opt-out DisableTelemetry option in the OpenID Connect middleware (#1140) --- .../OpenIdConnectHandler.cs | 3 +++ .../OpenIdConnectOptions.cs | 7 +++++++ .../OpenIdConnectChallengeTests.cs | 20 +++++++++++++++++-- .../OpenIdConnectMiddlewareTests.cs | 20 ++++++++++++++++++- .../OpenIdConnect/TestSettings.cs | 14 +++++++++++++ 5 files changed, 61 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs index a7c20f62ea..6b24996e78 100644 --- a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs +++ b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs @@ -161,6 +161,7 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect 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 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect 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 @@ namespace Microsoft.AspNetCore.Authentication.OpenIdConnect 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 a46c2956c6..8269acbd8f 100644 --- a/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectOptions.cs +++ b/src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectOptions.cs @@ -241,5 +241,12 @@ namespace Microsoft.AspNetCore.Builder /// 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 b9c0179aff..1912561b11 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs @@ -35,7 +35,23 @@ namespace Microsoft.AspNetCore.Authentication.Tests.OpenIdConnect OpenIdConnectParameterNames.ResponseType, OpenIdConnectParameterNames.ResponseMode, OpenIdConnectParameterNames.Scope, - OpenIdConnectParameterNames.RedirectUri); + OpenIdConnectParameterNames.RedirectUri, + OpenIdConnectParameterNames.SkuTelemetry, + OpenIdConnectParameterNames.VersionTelemetry); + } + + [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); } /* @@ -58,7 +74,7 @@ namespace Microsoft.AspNetCore.Authentication.Tests.OpenIdConnect */ [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 6c427c600e..a58d54b650 100644 --- a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs +++ b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs @@ -46,7 +46,25 @@ namespace Microsoft.AspNetCore.Authentication.Tests.OpenIdConnect 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] + 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] diff --git a/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/TestSettings.cs b/test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/TestSettings.cs index 3e50a7abee..a3bea3ebe7 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; 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 @@ namespace Microsoft.AspNetCore.Authentication.Tests.OpenIdConnect 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 @@ namespace Microsoft.AspNetCore.Authentication.Tests.OpenIdConnect 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,