dougdellolio / coinbasepro-csharp

The unofficial .NET/C# client library for the Coinbase Pro/GDAX API
MIT License
193 stars 90 forks source link

How to disable exception throw output messages from your API #298

Closed prozerran closed 2 years ago

prozerran commented 2 years ago

When calling your API, certain methods will throw an exception in the underlying HttpMethod calls. And with the exception, it will dump the HTTP request to the log, such as......

2022-01-04 16:25:12.433 +08:00 [ERR] REST request about to throw {"StatusCode":"NotFound","EndPoint":{"HttpMethod":{"Method":"DELETE","$type":"HttpMethod"},"Uri":"/orders/095a94ab-f9c9-4229-a663-78280c42054b","Content":null,"$type":"EndPoint"},"RequestMessage":{"Version":{"Major":1,"Minor":1,"Build":-1,"Revision":-1,"MajorRevision":-1,"MinorRevision":-1,"$type":"Version"},"Content":null,"Method":{"Method":"DELETE","$type":"HttpMethod"},"RequestUri":"https://api-public.sandbox.pro.coinbase.com/orders/095a94ab-f9c9-4229-a663-78280c42054b","Headers":[{"Key":"User-Agent","Value":["CoinbaseProClient"],"$type":"KeyValuePair`2"},{"Key":"CB-ACCESS-KEY","Value":["XXXXXX__MASK__XXXXXXXXX"],"$type":"KeyValuePair`2"},{"Key":"CB-ACCESS-TIMESTAMP","Value":["1641284712"],"$type":"KeyValuePair`2"},{"Key":"CB-ACCESS-SIGN","Value":["XXXXXX__MASK__XXXXXXXXX/aY="],"$type":"KeyValuePair`2"},{"Key":"CB-ACCESS-PASSPHRASE","Value":["XXXXXX__MASK__XXXXXXXXX"],"$type":"KeyValuePair`2"}],"Properties":{},"$type":"HttpRequestMessage"},"ResponseMessage":{"Version":{"Major":1,"Minor":1,"Build":-1,"Revision":-1,"MajorRevision":-1,"MinorRevision":-1,"$type":"Version"},"Content":{"Headers":[{"Key":"Content-Length","Value":["29"],"$type":"KeyValuePair`2"},{"Key":"Content-Type","Value":["application/json; charset=utf-8"],"$type":"KeyValuePair2"}],"$type":"StreamContent"},"StatusCode":"NotFound","ReasonPhrase":"Not Found","Headers":[{"Key":"Connection","Value":["keep-alive"],"$type":"KeyValuePair2"},{"Key":"Access-Control-Allow-Headers","Value":["Content-Type, Accept, cb-session, cb-fp, cb-form-factor"],"$type":"KeyValuePair2"},{"Key":"Access-Control-Allow-Methods","Value":["GET,POST,DELETE,PUT"],"$type":"KeyValuePair2"},{"Key":"Access-Control-Allow-Origin","Value":["*"],"$type":"KeyValuePair`2"},{"Key":"Access-Control-Expose-Headers","Value":["cb-before, cb-after, .................

The problem is, this is a MAJOR security flaw when it dumps the entire HTTP request. That includes information on CB-ACCESS-KEY, CB-ACCESS-SIGN, and CB-ACCESS-PASSPHRASE. I have masked my info here for security reasons, but the actual log CONTAINS these data.

We have several users/developers that may get access to this log file and we do NOT want them to get access to such information.

How do I disable all logs dumped by your API lib? We cannot allow such information to get in the hands of anyone but the master admin. It could be a potential financial disaster if such information were to get into the wrong hands.

Edit: I also use Serilog and have my own logging as well. So there needs to be a way to disable your Error logs while keeping my Errors logs working as is.

dougdellolio commented 2 years ago

hi @prozerran I see how this can be an issue.

While not an ideal solution, in the short term you can filter out the error log events. Here's an example of filtering by level. More examples here: https://stackoverflow.com/questions/52084651/how-to-exclude-specific-exception-types-from-serilog

Serilog.Log.Logger = new LoggerConfiguration()
                   .MinimumLevel.Debug()
                   .WriteTo.Console()
                   .Filter
                   .ByExcluding(logEvent => logEvent.Level == Serilog.Events.LogEventLevel.Error)
                   .CreateLogger();

I am also happy to accept and merge any PRs that would exclude these sensitive values from the response

prozerran commented 2 years ago

Alright, i guess i have no choice but to disable all Error logging including mines. But i suggest a fix be provided that only disable "your" logs, while keeping "my" logs in tack. So for now, I will chance my Error logs to Debug logs but that's really a bad solution to fix this problem. I hope a proper fix can be implemented.

prozerran commented 2 years ago

Suggested fix would be write a method that can be called with your api to accept the acceptable log levels to display...such as, UseLogLevel( LogEventLevel.Debug | LogEventLevel.Verbose | LogEventLevel.Information );

etc..... user can then fill or exclude which logs they want.

dougdellolio commented 2 years ago

feel free to open up a pull request with these changes. I would be happy to merge them in