Skip to content

Commit 9375982

Browse files
committed
If cookie parsing fails or if referenced session ID is missing direct the user to reauthentication.
1 parent 5a91372 commit 9375982

File tree

4 files changed

+100
-18
lines changed

4 files changed

+100
-18
lines changed

src/idpyoidc/server/cookie_handler.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -279,15 +279,15 @@ def parse_cookie(self, name: str, cookies: List[dict]) -> Optional[List[dict]]:
279279
LOGGER.debug("Looking for '{}' cookies".format(name))
280280
res = []
281281
for _cookie in cookies:
282-
LOGGER.debug("Cookie: {}".format(_cookie))
282+
LOGGER.debug(f"Cookie: {_cookie}")
283283
if "name" in _cookie and _cookie["name"] == name:
284284
_content = self._ver_dec_content(_cookie["value"].split("|"))
285285
if _content:
286-
payload, timestamp = self._ver_dec_content(_cookie["value"].split("|"))
286+
payload, timestamp = _content
287287
value, typ = payload.split("::")
288288
res.append({"value": value, "type": typ, "timestamp": timestamp})
289289
else:
290-
LOGGER.debug(f"Could not verify {name} cookie")
290+
LOGGER.debug(f"Could not verify '{name}' cookie")
291291
return res
292292

293293

src/idpyoidc/server/oauth2/authorization.py

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -603,21 +603,22 @@ def setup_auth(
603603
if _csi.is_active() is False:
604604
identity = None
605605

606-
authn_args = authn_args_gather(request, authn_class_ref, cinfo, **kwargs)
607606
_mngr = _context.session_manager
608607
_session_id = ""
609608

610609
# To authenticate or Not
611610
if not identity: # No!
612611
logger.info("No active authentication")
613-
logger.debug("Known clients: {}".format(list(_context.cdb.keys())))
612+
# logger.debug("Known clients: {}".format(list(_context.cdb.keys())))
614613

615614
if "prompt" in request and "none" in request["prompt"]:
616615
# Need to authenticate but not allowed
617616
return self._login_required_error(redirect_uri, request)
618617
else:
618+
authn_args = authn_args_gather(request, authn_class_ref, cinfo, **kwargs)
619619
return {"function": authn, "args": authn_args}
620620
else:
621+
authn_args = authn_args_gather(request, authn_class_ref, cinfo, **kwargs)
621622
logger.info(f"Active authentication: {identity}")
622623
if re_authenticate(request, authn):
623624
# demand re-authentication
@@ -994,7 +995,7 @@ def process_request(
994995
cinfo = _context.cdb[_cid]
995996
# logger.debug("client {}: {}".format(_cid, cinfo))
996997

997-
# this apply the default optionally deny_unknown_scopes policy
998+
# this applies the default optionally deny_unknown_scopes policy
998999
check_unknown_scopes_policy(request, _cid, _context)
9991000

10001001
if http_info is None:
@@ -1004,7 +1005,11 @@ def process_request(
10041005
if _cookies:
10051006
logger.debug("parse_cookie@process_request")
10061007
_session_cookie_name = _context.cookie_handler.name["session"]
1007-
_my_cookies = _context.cookie_handler.parse_cookie(_session_cookie_name, _cookies)
1008+
try:
1009+
_my_cookies = _context.cookie_handler.parse_cookie(_session_cookie_name, _cookies)
1010+
except Exception as err:
1011+
logger.info(f"Parse cookie failed due to: {err}")
1012+
_my_cookies = {}
10081013
else:
10091014
_my_cookies = {}
10101015

src/idpyoidc/server/user_authn/user.py

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,9 @@
1313

1414
from idpyoidc.server.exception import FailedAuthentication
1515
from idpyoidc.server.exception import ImproperlyConfigured
16+
from idpyoidc.server.exception import InconsistentDatabase
17+
from idpyoidc.server.exception import NoSuchClientSession
18+
from idpyoidc.server.exception import NoSuchGrant
1619
from idpyoidc.server.exception import OnlyForTestingWarning
1720
from idpyoidc.time_util import utc_time_sans_frac
1821
from idpyoidc.util import instantiate
@@ -64,8 +67,8 @@ def authenticated_as(self, client_id, cookie=None, **kwargs):
6467
return None, 0
6568
else:
6669
_info = self.cookie_info(cookie, client_id)
67-
logger.debug("authenticated_as: cookie info={}".format(_info))
6870
if _info:
71+
logger.debug("authenticated_as: cookie info={}".format(_info))
6972
if "max_age" in kwargs and kwargs["max_age"] != 0:
7073
_max_age = kwargs["max_age"]
7174
_now = utc_time_sans_frac()
@@ -74,6 +77,9 @@ def authenticated_as(self, client_id, cookie=None, **kwargs):
7477
"Too old by {} seconds".format(_now - (_info["timestamp"] + _max_age))
7578
)
7679
return None, 0
80+
else:
81+
logger.info("Failed to find session based on cookie")
82+
7783
return _info, utc_time_sans_frac()
7884

7985
def verify(self, *args, **kwargs):
@@ -109,9 +115,18 @@ def cookie_info(self, cookie: List[dict], client_id: str) -> dict:
109115
for val in cookie:
110116
_info = json.loads(val["value"])
111117
_info["timestamp"] = int(val["timestamp"])
118+
119+
# verify session ID
120+
try:
121+
_context.session_manager[_info["sid"]]
122+
except (KeyError, ValueError, InconsistentDatabase,
123+
NoSuchClientSession, NoSuchGrant) as err:
124+
logger.info(f"Verifying session ID fail due to {err}")
125+
return {}
126+
112127
session_id = _context.session_manager.decrypt_session_id(_info["sid"])
113128
logger.debug("cookie_info: session id={}".format(session_id))
114-
# _, cid, _ = _context.session_manager.decrypt_session_id(_info["sid"])
129+
115130
if session_id[1] != client_id:
116131
continue
117132
else:
@@ -138,13 +153,13 @@ class UserPassJinja2(UserAuthnMethod):
138153
url_endpoint = "/verify/user_pass_jinja"
139154

140155
def __init__(
141-
self,
142-
db,
143-
template_handler,
144-
template="user_pass.jinja2",
145-
server_get=None,
146-
verify_endpoint="",
147-
**kwargs,
156+
self,
157+
db,
158+
template_handler,
159+
template="user_pass.jinja2",
160+
server_get=None,
161+
verify_endpoint="",
162+
**kwargs,
148163
):
149164

150165
super(UserPassJinja2, self).__init__(server_get=server_get)

tests/test_server_24_oidc_authorization_endpoint.py

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1362,6 +1362,20 @@ def create_endpoint_context(self):
13621362
}
13631363
server = Server(OPConfiguration(conf=conf, base_path=BASEDIR), cwd=BASEDIR)
13641364
self.endpoint_context = server.endpoint_context
1365+
self.session_manager = self.endpoint_context.session_manager
1366+
self.user_id = "diana"
1367+
1368+
def _create_session(self, auth_req, sub_type="public", sector_identifier=""):
1369+
if sector_identifier:
1370+
authz_req = auth_req.copy()
1371+
authz_req["sector_identifier_uri"] = sector_identifier
1372+
else:
1373+
authz_req = auth_req
1374+
client_id = authz_req["client_id"]
1375+
ae = create_authn_event(self.user_id)
1376+
return self.session_manager.create_session(
1377+
ae, authz_req, self.user_id, client_id=client_id, sub_type=sub_type
1378+
)
13651379

13661380
def test_authenticated_as_without_cookie(self):
13671381
authn_item = self.endpoint_context.authn_broker.pick(INTERNETPROTOCOLPASSWORD)
@@ -1375,11 +1389,35 @@ def test_authenticated_as_with_cookie(self):
13751389
method = authn_item[0]["method"]
13761390

13771391
authn_req = {"state": "state_identifier", "client_id": "client 12345"}
1392+
session_id = self._create_session(authn_req)
1393+
13781394
_cookie = self.endpoint_context.new_cookie(
13791395
name=self.endpoint_context.cookie_handler.name["session"],
13801396
sub="diana",
1397+
sid=session_id,
1398+
state=authn_req["state"],
1399+
client_id=authn_req["client_id"],
1400+
)
1401+
1402+
# Parsed once before setup_auth
1403+
kakor = self.endpoint_context.cookie_handler.parse_cookie(
1404+
cookies=[_cookie], name=self.endpoint_context.cookie_handler.name["session"]
1405+
)
1406+
1407+
_info, _time_stamp = method.authenticated_as(client_id="client 12345", cookie=kakor)
1408+
assert _info["sub"] == "diana"
1409+
1410+
def test_authenticated_as_with_unknown_user(self):
1411+
authn_item = self.endpoint_context.authn_broker.pick(INTERNETPROTOCOLPASSWORD)
1412+
method = authn_item[0]["method"]
1413+
1414+
authn_req = {"state": "state_identifier", "client_id": "client 12345"}
1415+
session_id = self._create_session(authn_req)
1416+
_cookie = self.endpoint_context.new_cookie(
1417+
name=self.endpoint_context.cookie_handler.name["session"],
1418+
sub="adam",
13811419
sid=self.endpoint_context.session_manager.encrypted_session_id(
1382-
"diana", "client 12345", "abcdefgh"
1420+
"adam", "client 12345", "0123456789"
13831421
),
13841422
state=authn_req["state"],
13851423
client_id=authn_req["client_id"],
@@ -1391,4 +1429,28 @@ def test_authenticated_as_with_cookie(self):
13911429
)
13921430

13931431
_info, _time_stamp = method.authenticated_as(client_id="client 12345", cookie=kakor)
1394-
assert _info["sub"] == "diana"
1432+
assert _info == {}
1433+
1434+
def test_authenticated_as_with_goobledigook(self):
1435+
authn_item = self.endpoint_context.authn_broker.pick(INTERNETPROTOCOLPASSWORD)
1436+
method = authn_item[0]["method"]
1437+
1438+
authn_req = {"state": "state_identifier", "client_id": "client 12345"}
1439+
_ = self._create_session(authn_req)
1440+
_cookie = self.endpoint_context.new_cookie(
1441+
name=self.endpoint_context.cookie_handler.name["session"],
1442+
sub="adam",
1443+
sid=self.endpoint_context.session_manager.encrypted_session_id(
1444+
"adam", "client 12345", "0123456789"
1445+
),
1446+
state=authn_req["state"],
1447+
client_id=authn_req["client_id"],
1448+
)
1449+
1450+
kakor = [{
1451+
'value': '{"sub": "adam", "sid": "Z0FBQUFBQmlhVl", "state": "state_identifier", "client_id": "client 12345"}',
1452+
'type': '',
1453+
'timestamp': '1651070251'}]
1454+
1455+
_info, _time_stamp = method.authenticated_as(client_id="client 12345", cookie=kakor)
1456+
assert _info == {}

0 commit comments

Comments
 (0)