From a829726be8b1fc6167228de72cc0cbbfbc9679a3 Mon Sep 17 00:00:00 2001 From: Wojciech Bartosiak Date: Fri, 5 May 2017 05:19:57 +0200 Subject: [PATCH 1/3] Merge develop to v0.5.x (#179) * Log create_uri_response exceptions to logger.exception * Support grant type password - basics * Add tests for Resource Owner Password Credentials Flow * Password Grant -Response according to specification * Better tests for errors, disable grant type password by default * Add documentation for grant type password * User authentication failure to return 403 * Add id_token to response * skipping consent only works for confidential clients * fix URI fragment example not working URL `http://localhost:8100/#/auth/callback/` * OIDC_POST_END_SESSION_HOOK + tests * Explicit function naming * Remove print statements * No need for semicolons, this is Python * Update CHANGELOG.md * fixed logger message * Improved `exp` value calculation * rename OIDC_POST_END_SESSION_HOOK to OIDC_AFTER_END_SESSION_HOOK * added docs for OIDC_AFTER_END_SESSION_HOOK * Replaces `LOGIN_URL` with `OIDC_LOGIN_URL` so users can use a different login path for their oidc requests. * Adds a setting variable for custom template paths * Updates documentation * Fixed bad try/except/finally block * Adds test for OIDC_TEMPLATES settings * Determine value for op_browser_state from session_key or default * Do not use cookie for browser_state. It may not yet be there * Add docs on new setting OIDC_UNAUTHENTICATED_SESSION_MANAGEMENT_KEY * Fix compatibility for older versions of Django * solved merging typo for missing @property --- CHANGELOG.md | 3 + docs/index.rst | 2 + docs/sections/sessionmanagement.rst | 4 + docs/sections/settings.rst | 72 +++++++++- docs/sections/templates.rst | 3 + oidc_provider/lib/endpoints/authorize.py | 12 +- oidc_provider/lib/endpoints/token.py | 54 ++++++- oidc_provider/lib/errors.py | 15 ++ oidc_provider/lib/utils/common.py | 38 +++++ oidc_provider/middleware.py | 16 ++- oidc_provider/settings.py | 70 +++++++-- .../tests/test_authorize_endpoint.py | 52 ++++++- .../tests/test_end_session_endpoint.py | 10 +- oidc_provider/tests/test_middleware.py | 39 +++++ oidc_provider/tests/test_settings.py | 25 ++++ oidc_provider/tests/test_token_endpoint.py | 136 ++++++++++++++++-- oidc_provider/tests/test_utils.py | 24 +++- oidc_provider/views.py | 29 +++- 18 files changed, 554 insertions(+), 50 deletions(-) create mode 100644 oidc_provider/tests/test_middleware.py create mode 100644 oidc_provider/tests/test_settings.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 0850514..57c7499 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ All notable changes to this project will be documented in this file. ##### Added - Signals when user accept/decline the authorization page. +- `OIDC_AFTER_END_SESSION_HOOK` setting for additional business logic +- Feature granttype password - require_consent and reuse_consent are added to Client model. ##### Changed @@ -14,6 +16,7 @@ All notable changes to this project will be documented in this file. ##### Fixed - Timestamps with unixtime (instead of django timezone). - Field refresh_token cannot be primary key if null. +- `create_uri_exceptions` are now being logged at `Exception` level not `DEBUG` ### [0.4.4] - 2016-11-29 diff --git a/docs/index.rst b/docs/index.rst index cb4816a..1a15854 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -8,6 +8,7 @@ Also implements the following specifications: * `OpenID Connect Discovery 1.0 `_ * `OpenID Connect Session Management 1.0 `_ * `OAuth 2.0 for Native Apps `_ +* `OAuth 2.0 Resource Owner Password Credentials Grant `_ * `Proof Key for Code Exchange by OAuth Public Clients `_ -------------------------------------------------------------------------------- @@ -16,6 +17,7 @@ Before getting started there are some important things that you should know: * Despite that implementation MUST support TLS. You can make request without using SSL. There is no control on that. * Supports only for requesting Claims using Scope values. +* If you enable the Resource Owner Password Credentials Grant, you MUST implement protection against brute force attacks on the token endpoint -------------------------------------------------------------------------------- diff --git a/docs/sections/sessionmanagement.rst b/docs/sections/sessionmanagement.rst index c33cd5b..3c182b7 100644 --- a/docs/sections/sessionmanagement.rst +++ b/docs/sections/sessionmanagement.rst @@ -18,6 +18,10 @@ Somewhere in your Django ``settings.py``:: OIDC_SESSION_MANAGEMENT_ENABLE = True + +If you're in a multi-server setup, you might also want to add ``OIDC_UNAUTHENTICATED_SESSION_MANAGEMENT_KEY`` to your settings and set it to some random but fixed string. While authenticated clients have a session that can be used to calculate the browser state, there is no such thing for unauthenticated clients. Hence this value. By default a value is generated randomly on startup, so this will be different on each server. To get a consistent value across all servers you should set this yourself. + + Example RP iframe ================= diff --git a/docs/sections/settings.rst b/docs/sections/settings.rst index 7edf35b..82d1344 100644 --- a/docs/sections/settings.rst +++ b/docs/sections/settings.rst @@ -5,12 +5,12 @@ Settings Customize your provider so fit your project needs. -LOGIN_URL -========= +OIDC_LOGIN_URL +============== -REQUIRED. ``str``. Used to log the user in. `Read more in Django docs `_ +OPTIONAL. ``str``. Used to log the user in. By default Django's ``LOGIN_URL`` will be used. `Read more in Django docs `_ -``str``. Default is ``/accounts/login/``. +``str``. Default is ``/accounts/login/`` (Django's ``LOGIN_URL``). SITE_URL ======== @@ -36,6 +36,18 @@ Return ``None`` if you want to continue with the flow. The typical situation will be checking some state of the user or maybe redirect him somewhere. With request you have access to all OIDC parameters. Remember that if you redirect the user to another place then you need to take him back to the authorize endpoint (use ``request.get_full_path()`` as the value for a "next" parameter). +OIDC_AFTER_END_SESSION_HOOK +=========================== + +OPTIONAL. ``str``. A string with the location of your function. Provide a way to plug into the log out process just before calling Django's log out function, typically to perform some business logic. + +Default is:: + + def default_after_end_session_hook(request, id_token=None, post_logout_redirect_uri=None, state=None, client=None, next_page=None): + return None + +Return ``None`` if you want to continue with the flow. + OIDC_CODE_EXPIRE ================ @@ -98,6 +110,13 @@ OPTIONAL. ``bool``. Enables OpenID Connect Session Management 1.0 in your provid Default is ``False``. +OIDC_UNAUTHENTICATED_SESSION_MANAGEMENT_KEY +=========================================== + +OPTIONAL. Supply a fixed string to use as browser-state key for unauthenticated clients. Read :ref:`sessionmanagement` section. + +Default is a string generated at startup. + OIDC_SKIP_CONSENT_EXPIRE ======================== @@ -133,3 +152,48 @@ Example usage:: .. note:: Please **DO NOT** add extra keys or delete the existing ones in the ``claims`` dict. If you want to add extra claims to some scopes you can use the ``OIDC_EXTRA_SCOPE_CLAIMS`` setting. + +OIDC_GRANT_TYPE_PASSWORD_ENABLE +=============================== +OPTIONAL. A boolean to set whether to allow the Resource Owner Password +Credentials Grant. https://tools.ietf.org/html/rfc6749#section-4.3 + +.. important:: + From the specification: + "Since this access token request utilizes the resource owner's + password, the authorization server **MUST** protect the endpoint + against brute force attacks (e.g., using rate-limitation or + generating alerts)." + + There are many ways to implement brute force attack prevention. We cannot + decide what works best for you, so you will have to implement a solution for + this that suits your needs. + +OIDC_TEMPLATES +============== +OPTIONAL. A dictionary pointing to templates for authorize and error pages. +Default is:: + + { + 'authorize': 'oidc_provider/authorize.html', + 'error': 'oidc_provider/error.html' + } + +The following contexts will be passed to the ``authorize`` and ``error`` templates respectively:: + + # For authorize template + { + 'client': 'an instance of Client for the auth request', + 'hidden_inputs': 'a rendered html with all the hidden inputs needed for AuthorizeEndpoint', + 'params': 'a dict containing the params in the auth request', + 'scopes': 'a list of scopes' + } + + # For error template + { + 'error': 'string stating the error', + 'description': 'string stating description of the error' + } + +.. note:: + The templates that are not specified here will use the default ones. diff --git a/docs/sections/templates.rst b/docs/sections/templates.rst index 8047c2b..bd9cef5 100644 --- a/docs/sections/templates.rst +++ b/docs/sections/templates.rst @@ -33,3 +33,6 @@ You can copy the sample html here and edit them with your own styles.

{{ error }}

{{ description }}

+ +You can also customize paths to your custom templates by putting them in ``OIDC_TEMPLATES`` in the settings. + diff --git a/oidc_provider/lib/endpoints/authorize.py b/oidc_provider/lib/endpoints/authorize.py index 4be87c0..36b4b2d 100644 --- a/oidc_provider/lib/endpoints/authorize.py +++ b/oidc_provider/lib/endpoints/authorize.py @@ -30,8 +30,7 @@ from oidc_provider.models import ( UserConsent, ) from oidc_provider import settings -from oidc_provider.lib.utils.common import cleanup_url_from_query_string - +from oidc_provider.lib.utils.common import cleanup_url_from_query_string, get_browser_state_or_default logger = logging.getLogger(__name__) @@ -122,7 +121,7 @@ class AuthorizeEndpoint(object): def create_response_uri(self): uri = urlsplit(self.params['redirect_uri']) query_params = parse_qs(uri.query) - query_fragment = parse_qs(uri.fragment) + query_fragment = {} try: if self.grant_type in ['authorization_code', 'hybrid']: @@ -197,7 +196,7 @@ class AuthorizeEndpoint(object): session_state = '{client_id} {origin} {browser_state} {salt}'.format( client_id=self.client.client_id, origin=client_origin, - browser_state=self.request.COOKIES['op_browser_state'], + browser_state=get_browser_state_or_default(self.request), salt=salt) session_state = sha256(session_state.encode('utf-8')).hexdigest() session_state += '.' + salt @@ -207,11 +206,10 @@ class AuthorizeEndpoint(object): query_fragment['session_state'] = session_state except Exception as error: - logger.debug('[Authorize] Error when trying to create response uri: %s', error) + logger.exception('[Authorize] Error when trying to create response uri: %s', error) raise AuthorizeError(self.params['redirect_uri'], 'server_error', self.grant_type) - uri = uri._replace(query=urlencode(query_params, doseq=True)) - uri = uri._replace(fragment=urlencode(query_fragment, doseq=True)) + uri = uri._replace(query=urlencode(query_params, doseq=True), fragment=uri.fragment + urlencode(query_fragment, doseq=True)) return urlunsplit(uri) diff --git a/oidc_provider/lib/endpoints/token.py b/oidc_provider/lib/endpoints/token.py index 0bf52da..ae1eb98 100644 --- a/oidc_provider/lib/endpoints/token.py +++ b/oidc_provider/lib/endpoints/token.py @@ -2,7 +2,7 @@ from base64 import b64decode, urlsafe_b64encode import hashlib import logging import re - +from django.contrib.auth import authenticate from oidc_provider.lib.utils.common import cleanup_url_from_query_string try: @@ -14,6 +14,7 @@ from django.http import JsonResponse from oidc_provider.lib.errors import ( TokenError, + UserAuthError, ) from oidc_provider.lib.utils.token import ( create_id_token, @@ -27,15 +28,14 @@ from oidc_provider.models import ( ) from oidc_provider import settings - logger = logging.getLogger(__name__) class TokenEndpoint(object): - def __init__(self, request): self.request = request self.params = {} + self.user = None self._extract_params() def _extract_params(self): @@ -53,6 +53,9 @@ class TokenEndpoint(object): # PKCE parameter. self.params['code_verifier'] = self.request.POST.get('code_verifier') + self.params['username'] = self.request.POST.get('username', '') + self.params['password'] = self.request.POST.get('password', '') + def _extract_client_auth(self): """ Get client credentials using HTTP Basic Authentication method. @@ -103,8 +106,7 @@ class TokenEndpoint(object): if not (self.code.client == self.client) \ or self.code.has_expired(): - logger.debug('[Token] Invalid code: invalid client or code has expired', - self.params['redirect_uri']) + logger.debug('[Token] Invalid code: invalid client or code has expired') raise TokenError('invalid_grant') # Validate PKCE parameters. @@ -120,6 +122,20 @@ class TokenEndpoint(object): if not (new_code_challenge == self.code.code_challenge): raise TokenError('invalid_grant') + elif self.params['grant_type'] == 'password': + if not settings.get('OIDC_GRANT_TYPE_PASSWORD_ENABLE'): + raise TokenError('unsupported_grant_type') + + user = authenticate( + username=self.params['username'], + password=self.params['password'] + ) + + if not user: + raise UserAuthError() + + self.user = user + elif self.params['grant_type'] == 'refresh_token': if not self.params['refresh_token']: logger.debug('[Token] Missing refresh token') @@ -142,6 +158,34 @@ class TokenEndpoint(object): return self.create_code_response_dic() elif self.params['grant_type'] == 'refresh_token': return self.create_refresh_response_dic() + elif self.params['grant_type'] == 'password': + return self.create_access_token_response_dic() + + def create_access_token_response_dic(self): + token = create_token( + self.user, + self.client, + self.params['scope'].split(' ')) + + id_token_dic = create_id_token( + user=self.user, + aud=self.client.client_id, + nonce='self.code.nonce', + at_hash=token.at_hash, + request=self.request, + scope=self.params['scope'], + ) + + token.id_token = id_token_dic + token.save() + + return { + 'access_token': token.access_token, + 'refresh_token': token.refresh_token, + 'expires_in': settings.get('OIDC_TOKEN_EXPIRE'), + 'token_type': 'bearer', + 'id_token': encode_id_token(id_token_dic, token.client), + } def create_code_response_dic(self): token = create_token( diff --git a/oidc_provider/lib/errors.py b/oidc_provider/lib/errors.py index ce84811..47f4b10 100644 --- a/oidc_provider/lib/errors.py +++ b/oidc_provider/lib/errors.py @@ -16,6 +16,21 @@ class ClientIdError(Exception): description = 'The client identifier (client_id) is missing or invalid.' +class UserAuthError(Exception): + """ + Specific to the Resource Owner Password Credentials flow when + the Resource Owners credentials are not valid. + """ + error = 'access_denied' + description = 'The resource owner or authorization server denied ' \ + 'the request' + + def create_dict(self): + return { + 'error': self.error, + 'error_description': self.description, + } + class AuthorizeError(Exception): _errors = { diff --git a/oidc_provider/lib/utils/common.py b/oidc_provider/lib/utils/common.py index b7a6676..c4778bd 100644 --- a/oidc_provider/lib/utils/common.py +++ b/oidc_provider/lib/utils/common.py @@ -1,3 +1,5 @@ +from hashlib import sha224 + from django.core.urlresolvers import reverse from django.http import HttpResponse @@ -50,6 +52,7 @@ def get_site_url(site_url=None, request=None): 'or set `SITE_URL` in settings, ' 'or pass `request` object.') + def get_issuer(site_url=None, request=None): """ Construct the issuer full url. Basically is the site url with some path @@ -84,6 +87,33 @@ def default_after_userlogin_hook(request, user, client): """ return None + +def default_after_end_session_hook(request, id_token=None, post_logout_redirect_uri=None, state=None, client=None, next_page=None): + """ + Default function for setting OIDC_AFTER_END_SESSION_HOOK. + + :param request: Django request object + :type request: django.http.HttpRequest + + :param id_token: token passed by `id_token_hint` url query param - do NOT trust this param or validate token + :type id_token: str + + :param post_logout_redirect_uri: redirect url from url query param - do NOT trust this param + :type post_logout_redirect_uri: str + + :param state: state param from url query params + :type state: str + + :param client: If id_token has `aud` param and associated Client exists, this is an instance of it - do NOT trust this param + :type client: oidc_provider.models.Client + + :param next_page: calculated next_page redirection target + :type next_page: str + :return: + """ + return None + + def default_idtoken_processing_hook(id_token, user): """ Hook to perform some additional actions ti `id_token` dictionary just before serialization. @@ -98,3 +128,11 @@ def default_idtoken_processing_hook(id_token, user): :rtype dict """ return id_token + + +def get_browser_state_or_default(request): + """ + Determine value to use as session state. + """ + key = request.session.session_key or settings.get('OIDC_UNAUTHENTICATED_SESSION_MANAGEMENT_KEY') + return sha224(key.encode('utf-8')).hexdigest() diff --git a/oidc_provider/middleware.py b/oidc_provider/middleware.py index a359a95..3516bc4 100644 --- a/oidc_provider/middleware.py +++ b/oidc_provider/middleware.py @@ -1,17 +1,21 @@ -from hashlib import sha224 +try: + # https://docs.djangoproject.com/en/1.10/topics/http/middleware/#upgrading-pre-django-1-10-style-middleware + from django.utils.deprecation import MiddlewareMixin +except ImportError: + MiddlewareMixin = object -from django.conf import settings as django_settings -from django.utils.deprecation import MiddlewareMixin +from oidc_provider import settings +from oidc_provider.lib.utils.common import get_browser_state_or_default class SessionManagementMiddleware(MiddlewareMixin): """ Maintain a `op_browser_state` cookie along with the `sessionid` cookie that represents the End-User's login state at the OP. If the user is not logged - in then use `SECRET_KEY` value. + in then use the value of settings.OIDC_UNAUTHENTICATED_SESSION_MANAGEMENT_KEY. """ def process_response(self, request, response): - session_state = sha224((request.session.session_key or django_settings.SECRET_KEY).encode('utf-8')).hexdigest() - response.set_cookie('op_browser_state', session_state) + if settings.get('OIDC_SESSION_MANAGEMENT_ENABLE'): + response.set_cookie('op_browser_state', get_browser_state_or_default(request)) return response diff --git a/oidc_provider/settings.py b/oidc_provider/settings.py index 69f0f3b..fc2b4c9 100644 --- a/oidc_provider/settings.py +++ b/oidc_provider/settings.py @@ -1,19 +1,22 @@ import importlib +import random +import string from django.conf import settings class DefaultSettings(object): - required_attrs = ( - 'LOGIN_URL', - ) + required_attrs = () + + def __init__(self): + self._unauthenticated_session_management_key = None @property - def LOGIN_URL(self): + def OIDC_LOGIN_URL(self): """ - REQUIRED. Used to log the user in. + REQUIRED. Used to log the user in. By default Django's LOGIN_URL will be used. """ - return None + return settings.LOGIN_URL @property def SITE_URL(self): @@ -30,6 +33,14 @@ class DefaultSettings(object): """ return 'oidc_provider.lib.utils.common.default_after_userlogin_hook' + @property + def OIDC_AFTER_END_SESSION_HOOK(self): + """ + OPTIONAL. Provide a way to plug into the end session process just before calling + Django's logout function, typically to perform some business logic. + """ + return 'oidc_provider.lib.utils.common.default_after_end_session_hook' + @property def OIDC_CODE_EXPIRE(self): """ @@ -68,6 +79,18 @@ class DefaultSettings(object): """ return False + @property + def OIDC_UNAUTHENTICATED_SESSION_MANAGEMENT_KEY(self): + """ + OPTIONAL. Supply a fixed string to use as browser-state key for unauthenticated clients. + """ + + # Memoize generated value + if not self._unauthenticated_session_management_key: + self._unauthenticated_session_management_key = ''.join( + random.choice(string.ascii_uppercase + string.digits) for _ in range(100)) + return self._unauthenticated_session_management_key + @property def OIDC_SKIP_CONSENT_EXPIRE(self): """ @@ -99,6 +122,29 @@ class DefaultSettings(object): """ return 'oidc_provider.lib.utils.common.default_idtoken_processing_hook' + @property + def OIDC_GRANT_TYPE_PASSWORD_ENABLE(self): + """ + OPTIONAL. A boolean to set whether to allow the Resource Owner Password + Credentials Grant. https://tools.ietf.org/html/rfc6749#section-4.3 + + From the specification: + Since this access token request utilizes the resource owner's + password, the authorization server MUST protect the endpoint + against brute force attacks (e.g., using rate-limitation or + generating alerts). + + How you do this, is up to you. + """ + return False + + @property + def OIDC_TEMPLATES(self): + return { + 'authorize': 'oidc_provider/authorize.html', + 'error': 'oidc_provider/error.html' + } + default_settings = DefaultSettings() @@ -121,13 +167,19 @@ def get(name, import_str=False): Helper function to use inside the package. """ value = None + default_value = getattr(default_settings, name) + try: - value = getattr(default_settings, name) value = getattr(settings, name) except AttributeError: - if value is None and name in default_settings.required_attrs: + if name in default_settings.required_attrs: raise Exception('You must set ' + name + ' in your settings.') - value = import_from_str(value) if import_str else value + if isinstance(default_value, dict) and value: + default_value.update(value) + value = default_value + else: + value = value or default_value + value = import_from_str(value) if import_str else value return value diff --git a/oidc_provider/tests/test_authorize_endpoint.py b/oidc_provider/tests/test_authorize_endpoint.py index 71fbb77..6cb83f7 100644 --- a/oidc_provider/tests/test_authorize_endpoint.py +++ b/oidc_provider/tests/test_authorize_endpoint.py @@ -7,6 +7,7 @@ try: except ImportError: from urlparse import parse_qs, urlsplit import uuid +from mock import patch, mock from django.contrib.auth.models import AnonymousUser from django.core.management import call_command @@ -26,6 +27,7 @@ from oidc_provider.tests.app.utils import ( is_code_valid, ) from oidc_provider.views import AuthorizeView +from oidc_provider.lib.endpoints.authorize import AuthorizeEndpoint class AuthorizeEndpointMixin(object): @@ -122,7 +124,7 @@ class AuthorizationCodeFlowTestCase(TestCase, AuthorizeEndpointMixin): response = self._auth_request('get', data) # Check if user was redirected to the login view. - self.assertIn(settings.get('LOGIN_URL'), response['Location']) + self.assertIn(settings.get('OIDC_LOGIN_URL'), response['Location']) def test_user_consent_inputs(self): """ @@ -498,3 +500,51 @@ class AuthorizationHybridFlowTestCase(TestCase, AuthorizeEndpointMixin): response = self._auth_request('post', self.data, is_user_authenticated=True) self.assertIn('expires_in=36000', response['Location']) + + +class TestCreateResponseURI(TestCase): + def setUp(self): + url = reverse('oidc_provider:authorize') + user = create_fake_user() + client = create_fake_client(response_type='code', is_public=True) + + # Base data to create a uri response + data = { + 'client_id': client.client_id, + 'redirect_uri': client.default_redirect_uri, + 'response_type': client.response_type, + } + + factory = RequestFactory() + self.request = factory.post(url, data=data) + self.request.user = user + + @patch('oidc_provider.lib.endpoints.authorize.create_code') + @patch('oidc_provider.lib.endpoints.authorize.logger.exception') + def test_create_response_uri_logs_to_error(self, log_exception, create_code): + """ + A lot can go wrong when creating a response uri and this is caught with a general Exception error. The + information contained within this error should show up in the error log so production servers have something + to work with when things don't work as expected. + """ + exception = Exception("Something went wrong!") + create_code.side_effect = exception + + authorization_endpoint = AuthorizeEndpoint(self.request) + authorization_endpoint.validate_params() + + with self.assertRaises(Exception): + authorization_endpoint.create_response_uri() + + log_exception.assert_called_once_with('[Authorize] Error when trying to create response uri: %s', exception) + + @override_settings(OIDC_SESSION_MANAGEMENT_ENABLE=True) + def test_create_response_uri_generates_session_state_if_session_management_enabled(self): + # RequestFactory doesn't support sessions, so we mock it + self.request.session = mock.Mock(session_key=None) + + authorization_endpoint = AuthorizeEndpoint(self.request) + authorization_endpoint.validate_params() + + uri = authorization_endpoint.create_response_uri() + self.assertIn('session_state=', uri) diff --git a/oidc_provider/tests/test_end_session_endpoint.py b/oidc_provider/tests/test_end_session_endpoint.py index 46d9cc0..b416762 100644 --- a/oidc_provider/tests/test_end_session_endpoint.py +++ b/oidc_provider/tests/test_end_session_endpoint.py @@ -11,6 +11,7 @@ from oidc_provider.tests.app.utils import ( create_fake_client, create_fake_user, ) +import mock class EndSessionTestCase(TestCase): @@ -35,7 +36,7 @@ class EndSessionTestCase(TestCase): } response = self.client.get(self.url, query_params) # With no id_token the OP MUST NOT redirect to the requested redirect_uri. - self.assertRedirects(response, settings.get('LOGIN_URL'), fetch_redirect_response=False) + self.assertRedirects(response, settings.get('OIDC_LOGIN_URL'), fetch_redirect_response=False) id_token_dic = create_id_token(user=self.user, aud=self.oidc_client.client_id) id_token = encode_id_token(id_token_dic, self.oidc_client) @@ -44,3 +45,10 @@ class EndSessionTestCase(TestCase): response = self.client.get(self.url, query_params) self.assertRedirects(response, self.LOGOUT_URL, fetch_redirect_response=False) + + @mock.patch(settings.get('OIDC_AFTER_END_SESSION_HOOK')) + def test_call_post_end_session_hook(self, hook_function): + self.client.get(self.url) + self.assertTrue(hook_function.called, 'OIDC_AFTER_END_SESSION_HOOK should be called') + self.assertTrue(hook_function.call_count == 1, 'OIDC_AFTER_END_SESSION_HOOK should be called once but was {}'.format(hook_function.call_count)) + diff --git a/oidc_provider/tests/test_middleware.py b/oidc_provider/tests/test_middleware.py new file mode 100644 index 0000000..c2a02df --- /dev/null +++ b/oidc_provider/tests/test_middleware.py @@ -0,0 +1,39 @@ +from django.conf.urls import url +from django.test import TestCase, override_settings +from django.views.generic import View +from mock import mock + + +class StubbedViews: + class SampleView(View): + pass + + urlpatterns = [url('^test/', SampleView.as_view())] + +MW_CLASSES = ('django.contrib.sessions.middleware.SessionMiddleware', + 'oidc_provider.middleware.SessionManagementMiddleware') + + +@override_settings(ROOT_URLCONF=StubbedViews, + MIDDLEWARE=MW_CLASSES, + MIDDLEWARE_CLASSES=MW_CLASSES, + OIDC_SESSION_MANAGEMENT_ENABLE=True) +class MiddlewareTestCase(TestCase): + + def setUp(self): + patcher = mock.patch('oidc_provider.middleware.get_browser_state_or_default') + self.mock_get_state = patcher.start() + + def test_session_management_middleware_sets_cookie_on_response(self): + response = self.client.get('/test/') + + self.assertIn('op_browser_state', response.cookies) + self.assertEqual(response.cookies['op_browser_state'].value, + str(self.mock_get_state.return_value)) + self.mock_get_state.assert_called_once_with(response.wsgi_request) + + @override_settings(OIDC_SESSION_MANAGEMENT_ENABLE=False) + def test_session_management_middleware_does_not_set_cookie_if_session_management_disabled(self): + response = self.client.get('/test/') + + self.assertNotIn('op_browser_state', response.cookies) diff --git a/oidc_provider/tests/test_settings.py b/oidc_provider/tests/test_settings.py new file mode 100644 index 0000000..e8c252a --- /dev/null +++ b/oidc_provider/tests/test_settings.py @@ -0,0 +1,25 @@ +from django.test import TestCase, override_settings + +from oidc_provider import settings + +CUSTOM_TEMPLATES = { + 'authorize': 'custom/authorize.html', + 'error': 'custom/error.html' +} + + +class SettingsTest(TestCase): + + @override_settings(OIDC_TEMPLATES=CUSTOM_TEMPLATES) + def test_override_templates(self): + self.assertEqual(settings.get('OIDC_TEMPLATES'), CUSTOM_TEMPLATES) + + def test_unauthenticated_session_management_key_has_default(self): + key = settings.get('OIDC_UNAUTHENTICATED_SESSION_MANAGEMENT_KEY') + self.assertRegexpMatches(key, r'[a-zA-Z0-9]+') + self.assertGreater(len(key), 50) + + def test_unauthenticated_session_management_key_has_constant_value(self): + key1 = settings.get('OIDC_UNAUTHENTICATED_SESSION_MANAGEMENT_KEY') + key2 = settings.get('OIDC_UNAUTHENTICATED_SESSION_MANAGEMENT_KEY') + self.assertEqual(key1, key2) diff --git a/oidc_provider/tests/test_token_endpoint.py b/oidc_provider/tests/test_token_endpoint.py index 7660240..46e96e4 100644 --- a/oidc_provider/tests/test_token_endpoint.py +++ b/oidc_provider/tests/test_token_endpoint.py @@ -18,7 +18,7 @@ from django.test import TestCase from jwkest.jwk import KEYS from jwkest.jws import JWS from jwkest.jwt import JWT -from mock import patch +from mock import patch, Mock from oidc_provider.lib.utils.token import create_code from oidc_provider.models import Token @@ -50,6 +50,14 @@ class TokenTestCase(TestCase): self.user = create_fake_user() self.client = create_fake_client(response_type='code') + def _password_grant_post_data(self): + return { + 'username': 'johndoe', + 'password': '1234', + 'grant_type': 'password', + 'scope': 'openid email', + } + def _auth_code_post_data(self, code): """ All the data that will be POSTed to the Token Endpoint. @@ -127,6 +135,123 @@ class TokenTestCase(TestCase): return userinfo(request) + def _password_grant_auth_header(self): + user_pass = self.client.client_id + ':' + self.client.client_secret + auth = b'Basic ' + b64encode(user_pass.encode('utf-8')) + auth_header = {'HTTP_AUTHORIZATION': auth.decode('utf-8')} + return auth_header + + # Resource Owner Password Credentials Grant + # requirements to satisfy in all test_password_grant methods + # https://tools.ietf.org/html/rfc6749#section-4.3.2 + # + # grant_type + # REQUIRED. Value MUST be set to "password". + # username + # REQUIRED. The resource owner username. + # password + # REQUIRED. The resource owner password. + # scope + # OPTIONAL. The scope of the access request as described by + # Section 3.3. + # + # The authorization server MUST: + # o require client authentication for confidential clients or for any + # client that was issued client credentials (or with other + # authentication requirements), + # o authenticate the client if client authentication is included, and + # o validate the resource owner password credentials using its + # existing password validation algorithm. + + def test_default_setting_does_not_allow_grant_type_password(self): + post_data = self._password_grant_post_data() + + response = self._post_request( + post_data=post_data, + extras=self._password_grant_auth_header() + ) + + response_dict = json.loads(response.content.decode('utf-8')) + + self.assertEqual(400, response.status_code) + self.assertEqual('unsupported_grant_type', response_dict['error']) + + @override_settings(OIDC_GRANT_TYPE_PASSWORD_ENABLE=True) + def test_password_grant_get_access_token_without_scope(self): + post_data = self._password_grant_post_data() + del (post_data['scope']) + + response = self._post_request( + post_data=post_data, + extras=self._password_grant_auth_header() + ) + + response_dict = json.loads(response.content.decode('utf-8')) + self.assertIn('access_token', response_dict) + + @override_settings(OIDC_GRANT_TYPE_PASSWORD_ENABLE=True) + def test_password_grant_get_access_token_with_scope(self): + response = self._post_request( + post_data=self._password_grant_post_data(), + extras=self._password_grant_auth_header() + ) + + response_dict = json.loads(response.content.decode('utf-8')) + self.assertIn('access_token', response_dict) + + @override_settings(OIDC_GRANT_TYPE_PASSWORD_ENABLE=True) + def test_password_grant_get_access_token_invalid_user_credentials(self): + invalid_post = self._password_grant_post_data() + invalid_post['password'] = 'wrong!' + + response = self._post_request( + post_data=invalid_post, + extras=self._password_grant_auth_header() + ) + + response_dict = json.loads(response.content.decode('utf-8')) + + self.assertEqual(403, response.status_code) + self.assertEqual('access_denied', response_dict['error']) + + def test_password_grant_get_access_token_invalid_client_credentials(self): + self.client.client_id = 'foo' + self.client.client_secret = 'bar' + + response = self._post_request( + post_data=self._password_grant_post_data(), + extras=self._password_grant_auth_header() + ) + + response_dict = json.loads(response.content.decode('utf-8')) + + self.assertEqual(400, response.status_code) + self.assertEqual('invalid_client', response_dict['error']) + + @patch('oidc_provider.lib.utils.token.uuid') + @override_settings(OIDC_TOKEN_EXPIRE=120, + OIDC_GRANT_TYPE_PASSWORD_ENABLE=True) + def test_password_grant_full_response(self, mock_uuid): + test_hex = 'fake_token' + mock_uuid4 = Mock(spec=uuid.uuid4) + mock_uuid4.hex = test_hex + mock_uuid.uuid4.return_value = mock_uuid4 + + response = self._post_request( + post_data=self._password_grant_post_data(), + extras=self._password_grant_auth_header() + ) + + response_dict = json.loads(response.content.decode('utf-8')) + id_token = JWS().verify_compact(response_dict['id_token'].encode('utf-8'), self._get_keys()) + + self.assertEqual(response_dict['access_token'], 'fake_token') + self.assertEqual(response_dict['refresh_token'], 'fake_token') + self.assertEqual(response_dict['expires_in'], 120) + self.assertEqual(response_dict['token_type'], 'bearer') + self.assertEqual(id_token['sub'], str(self.user.id)) + self.assertEqual(id_token['aud'], self.client.client_id) + @override_settings(OIDC_TOKEN_EXPIRE=720) def test_authorization_code(self): """ @@ -150,7 +275,7 @@ class TokenTestCase(TestCase): self.assertEqual(response_dic['token_type'], 'bearer') self.assertEqual(response_dic['expires_in'], 720) self.assertEqual(id_token['sub'], str(self.user.id)) - self.assertEqual(id_token['aud'], self.client.client_id); + self.assertEqual(id_token['aud'], self.client.client_id) def test_refresh_token(self): """ @@ -308,12 +433,7 @@ class TokenTestCase(TestCase): del basicauth_data['client_id'] del basicauth_data['client_secret'] - # Generate HTTP Basic Auth header with id and secret. - user_pass = self.client.client_id + ':' + self.client.client_secret - auth_header = b'Basic ' + b64encode(user_pass.encode('utf-8')) - response = self._post_request(basicauth_data, { - 'HTTP_AUTHORIZATION': auth_header.decode('utf-8'), - }) + response = self._post_request(basicauth_data, self._password_grant_auth_header()) response.content.decode('utf-8') self.assertEqual('invalid_client' in response.content.decode('utf-8'), diff --git a/oidc_provider/tests/test_utils.py b/oidc_provider/tests/test_utils.py index fd09c40..b09ff46 100644 --- a/oidc_provider/tests/test_utils.py +++ b/oidc_provider/tests/test_utils.py @@ -1,10 +1,13 @@ import time from datetime import datetime +from hashlib import sha224 -from django.test import TestCase +from django.http import HttpRequest +from django.test import TestCase, override_settings from django.utils import timezone +from mock import mock -from oidc_provider.lib.utils.common import get_issuer +from oidc_provider.lib.utils.common import get_issuer, get_browser_state_or_default from oidc_provider.lib.utils.token import create_id_token from oidc_provider.tests.app.utils import create_fake_user @@ -59,6 +62,7 @@ class TokenTest(TestCase): def setUp(self): self.user = create_fake_user() + @override_settings(OIDC_IDTOKEN_EXPIRE=600) def test_create_id_token(self): start_time = int(time.time()) login_timestamp = start_time - 1234 @@ -76,3 +80,19 @@ class TokenTest(TestCase): 'iss': 'http://localhost:8000/openid', 'sub': str(self.user.id), }) + + +class BrowserStateTest(TestCase): + + @override_settings(OIDC_UNAUTHENTICATED_SESSION_MANAGEMENT_KEY='my_static_key') + def test_get_browser_state_uses_value_from_settings_to_calculate_browser_state(self): + request = HttpRequest() + request.session = mock.Mock(session_key=None) + state = get_browser_state_or_default(request) + self.assertEqual(state, sha224('my_static_key'.encode('utf-8')).hexdigest()) + + def test_get_browser_state_uses_session_key_to_calculate_browser_state_if_available(self): + request = HttpRequest() + request.session = mock.Mock(session_key='my_session_key') + state = get_browser_state_or_default(request) + self.assertEqual(state, sha224('my_session_key'.encode('utf-8')).hexdigest()) diff --git a/oidc_provider/views.py b/oidc_provider/views.py index 996c968..7fdbd11 100644 --- a/oidc_provider/views.py +++ b/oidc_provider/views.py @@ -28,7 +28,7 @@ from oidc_provider.lib.errors import ( ClientIdError, RedirectUriError, TokenError, -) + UserAuthError) from oidc_provider.lib.utils.common import ( redirect, get_site_url, @@ -47,6 +47,8 @@ from oidc_provider import signals logger = logging.getLogger(__name__) +OIDC_TEMPLATES = settings.get('OIDC_TEMPLATES') + class AuthorizeView(View): @@ -79,7 +81,7 @@ class AuthorizeView(View): raise AuthorizeError(authorize.params['redirect_uri'], 'interaction_required', authorize.grant_type) if authorize.params['prompt'] == 'login': - return redirect_to_login(request.get_full_path()) + return redirect_to_login(request.get_full_path(), settings.get('OIDC_LOGIN_URL')) if authorize.params['prompt'] == 'select_account': # TODO: see how we can support multiple accounts for the end-user. @@ -103,12 +105,12 @@ class AuthorizeView(View): 'scopes': authorize.get_scopes_information(), } - return render(request, 'oidc_provider/authorize.html', context) + return render(request, OIDC_TEMPLATES['authorize'], context) else: if authorize.params['prompt'] == 'none': raise AuthorizeError(authorize.params['redirect_uri'], 'login_required', authorize.grant_type) - return redirect_to_login(request.get_full_path()) + return redirect_to_login(request.get_full_path(), settings.get('OIDC_LOGIN_URL')) except (ClientIdError, RedirectUriError) as error: context = { @@ -116,7 +118,7 @@ class AuthorizeView(View): 'description': error.description, } - return render(request, 'oidc_provider/error.html', context) + return render(request, OIDC_TEMPLATES['error'], context) except (AuthorizeError) as error: uri = error.create_uri( @@ -167,8 +169,10 @@ class TokenView(View): return TokenEndpoint.response(dic) - except (TokenError) as error: + except TokenError as error: return TokenEndpoint.response(error.create_dict(), status=400) + except UserAuthError as error: + return TokenEndpoint.response(error.create_dict(), status=403) @require_http_methods(['GET', 'POST']) @@ -264,8 +268,10 @@ class EndSessionView(View): id_token_hint = request.GET.get('id_token_hint', '') post_logout_redirect_uri = request.GET.get('post_logout_redirect_uri', '') state = request.GET.get('state', '') + client = None - next_page = settings.get('LOGIN_URL') + next_page = settings.get('OIDC_LOGIN_URL') + after_end_session_hook = settings.get('OIDC_AFTER_END_SESSION_HOOK', import_str=True) if id_token_hint: client_id = client_id_from_id_token(id_token_hint) @@ -283,6 +289,15 @@ class EndSessionView(View): except Client.DoesNotExist: pass + after_end_session_hook( + request=request, + id_token=id_token_hint, + post_logout_redirect_uri=post_logout_redirect_uri, + state=state, + client=client, + next_page=next_page + ) + return logout(request, next_page=next_page) From baad8246c790b23caba246600e189441b97b414a Mon Sep 17 00:00:00 2001 From: Wojciech Bartosiak Date: Fri, 5 May 2017 05:43:39 +0200 Subject: [PATCH 2/3] added TOX for Django 1.11 and PYthon 3.6 --- tox.ini | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tox.ini b/tox.ini index 849f411..ef539ff 100644 --- a/tox.ini +++ b/tox.ini @@ -2,9 +2,10 @@ envlist= clean, - py27-django{17,18,19,110}, - py34-django{17,18,19,110}, - py35-django{18,19,110}, + py27-django{17,18,19,110,111}, + py34-django{17,18,19,110,111}, + py35-django{18,19,110,111}, + py36-django{18,19,110,111}, [testenv] @@ -13,6 +14,7 @@ deps = django18: django>=1.8,<1.9 django19: django>=1.9,<1.10 django110: django>=1.10,<1.11 + django111: django>=1.11,<1.12 coverage mock From 2e36d2a16177b5eeaa9b2192737d8758300cc83f Mon Sep 17 00:00:00 2001 From: Wojciech Bartosiak Date: Mon, 8 May 2017 16:25:44 +0200 Subject: [PATCH 3/3] added python 3.6 and django 1.11 --- .travis.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.travis.yml b/.travis.yml index 2245af5..6c39c62 100644 --- a/.travis.yml +++ b/.travis.yml @@ -3,15 +3,19 @@ python: - "2.7" - "3.4" - "3.5" + - "3.6" env: - DJANGO=1.7 - DJANGO=1.8 - DJANGO=1.9 - DJANGO=1.10 + - DJANGO=1.11 matrix: exclude: - python: "3.5" env: DJANGO=1.7 + - python: "3.6" + env: DJANGO=1.7 install: - pip install tox coveralls script: