Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

Remove WARN log lines for authentication failures #34

Open
hjacobs opened this issue Oct 28, 2015 · 4 comments
Open

Remove WARN log lines for authentication failures #34

hjacobs opened this issue Oct 28, 2015 · 4 comments

Comments

@hjacobs
Copy link
Contributor

hjacobs commented Oct 28, 2015

We should not spam the log with WARNINGS about client's missing permissions. I propose changing these log lines to "INFO" and delete some places (e.g. if no auth was given).

@hjacobs
Copy link
Contributor Author

hjacobs commented Nov 3, 2015

This needs to be changed in Swagger1st too, e.g. here: https://github.com/sarnowski/swagger1st/blob/master/src/io/sarnowski/swagger1st/util/security.clj#L14

@sarnowski
Copy link
Contributor

I am not sure why we want to alter WARN to INFO here? According to our internal Logging guidelines, we have to log it either with WARNING (an error occured but we could handle it somehow) or not log it at all which could be a problem security wise (to detect anomalies).

@hjacobs
Copy link
Contributor Author

hjacobs commented Nov 3, 2015

I think it's discussible whether an "error occured" --- all spiders/crawlers around the world will attempt requests without authorization, i.e. it's no error, but usually behavior.

Logging as INFO still makes anomaly detection possible (also number of successful logins can be tracked).

I would strongly advise to remove all WARNings altogether, see Schlomo's presentation: https://twitter.com/s0enke/status/658627097162874880

WARNings suggest that something needs to be done, but in case of client errors there is nothing to be done.

@LappleApple
Copy link

Hey @dryewo, what are your plans here? Should we consider adding a "Help Wanted" label?

@Otann Otann added this to the 2.0 Release milestone Jun 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants