IdentityServer / IdentityServer3

OpenID Connect Provider and OAuth 2.0 Authorization Server Framework for ASP.NET 4.x/Katana
https://identityserver.github.io/Documentation/
Apache License 2.0
2.01k stars 764 forks source link

TokenRequestValidator logs every check failure as error #3244

Closed dzintaras closed 7 years ago

dzintaras commented 8 years ago

TokenRequestValidator logs every check failure as error - such as username is missing or username/password invalid, etc. I would not consider invalid input from user as error to be logged - from my perspective it could be max as a warning (ideally I would make that debug level logging as it's bad input problem which don't want to fill your logs unless you're debugging stuff). For example AuthenticationController uses warning level for when user authentication fails.

Why I'm concerned with that - we have to support legacy system with "old fashioned" login system and for this we have to use special client with ResourceOwner flow to proxy calls with username/password so we have single place authenticating users. And this means when invalid user data is passed we get errors logged and we can not control this.

I know that logging in production is discouraged by your documentation but we use it as supplement to events provided and it works seamlessly with our monitoring system.

Would you accept making all those errors logged as warning level or lower or would you have other suggestions?

leastprivilege commented 8 years ago

Could you please send me the lines in code where you think logging should be changed?

dzintaras commented 8 years ago

https://github.com/IdentityServer/IdentityServer3/blob/master/source/Core/Validation/TokenRequestValidator.cs

Line 511: LogError("User authentication failed: " + error);

leastprivilege commented 8 years ago

OK - we'll look into it for the next release.