search

DuendeSoftware / IdentityServer

The most flexible and standards-compliant OpenID Connect and OAuth 2.x framework for ASP.NET Core
https://duendesoftware.com/products/identityserver
Other
1.49k stars 344 forks source link

Re-work logging and auditing #36

Open leastprivilege opened 3 years ago

leastprivilege commented 3 years ago

https://github.com/IdentityServer/IdentityServer4/issues/3518

leastprivilege commented 3 years ago

https://github.com/IdentityServer/IdentityServer4/issues/3825

leastprivilege commented 3 years ago

https://github.com/IdentityServer/IdentityServer4/issues/4359

leastprivilege commented 3 years ago

https://github.com/IdentityServer/IdentityServer4/issues/4652

leastprivilege commented 3 years ago

https://github.com/IdentityServer/IdentityServer4/issues/4725

brockallen commented 3 years ago

@pbhughes is a customer that might have some feedback on this epic

pbhughes commented 3 years ago

I believe logging and "eventing" to be separate actions performed at separate rates for separate audiences. I would like to see them remain separated. Token issued events and Successful login events carry weight in the audit world, the debug or verbose log entries do not. Having this separation allows identity server developers to capture events in a distinct way and use modern event bus technology to build audit trails without log messages intermixed.

leastprivilege commented 3 years ago

https://github.com/DuendeSoftware/IdentityServer/discussions/249

leastprivilege commented 3 years ago

Use this? https://docs.microsoft.com/en-gb/dotnet/core/extensions/logger-message-generator

brockallen commented 3 years ago

@pbhughes One extra bit of info for you -- the line between logging and evening is blurred given that logging allows for structured data. So our high level thinking was that we'd move everything to the same approach (whatever that ends up being) and the message Id or message name or message prefix (or something) will be your indicator in your logging sink that this message was more like the "event" style. So your sink might just have to filter for the specific types it wanted to use for your high level audit and dashboard style functionality.

At least this is my last recollection of our plan... it might have changed since @leastprivilege has been doing more research.

brockallen commented 2 years ago

We'll consider OTEL (OpenTelemetry Logging Instrumentation) once it's part of .NET Core.

krosn commented 2 years ago

Hey Dominick, @brockallen asked me to add my company's use case here. We have two scenarios that may benefit from the upcoming logging changes:

  1. We have a 24/7 team monitoring errors and criticals that get logged to Application Insights by our services, and we get notified (sometimes outside of business hours) when they pop up. Currently a lot of the message severities in IdentityServer are hardcoded as errors (ex. an error is logged because a customer has an incorrect Client Id or Secret). Unfortunately without a way to customize or intercept this, we've had to take the blanket approach of overriding the severities on all traces generated by certain IdentityServer logging categories with an ITelemetryInitializer. This isn't ideal as we're potentially hiding other errors generated in the classes that we might want to know about.

  2. We have automated alerts in place to try and proactively detect PII leaks. While we thankfully haven't had any actual leaks, we were seeing false-positives due to customers putting normally sensitive information like names and email addresses into Client names. We didn't want to restrict their use-case here, but we also didn't want to keep seeing alerts that our AI logs contained possible PII. We removed the client name from the logs we generate, but the "token request validation success" messages generate by IdentityServer contain the client Id. I tried making use of the TokenRequestSensitiveValuesFilter logging options, but they only allow redacting values in the "Raw" dictionary, not the top-level entries like ClientId (see TokenRequestValidator and TokenRequestValidationLog). Again we ended up using an ITelemetryInitializer to redact that information ourselves.

Both of these items have adequate solutions for now, but I just wanted to highlight these as use-cases we have for possible logging changes.

And thanks again for all the great work! We enjoy making use of IdentityServer and working with both of you.

brockallen commented 2 months ago

See also: https://github.com/DuendeSoftware/IdentityServer/issues/1359

brockallen commented 2 months ago

Also: https://github.com/DuendeSoftware/IdentityServer/issues/1547