INCR-40 use ipaddress instead of ipaddr

This commit is contained in:
singuliere
2019-02-07 12:10:34 +01:00
parent 33d16d8e24
commit d00cb9bda8
9 changed files with 39 additions and 42 deletions

View File

@@ -2,7 +2,7 @@
Defines forms for providing validation of embargo admin details.
"""
import ipaddr
import ipaddress
from django import forms
from django.utils.translation import ugettext as _
from opaque_keys import InvalidKeyError
@@ -66,7 +66,7 @@ class IPFilterForm(forms.ModelForm):
"""Whether or not address is a valid ipv4 address or ipv6 address"""
try:
# Is this an valid ip address?
ipaddr.IPNetwork(address)
ipaddress.ip_network(address)
except ValueError:
return False
return True

View File

@@ -14,7 +14,7 @@ file and check it in at the same time as your model changes. To do that,
import json
import logging
import ipaddr
import ipaddress
from config_models.models import ConfigurationModel
from django.core.cache import cache
from django.urls import reverse
@@ -684,7 +684,7 @@ class IPFilter(ConfigurationModel):
"""
def __init__(self, ips):
self.networks = [ipaddr.IPNetwork(ip) for ip in ips]
self.networks = [ipaddress.ip_network(ip) for ip in ips]
def __iter__(self):
for network in self.networks:
@@ -692,12 +692,12 @@ class IPFilter(ConfigurationModel):
def __contains__(self, ip_addr):
try:
ip_addr = ipaddr.IPAddress(ip_addr)
ip_addr = ipaddress.ip_address(ip_addr)
except ValueError:
return False
for network in self.networks:
if network.Contains(ip_addr):
if ip_addr in network:
return True
return False

View File

@@ -68,31 +68,31 @@ class IPFilterFormTest(TestCase):
# should be able to do both ipv4 and ipv6
# spacing should not matter
form_data = {
'whitelist': '127.0.0.1, 2003:dead:beef:4dad:23:46:bb:101, 1.1.0.1/32, 1.0.0.0/24',
'blacklist': ' 18.244.1.5 , 2002:c0a8:101::42, 18.36.22.1, 1.0.0.0/16'
'whitelist': u'127.0.0.1, 2003:dead:beef:4dad:23:46:bb:101, 1.1.0.1/32, 1.0.0.0/24',
'blacklist': u' 18.244.1.5 , 2002:c0a8:101::42, 18.36.22.1, 1.0.0.0/16'
}
form = IPFilterForm(data=form_data)
self.assertTrue(form.is_valid())
form.save()
whitelist = IPFilter.current().whitelist_ips
blacklist = IPFilter.current().blacklist_ips
for addr in '127.0.0.1, 2003:dead:beef:4dad:23:46:bb:101'.split(','):
for addr in u'127.0.0.1, 2003:dead:beef:4dad:23:46:bb:101'.split(','):
self.assertIn(addr.strip(), whitelist)
for addr in '18.244.1.5, 2002:c0a8:101::42, 18.36.22.1'.split(','):
for addr in u'18.244.1.5, 2002:c0a8:101::42, 18.36.22.1'.split(','):
self.assertIn(addr.strip(), blacklist)
# Network tests
# ips not in whitelist network
for addr in ['1.1.0.2', '1.0.1.0']:
for addr in [u'1.1.0.2', u'1.0.1.0']:
self.assertNotIn(addr.strip(), whitelist)
# ips in whitelist network
for addr in ['1.1.0.1', '1.0.0.100']:
for addr in [u'1.1.0.1', u'1.0.0.100']:
self.assertIn(addr.strip(), whitelist)
# ips not in blacklist network
for addr in ['2.0.0.0', '1.1.0.0']:
for addr in [u'2.0.0.0', u'1.1.0.0']:
self.assertNotIn(addr.strip(), blacklist)
# ips in blacklist network
for addr in ['1.0.100.0', '1.0.0.10']:
for addr in [u'1.0.100.0', u'1.0.0.10']:
self.assertIn(addr.strip(), blacklist)
# Test clearing by adding an empty list is OK too
@@ -109,8 +109,8 @@ class IPFilterFormTest(TestCase):
def test_add_invalid_ips(self):
# test adding invalid ip addresses
form_data = {
'whitelist': '.0.0.1, :dead:beef:::, 1.0.0.0/55',
'blacklist': ' 18.244.* , 999999:c0a8:101::42, 1.0.0.0/'
'whitelist': u'.0.0.1, :dead:beef:::, 1.0.0.0/55',
'blacklist': u' 18.244.* , 999999:c0a8:101::42, 1.0.0.0/'
}
form = IPFilterForm(data=form_data)
self.assertFalse(form.is_valid())

View File

@@ -82,14 +82,14 @@ class EmbargoMiddlewareAccessTests(UrlResetMixin, ModuleStoreTestCase):
@patch.dict(settings.FEATURES, {'EMBARGO': True})
@ddt.data(
# request_ip, blacklist, whitelist, is_enabled, allow_access
('173.194.123.35', ['173.194.123.35'], [], True, False),
('173.194.123.35', ['173.194.0.0/16'], [], True, False),
('173.194.123.35', ['127.0.0.0/32', '173.194.0.0/16'], [], True, False),
('173.195.10.20', ['173.194.0.0/16'], [], True, True),
('173.194.123.35', ['173.194.0.0/16'], ['173.194.0.0/16'], True, False),
('173.194.123.35', [], ['173.194.0.0/16'], True, True),
('192.178.2.3', [], ['173.194.0.0/16'], True, True),
('173.194.123.35', ['173.194.123.35'], [], False, True),
(u'173.194.123.35', ['173.194.123.35'], [], True, False),
(u'173.194.123.35', ['173.194.0.0/16'], [], True, False),
(u'173.194.123.35', ['127.0.0.0/32', '173.194.0.0/16'], [], True, False),
(u'173.195.10.20', ['173.194.0.0/16'], [], True, True),
(u'173.194.123.35', ['173.194.0.0/16'], ['173.194.0.0/16'], True, False),
(u'173.194.123.35', [], ['173.194.0.0/16'], True, True),
(u'192.178.2.3', [], ['173.194.0.0/16'], True, True),
(u'173.194.123.35', ['173.194.123.35'], [], False, True),
)
@ddt.unpack
def test_ip_access_rules(self, request_ip, blacklist, whitelist, is_enabled, allow_access):
@@ -155,7 +155,7 @@ class EmbargoMiddlewareAccessTests(UrlResetMixin, ModuleStoreTestCase):
def test_whitelist_ip_skips_country_access_checks(self):
# Whitelist an IP address
IPFilter.objects.create(
whitelist="192.168.10.20",
whitelist=u"192.168.10.20",
enabled=True
)
@@ -165,8 +165,8 @@ class EmbargoMiddlewareAccessTests(UrlResetMixin, ModuleStoreTestCase):
# Make a request from the whitelisted IP address
response = self.client.get(
self.courseware_url,
HTTP_X_FORWARDED_FOR="192.168.10.20",
REMOTE_ADDR="192.168.10.20"
HTTP_X_FORWARDED_FOR=u"192.168.10.20",
REMOTE_ADDR=u"192.168.10.20"
)
# Expect that we were still able to access the page,

View File

@@ -74,8 +74,8 @@ class EmbargoModelsTest(CacheIsolationTestCase):
self.assertIn(state, currently_blocked)
def test_ip_blocking(self):
whitelist = '127.0.0.1'
blacklist = '18.244.51.3'
whitelist = u'127.0.0.1'
blacklist = u'18.244.51.3'
cwhitelist = IPFilter.current().whitelist_ips
self.assertNotIn(whitelist, cwhitelist)
@@ -90,20 +90,20 @@ class EmbargoModelsTest(CacheIsolationTestCase):
self.assertIn(blacklist, cblacklist)
def test_ip_network_blocking(self):
whitelist = '1.0.0.0/24'
blacklist = '1.1.0.0/16'
whitelist = u'1.0.0.0/24'
blacklist = u'1.1.0.0/16'
IPFilter(whitelist=whitelist, blacklist=blacklist).save()
cwhitelist = IPFilter.current().whitelist_ips
self.assertIn('1.0.0.100', cwhitelist)
self.assertIn('1.0.0.10', cwhitelist)
self.assertNotIn('1.0.1.0', cwhitelist)
self.assertIn(u'1.0.0.100', cwhitelist)
self.assertIn(u'1.0.0.10', cwhitelist)
self.assertNotIn(u'1.0.1.0', cwhitelist)
cblacklist = IPFilter.current().blacklist_ips
self.assertIn('1.1.0.0', cblacklist)
self.assertIn('1.1.0.1', cblacklist)
self.assertIn('1.1.1.0', cblacklist)
self.assertNotIn('1.2.0.0', cblacklist)
self.assertIn(u'1.1.0.0', cblacklist)
self.assertIn(u'1.1.0.1', cblacklist)
self.assertIn(u'1.1.1.0', cblacklist)
self.assertNotIn(u'1.2.0.0', cblacklist)
class RestrictedCourseTest(CacheIsolationTestCase):

View File

@@ -96,7 +96,7 @@ glob2 # Enhanced glob module, used in openedx.core
gunicorn==19.0
help-tokens
html5lib # HTML parser, used for capa problems
ipaddr==2.1.11 # Ip network support for Embargo feature
ipaddress # Ip network support for Embargo feature
jsonfield # Django model field for validated JSON; used in several apps
mailsnake # Needed for mailchimp (mailing djangoapp)
mako==1.0.2 # Primary template language used for server-side page rendering

View File

@@ -144,7 +144,6 @@ help-tokens==1.0.3
html5lib==1.0.1
httplib2==0.12.0 # via oauth2, zendesk
idna==2.8
ipaddr==2.1.11
ipaddress==1.0.22
isodate==0.6.0 # via python3-saml
itypes==1.1.0 # via coreapi

View File

@@ -184,7 +184,6 @@ idna==2.8
imagesize==1.1.0 # via sphinx
incremental==17.5.0
inflect==2.1.0
ipaddr==2.1.11
ipaddress==1.0.22
isodate==0.6.0
isort==4.3.4

View File

@@ -177,7 +177,6 @@ httpretty==0.9.6
idna==2.8
incremental==17.5.0 # via twisted
inflect==2.1.0
ipaddr==2.1.11
ipaddress==1.0.22
isodate==0.6.0
isort==4.3.4