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

Commit

Permalink
Corrected nonce usage
Browse files Browse the repository at this point in the history
The nonce passed to the initial authorization request is now set in the ID token when an access token is requested. This conforms to the OpenID Connect spec.

LEARNER-693
  • Loading branch information
Clinton Blackburn committed Apr 25, 2017
1 parent a1f6080 commit cd5e504
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 24 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
requirements:
pip install -r requirements.txt
pip install --process-dependency-links -r requirements.txt

test:
coverage run ./manage.py test
Expand Down
2 changes: 1 addition & 1 deletion edx_oauth2_provider/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,4 @@
for the edx-platform.
"""
__version__ = '1.2.0'
__version__ = '1.2.1'
5 changes: 5 additions & 0 deletions edx_oauth2_provider/tests/base.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# pylint: disable=missing-docstring
import json
import uuid
from urlparse import urlparse

from django.core.urlresolvers import reverse
Expand All @@ -23,6 +24,8 @@

class BaseTestCase(TestCase):
def setUp(self):
super(BaseTestCase, self).setUp()

self.client_secret = 'some_secret'
self.auth_client = ClientFactory(client_secret=self.client_secret)

Expand All @@ -49,6 +52,7 @@ def set_trusted(self, client, trusted=True):
class OAuth2TestCase(BaseTestCase):
def setUp(self):
super(OAuth2TestCase, self).setUp()
self.nonce = str(uuid.uuid4())

def login_and_authorize(self, scope=None, claims=None, trusted=False, validate_session=True):
""" Login into client using OAuth2 authorization flow. """
Expand All @@ -62,6 +66,7 @@ def login_and_authorize(self, scope=None, claims=None, trusted=False, validate_s
'redirect_uri': self.auth_client.redirect_uri,
'response_type': 'code',
'state': 'some_state',
'nonce': self.nonce,
}
_add_values(payload, 'id_token', scope, claims)

Expand Down
30 changes: 20 additions & 10 deletions edx_oauth2_provider/tests/test_oidc.py
Original file line number Diff line number Diff line change
@@ -1,21 +1,19 @@
# pylint: disable=missing-docstring

import datetime
import json
import uuid

from django.conf import settings
from django.test.utils import override_settings

import datetime
import jwt
import mock
from django.conf import settings
from django.test.utils import override_settings

from provider.oauth2.models import Grant
from provider.scope import check
from .. import oidc
from .. import constants
from ..oidc.core import IDToken
from .base import OAuth2TestCase
from .factories import AccessTokenFactory
from .. import constants
from .. import oidc
from ..oidc.core import IDToken

BASE_DATETIME = datetime.datetime(1970, 1, 1)

Expand All @@ -25,7 +23,6 @@
class BaseTestCase(OAuth2TestCase):
def setUp(self):
super(BaseTestCase, self).setUp()
self.nonce = unicode(uuid.uuid4())
self.access_token = AccessTokenFactory(user=self.user, client=self.auth_client)


Expand Down Expand Up @@ -102,6 +99,18 @@ def test_get_id_token_with_profile_and_email(self):
expected = self._get_expected_claims(self.access_token, self.nonce)
self.assertEqual(claims, expected)

def test_id_token_nonce(self):
""" The nonce in the ID token should be the same nonce passed to the initial authorization call. """
response = self.get_access_token_response(scope='openid')

# Validate Grant has an associated nonce
self.assertEqual(Grant.objects.filter(client=self.auth_client, user=self.user, nonce=self.nonce).count(), 1)

# Validate ID token nonce
response = json.loads(response.content)
id_token = jwt.decode(response['id_token'], verify=False)
self.assertEqual(id_token['nonce'], self.nonce)


class UserInfoTest(BaseTestCase):
def assertIncludedClaims(self, claims, expected_scope=None, expected_claims=None):
Expand Down Expand Up @@ -146,6 +155,7 @@ def test_not_recognized_values(self):

def test_arguments(self):
""" Test if the responses contain the requested claims according to permissions"""

# TODO: replace with DDT test

def userinfo_req(req):
Expand Down
11 changes: 4 additions & 7 deletions edx_oauth2_provider/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def get_authorization_form(self, _request, client, data, client_data):
trusted = TrustedClient.objects.filter(client=client).exists()
if trusted:
scope_names = provider.scope.to_names(client_data['scope'])
data = {'authorize': [u'Authorize'], 'scope': scope_names}
data = {'authorize': ['Authorize'], 'scope': scope_names, 'nonce': client_data.get('nonce', '')}

form = AuthorizationForm(data)
return form
Expand Down Expand Up @@ -101,7 +101,7 @@ def get_password_grant(self, _request, data, client):
return form.cleaned_data

# pylint: disable=super-on-old-class
def access_token_response_data(self, access_token):
def access_token_response_data(self, access_token, response_type=None, nonce=''):
"""
Return `access_token` fields for OAuth2, and add `id_token` fields for
OpenID Connect according to the `access_token` scope.
Expand All @@ -121,7 +121,7 @@ def access_token_response_data(self, access_token):
# requested, as required by OpenID Connect specification.

if provider.scope.check(constants.OPEN_ID_SCOPE, access_token.scope):
id_token = self.get_id_token(access_token)
id_token = self.get_id_token(access_token, nonce)
extra_data['id_token'] = self.encode_id_token(id_token)
scope = provider.scope.to_int(*id_token.scopes)

Expand All @@ -138,15 +138,12 @@ def access_token_response_data(self, access_token):

return response_data

def get_id_token(self, access_token):
def get_id_token(self, access_token, nonce):
""" Return an ID token for the given Access Token. """

claims_string = self.request.POST.get('claims')
claims_request = json.loads(claims_string) if claims_string else {}

# Use a nonce to prevent replay attacks.
nonce = self.request.POST.get('nonce')

return oidc.id_token(access_token, nonce, claims_request)

def encode_id_token(self, id_token):
Expand Down
8 changes: 4 additions & 4 deletions requirements.txt
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
-e .
coverage==4.1
ddt==1.1.0
django-nose==1.4.3
factory_boy==2.7.0
coverage==4.3.4
ddt==1.1.1
django-nose==1.4.4
factory_boy==2.8.1
mock==2.0.0
2 changes: 1 addition & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
packages=find_packages(exclude=['tests']),
install_requires=[
'django>=1.8.7,<1.9',
'edx-django-oauth2-provider>=0.3.0,<2.0.0',
'edx-django-oauth2-provider>=1.2.0,<2.0.0',
'PyJWT>=1.4.0,<2.0.0'
]
)

0 comments on commit cd5e504

Please sign in to comment.