From 7305aa210e6ea11e0dd7d2648e997dd5c7ea42aa Mon Sep 17 00:00:00 2001 From: peppelinux Date: Sat, 5 Jun 2021 15:30:56 +0200 Subject: [PATCH 1/2] Cookies improvements * BREAKAGE: Cookies flags in cookie_handler.make_cookie_content now are lowercased by default, they doesn't being correctly loaded by flask and django .set_cookie * chore: flask_op views _add_cookie generalization * feat: additional cookie_handler parameter called `flags` to configure whatever cookie flag we desire * feat: Cookie default flags SameSite, HttpOnly and Secure set to True by default --- example/flask_op/views.py | 7 +++---- src/oidcop/cookie_handler.py | 17 ++++++++++++++-- src/oidcop/endpoint_context.py | 3 ++- tests/test_09_cookie_handler.py | 32 +++++++++++++++++++++++-------- tests/test_30_oidc_end_session.py | 2 +- 5 files changed, 45 insertions(+), 16 deletions(-) diff --git a/example/flask_op/views.py b/example/flask_op/views.py index 06a7f180..f8e485b6 100644 --- a/example/flask_op/views.py +++ b/example/flask_op/views.py @@ -28,10 +28,9 @@ def _add_cookie(resp, cookie_spec): - kwargs = {'value': cookie_spec["value"]} - for param in ['expires', 'max-age']: - if param in cookie_spec: - kwargs[param] = cookie_spec[param] + kwargs = {k:v + for k,v in cookie_spec.items() + if k not in ('name',)} kwargs["path"] = "/" resp.set_cookie(cookie_spec["name"], **kwargs) diff --git a/src/oidcop/cookie_handler.py b/src/oidcop/cookie_handler.py index 4524c94f..b36a0c33 100755 --- a/src/oidcop/cookie_handler.py +++ b/src/oidcop/cookie_handler.py @@ -37,6 +37,7 @@ def __init__( keys: Optional[dict] = None, sign_alg: [str] = "SHA256", name: Optional[dict] = None, + **kwargs ): if keys: @@ -77,6 +78,15 @@ def __init__( else: self.name = name + self.flags = kwargs.get( + 'flags', + { + "samesite": "None", + "httponly": True, + "secure": True, + } + ) + def _sign_enc_payload(self, payload: str, timestamp: Optional[Union[int, str]] = 0): """ Creates signed and/or encrypted information. @@ -211,9 +221,12 @@ def make_cookie_content( content = {"name": name, "value": _cookie_value} if max_age == -1: - content["Expires"] = "Thu, 01 Jan 1970 00:00:00 GMT;" + content["expires"] = "Thu, 01 Jan 1970 00:00:00 GMT;" elif max_age: - content["Max-Age"] = epoch_in_a_while(seconds=max_age) + content["max-age"] = epoch_in_a_while(seconds=max_age) + + for k,v in self.flags.items(): + content[k] = v return content diff --git a/src/oidcop/endpoint_context.py b/src/oidcop/endpoint_context.py index e68cc99c..e3c19dbe 100755 --- a/src/oidcop/endpoint_context.py +++ b/src/oidcop/endpoint_context.py @@ -232,9 +232,10 @@ def __init__( self.claims_interface = None def new_cookie(self, name: str, max_age: Optional[int] = 0, **kwargs): - return self.cookie_handler.make_cookie_content( + cookie_cont = self.cookie_handler.make_cookie_content( name=name, value=json.dumps(kwargs), max_age=max_age ) + return cookie_cont def set_scopes_handler(self): _spec = self.conf.get("scopes_handler") diff --git a/tests/test_09_cookie_handler.py b/tests/test_09_cookie_handler.py index b170faf2..b8b4147e 100644 --- a/tests/test_09_cookie_handler.py +++ b/tests/test_09_cookie_handler.py @@ -25,7 +25,9 @@ def test_init(self): def test_make_cookie_content(self): _cookie_info = self.cookie_handler.make_cookie_content("oidcop", "value", "sso") assert _cookie_info - assert set(_cookie_info.keys()) == {"name", "value"} + assert set(_cookie_info.keys()) == { + "name", "value", "samesite", "httponly", "secure" + } assert len(_cookie_info["value"].split("|")) == 3 def test_make_cookie_content_max_age(self): @@ -33,7 +35,9 @@ def test_make_cookie_content_max_age(self): "oidcop", "value", "sso", max_age=3600 ) assert _cookie_info - assert set(_cookie_info.keys()) == {"name", "value", "Max-Age"} + assert set(_cookie_info.keys()) == { + 'name', 'value', 'max-age', 'samesite', 'httponly', 'secure' + } assert len(_cookie_info["value"].split("|")) == 3 def test_read_cookie_info(self): @@ -72,7 +76,9 @@ def make_cookie_handler(self): def test_make_cookie_content(self): _cookie_info = self.cookie_handler.make_cookie_content("oidcop", "value", "sso") assert _cookie_info - assert set(_cookie_info.keys()) == {"name", "value"} + assert set(_cookie_info.keys()) == { + 'name', 'value', 'samesite', 'httponly', 'secure' + } assert len(_cookie_info["value"].split("|")) == 4 def test_make_cookie_content_max_age(self): @@ -80,7 +86,9 @@ def test_make_cookie_content_max_age(self): "oidcop", "value", "sso", max_age=3600 ) assert _cookie_info - assert set(_cookie_info.keys()) == {"name", "value", "Max-Age"} + assert set(_cookie_info.keys()) == { + 'name', 'value', 'max-age', 'samesite', 'httponly', 'secure' + } assert len(_cookie_info["value"].split("|")) == 4 def test_read_cookie_info(self): @@ -118,7 +126,9 @@ def make_cookie_content_handler(self): def test_make_cookie_content(self): _cookie_info = self.cookie_handler.make_cookie_content("oidcop", "value", "sso") assert _cookie_info - assert set(_cookie_info.keys()) == {"name", "value"} + assert set(_cookie_info.keys()) == { + 'name', 'value', 'samesite', 'httponly', 'secure' + } assert len(_cookie_info["value"].split("|")) == 4 def test_make_cookie_content_max_age(self): @@ -126,7 +136,9 @@ def test_make_cookie_content_max_age(self): "oidcop", "value", "sso", max_age=3600 ) assert _cookie_info - assert set(_cookie_info.keys()) == {"name", "value", "Max-Age"} + assert set(_cookie_info.keys()) == { + 'name', 'value', 'max-age', 'samesite', 'httponly', 'secure' + } assert len(_cookie_info["value"].split("|")) == 4 def test_read_cookie_info(self): @@ -168,7 +180,9 @@ def make_cookie_handler(self): def test_make_cookie_content(self): _cookie_info = self.cookie_handler.make_cookie_content("oidcop", "value", "sso") assert _cookie_info - assert set(_cookie_info.keys()) == {"name", "value"} + assert set(_cookie_info.keys()) == { + 'name', 'value', 'samesite', 'httponly', 'secure' + } assert len(_cookie_info["value"].split("|")) == 4 def test_make_cookie_content_max_age(self): @@ -176,7 +190,9 @@ def test_make_cookie_content_max_age(self): "oidcop", "value", "sso", max_age=3600 ) assert _cookie_info - assert set(_cookie_info.keys()) == {"name", "value", "Max-Age"} + assert set(_cookie_info.keys()) == { + 'name', 'value', 'max-age', 'samesite', 'httponly', 'secure' + } assert len(_cookie_info["value"].split("|")) == 4 def test_read_cookie_info(self): diff --git a/tests/test_30_oidc_end_session.py b/tests/test_30_oidc_end_session.py index 1d827a48..5690cf75 100644 --- a/tests/test_30_oidc_end_session.py +++ b/tests/test_30_oidc_end_session.py @@ -605,7 +605,7 @@ def test_kill_cookies(self): assert set(_names) == {"oidc_op_sman", "oidc_op"} _values = [ci["value"] for ci in _info] assert set(_values) == {"", ""} - _exps = [ci["Expires"] for ci in _info] + _exps = [ci["expires"] for ci in _info] assert set(_exps) == { "Thu, 01 Jan 1970 00:00:00 GMT;", "Thu, 01 Jan 1970 00:00:00 GMT;", From 18ff64c0f286ddaeaa30c9bbec67d04303fb9e8d Mon Sep 17 00:00:00 2001 From: peppelinux Date: Sat, 5 Jun 2021 15:36:13 +0200 Subject: [PATCH 2/2] fix: Documentation about cookie_handler --- docs/source/contents/conf.rst | 46 ++++++++++++++--------------------- 1 file changed, 18 insertions(+), 28 deletions(-) diff --git a/docs/source/contents/conf.rst b/docs/source/contents/conf.rst index d12ba6e1..281394bb 100644 --- a/docs/source/contents/conf.rst +++ b/docs/source/contents/conf.rst @@ -150,42 +150,32 @@ An example:: check_session_iframe: https://127.0.0.1:5000/check_session_iframe ------------ -cookie_name ------------ - -An example:: - - "cookie_name": { - "session": "oidc_op", - "register": "oidc_op_rp", - "session_management": "sman" - }, - ------------- -cookie_dealer +cookie_handler ------------- An example:: - "cookie_dealer": { - "class": "oidcop.cookie.CookieDealer", + "cookie_handler": { + "class": "oidcop.cookie_handler.CookieHandler", "kwargs": { - "sign_jwk": { - "filename": "private/cookie_sign_jwk.json", - "type": "OCT", - "kid": "cookie_sign_key_id" + "keys": { + "private_path": f"{OIDC_JWKS_PRIVATE_PATH}/cookie_jwks.json", + "key_defs": [ + {"type": "OCT", "use": ["enc"], "kid": "enc"}, + {"type": "OCT", "use": ["sig"], "kid": "sig"} + ], + "read_only": False }, - "enc_jwk": { - "filename": "private/cookie_enc_jwk.json", - "type": "OCT", - "kid": "cookie_enc_key_id" + "flags": { + "samesite": "None", + "httponly": True, + "secure": True, }, - "default_values": { - "name": "oidc_op", - "domain": "127.0.0.1", - "path": "/", - "max_age": 3600 + "name": { + "session": "oidc_op", + "register": "oidc_op_rp", + "session_management": "sman" } } },