Implement CMS_INTERNAL_IPS #5173
Conversation
| @@ -47,13 +47,34 @@ def toolbar_plugin_processor(instance, placeholder, rendered_content, original_c | |||
| return output | |||
|
|
|||
|
|
|||
| def get_client_ip(request): | |||
There was a problem hiding this comment.
I suggest to implement something like https://github.com/Gidsy/django-geoip-utils/blob/master/geoip_utils/utils.py because there could be other headers that point to the IP and so is best to give developers the option.
There was a problem hiding this comment.
Perhaps, but please bear in mind that Django itself only checks the REMOTE_ADDR (https://github.com/django/django/blob/ecb59cc6579402b68ddfd4499bf30edacf5963be/django/contrib/admindocs/middleware.py#L20-L21).
Considering what this is used for, does it make sense to implement something as complex in CMS whereas Django itself uses only the simple check?
There was a problem hiding this comment.
I agree. Let's be consistent with upstream: HTTP_X_FORWARDED_FOR and REMOTE_ADDR are the standard ones, and you have to comply with those for a lot of different reasons anyway. Maybe adding the header used to check those can help clarify any issue users might have in not standard setups. In all cases it's just a matter of adding a configuration variable to the webserver to use the correct header
|
Ok to merge |
| @@ -14,6 +14,9 @@ | |||
| * Removed frontend integration tests (written with Selenium) | |||
| * Provides the ability to declare cache expiration periods on a per-plugin basis | |||
| * Minor UI improvements | |||
| * Adds new setting CMS_INTERNAL_IPS for defining a set of IP address for which | |||
| the toolbar will appear for authorized users. If left unset, does nothing. | |||
|
|
|||
There was a problem hiding this comment.
Shouldn't "If left unset, does nothing" be better explained as "If left unset, every IP address is authorized"?
|
Ok to merge |
No description provided.