Add an opt-out DisableTelemetry option in the OpenID Connect middleware #1140
Conversation
|
@PinpointTownes, |
Eilon
left a comment
Eilon
left a comment
There was a problem hiding this comment.
I think this is good, just a small doc comment tweak.
| public bool SkipUnrecognizedRequests { get; set; } = false; | ||
|
|
||
| /// <summary> | ||
| /// Indicates whether telemetry should be disabled. When this feature is disabled, |
There was a problem hiding this comment.
Perhaps change the second sentence to be worded with positives instead of the double-negative? E.g. "When this feature is enabled, blah blah is sent to the blah blah". I just think it's a bit easier to read that way.
| 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); |
There was a problem hiding this comment.
Could use one positive test they verifies the presence of these parameters. Could be added to an existing test.
There was a problem hiding this comment.
Done. FYI, I didn't update ChallengeIssuedCorrectlyForFormPost because the telemetry parameters are never sent when opting for POST authorization requests.
4 participants
Fixes #1035.
/cc @Eilon @Tratcher @brentschmaltz @polita