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

Compatible with Django 3? #82

Open
nh916 opened this issue Jun 1, 2020 · 15 comments
Open

Compatible with Django 3? #82

nh916 opened this issue Jun 1, 2020 · 15 comments

Comments

@nh916
Copy link

nh916 commented Jun 1, 2020

is this compatible with django 3.x?
also side question, would this play well with django allauth?

@mgrouchy
Copy link
Owner

It should be compatible with Django 3, but I haven't wrote tests for that. I will add it to the todo list.

@nh916
Copy link
Author

nh916 commented Jul 17, 2020

for sure let me know! :)

@thoward27
Copy link

I'm on Django 3.1, trying to use Stronghold, it unfortunately appears to have no effect on access to my site. Do you have any tips for debugging @nh916?

@nh916
Copy link
Author

nh916 commented Sep 4, 2020

@thoward27 I have not used it yet. I was waiting to get a reply back that it works before i start trying it. if you do figure it out thought please feel free to post for anyone who might run into that problem in the future!

@thoward27
Copy link

@mgrouchy do you have any tips for figuring out why views may not be secured?

@drholera
Copy link

Hello.
I've tested it with the latest Django v3.1 and it works ok for me. By default, it redirects me to /accounts/login URL.

@thoward27
Copy link

I can say that I upgrade my application to django 3.1, and stronghold stopped working, downgrading django solved my problem. @drholera did you start from scratch? Or were you upgrading to 3.1?

@drholera
Copy link

Yes, I started a new project from scratch.

@christopherpickering
Copy link

It is not working for me in django 3.1, I am using a custom backend and user model.

@wesleykendall
Copy link

wesleykendall commented Apr 1, 2021

@thoward27 @mgrouchy @christopherpickering I was having issues with Django 3.1 and think I found an issue. Django may have changed the default value for settings.MEDIA_URL. It defaults to / for me even though I don't have it set in my settings.py. I'm assuming Django used to not set this setting based on the stronghold code.

It appears that stronghold ignores the media URL (and everything under it) by default if it is set, causing none of my URLs to be protected after the upgrade.

I haven't investigated this myself to verify older behavior of Django, but this could end up being a pretty substantial security issue for people that upgrade to Django 3.1 without a MEDIA_URL and inadvertently have all of their URLs exposed by default (edit - just checked and this is only enabled in debug mode, so likely not a security issue)

This is mostly up to @mgrouchy in regards to design, but I would disallow URLs of / for static/media if those are now the defaults for Django. For reference, here is the line of code - https://github.com/mgrouchy/django-stronghold/blob/master/stronghold/conf.py#L55

@wesleykendall
Copy link

@mgrouchy I just downgraded to Django 3.0.3 and MEDIA_URL was the empty string (as opposed to / in 3.1). I verified that the downgrade fixed my issue.

Oddly enough, when I set my MEDIA_URL to the empty string in 3.1, I still get the same behavior. However, setting it to None results in the expected behavior.

@christopherpickering @thoward27 check if you have a MEDIA_URL in your settings. If not, set it to None for now and I believe it will fix your issue

@wesleykendall
Copy link

Oops, one more side note. I may have jumped the gun on labeling this a possible security issue. It seems like the media URLs are only included as public URLs as a convenience in debug mode. This may likely not have an impact on production code. I have only ran my experiments in debug mode

@JulianVolodia
Copy link

Oops, one more side note. I may have jumped the gun on labeling this a possible security issue. ... in debug mode.

But Debug mode is not secure and any issues found during that are not the sec issue itself if You cant reproduce w/o it.

@JulianVolodia
Copy link

@wesleykendall there is working for 3.1 and 3.2 fork of stronghold - django-require-login https://pypi.org/project/django-require-login/

@sphuber
Copy link

sphuber commented Oct 4, 2021

I confirm that on Django v3.2.7 not having MEDIA_URL defined in the settings, leads to all URLs being public. The reason seems to be that the regex r'^.*$' ends up in the public_view_urls of the LoginRequiredMiddleware mixin, which of course matches all URLs. The bug is most likely that the non-defined value is not being caught as a special case and the default media URL is added but interpolated from None, leading to the regex matching everything.

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

No branches or pull requests

8 participants