Skip to content
This repository was archived by the owner on Jun 12, 2021. It is now read-only.

Commit 5a0f27a

Browse files
committed
A bit of refactoring around client authentication.
Fixed the tests. Made sure the introspection module also got build with the rest.
1 parent 374c04b commit 5a0f27a

8 files changed

Lines changed: 48 additions & 28 deletions

File tree

setup.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ def run_tests(self):
5050
license="Apache 2.0",
5151
url='https://github.com/IdentityPython/oicsrv',
5252
packages=["oidcendpoint", 'oidcendpoint/oidc', 'oidcendpoint/authz',
53-
'oidcendpoint/user_authn', 'oidcendpoint/user_info'],
53+
'oidcendpoint/user_authn', 'oidcendpoint/user_info',
54+
'oidcendpoint/oauth2'],
5455
package_dir={"": "src"},
5556
classifiers=[
5657
"Development Status :: 4 - Beta",

src/oidcendpoint/client_authn.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ def valid_client_info(cinfo):
191191
return True
192192

193193

194-
def verify_client(endpoint_context, request, authorization_info):
194+
def verify_client(endpoint_context, request, authorization_info=None):
195195
"""
196196
Initiated Guessing !
197197
@@ -202,7 +202,7 @@ def verify_client(endpoint_context, request, authorization_info):
202202
possibly access token.
203203
"""
204204

205-
if not authorization_info:
205+
if authorization_info is None:
206206
if 'client_id' in request and 'client_secret' in request:
207207
auth_info = ClientSecretPost(endpoint_context).verify(request)
208208
auth_info['method'] = 'client_secret_post'
@@ -232,7 +232,6 @@ def verify_client(endpoint_context, request, authorization_info):
232232
try:
233233
client_id = auth_info['client_id']
234234
except KeyError:
235-
client_id = ''
236235
try:
237236
_token = auth_info['token']
238237
except KeyError:
@@ -258,7 +257,7 @@ def verify_client(endpoint_context, request, authorization_info):
258257
logger.warning('Client registration has timed out')
259258
raise ValueError('Not valid client')
260259
else:
261-
# check that the expected authz method was used
260+
# store what authn method was used
262261
try:
263262
endpoint_context.cdb[client_id]['auth_method'][
264263
request.__class__.__name__] = auth_info['method']

src/oidcendpoint/endpoint.py

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515

1616
__author__ = 'Roland Hedberg'
1717

18-
logger = logging.getLogger(__name__)
18+
LOGGER = logging.getLogger(__name__)
1919

2020
"""
2121
method call structure for Endpoints:
@@ -62,7 +62,7 @@ class Endpoint(object):
6262
request_placement = 'query'
6363
response_format = 'json'
6464
response_placement = 'body'
65-
client_auth_method = ''
65+
client_authn_method = ''
6666

6767
def __init__(self, endpoint_context, **kwargs):
6868
self.endpoint_context = endpoint_context
@@ -81,8 +81,8 @@ def parse_request(self, request, auth=None, **kwargs):
8181
:param kwargs: extra keyword arguments
8282
:return:
8383
"""
84-
logger.debug("- {} -".format(self.endpoint_name))
85-
logger.info("Request: %s" % sanitize(request))
84+
LOGGER.debug("- {} -".format(self.endpoint_name))
85+
LOGGER.info("Request: %s" % sanitize(request))
8686

8787
if request:
8888
if isinstance(request, dict):
@@ -108,7 +108,8 @@ def parse_request(self, request, auth=None, **kwargs):
108108
try:
109109
auth_info = self.client_authentication(req, auth, **kwargs)
110110
except UnknownOrNoAuthnMethod:
111-
if not self.client_auth_method:
111+
# If there is no required client authentication method
112+
if not self.client_authn_method:
112113
try:
113114
_client_id = req['client_id']
114115
except KeyError:
@@ -138,7 +139,7 @@ def parse_request(self, request, auth=None, **kwargs):
138139
return self.error_cls(error="invalid_request",
139140
error_description="%s" % err)
140141

141-
logger.info("Parsed and verified request: %s" % sanitize(req))
142+
LOGGER.info("Parsed and verified request: %s" % sanitize(req))
142143

143144
# Do any endpoint specific parsing
144145
return self.do_post_parse_request(req, _client_id, **kwargs)
@@ -154,7 +155,13 @@ def client_authentication(self, request, auth=None, **kwargs):
154155
:return: client_id or raise an exception
155156
"""
156157

157-
return verify_client(self.endpoint_context, request, auth)
158+
authn_info = verify_client(self.endpoint_context, request, auth)
159+
160+
if authn_info['method'] not in self.client_authn_method:
161+
LOGGER.warning("Wrong client authentication method was used")
162+
raise UnknownOrNoAuthnMethod("Wrong authn method")
163+
164+
return authn_info
158165

159166
def do_post_parse_request(self, request, client_id='', **kwargs):
160167
for meth in self.post_parse_request:
@@ -197,7 +204,7 @@ def construct(self, response_args, request, **kwargs):
197204
"""
198205
response_args = self.do_pre_construct(response_args, request, **kwargs)
199206

200-
# logger.debug("kwargs: %s" % sanitize(kwargs))
207+
# LOGGER.debug("kwargs: %s" % sanitize(kwargs))
201208
response = self.response_cls(**response_args)
202209

203210
return self.do_post_construct(response, request, **kwargs)

src/oidcendpoint/oauth2/introspection.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,13 +34,11 @@ def client_authentication(self, request, auth=None, **kwargs):
3434
except Exception as err:
3535
msg = "Failed to verify client due to: {}".format(err)
3636
LOGGER.error(msg)
37-
return self.error_cls(error="unauthorized_client",
38-
error_description=msg)
37+
return self.error_cls(error="unauthorized_client")
3938
else:
4039
if 'client_id' not in auth_info:
4140
LOGGER.error('No client_id, authentication failed')
42-
return self.error_cls(error="unauthorized_client",
43-
error_description='unknown client')
41+
return self.error_cls(error="unauthorized_client")
4442

4543
return auth_info
4644

src/oidcendpoint/util.py

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,11 +79,9 @@ def build_endpoints(conf, endpoint_context, client_authn_method, issuer):
7979
_instance.provider_info = spec['provider_info']
8080

8181
try:
82-
_client_authn_method = kwargs['client_authn_method']
82+
_instance.client_authn_method = kwargs['client_authn_method']
8383
except KeyError:
84-
_instance.client_auth_method = client_authn_method
85-
else:
86-
_instance.client_auth_method = _client_authn_method
84+
_instance.client_authn_method = client_authn_method
8785

8886
endpoint[name] = _instance
8987

tests/test_02_client_authn.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -299,7 +299,7 @@ def test_verify_client_jws_authn_method():
299299
'client_assertion_type': JWT_BEARER
300300
}
301301

302-
res = verify_client(endpoint_context, request, '')
302+
res = verify_client(endpoint_context, request)
303303
assert res['method'] == 'private_key_jwt'
304304
assert res['client_id'] == 'client_id'
305305

@@ -308,14 +308,14 @@ def test_verify_client_bearer_body():
308308
request = {'access_token': '1234567890', 'client_id': client_id}
309309
sinfo = SessionInfo(authn_req=AuthorizationRequest(client_id= client_id))
310310
endpoint_context.sdb['1234567890'] = sinfo
311-
res = verify_client(endpoint_context, request, '')
311+
res = verify_client(endpoint_context, request)
312312
assert set(res.keys()) == {'token', 'method','client_id'}
313313
assert res['method'] == 'bearer_body'
314314

315315

316316
def test_verify_client_client_secret_post():
317317
request = {'client_id': client_id, 'client_secret': client_secret}
318-
res = verify_client(endpoint_context, request, '')
318+
res = verify_client(endpoint_context, request)
319319
assert set(res.keys()) == {'method','client_id'}
320320
assert res['method'] == 'client_secret_post'
321321

tests/test_26_userinfo_endpoint.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ def create_endpoint(self):
131131
'token_endpoint_auth_method': 'client_secret_post',
132132
'response_types': ['code', 'token', 'code id_token', 'id_token']
133133
}
134-
self.endpoint = userinfo.UserInfo(endpoint_context)
134+
self.endpoint = endpoint_context.endpoint['userinfo']
135135

136136
def test_init(self):
137137
assert self.endpoint

tests/test_31_introspection.py

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from oidcmsg.oauth2 import TokenIntrospectionRequest
77
from oidcmsg.oidc import AuthorizationRequest
88

9+
from oidcendpoint.client_authn import ClientSecretPost
910
from oidcendpoint.client_authn import verify_client
1011
from oidcendpoint.endpoint_context import EndpointContext
1112
from oidcendpoint.oauth2.introspection import Introspection
@@ -77,8 +78,11 @@ def create_endpoint(self):
7778
'path': '{}/intro',
7879
'class': Introspection,
7980
'kwargs': {
80-
"release": ['username']
81-
}
81+
"release": ['username'],
82+
"client_authn_method": {
83+
'client_secrert_post': ClientSecretPost
84+
}
85+
},
8286
}
8387
},
8488
"authentication": {
@@ -91,7 +95,7 @@ def create_endpoint(self):
9195
'userinfo': {
9296
'path': '{}/userinfo',
9397
'class': UserInfo,
94-
'kwargs': {'db_file': 'users.json'}
98+
'kwargs': {'db_file': full_path('users.json')}
9599
},
96100
'client_authn': verify_client,
97101
'template_dir': 'template'
@@ -130,6 +134,19 @@ def test_parse(self):
130134
assert isinstance(_req, TokenIntrospectionRequest)
131135
assert set(_req.keys()) == {"token"}
132136

137+
def test_parse_with_client_auth_in_req(self):
138+
_context = self.introspection_endpoint.endpoint_context
139+
_ = setup_session(_context, AUTH_REQ, uid='diana')
140+
_token = self._create_jwt('diana')
141+
_req = self.introspection_endpoint.parse_request(
142+
{
143+
"token": _token, 'client_id': 'client_1',
144+
'client_secret': _context.cdb['client_1']['client_secret']
145+
})
146+
147+
assert isinstance(_req, TokenIntrospectionRequest)
148+
assert set(_req.keys()) == {"token", "client_id", "client_secret"}
149+
133150
def test_process_request(self):
134151
_ = setup_session(self.introspection_endpoint.endpoint_context,
135152
AUTH_REQ, uid='diana')

0 commit comments

Comments
 (0)