Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

Add an opt-out DisableTelemetry option in the OpenID Connect middleware #1140

Merged
merged 2 commits into from
Mar 13, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -309,6 +310,7 @@ protected override async Task<bool> 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,
Expand Down Expand Up @@ -1023,6 +1025,7 @@ private async Task<AuthorizationCodeReceivedContext> RunAuthorizationCodeReceive
ClientSecret = Options.ClientSecret,
Code = authorizationResponse.Code,
GrantType = OpenIdConnectGrantTypes.AuthorizationCode,
EnableTelemetryParameters = !Options.DisableTelemetry,
RedirectUri = properties.Items[OpenIdConnectDefaults.RedirectUriForCodePropertiesKey]
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,5 +241,12 @@ public OpenIdConnectOptions(string authenticationScheme)
/// This is disabled by default.
/// </summary>
public bool SkipUnrecognizedRequests { get; set; } = false;

/// <summary>
/// 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.
/// </summary>
public bool DisableTelemetry { get; set; }
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,23 @@ public async Task ChallengeIsIssuedCorrectly()
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use one positive test they verifies the presence of these parameters. Could be added to an existing test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. FYI, I didn't update ChallengeIssuedCorrectlyForFormPost because the telemetry parameters are never sent when opting for POST authorization requests.

}

/*
Expand All @@ -58,7 +74,7 @@ Example of a form post
</body>
*/
[Fact]
public async Task ChallengeIssueedCorrectlyForFormPost()
public async Task ChallengeIssuedCorrectlyForFormPost()
{
var settings = new TestSettings(
opt => opt.AuthenticationMethod = OpenIdConnectRedirectBehavior.FormPost);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,25 @@ 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]
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]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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}\".");
}
Expand Down Expand Up @@ -201,6 +208,13 @@ private void ValidateResource(IDictionary<string, string> actualQuery, ICollecti
private void ValidateState(IDictionary<string, string> actualQuery, ICollection<string> errors, bool htmlEncoded) =>
ValidateQueryParameter(OpenIdConnectParameterNames.State, ExpectedState, actualQuery, errors, htmlEncoded);

private void ValidateSkuTelemetry(IDictionary<string, string> actualQuery, ICollection<string> errors, bool htmlEncoded) =>
ValidateQueryParameter(OpenIdConnectParameterNames.SkuTelemetry, "ID_NET", actualQuery, errors, htmlEncoded);

private void ValidateVersionTelemetry(IDictionary<string, string> actualQuery, ICollection<string> errors, bool htmlEncoded) =>
ValidateQueryParameter(OpenIdConnectParameterNames.VersionTelemetry,
typeof(OpenIdConnectMessage).GetTypeInfo().Assembly.GetName().Version.ToString(), actualQuery, errors, htmlEncoded);

private void ValidateQueryParameter(
string parameterName,
string expectedValue,
Expand Down