From 9bef3e251f6429ea20f3e6f824c1dd4124ffbb78 Mon Sep 17 00:00:00 2001 From: Josue Kouka Date: Thu, 7 Sep 2017 10:02:17 +0200 Subject: [PATCH 2/2] add redirects uris field checking (#14147) --- fargo/oauth2/admin.py | 5 ++-- fargo/oauth2/migrations/0001_initial.py | 10 ++++++- fargo/oauth2/migrations/0002_oauth2tempfile.py | 23 --------------- fargo/oauth2/models.py | 28 ++++++++++++++---- fargo/oauth2/views.py | 5 ++-- tests/test_oauth2.py | 40 ++++++++++++++++++++------ 6 files changed, 70 insertions(+), 41 deletions(-) delete mode 100644 fargo/oauth2/migrations/0002_oauth2tempfile.py diff --git a/fargo/oauth2/admin.py b/fargo/oauth2/admin.py index 782ba4f..91cf12a 100644 --- a/fargo/oauth2/admin.py +++ b/fargo/oauth2/admin.py @@ -20,8 +20,9 @@ from .models import OAuth2Client class OAuth2ClientAdmin(admin.ModelAdmin): - fields = ('client_name', 'client_id', 'client_secret') - list_display = ['client_name', 'client_id', 'client_secret'] + fields = ('client_name', 'client_id', 'client_secret', 'redirect_uris') + list_display = ['client_name', 'client_id', 'client_secret', 'redirect_uris'] readonly_fields = ['client_id', 'client_secret'] + admin.site.register(OAuth2Client, OAuth2ClientAdmin) diff --git a/fargo/oauth2/migrations/0001_initial.py b/fargo/oauth2/migrations/0001_initial.py index 2b0e815..d6e5f92 100644 --- a/fargo/oauth2/migrations/0001_initial.py +++ b/fargo/oauth2/migrations/0001_initial.py @@ -29,7 +29,15 @@ class Migration(migrations.Migration): ('client_secret', models.CharField(default=fargo.oauth2.models.generate_uuid, max_length=255)), ('client_id', models.CharField(default=fargo.oauth2.models.generate_uuid, max_length=255)), ('client_name', models.CharField(max_length=255)), - ('redirect_uri', models.TextField(default=b'')), + ('redirect_uris', models.TextField(verbose_name='redirect URIs', validators=[fargo.oauth2.models.validate_https_url])), + ], + ), + migrations.CreateModel( + name='OAuth2TempFile', + fields=[ + ('hash_key', models.CharField(max_length=128, serialize=False, primary_key=True)), + ('filename', models.CharField(max_length=512)), + ('document', models.ForeignKey(to='fargo.Document')), ], ), ] diff --git a/fargo/oauth2/migrations/0002_oauth2tempfile.py b/fargo/oauth2/migrations/0002_oauth2tempfile.py deleted file mode 100644 index 9500186..0000000 --- a/fargo/oauth2/migrations/0002_oauth2tempfile.py +++ /dev/null @@ -1,23 +0,0 @@ -# -*- coding: utf-8 -*- -from __future__ import unicode_literals - -from django.db import models, migrations - - -class Migration(migrations.Migration): - - dependencies = [ - ('fargo', '0013_document_mime_type'), - ('oauth2', '0001_initial'), - ] - - operations = [ - migrations.CreateModel( - name='OAuth2TempFile', - fields=[ - ('hash_key', models.CharField(max_length=128, serialize=False, primary_key=True)), - ('filename', models.CharField(max_length=512)), - ('document', models.ForeignKey(to='fargo.Document')), - ], - ), - ] diff --git a/fargo/oauth2/models.py b/fargo/oauth2/models.py index c624072..fbf974e 100644 --- a/fargo/oauth2/models.py +++ b/fargo/oauth2/models.py @@ -16,7 +16,10 @@ import uuid +from django.core.validators import URLValidator +from django.core.exceptions import ValidationError from django.db import models +from django.utils.translation import ugettext_lazy as _ from fargo.fargo.models import Document, UserDocument @@ -25,6 +28,20 @@ def generate_uuid(): return unicode(uuid.uuid4()) +def validate_https_url(data): + errors = [] + data = data.strip() + if not data: + return + for url in data.split(): + try: + URLValidator(schemes=['http', 'https'])(url) + except ValidationError as e: + errors.append(e) + if errors: + raise ValidationError(errors) + + class OAuth2Authorize(models.Model): user_document = models.ForeignKey(UserDocument) access_token = models.CharField(max_length=255, default=generate_uuid) @@ -39,17 +56,18 @@ class OAuth2Client(models.Model): client_secret = models.CharField(max_length=255, default=generate_uuid) client_id = models.CharField(max_length=255, default=generate_uuid) client_name = models.CharField(max_length=255) - redirect_uri = models.TextField(default='') + redirect_uris = models.TextField( + verbose_name=_('redirect URIs'), + validators=[validate_https_url]) def __repr__(self): return 'OAuth2Client name: %s with id: %s' % (self.client_name, self.client_id) def get_redirect_uris(self): - return self.redirect_uri.split() + return self.redirect_uris.split() - def add_redirect_uri(self, redirect_uri): - self.redirect_uri += ' %s' % redirect_uri - self.save() + def check_redirect_uri(self, redirect_uri): + return redirect_uri in self.redirect_uris.strip().split() class OAuth2TempFile(models.Model): diff --git a/fargo/oauth2/views.py b/fargo/oauth2/views.py index 175791d..3949313 100644 --- a/fargo/oauth2/views.py +++ b/fargo/oauth2/views.py @@ -54,9 +54,10 @@ class OAuth2AuthorizeView(FormView): uri = self.error_redirect(redirect_uri, 'unsupported_response_type') return HttpResponseRedirect(uri) try: - client = OAuth2Client.objects.get(client_id=client_id) - client.add_redirect_uri(redirect_uri) + if not client.check_redirect_uri(redirect_uri): + uri = self.error_redirect(redirect_uri, 'invalid_redirect_uri') + return HttpResponseRedirect(uri) except OAuth2Client.DoesNotExist: uri = self.error_redirect(redirect_uri, 'unauthorized_client') return HttpResponseRedirect(uri) diff --git a/tests/test_oauth2.py b/tests/test_oauth2.py index 1051afc..0705218 100644 --- a/tests/test_oauth2.py +++ b/tests/test_oauth2.py @@ -1,7 +1,6 @@ import pytest -import os -import base64 from urllib import quote +import urlparse from django.core.files.base import ContentFile from django.core.urlresolvers import reverse @@ -14,9 +13,13 @@ from test_manager import login pytestmark = pytest.mark.django_db + @pytest.fixture def oauth2_client(): - return OAuth2Client.objects.create(client_name='test_oauth2') + return OAuth2Client.objects.create( + client_name='test_oauth2', client_id='client-id', client_secret='client-secret', + redirect_uris='https://example.net/document https://doc.example.net/ https://example.com') + @pytest.fixture def document(): @@ -25,23 +28,43 @@ def document(): return Document.objects.get_by_file(content) + @pytest.fixture def user_doc(document, john_doe): return UserDocument.objects.create(user=john_doe, document=document, filename='Baudelaire.txt') + +def assert_error_redirect(url, error): + assert urlparse.urlparse(url).query == 'error=%s' % error + + def test_get_document_oauth2(app, john_doe, oauth2_client, user_doc): login(app, user=john_doe) url = reverse('oauth2-authorize') params = { - 'client_id': oauth2_client.client_id, 'client_secret': oauth2_client.client_secret, - 'redirect_uri': 'https://example.com', 'response_type': 'code', 'state': 'achipeachope' } - - url += '?%s' % urlencode(params) - resp = app.get(url) + # test missing redirect_uri + resp = app.get(url, params={}, status=400) + assert resp.content == 'missing parameter redirect_uri' + # test missing client id + params['redirect_uri'] = 'https://toto.example.com' + resp = app.get(url, params=params, status=302) + assert_error_redirect(resp.url, 'invalid_request') + # test invalid response type + params['client_id'] = oauth2_client.client_id + params['response_type'] = 'token' + resp = app.get(url, params=params, status=302) + assert_error_redirect(resp.url, 'unsupported_response_type') + # test invalid redirect uri + params['response_type'] = 'code' + resp = app.get(url, params=params, status=302) + assert_error_redirect(resp.url, 'invalid_redirect_uri') + + params['redirect_uri'] = 'https://example.com' + resp = app.get(url, params=params) assert resp.status_code == 200 assert len(resp.forms[0]['document'].options) == 2 @@ -123,6 +146,7 @@ def test_put_document(app, john_doe, oauth2_client): assert user_document.filename == 'Baudelaire.txt' + def test_confirm_put_document_file_exception(app, john_doe, user_doc): login(app, user=john_doe) oauth_tmp_file = OAuth2TempFile.objects.create(document=user_doc.document, filename=user_doc.filename) -- 2.11.0