Skip to content
This repository has been archived by the owner on Mar 3, 2020. It is now read-only.

Commit

Permalink
Disable url validation in client creation
Browse files Browse the repository at this point in the history
In networks created by docker, the internal url for containers is often
of the form "http://myservice". For instance, the "notes" service might
reside at url "http://notes:8000". However, the client creation command
did not recognize "http://notes:8000" as a valid url. To reproduce, run:

    from django.core.validators import URLValidator
    URLValidator()('http://notes:8000')

This piece of code raises a django.core.exceptions.ValidationError. As a
consequence, the following command fails:

    ./manage.py lms create_oauth2_client "http://notes:8000" ...

An alternative was to use the external service url as the oauth2 client
url, but this is failing on local instances, as the domain name might
not be valid.

Another alternative was to add aliases for the service names, but that
made us jump through too many hoops, in particular for Kubernetes
deployment.

I'm not sure whether this is an upstream Django issue caused by
incorrect URLValidator implementation. Most recent version (v2.2) still
raises validation error. It's definitely more simple to solve this in
the present repo.
  • Loading branch information
regisb committed Aug 2, 2019
1 parent 33cff73 commit a056bdc
Show file tree
Hide file tree
Showing 3 changed files with 1 addition and 29 deletions.
2 changes: 1 addition & 1 deletion edx_oauth2_provider/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,4 @@
"""
from __future__ import unicode_literals

__version__ = '1.2.3'
__version__ = '1.3.0'
17 changes: 0 additions & 17 deletions edx_oauth2_provider/management/commands/create_oauth2_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,7 @@
import json

from django.contrib.auth import get_user_model
from django.core.exceptions import ValidationError
from django.core.management.base import BaseCommand, CommandError
from django.core.validators import URLValidator
from provider.constants import CONFIDENTIAL, PUBLIC
from provider.oauth2.models import Client

Expand Down Expand Up @@ -117,13 +115,6 @@ def _clean_required_args(self, url, redirect_uri, client_type):
Raises:
CommandError, if the URLs provided are invalid, or if the client type provided is invalid.
"""
# Validate URLs
for url_to_validate in (url, redirect_uri):
try:
URLValidator()(url_to_validate)
except ValidationError:
raise CommandError("URLs provided are invalid. Please provide valid application and redirect URLs.")

# Validate and map client type to the appropriate django-oauth2-provider constant
client_type = client_type.lower()
client_type = {
Expand Down Expand Up @@ -167,11 +158,3 @@ def _parse_options(self, options):
client_name = self.fields.pop('client_name', None)
if client_name is not None:
self.fields['name'] = client_name

logout_uri = self.fields.get('logout_uri')

if logout_uri:
try:
URLValidator()(logout_uri)
except ValidationError:
raise CommandError("The logout_uri is invalid.")
11 changes: 0 additions & 11 deletions edx_oauth2_provider/tests/test_create_oauth2_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -111,17 +111,6 @@ def test_argument_cardinality(self, args, err_msg):
self._call_command(args, {})
self.assertIn(err_msg, str(exc.exception))

@ddt.data(
('invalid', REDIRECT_URI, CLIENT_TYPES[0]),
(URL, 'invalid', CLIENT_TYPES[0]),
)
def test_url_validation(self, args):
"""Verify that the command fails when the provided URLs are invalid."""
with self.assertRaises(CommandError) as exc:
self._call_command(args)

self.assertIn('URLs provided are invalid.', str(exc.exception))

def test_client_type_validation(self):
"""Verify that the command fails when the provided client type is invalid."""
with self.assertRaises(CommandError) as exc:
Expand Down

0 comments on commit a056bdc

Please sign in to comment.