entrouvert
/
authentic
16
0
Fork 0
Code Issues Pull Requests 33 Releases Activity

idp_oidc: check length of authorize's redirect_uri (#44589)

This commit is contained in:
Benjamin Dauvergne 2020-06-30 10:55:38 +02:00
parent 049a37b53c
commit 13c23e25b4
4 changed files with 51 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/authentic2_idp_oidc/app_settings.py
View File

@ -61,6 +61,10 @@ class AppSettings(object):
def PASSWORD_GRANT_RATELIMIT(self):
return self._setting('PASSWORD_GRANT_RATELIMIT', '100/m')
@property
def REDIRECT_URI_MAX_LENGTH(self):
return self._setting('REDIRECT_URI_MAX_LENGTH', 1024)
app_settings = AppSettings('A2_IDP_OIDC_')
app_settings.__name__ = __name__
sys.modules[__name__] = app_settings

11
src/authentic2_idp_oidc/models.py
View File

@ -32,7 +32,7 @@ from authentic2.a2_rbac.models import OrganizationalUnit
from authentic2.models import Service
from authentic2.utils import to_iter
from . import utils, managers
from . import utils, managers, app_settings
def generate_uuid():
@ -192,7 +192,10 @@ class OIDCClient(Service):
def get_wanted_attributes(self):
return self.oidcclaim_set.filter(name__isnull=False).values_list('value', flat=True)
def is_valid_redirect_uri(self, redirect_uri):
def validate_redirect_uri(self, redirect_uri):
if len(redirect_uri) > app_settings.REDIRECT_URI_MAX_LENGTH:
raise ValueError('redirect_uri length > %s' % app_settings.REDIRECT_URI_MAX_LENGTH)
parsed_uri = urlparse.urlparse(redirect_uri)
for valid_redirect_uri in self.redirect_uris.split():
parsed_valid_uri = urlparse.urlparse(valid_redirect_uri)
@ -214,8 +217,8 @@ class OIDCClient(Service):
else:
if parsed_uri.path.rstrip('/') != parsed_valid_uri.path.rstrip('/'):
continue
return True
return False
return
raise ValueError('redirect_uri is not declared')
def scope_set(self):
return utils.scope_set(self.scope)

10
src/authentic2_idp_oidc/views.py
View File

@ -141,10 +141,12 @@ def authorize(request, *args, **kwargs):
error_description='authz endpoint is configured '
'for resource owner password credential grant type')
if not client.is_valid_redirect_uri(redirect_uri):
messages.warning(request, _('Authorization request is invalid'))
logger.warning(u'idp_oidc: authorization request error, unknown redirect_uri redirect_uri=%r client_id=%r',
redirect_uri, client_id)
try:
client.validate_redirect_uri(redirect_uri)
except ValueError as e:
messages.warning(request, _('Authorization request is invalid: %s') % e)
logger.warning(u'idp_oidc: authorization request error, invalid redirect_uri %r (client_id=%r): %s',
redirect_uri, client_id, e)
return redirect(request, 'auth_homepage')
fragment = client.authorization_flow == client.FLOW_IMPLICIT

58
tests/test_idp_oidc.py
View File

@ -1236,36 +1236,46 @@ def test_claim_templated(oidc_settings, normal_oidc_client, simple_user, app):
assert user_info['family_name'].endswith(simple_user.last_name)
def test_client_is_valid_redirect_uri():
def test_client_validate_redirect_uri():
client = OIDCClient(redirect_uris='''http://example.com
http://example2.com/
http://example3.com/toto
http://*example4.com/
http://example5.com/toto*
''')
assert client.is_valid_redirect_uri('http://example.com')
assert client.is_valid_redirect_uri('http://example.com/')
assert not client.is_valid_redirect_uri('http://coin.example.com/')
assert not client.is_valid_redirect_uri('http://example.com/toto/')
assert not client.is_valid_redirect_uri('http://coin.example.com')
assert client.is_valid_redirect_uri('http://example2.com')
assert client.is_valid_redirect_uri('http://example2.com/')
assert not client.is_valid_redirect_uri('http://example3.com/')
assert not client.is_valid_redirect_uri('http://example3.com')
assert client.is_valid_redirect_uri('http://example3.com/toto')
assert client.is_valid_redirect_uri('http://example3.com/toto/')
assert client.is_valid_redirect_uri('http://example4.com/')
assert client.is_valid_redirect_uri('http://example4.com')
assert client.is_valid_redirect_uri('http://coin.example4.com')
assert client.is_valid_redirect_uri('http://coin.example4.com/')
assert not client.is_valid_redirect_uri('http://coinexample4.com')
assert not client.is_valid_redirect_uri('http://coinexample4.com/')
assert client.is_valid_redirect_uri('http://example5.com/toto')
assert client.is_valid_redirect_uri('http://example5.com/toto/')
assert client.is_valid_redirect_uri('http://example5.com/toto/tata')
assert client.is_valid_redirect_uri('http://example5.com/toto/tata/')
assert not client.is_valid_redirect_uri('http://example5.com/tototata/')
assert not client.is_valid_redirect_uri('http://example5.com/tototata')
# ok
for uri in [
'http://example.com',
'http://example.com/',
'http://example2.com',
'http://example2.com/',
'http://example3.com/toto',
'http://example3.com/toto/',
'http://example4.com/',
'http://example4.com',
'http://coin.example4.com',
'http://coin.example4.com/',
'http://example5.com/toto',
'http://example5.com/toto/',
'http://example5.com/toto/tata',
'http://example5.com/toto/tata/']:
client.validate_redirect_uri(uri)
# nok
for uri in [
'http://coin.example.com/',
'http://example.com/toto/',
'http://coin.example.com',
'http://example3.com/',
'http://example3.com',
'http://coinexample4.com',
'http://coinexample4.com/',
'http://example5.com/tototata/',
'http://example5.com/tototata']:
with pytest.raises(ValueError, match=r'is not declared'):
client.validate_redirect_uri(uri)
client.validate_redirect_uri('http://example5.com/toto/' + 'a' * 500)
with pytest.raises(ValueError, match=r'redirect_uri length >'):
client.validate_redirect_uri('http://example5.com/toto/' + 'a' * 1024)
def test_filter_api_users(app, oidc_client, admin, simple_user, role_random):