Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Access control failures must be logged at WARN level #1074

Closed
leplatrem opened this issue Feb 14, 2017 · 4 comments
Closed

Access control failures must be logged at WARN level #1074

leplatrem opened this issue Feb 14, 2017 · 4 comments

Comments

@leplatrem
Copy link
Contributor

Ref mozilla-services/kinto-dist#108

@sk0g
Copy link

sk0g commented Feb 18, 2017

I'd like to solve this issue, if I could. I checked the logs.py, and from what I saw, the access control failures aren't tested for right now. Am I off track already?

@leplatrem
Copy link
Contributor Author

Thanks for looking at this!

Don't worry, you're not off the track. logs.py is the place where the logger are instantiated. They are then use throughout the application.

In this issue, we'd like to log access control failures with warn. It means we want to log something when:

  • a user authentication is not valid
  • a user tries to access/modify an object without permissions.

The first part is more complex and can be done in a second step/ticket, but the second happens here in the authorization code:
https://github.com/Kinto/kinto/blob/5.3.5/kinto/core/authorization.py#L53
Basically we'd like to log warn something when the permits method returns False

Let's know! Don't hesitate to submit a draft ;)

Thanks again!

@sk0g
Copy link

sk0g commented Feb 20, 2017

So you're finding the permission type in 50+, but at 53 it's testing whether the user is allowed to access/ change the file, or whether further testing has to be done (in case the permission is dynamic, and the permission will be checked while running.) Then some of the fail cases show up at 77, 85.
Is there an actual permits method, or did you mean permission?

Would a block of code saying if not allowed or not allowed_to_create, log something work there? Obviously, after determining the need to actually log something, we could again check to see which one to log, and then add on log statements in logs.py to facilitate logging.

@leplatrem
Copy link
Contributor Author

Is there an actual permits method, or did you mean permission?

I sent you a link that points to it ;)

add on log statements in logs.py to facilitate logging

I don't follow you. Wouldn't something like that be sufficient ?

if not allowed:
    logger.warn("{userid} is not allowed to {permission} {object_id}".format(...))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants