From 36018d19aee0d065b793c9f8e4e3b824fe24b5dd Mon Sep 17 00:00:00 2001 From: Andy Clayton Date: Wed, 15 Aug 2018 15:43:48 -0500 Subject: [PATCH 1/4] support multiple response types per client The Dynamic Client Registration spec specifies multiple response_types and grant_types per client (https://openid.net/specs/openid-connect-registration-1_0.html#ClientMetadata). Since grant_types can be inferred from response_types we should be able to support both without needing to store grant_types. This also helps with oidc-client-js which expects a client that supports both "id_token" and "id_token token". --- docs/sections/changelog.rst | 2 + oidc_provider/admin.py | 4 +- oidc_provider/lib/endpoints/authorize.py | 3 +- .../0026_client_multiple_response_types.py | 51 ++++++++++++++++ oidc_provider/models.py | 23 +++++++- oidc_provider/tests/app/utils.py | 9 ++- .../tests/cases/test_authorize_endpoint.py | 59 ++++++++++++++----- oidc_provider/views.py | 7 +-- 8 files changed, 132 insertions(+), 26 deletions(-) create mode 100644 oidc_provider/migrations/0026_client_multiple_response_types.py diff --git a/docs/sections/changelog.rst b/docs/sections/changelog.rst index e1b4cec..905a8ff 100644 --- a/docs/sections/changelog.rst +++ b/docs/sections/changelog.rst @@ -8,6 +8,8 @@ All notable changes to this project will be documented in this file. Unreleased ========== +* Added: support multiple response types per client. + 0.6.2 ===== diff --git a/oidc_provider/admin.py b/oidc_provider/admin.py index 7718897..8525de8 100644 --- a/oidc_provider/admin.py +++ b/oidc_provider/admin.py @@ -52,7 +52,7 @@ class ClientAdmin(admin.ModelAdmin): fieldsets = [ [_(u''), { 'fields': ( - 'name', 'owner', 'client_type', 'response_type', '_redirect_uris', 'jwt_alg', + 'name', 'owner', 'client_type', 'response_types', '_redirect_uris', 'jwt_alg', 'require_consent', 'reuse_consent'), }], [_(u'Credentials'), { @@ -66,7 +66,7 @@ class ClientAdmin(admin.ModelAdmin): }], ] form = ClientForm - list_display = ['name', 'client_id', 'response_type', 'date_created'] + list_display = ['name', 'client_id', 'response_type_descriptions', 'date_created'] readonly_fields = ['date_created'] search_fields = ['name'] raw_id_fields = ['owner'] diff --git a/oidc_provider/lib/endpoints/authorize.py b/oidc_provider/lib/endpoints/authorize.py index 569d1f2..2f214c9 100644 --- a/oidc_provider/lib/endpoints/authorize.py +++ b/oidc_provider/lib/endpoints/authorize.py @@ -115,7 +115,8 @@ class AuthorizeEndpoint(object): raise AuthorizeError(self.params['redirect_uri'], 'invalid_request', self.grant_type) # Response type parameter validation. - if self.is_authentication and self.params['response_type'] != self.client.response_type: + if self.is_authentication \ + and self.params['response_type'] not in self.client.response_type_values(): raise AuthorizeError(self.params['redirect_uri'], 'invalid_request', self.grant_type) # PKCE validation of the transformation method. diff --git a/oidc_provider/migrations/0026_client_multiple_response_types.py b/oidc_provider/migrations/0026_client_multiple_response_types.py new file mode 100644 index 0000000..f572f0c --- /dev/null +++ b/oidc_provider/migrations/0026_client_multiple_response_types.py @@ -0,0 +1,51 @@ +# Generated by Django 2.0.7 on 2018-08-15 20:44 + +from django.db import migrations, models + + +def migrate_response_type(apps, schema_editor): + RESPONSE_TYPES = [ + ('code', 'code (Authorization Code Flow)'), + ('id_token', 'id_token (Implicit Flow)'), + ('id_token token', 'id_token token (Implicit Flow)'), + ('code token', 'code token (Hybrid Flow)'), + ('code id_token', 'code id_token (Hybrid Flow)'), + ('code id_token token', 'code id_token token (Hybrid Flow)'), + ] + # ensure we get proper, versioned model with the deleted response_type field; + # importing directly yields the latest without response_type + ResponseType = apps.get_model('oidc_provider', 'ResponseType') + Client = apps.get_model('oidc_provider', 'Client') + for value, description in RESPONSE_TYPES: + ResponseType.objects.create(value=value, description=description) + for client in Client.objects.all(): + client.response_types.add(ResponseType.objects.get(value=client.response_type)) + + +class Migration(migrations.Migration): + + dependencies = [ + ('oidc_provider', '0025_user_field_codetoken'), + ] + + operations = [ + migrations.CreateModel( + name='ResponseType', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('value', models.CharField(choices=[('code', 'code (Authorization Code Flow)'), ('id_token', 'id_token (Implicit Flow)'), ('id_token token', 'id_token token (Implicit Flow)'), ('code token', 'code token (Hybrid Flow)'), ('code id_token', 'code id_token (Hybrid Flow)'), ('code id_token token', 'code id_token token (Hybrid Flow)')], max_length=30, unique=True, verbose_name='Response Type Value')), + ('description', models.CharField(max_length=50)), + ], + ), + migrations.AddField( + model_name='client', + name='response_types', + field=models.ManyToManyField(to='oidc_provider.ResponseType'), + ), + # omitting reverse migrate_response_type since removing response_type is irreversible (nonnull and no default) + migrations.RunPython(migrate_response_type), + migrations.RemoveField( + model_name='client', + name='response_type', + ), + ] diff --git a/oidc_provider/models.py b/oidc_provider/models.py index dd51c90..26f2990 100644 --- a/oidc_provider/models.py +++ b/oidc_provider/models.py @@ -30,6 +30,20 @@ JWT_ALGS = [ ] +class ResponseType(models.Model): + value = models.CharField( + max_length=30, + choices=RESPONSE_TYPE_CHOICES, + unique=True, + verbose_name=_(u'Response Type Value')) + description = models.CharField( + max_length=50, + ) + + def __str__(self): + return u'{0}'.format(self.description) + + class Client(models.Model): name = models.CharField(max_length=100, default='', verbose_name=_(u'Name')) @@ -45,8 +59,7 @@ class Client(models.Model): u' of their credentials. Public clients are incapable.')) client_id = models.CharField(max_length=255, unique=True, verbose_name=_(u'Client ID')) client_secret = models.CharField(max_length=255, blank=True, verbose_name=_(u'Client SECRET')) - response_type = models.CharField( - max_length=30, choices=RESPONSE_TYPE_CHOICES, verbose_name=_(u'Response Type')) + response_types = models.ManyToManyField(ResponseType) jwt_alg = models.CharField( max_length=10, choices=JWT_ALGS, @@ -99,6 +112,12 @@ class Client(models.Model): def __unicode__(self): return self.__str__() + def response_type_values(self): + return (response_type.value for response_type in self.response_types.all()) + + def response_type_descriptions(self): + return [response_type.description for response_type in self.response_types.all()] + @property def redirect_uris(self): return self._redirect_uris.splitlines() diff --git a/oidc_provider/tests/app/utils.py b/oidc_provider/tests/app/utils.py index 0fc33b0..5a31afd 100644 --- a/oidc_provider/tests/app/utils.py +++ b/oidc_provider/tests/app/utils.py @@ -15,7 +15,8 @@ from django.contrib.auth.models import User from oidc_provider.models import ( Client, Code, - Token) + Token, + ResponseType) FAKE_NONCE = 'cb584e44c43ed6bd0bc2d9c7e242837d' @@ -58,12 +59,16 @@ def create_fake_client(response_type, is_public=False, require_consent=True): client.client_secret = '' else: client.client_secret = str(random.randint(1, 999999)).zfill(6) - client.response_type = response_type client.redirect_uris = ['http://example.com/'] client.require_consent = require_consent client.save() + if isinstance(response_type, ("".__class__, u"".__class__)): + response_type = (response_type,) + for value in response_type: + client.response_types.add(ResponseType.objects.get(value=value)) + return client diff --git a/oidc_provider/tests/cases/test_authorize_endpoint.py b/oidc_provider/tests/cases/test_authorize_endpoint.py index d589274..a062856 100644 --- a/oidc_provider/tests/cases/test_authorize_endpoint.py +++ b/oidc_provider/tests/cases/test_authorize_endpoint.py @@ -349,7 +349,7 @@ class AuthorizationCodeFlowTestCase(TestCase, AuthorizeEndpointMixin): """ data = { 'client_id': self.client.client_id, - 'response_type': self.client.response_type, + 'response_type': next(self.client.response_type_values()), 'redirect_uri': self.client.default_redirect_uri, 'scope': 'openid email', 'state': self.state, @@ -376,7 +376,7 @@ class AuthorizationCodeFlowTestCase(TestCase, AuthorizeEndpointMixin): """ data = { 'client_id': self.client.client_id, - 'response_type': self.client.response_type, + 'response_type': next(self.client.response_type_values()), 'redirect_uri': self.client.default_redirect_uri, 'scope': 'openid email', 'state': self.state, @@ -410,7 +410,7 @@ class AuthorizationCodeFlowTestCase(TestCase, AuthorizeEndpointMixin): """ data = { 'client_id': self.client.client_id, - 'response_type': self.client.response_type, + 'response_type': next(self.client.response_type_values()), 'redirect_uri': self.client.default_redirect_uri, 'scope': 'openid email', 'state': self.state, @@ -432,7 +432,7 @@ class AuthorizationCodeFlowTestCase(TestCase, AuthorizeEndpointMixin): """ data = { 'client_id': self.client.client_id, - 'response_type': self.client.response_type, + 'response_type': next(self.client.response_type_values()), 'redirect_uri': self.client.default_redirect_uri, 'scope': 'openid email', 'state': self.state, @@ -455,7 +455,7 @@ class AuthorizationCodeFlowTestCase(TestCase, AuthorizeEndpointMixin): """ data = { 'client_id': self.client.client_id, - 'response_type': self.client.response_type, + 'response_type': next(self.client.response_type_values()), 'redirect_uri': self.client.default_redirect_uri, 'scope': 'openid email', 'state': self.state, @@ -485,6 +485,8 @@ class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): require_consent=False) self.client_no_access = create_fake_client(response_type='id_token') self.client_public_no_access = create_fake_client(response_type='id_token', is_public=True) + self.client_multiple_response_types = create_fake_client( + response_type=('id_token', 'id_token token')) self.state = uuid.uuid4().hex self.nonce = uuid.uuid4().hex @@ -494,7 +496,7 @@ class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): """ data = { 'client_id': self.client.client_id, - 'response_type': self.client.response_type, + 'response_type': next(self.client.response_type_values()), 'redirect_uri': self.client.default_redirect_uri, 'scope': 'openid email', 'state': self.state, @@ -512,7 +514,7 @@ class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): data = { 'client_id': self.client.client_id, 'redirect_uri': self.client.default_redirect_uri, - 'response_type': self.client.response_type, + 'response_type': next(self.client.response_type_values()), 'scope': 'openid email', 'state': self.state, 'nonce': self.nonce, @@ -527,7 +529,7 @@ class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): # same for public client data['client_id'] = self.client_public.client_id, data['redirect_uri'] = self.client_public.default_redirect_uri, - data['response_type'] = self.client_public.response_type, + data['response_type'] = next(self.client_public.response_type_values()), response = self._auth_request('post', data, is_user_authenticated=True) @@ -542,7 +544,7 @@ class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): data = { 'client_id': self.client_no_access.client_id, 'redirect_uri': self.client_no_access.default_redirect_uri, - 'response_type': self.client_no_access.response_type, + 'response_type': next(self.client_no_access.response_type_values()), 'scope': 'openid email', 'state': self.state, 'nonce': self.nonce, @@ -557,7 +559,7 @@ class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): # same for public client data['client_id'] = self.client_public_no_access.client_id, data['redirect_uri'] = self.client_public_no_access.default_redirect_uri, - data['response_type'] = self.client_public_no_access.response_type, + data['response_type'] = next(self.client_public_no_access.response_type_values()), response = self._auth_request('post', data, is_user_authenticated=True) @@ -572,7 +574,7 @@ class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): data = { 'client_id': self.client.client_id, 'redirect_uri': self.client.default_redirect_uri, - 'response_type': self.client.response_type, + 'response_type': next(self.client.response_type_values()), 'scope': 'openid email', 'state': self.state, 'nonce': self.nonce, @@ -598,7 +600,7 @@ class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): data = { 'client_id': self.client_no_access.client_id, 'redirect_uri': self.client_no_access.default_redirect_uri, - 'response_type': self.client_no_access.response_type, + 'response_type': next(self.client_no_access.response_type_values()), 'scope': 'openid email', 'state': self.state, 'nonce': self.nonce, @@ -622,7 +624,7 @@ class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): """ data = { 'client_id': self.client_public_no_consent.client_id, - 'response_type': self.client_public_no_consent.response_type, + 'response_type': next(self.client_public_no_consent.response_type_values()), 'redirect_uri': self.client_public_no_consent.default_redirect_uri, 'scope': 'openid email', 'state': self.state, @@ -638,6 +640,33 @@ class AuthorizationImplicitFlowTestCase(TestCase, AuthorizeEndpointMixin): self.assertIn('id_token', fragment) self.assertIn('expires_in', fragment) + def test_multiple_response_types(self): + """ + Clients should be able to be configured for multiple response types. + """ + data = { + 'client_id': self.client_multiple_response_types.client_id, + 'redirect_uri': self.client_multiple_response_types.default_redirect_uri, + 'response_type': 'id_token', + 'scope': 'openid email', + 'state': self.state, + 'nonce': self.nonce, + 'allow': 'Accept', + } + + response = self._auth_request('post', data, is_user_authenticated=True) + + self.assertNotIn('access_token', response['Location']) + self.assertIn('id_token', response['Location']) + + # should also support "id_token token" response_type + data['response_type'] = 'id_token token' + + response = self._auth_request('post', data, is_user_authenticated=True) + + self.assertIn('access_token', response['Location']) + self.assertIn('id_token', response['Location']) + class AuthorizationHybridFlowTestCase(TestCase, AuthorizeEndpointMixin): """ @@ -657,7 +686,7 @@ class AuthorizationHybridFlowTestCase(TestCase, AuthorizeEndpointMixin): self.data = { 'client_id': self.client_code_idtoken_token.client_id, 'redirect_uri': self.client_code_idtoken_token.default_redirect_uri, - 'response_type': self.client_code_idtoken_token.response_type, + 'response_type': next(self.client_code_idtoken_token.response_type_values()), 'scope': 'openid email', 'state': self.state, 'nonce': self.nonce, @@ -703,7 +732,7 @@ class TestCreateResponseURI(TestCase): data = { 'client_id': client.client_id, 'redirect_uri': client.default_redirect_uri, - 'response_type': client.response_type, + 'response_type': next(client.response_type_values()), } factory = RequestFactory() diff --git a/oidc_provider/views.py b/oidc_provider/views.py index 17965f4..b18cd5d 100644 --- a/oidc_provider/views.py +++ b/oidc_provider/views.py @@ -49,9 +49,8 @@ from oidc_provider.lib.utils.oauth2 import protected_resource_view from oidc_provider.lib.utils.token import client_id_from_id_token from oidc_provider.models import ( Client, - RESPONSE_TYPE_CHOICES, RSAKey, -) + ResponseType) from oidc_provider import settings from oidc_provider import signals @@ -105,7 +104,7 @@ class AuthorizeView(View): implicit_flow_resp_types = {'id_token', 'id_token token'} allow_skipping_consent = ( authorize.client.client_type != 'public' or - authorize.client.response_type in implicit_flow_resp_types) + authorize.params['response_type'] in implicit_flow_resp_types) if not authorize.client.require_consent and ( allow_skipping_consent and @@ -283,7 +282,7 @@ class ProviderInfoView(View): dic['end_session_endpoint'] = site_url + reverse('oidc_provider:end-session') dic['introspection_endpoint'] = site_url + reverse('oidc_provider:token-introspection') - types_supported = [x[0] for x in RESPONSE_TYPE_CHOICES] + types_supported = [response_type.value for response_type in ResponseType.objects.all()] dic['response_types_supported'] = types_supported dic['jwks_uri'] = site_url + reverse('oidc_provider:jwks') From 036c4fc9b360cfe3470ad337c8cd0ea7c7468990 Mon Sep 17 00:00:00 2001 From: Andy Clayton Date: Thu, 16 Aug 2018 15:46:50 -0500 Subject: [PATCH 2/4] document non-obvious string check --- oidc_provider/tests/app/utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/oidc_provider/tests/app/utils.py b/oidc_provider/tests/app/utils.py index 5a31afd..6a65b64 100644 --- a/oidc_provider/tests/app/utils.py +++ b/oidc_provider/tests/app/utils.py @@ -64,6 +64,7 @@ def create_fake_client(response_type, is_public=False, require_consent=True): client.save() + # check if response_type is a string in a python 2 and 3 compatible way if isinstance(response_type, ("".__class__, u"".__class__)): response_type = (response_type,) for value in response_type: From 64a8b935e6355d9439b293c9dd50815bef77f45d Mon Sep 17 00:00:00 2001 From: Andy Clayton Date: Thu, 16 Aug 2018 15:47:47 -0500 Subject: [PATCH 3/4] document response_type_descriptions needs to be a list --- oidc_provider/models.py | 1 + 1 file changed, 1 insertion(+) diff --git a/oidc_provider/models.py b/oidc_provider/models.py index 26f2990..f49689c 100644 --- a/oidc_provider/models.py +++ b/oidc_provider/models.py @@ -116,6 +116,7 @@ class Client(models.Model): return (response_type.value for response_type in self.response_types.all()) def response_type_descriptions(self): + # return as a list, rather than a generator, so descriptions display correctly in admin return [response_type.description for response_type in self.response_types.all()] @property From 988b728fb22bcdfd02f6dc89581f64e479c8f1e7 Mon Sep 17 00:00:00 2001 From: Andy Clayton Date: Thu, 16 Aug 2018 16:42:18 -0500 Subject: [PATCH 4/4] update docs for multiple response types per client --- docs/sections/accesstokens.rst | 2 +- docs/sections/relyingparties.rst | 9 +++++---- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/sections/accesstokens.rst b/docs/sections/accesstokens.rst index 0472853..b6835db 100644 --- a/docs/sections/accesstokens.rst +++ b/docs/sections/accesstokens.rst @@ -10,7 +10,7 @@ Access tokens generally have a lifetime of only a couple of hours. You can use ` Obtaining an Access Token ========================= -Go to the admin site and create a confidential client with ``response_type = code`` and ``redirect_uri = http://example.org/``. +Go to the admin site and create a confidential client with ``response_types = code`` and ``redirect_uri = http://example.org/``. Open your browser and accept consent at:: diff --git a/docs/sections/relyingparties.rst b/docs/sections/relyingparties.rst index d99497a..3a3028c 100644 --- a/docs/sections/relyingparties.rst +++ b/docs/sections/relyingparties.rst @@ -19,8 +19,8 @@ Properties * ``client_type``: Values are ``confidential`` and ``public``. * ``client_id``: Client unique identifier. * ``client_secret``: Client secret for confidential applications. -* ``response_type``: Values depends of wich flow you want use. -* ``jwt_alg``: Clients can choose wich algorithm will be used to sign id_tokens. Values are ``HS256`` and ``RS256``. +* ``response_types``: The flows and associated ```response_type``` values that can be used by the client. +* ``jwt_alg``: Clients can choose which algorithm will be used to sign id_tokens. Values are ``HS256`` and ``RS256``. * ``date_created``: Date automatically added when created. * ``redirect_uris``: List of redirect URIs. * ``require_consent``: If checked, the Server will never ask for consent (only applies to confidential clients). @@ -58,8 +58,9 @@ Programmatically You can create a Client programmatically with Django shell ``python manage.py shell``:: - >>> from oidc_provider.models import Client - >>> c = Client(name='Some Client', client_id='123', client_secret='456', response_type='code', redirect_uris=['http://example.com/']) + >>> from oidc_provider.models import Client, ResponseType + >>> c = Client(name='Some Client', client_id='123', client_secret='456', redirect_uris=['http://example.com/']) >>> c.save() + >>> c.response_types.add(ResponseType.objects.get(value='code')) `Read more about client creation in the OAuth2 spec `_