From 3ca899fe495276f33ac13de9458637f3a98a4675 Mon Sep 17 00:00:00 2001 From: trbs Date: Mon, 18 May 2015 09:00:21 +0200 Subject: [PATCH 1/6] Issue #196 Add permission checking to application registration Add an addition setting called APPLICATION_REGISTRATION_PERMISSIONS which controls which permissions must be set on a user to allow the creation of applications. By default (in this PR) this is the 'add_application' permission which regretfully is not backwards compatible but it does seem to make the most sense from a security perspective as a default. Reverting to the old behaviour is easily done by using None for the settings key. --- docs/settings.rst | 9 +++++++++ oauth2_provider/settings.py | 3 +++ oauth2_provider/tests/test_application_views.py | 6 ++++++ oauth2_provider/views/application.py | 11 +++++++++-- 4 files changed, 27 insertions(+), 2 deletions(-) diff --git a/docs/settings.rst b/docs/settings.rst index 6fc46da46..5da6db3db 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -38,6 +38,15 @@ The import string of the class (model) representing your applications. Overwrite this value if you wrote your own implementation (subclass of ``oauth2_provider.models.Application``). +APPLICATION_REGISTRATION_PERMISSIONS +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +Which permissions are required to be able to register an application. By default +only superusers or users with ```oauth2_provider | application | Can add application``` +permission are allowed to register applications. If this setting is set to ```None``` +then all authenticated users are allowed to register applications. For more information +about setting more complex permissions see +`MultiplePermissionsRequiredMixin `_. + AUTHORIZATION_CODE_EXPIRE_SECONDS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The number of seconds an authorization code remains valid. Requesting an access diff --git a/oauth2_provider/settings.py b/oauth2_provider/settings.py index db5768686..3b856f145 100644 --- a/oauth2_provider/settings.py +++ b/oauth2_provider/settings.py @@ -41,6 +41,9 @@ 'AUTHORIZATION_CODE_EXPIRE_SECONDS': 60, 'ACCESS_TOKEN_EXPIRE_SECONDS': 36000, 'APPLICATION_MODEL': getattr(settings, 'OAUTH2_PROVIDER_APPLICATION_MODEL', 'oauth2_provider.Application'), + 'APPLICATION_REGISTRATION_PERMISSIONS': { + 'all': ('oauth2_provider.add_application', ), + }, 'REQUEST_APPROVAL_PROMPT': 'force', 'ALLOWED_REDIRECT_URI_SCHEMES': ['http', 'https'], diff --git a/oauth2_provider/tests/test_application_views.py b/oauth2_provider/tests/test_application_views.py index 32495ca27..b226747c0 100644 --- a/oauth2_provider/tests/test_application_views.py +++ b/oauth2_provider/tests/test_application_views.py @@ -1,6 +1,8 @@ from __future__ import unicode_literals from django.core.urlresolvers import reverse +from django.contrib.auth.models import Permission +from django.contrib.contenttypes.models import ContentType from django.test import TestCase from ..models import get_application_model @@ -15,6 +17,10 @@ def setUp(self): self.foo_user = UserModel.objects.create_user("foo_user", "test@user.com", "123456") self.bar_user = UserModel.objects.create_user("bar_user", "dev@user.com", "123456") + application_content_type = ContentType.objects.get_for_model(Application) + add_application_permission = Permission.objects.get(content_type=application_content_type, codename='add_application') + self.foo_user.user_permissions.add(add_application_permission) + def tearDown(self): self.foo_user.delete() self.bar_user.delete() diff --git a/oauth2_provider/views/application.py b/oauth2_provider/views/application.py index 0a3598709..15fc62cfb 100644 --- a/oauth2_provider/views/application.py +++ b/oauth2_provider/views/application.py @@ -1,10 +1,11 @@ from django.core.urlresolvers import reverse_lazy from django.views.generic import CreateView, DetailView, DeleteView, ListView, UpdateView -from braces.views import LoginRequiredMixin +from braces.views import LoginRequiredMixin, MultiplePermissionsRequiredMixin from ..forms import RegistrationForm from ..models import get_application_model +from ..settings import oauth2_settings class ApplicationOwnerIsUserMixin(LoginRequiredMixin): @@ -17,13 +18,19 @@ def get_queryset(self): return get_application_model().objects.filter(user=self.request.user) -class ApplicationRegistration(LoginRequiredMixin, CreateView): +class ApplicationRegistration(LoginRequiredMixin, MultiplePermissionsRequiredMixin, CreateView): """ View used to register a new Application for the request.user """ form_class = RegistrationForm + permissions = oauth2_settings.APPLICATION_REGISTRATION_PERMISSIONS template_name = "oauth2_provider/application_registration_form.html" + def check_permissions(self, request): + if getattr(self, 'permissions', None) is None: + return True + return super(ApplicationRegistration, self).check_permissions(request) + def form_valid(self, form): form.instance.user = self.request.user return super(ApplicationRegistration, self).form_valid(form) From 6e60c385eee87ed70d741830c61ee6b7ad7265e7 Mon Sep 17 00:00:00 2001 From: trbs Date: Mon, 18 May 2015 15:13:38 +0200 Subject: [PATCH 2/6] make default value backwards compatible --- oauth2_provider/settings.py | 1 + 1 file changed, 1 insertion(+) diff --git a/oauth2_provider/settings.py b/oauth2_provider/settings.py index 3b856f145..1d192b1d9 100644 --- a/oauth2_provider/settings.py +++ b/oauth2_provider/settings.py @@ -41,6 +41,7 @@ 'AUTHORIZATION_CODE_EXPIRE_SECONDS': 60, 'ACCESS_TOKEN_EXPIRE_SECONDS': 36000, 'APPLICATION_MODEL': getattr(settings, 'OAUTH2_PROVIDER_APPLICATION_MODEL', 'oauth2_provider.Application'), + 'APPLICATION_REGISTRATION_PERMISSIONS': None, 'APPLICATION_REGISTRATION_PERMISSIONS': { 'all': ('oauth2_provider.add_application', ), }, From f02a1c89158112b09caa63879cb98a2030d4ee35 Mon Sep 17 00:00:00 2001 From: trbs Date: Mon, 18 May 2015 15:14:22 +0200 Subject: [PATCH 3/6] update documentation --- docs/settings.rst | 27 ++++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-) diff --git a/docs/settings.rst b/docs/settings.rst index 5da6db3db..79299509f 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -40,13 +40,30 @@ this value if you wrote your own implementation (subclass of APPLICATION_REGISTRATION_PERMISSIONS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -Which permissions are required to be able to register an application. By default -only superusers or users with ```oauth2_provider | application | Can add application``` -permission are allowed to register applications. If this setting is set to ```None``` -then all authenticated users are allowed to register applications. For more information -about setting more complex permissions see +Defines which permissions are required to be able to register an application. +By default every authenticated user is able to register applications. + +A common setup for none public sites might be to only allow superusers and users +with the ```oauth2_provider | application | Can add application``` permission to be allowed +to register applications. + +This can be done with the following configuration:: + + OAUTH2_PROVIDER = { + ... + 'APPLICATION_REGISTRATION_PERMISSIONS': { + 'all': ('oauth2_provider.add_application', ), + }, + ... + } + +Following Django's auth system you can now either grant the ```oauth2_provider | application | Can add application``` +the a specific user or group to allow access to the application registration page. + +For more information about setting more complex permissions see `MultiplePermissionsRequiredMixin `_. + AUTHORIZATION_CODE_EXPIRE_SECONDS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ The number of seconds an authorization code remains valid. Requesting an access From ffed5389c7f3da08a8df37013483f56626ce3acc Mon Sep 17 00:00:00 2001 From: trbs Date: Mon, 18 May 2015 15:15:13 +0200 Subject: [PATCH 4/6] add provider register permissions tests --- .../tests/test_application_views.py | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/oauth2_provider/tests/test_application_views.py b/oauth2_provider/tests/test_application_views.py index b226747c0..cd278b788 100644 --- a/oauth2_provider/tests/test_application_views.py +++ b/oauth2_provider/tests/test_application_views.py @@ -5,6 +5,7 @@ from django.contrib.contenttypes.models import ContentType from django.test import TestCase +from ..settings import oauth2_settings from ..models import get_application_model from ..compat import get_user_model @@ -46,6 +47,58 @@ def test_application_registration_user(self): self.assertEqual(app.user.username, "foo_user") +class TestApplicationRegistrationViewPermissions(BaseTest): + def setUp(self): + super(TestApplicationRegistrationViewPermissions, self).setUp() + oauth2_settings.APPLICATION_REGISTRATION_PERMISSIONS = { + 'all': ('oauth2_provider.add_application', ), + } + + def tearDown(self): + super(TestApplicationRegistrationViewPermissions, self).tearDown() + oauth2_settings.APPLICATION_REGISTRATION_PERMISSIONS = None + + def test_application_registration_user_with_permission(self): + self.client.login(username="foo_user", password="123456") + + form_data = { + 'name': 'Foo app', + 'client_id': 'client_id', + 'client_secret': 'client_secret', + 'client_type': Application.CLIENT_CONFIDENTIAL, + 'redirect_uris': 'http://example.com', + 'authorization_grant_type': Application.GRANT_AUTHORIZATION_CODE + } + + response = self.client.post(reverse('oauth2_provider:register'), form_data) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.has_header('Location'), True) + self.assertEqual(response['Location'].endswith("/o/applications/1/"), True) + + app = Application.objects.get(name="Foo app") + self.assertEqual(app.user.username, "foo_user") + + def test_application_registration_user_without_permission(self): + self.client.login(username="bar_user", password="123456") + + form_data = { + 'name': 'Bar app', + 'client_id': 'client_id', + 'client_secret': 'client_secret', + 'client_type': Application.CLIENT_CONFIDENTIAL, + 'redirect_uris': 'http://example.com', + 'authorization_grant_type': Application.GRANT_AUTHORIZATION_CODE + } + + response = self.client.post(reverse('oauth2_provider:register'), form_data) + self.assertEqual(response.status_code, 302) + self.assertEqual(response.has_header('Location'), True) + self.assertEqual(response['Location'].endswith("/accounts/login/?next=/o/applications/register/"), True) + + with self.assertRaises(Application.DoesNotExist): + app = Application.objects.get(name="Bar app") + + class TestApplicationViews(BaseTest): def _create_application(self, name, user): app = Application.objects.create( From a0ce6a9964eea197cb6fb9efc05df4d82703017a Mon Sep 17 00:00:00 2001 From: trbs Date: Mon, 25 May 2015 01:25:57 +0200 Subject: [PATCH 5/6] fix typo --- docs/settings.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/settings.rst b/docs/settings.rst index 79299509f..4e5b05110 100644 --- a/docs/settings.rst +++ b/docs/settings.rst @@ -43,7 +43,7 @@ APPLICATION_REGISTRATION_PERMISSIONS Defines which permissions are required to be able to register an application. By default every authenticated user is able to register applications. -A common setup for none public sites might be to only allow superusers and users +A common setup for non public sites might be to only allow superusers and users with the ```oauth2_provider | application | Can add application``` permission to be allowed to register applications. From 66525d53ff986229781ce8837213785d8bf19948 Mon Sep 17 00:00:00 2001 From: trbs Date: Mon, 25 May 2015 01:26:48 +0200 Subject: [PATCH 6/6] remove example permissions for only allowing registrations from super users or users with add_application permission --- oauth2_provider/settings.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/oauth2_provider/settings.py b/oauth2_provider/settings.py index 1d192b1d9..49d0e3f7f 100644 --- a/oauth2_provider/settings.py +++ b/oauth2_provider/settings.py @@ -42,9 +42,6 @@ 'ACCESS_TOKEN_EXPIRE_SECONDS': 36000, 'APPLICATION_MODEL': getattr(settings, 'OAUTH2_PROVIDER_APPLICATION_MODEL', 'oauth2_provider.Application'), 'APPLICATION_REGISTRATION_PERMISSIONS': None, - 'APPLICATION_REGISTRATION_PERMISSIONS': { - 'all': ('oauth2_provider.add_application', ), - }, 'REQUEST_APPROVAL_PROMPT': 'force', 'ALLOWED_REDIRECT_URI_SCHEMES': ['http', 'https'],