Skip to content
This repository has been archived by the owner on Apr 11, 2024. It is now read-only.

BAYA_ALLOW_ALL = True allows unauthenticated users #4

Open
sbuss opened this issue Feb 5, 2016 · 2 comments
Open

BAYA_ALLOW_ALL = True allows unauthenticated users #4

sbuss opened this issue Feb 5, 2016 · 2 comments

Comments

@sbuss
Copy link

sbuss commented Feb 5, 2016

(Copied from internal github, original issue by rxia)

@sbuss

I'm not sure if this was intentional, but if you set BAYA_ALLOW_ALL = True, then views decorated with @requires() will allow unauthenticated users to log in. Do you think it's reasonable to change this so that it still forces you to be logged in as someone, but not do any actual checking on the groups?

I know that the use of BAYA_ALLOW_ALL = True is discouraged, but I was making changes in an app I'm not familiar with, and I didn't want to figure out the exact permission required. One of the pages I visited was raising an exception because it assumed that request.user was a valid, authenticated user, which is normally safe to make because of the @requires() decorator. However, because I had BAYA_ALLOW_ALL = True, the decorator actually let an unauthenticated user through.

I think the only thing required to make this change is to move this block of code over to this line. Alternatively, we could also redefine _has_permission() to mean "is logged in and has permission" and short-circuit before the group check if the user is not logged in. I'd be more than happy to submit a PR if you agree with this change.

@sbuss
Copy link
Author

sbuss commented Feb 5, 2016

(response copied from internal github)

Yes, that was intentional. See the README:

# settings.py
BAYA_ALLOW_ALL = True

This will allow all requests to your protected views and is useful if you're just testing that your view works, but don't currently care about the access restrictions.

Your use case certainly isn't unique and this has bitten me before. I think adding a check for BAYA_ALLOW_ALL in _has_permission is a suitable fix.

@sbuss
Copy link
Author

sbuss commented Feb 5, 2016

This needs a bit more thought. This behavior has existed for a while now, so I'm not sure if changing it is correct. I'm interested to see what the community thinks.

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

No branches or pull requests

1 participant