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

Heroku fix #1

Closed
wants to merge 1 commit into from
Closed

Heroku fix #1

wants to merge 1 commit into from

Conversation

petedermott
Copy link

Changes as per PR #54 that should fix iprestrict errors when using Heroku

@sztamas
Copy link
Owner

sztamas commented Aug 27, 2020

Hi @petedermott ,

Sorry, but I wasn't getting notification on the repository for some reason (although it is my repository, so I thought that would be the default). and didn't notice you created a PR.

I've seen this problem reported on the muccg repository, but I didn't move this fix over on purpose as I'm not sure it is the right solution.

I don't like that it just ignores "unknown" proxies as I like to keep the default configuration as secure as possible (but give the possibility to opt out of that explicitly).

We will probably need a separate setting by which the users will confirm they are fine with accepting an IP that was reported through an unknown proxy.

Also, while looking at the RFC, I've noticed that beside the "unknown" value X_FORWARDED_FOR could also have IPs with ports (ex. 1.1.1.1:8080), and IPv6 adresses could be quoted.

Given all that, the parsing of the header should be improved to handle all those cases.

I hope that explains why I didn't port that fix over yet.

Are you experiencing that Heroku problem as well? If so is it something that happens just ocassionaly or all the time?

Thanks,

Tamas

@petedermott
Copy link
Author

Hi Tamas,

No worries with the delay, I'm just glad this is active again 👍

I think a config setting might be the best solution, that way Heroku users who will run into this can turn the "ignore" functionality on but its off for everyone else by default.

Our Heroku apps aren't massive in terms of scale but we get at least a couple of errors every day regarding this issue.

I'll look at updating this PR next week with the possible changes.

sztamas added a commit that referenced this pull request Aug 30, 2020
Fixes muccg/django-iprestrict#53
References:
 * muccg/djang-iprestrict#54
 * #1
@sztamas
Copy link
Owner

sztamas commented Aug 30, 2020

Fixed in https://github.com/sztamas/django-iprestrict-redux/releases/tag/1.9.0

You probably want to set IPRESTRICT_USE_PROXY_IF_UNKNOWN to True in settings.py.

@sztamas sztamas closed this Aug 30, 2020
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

Successfully merging this pull request may close these issues.

2 participants