Skip to content

Commit

Permalink
Store the signed ID Token in user session.
Browse files Browse the repository at this point in the history
Previously the real signature was removed, and a plain (unsigned) JWT
containing the same claims as the original ID Token was stored in the
session. This prevented it from being properly validated when forwarded
as 'id_token_hint' in logout requests.
  • Loading branch information
zamzterz committed Sep 25, 2018
1 parent 63ff1a5 commit ea97af2
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 23 deletions.
8 changes: 5 additions & 3 deletions src/flask_pyoidc/flask_pyoidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,6 @@ def _handle_authentication_response(self):
access_token = token_resp['access_token']

id_token_claims = None
id_token_jwt = None
if 'id_token' in token_resp:
id_token = token_resp['id_token']
logger.debug('received id token: %s', id_token.to_json())
Expand All @@ -128,7 +127,6 @@ def _handle_authentication_response(self):
raise ValueError('The \'nonce\' parameter does not match.')

id_token_claims = id_token.to_dict()
id_token_jwt = id_token.to_jwt()
# set the session as requested by the OP if we have no default
if current_app.config.get('OIDC_SESSION_PERMANENT', True):
flask.session.permanent = True
Expand All @@ -143,7 +141,11 @@ def _handle_authentication_response(self):
if id_token_claims and userinfo_claims and userinfo_claims['sub'] != id_token_claims['sub']:
raise ValueError('The \'sub\' of userinfo does not match \'sub\' of ID Token.')

UserSession(flask.session).update(time.time(), access_token, id_token_claims, id_token_jwt, userinfo_claims)
UserSession(flask.session).update(time.time(),
access_token,
id_token_claims,
token_resp.get('id_token_jwt'),
userinfo_claims)

destination = flask.session.pop('destination')
return redirect(destination)
Expand Down
5 changes: 4 additions & 1 deletion src/flask_pyoidc/pyoidc_facade.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import base64
import json

import logging
from oic.oic import Client, ProviderConfigurationResponse, RegistrationResponse, AuthorizationResponse, \
Expand Down Expand Up @@ -126,14 +127,16 @@ def token_request(self, authorization_code):
data=request,
headers=auth_header) \
.json()
logger.debug('received token response: %s', json.dumps(resp))

if 'error' in resp:
token_resp = TokenErrorResponse(**resp)
else:
token_resp = AccessTokenResponse(**resp)
token_resp.verify(keyjar=self._client.keyjar)
if 'id_token' in resp:
token_resp['id_token_jwt'] = resp['id_token']

logger.debug('received token response: %s', token_resp.to_json())
return token_resp

def userinfo_request(self, access_token):
Expand Down
Empty file added tests/__init__.py
Empty file.
25 changes: 16 additions & 9 deletions tests/test_flask_pyoidc.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from flask_pyoidc.provider_configuration import ProviderConfiguration, ProviderMetadata, ClientMetadata, \
ClientRegistrationInfo
from flask_pyoidc.user_session import UserSession
from .util import signed_id_token


class TestOIDCAuthentication(object):
Expand Down Expand Up @@ -148,16 +149,22 @@ def test_handle_authentication_response(self, time_mock, utc_time_sans_frac_mock
user_id = 'user1'
exp_time = 10
nonce = 'test_nonce'
id_token = IdToken(iss=self.PROVIDER_BASEURL,
aud=self.CLIENT_ID,
sub=user_id,
exp=int(timestamp) + exp_time,
iat=int(timestamp),
nonce=nonce)
id_token_claims = {
'iss': self.PROVIDER_BASEURL,
'aud': [self.CLIENT_ID],
'sub': user_id,
'exp': int(timestamp) + exp_time,
'iat': int(timestamp),
'nonce': nonce
}
id_token_jwt, id_token_signing_key = signed_id_token(id_token_claims)
access_token = 'test_access_token'
token_response = {'access_token': access_token, 'token_type': 'Bearer', 'id_token': id_token.to_jwt()}
token_response = {'access_token': access_token, 'token_type': 'Bearer', 'id_token': id_token_jwt}
token_endpoint = self.PROVIDER_BASEURL + '/token'
responses.add(responses.POST, token_endpoint, json=token_response)
responses.add(responses.GET,
self.PROVIDER_BASEURL + '/jwks',
json={'keys': [id_token_signing_key.serialize()]})

# mock userinfo response
userinfo = {'sub': user_id, 'name': 'Test User'}
Expand All @@ -175,8 +182,8 @@ def test_handle_authentication_response(self, time_mock, utc_time_sans_frac_mock
authn._handle_authentication_response()
session = UserSession(flask.session)
assert session.access_token == access_token
assert session.id_token == id_token.to_dict()
assert IdToken().from_jwt(session.id_token_jwt) == id_token
assert session.id_token == id_token_claims
assert session.id_token_jwt == id_token_jwt
assert session.userinfo == userinfo

@patch('time.time')
Expand Down
28 changes: 18 additions & 10 deletions tests/test_pyoidc_facade.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
import time

import base64
import json
import pytest
import responses
from oic.oic import AuthorizationResponse, AccessTokenResponse, IdToken, TokenErrorResponse, OpenIDSchema
from oic.oic import AuthorizationResponse, AccessTokenResponse, TokenErrorResponse, OpenIDSchema
from six.moves.urllib.parse import parse_qsl, urlparse

from flask_pyoidc.provider_configuration import ProviderConfiguration, ClientMetadata, ProviderMetadata, \
ClientRegistrationInfo
from flask_pyoidc.pyoidc_facade import PyoidcFacade, _ClientAuthentication
from .util import signed_id_token


class TestPyoidcFacade(object):
Expand Down Expand Up @@ -92,15 +92,19 @@ def test_parse_authentication_response(self):
@responses.activate
def test_token_request(self):
token_endpoint = self.PROVIDER_BASEURL + '/token'
id_token = IdToken(iss=self.PROVIDER_METADATA['issuer'],
sub='test_user',
aud=self.CLIENT_METADATA['client_id'],
exp=time.time() + 1,
iat=time.time(),
nonce='test_nonce')
now = int(time.time())
id_token_claims = {
'iss': self.PROVIDER_METADATA['issuer'],
'sub': 'test_user',
'aud': [self.CLIENT_METADATA['client_id']],
'exp': now + 1,
'iat': now,
'nonce': 'test_nonce'
}
id_token_jwt, id_token_signing_key = signed_id_token(id_token_claims)
token_response = AccessTokenResponse(access_token='test_access_token',
token_type='Bearer',
id_token=id_token.to_jwt())
id_token=id_token_jwt)
responses.add(responses.POST, token_endpoint, json=token_response.to_dict())

provider_metadata = self.PROVIDER_METADATA.copy(token_endpoint=token_endpoint)
Expand All @@ -109,10 +113,14 @@ def test_token_request(self):
self.REDIRECT_URI)

auth_code = 'auth_code-1234'
responses.add(responses.GET,
self.PROVIDER_METADATA['jwks_uri'],
json={'keys': [id_token_signing_key.serialize()]})
token_response = facade.token_request(auth_code)

expected_token_response = token_response.to_dict()
expected_token_response['id_token'] = id_token.to_dict()
expected_token_response['id_token'] = id_token_claims
expected_token_response['id_token_jwt'] = id_token_jwt
assert token_response.to_dict() == expected_token_response

token_request = dict(parse_qsl(responses.calls[0].request.body))
Expand Down
10 changes: 10 additions & 0 deletions tests/util.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
from jwkest.jwk import SYMKey
from oic import rndstr
from oic.oic import IdToken


def signed_id_token(claims):
id_token = IdToken(**claims)
signing_key = SYMKey(alg='HS256', key=rndstr())
jws = id_token.to_jwt(key=[signing_key], algorithm=signing_key.alg)
return jws, signing_key

0 comments on commit ea97af2

Please sign in to comment.