From bac06a4fc580571414dcadea7eceabddb6cc0153 Mon Sep 17 00:00:00 2001 From: Roland Hedberg Date: Sat, 1 Feb 2020 15:48:45 +0100 Subject: [PATCH] Session cookie need to be visible to OP IFrame Javascript script. (#731) * Session cookie need to be visible to OP IFrame Javascript script. Even if no back-/frontchannel logout is defined the session cookie should be removed. * Added SameSite. --- CHANGELOG.md | 2 ++ src/oic/oauth2/provider.py | 20 ++++++++++++++++++-- src/oic/oic/provider.py | 17 +++++++++++++---- src/oic/utils/http_util.py | 16 +++++++++++++--- tests/test_oic_provider_logout.py | 3 ++- 5 files changed, 48 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ee09f98b2..77b811b02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,10 +13,12 @@ The format is based on the [KeepAChangeLog] project. ### Added - [#719] Add support for JWT registration tokens - [#728] OAuth client request using Extension grant +- [#731] Session cookie need to be visible to OP IFrame. [#719]: https://github.com/OpenIDC/pyoidc/pull/719 [#727]: https://github.com/OpenIDC/pyoidc/issues/727 [#728]: https://github.com/OpenIDC/pyoidc/issues/728 +[#731]: https://github.com/OpenIDC/pyoidc/pull/731 ## 1.1.2 [2019-11-23] diff --git a/src/oic/oauth2/provider.py b/src/oic/oauth2/provider.py index 58a9f6f2d..b0692190e 100644 --- a/src/oic/oauth2/provider.py +++ b/src/oic/oauth2/provider.py @@ -1115,8 +1115,15 @@ def verify_endpoint(self, request="", cookie=None, **kwargs): kwargs["cookie"] = cookie return authn.verify(_req, **kwargs) - def write_session_cookie(self, value): - return make_cookie(self.session_cookie_name, value, self.seed, path="/") + def write_session_cookie(self, value, http_only=True, same_site=""): + return make_cookie( + self.session_cookie_name, + value, + self.seed, + path="/", + httponly=http_only, + same_site=same_site, + ) def delete_session_cookie(self): return make_cookie(self.session_cookie_name, "", b"", path="/", expire=-1) @@ -1124,5 +1131,14 @@ def delete_session_cookie(self): def _compute_session_state(self, state, salt, client_id, redirect_uri): parsed_uri = urlparse(redirect_uri) rp_origin_url = "{uri.scheme}://{uri.netloc}".format(uri=parsed_uri) + + logger.debug( + "Calculating sessions state using, client_id:%s origin:%s state:%s salt:%s", + client_id, + rp_origin_url, + state, + salt, + ) + session_str = client_id + " " + rp_origin_url + " " + state + " " + salt return hashlib.sha256(session_str.encode("utf-8")).hexdigest() + "." + salt diff --git a/src/oic/oic/provider.py b/src/oic/oic/provider.py index d01c9c9fa..7adc8ddcd 100644 --- a/src/oic/oic/provider.py +++ b/src/oic/oic/provider.py @@ -679,7 +679,9 @@ def authz_part2(self, user, areq, sid, **kwargs): aresp["session_state"] = self._compute_session_state( state, salt, areq["client_id"], redirect_uri ) - headers.append(self.write_session_cookie(state)) + headers.append( + self.write_session_cookie(state, http_only=False, same_site="None") + ) # as per the mix-up draft don't add iss and client_id if they are # already in the id_token. @@ -2256,7 +2258,7 @@ def unpack_signed_jwt(self, sjwt: str): def do_verified_logout( self, sid: str, client_id: str, alla: bool = False, **kwargs - ) -> Dict: + ) -> Union[dict, Dict[str, list]]: """ Perform back channel logout and prepares the information needed for front channel logout. @@ -2281,7 +2283,14 @@ def do_verified_logout( self.events.store("object args", "{}".format(logout_spec)) if not logout_spec["back_channel"] and not logout_spec["front_channel"]: - return {} + # kill cookies + kaka1 = self.write_session_cookie( + "removed", http_only=False, same_site="None" + ) + kaka2 = self.cookie_func( + "", typ="sso", cookie_name=self.sso_cookie_name, kill=True + ) + return {"cookie": [kaka1, kaka2]} # take care of Back channel logout first if logout_spec["back_channel"]: @@ -2321,7 +2330,7 @@ def do_verified_logout( return {} # kill cookies - kaka1 = self.write_session_cookie("removed") + kaka1 = self.write_session_cookie("removed", http_only=False, same_site="None") kaka2 = self.cookie_func( "", typ="sso", cookie_name=self.sso_cookie_name, kill=True ) diff --git a/src/oic/utils/http_util.py b/src/oic/utils/http_util.py index 69348b1c3..7f75ea050 100644 --- a/src/oic/utils/http_util.py +++ b/src/oic/utils/http_util.py @@ -6,10 +6,8 @@ import time from http import client from http.cookies import SimpleCookie -from typing import Dict # noqa from typing import List # noqa from typing import Tuple # noqa -from typing import Union # noqa from urllib.parse import quote from jwkest import as_unicode @@ -324,6 +322,7 @@ def make_cookie( enc_key=None, secure=True, httponly=True, + same_site="", ): """ Create and return a cookie. @@ -354,6 +353,13 @@ def make_cookie( :type timestamp: text :param enc_key: The key to use for cookie encryption. :type enc_key: byte string + :param secure: A secure cookie is only sent to the server with an encrypted request over the + HTTPS protocol. + :type enc_key: boolean + :param httponly: HttpOnly cookies are inaccessible to JavaScript's Document.cookie API + :type enc_key: boolean + :param same_site: Whether SameSite (None,Strict or Lax) should be added to the cookie + :type enc_key: byte string :return: A tuple to be added to headers """ cookie = SimpleCookie() # type: SimpleCookie @@ -394,6 +400,8 @@ def make_cookie( ] cookie[name] = (b"|".join(cookie_payload)).decode("utf-8") + cookie[name]._reserved[str("samesite")] = str("SameSite") # type: ignore + if path: cookie[name]["path"] = path if domain: @@ -401,9 +409,11 @@ def make_cookie( if expire: cookie[name]["expires"] = _expiration(expire, "%a, %d-%b-%Y %H:%M:%S GMT") if secure: - cookie[name]["secure"] = secure + cookie[name]["Secure"] = secure if httponly: cookie[name]["httponly"] = httponly + if same_site: + cookie[name]["SameSite"] = same_site return tuple(cookie.output().split(": ", 1)) diff --git a/tests/test_oic_provider_logout.py b/tests/test_oic_provider_logout.py index 903507e68..9ebcd2693 100644 --- a/tests/test_oic_provider_logout.py +++ b/tests/test_oic_provider_logout.py @@ -982,7 +982,8 @@ def test_no_back_or_front_channel_logout(self): sid=list(self.provider.sdb._db.storage.keys())[0], client_id="number5" ) - assert resp == {} + # only cookies + assert set(resp.keys()) == {"cookie"} def test_back_channel_logout_fails(self): self._code_auth()