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

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4e29844
Add an opt-out DisableTelemetry option in the OpenID Connect middleware
kevinchalet Mar 9, 2017
5de17fc
Add a positive test verifying the telemetry parameters are sent by de…
kevinchalet Mar 13, 2017
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
3 changes: 3 additions & 0 deletions src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectHandler.cs
View file
Open in desktop
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
7 changes: 7 additions & 0 deletions src/Microsoft.AspNetCore.Authentication.OpenIdConnect/OpenIdConnectOptions.cs
View file
Open in desktop
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; }
}
}
16 changes: 15 additions & 1 deletion test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectChallengeTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
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.

}

/*
Example of a form post
<body>
Expand All @@ -58,7 +72,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
15 changes: 15 additions & 0 deletions test/Microsoft.AspNetCore.Authentication.Test/OpenIdConnect/OpenIdConnectMiddlewareTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down