Skip to content

Commit

Permalink
Session cookie need to be visible to OP IFrame Javascript script. (#731)
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
rohe committed Feb 1, 2020
1 parent 16167f0 commit bac06a4
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 10 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down
20 changes: 18 additions & 2 deletions src/oic/oauth2/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -1115,14 +1115,30 @@ 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)

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
17 changes: 13 additions & 4 deletions src/oic/oic/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand All @@ -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"]:
Expand Down Expand Up @@ -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
)
Expand Down
16 changes: 13 additions & 3 deletions src/oic/utils/http_util.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -324,6 +322,7 @@ def make_cookie(
enc_key=None,
secure=True,
httponly=True,
same_site="",
):
"""
Create and return a cookie.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -394,16 +400,20 @@ 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:
cookie[name]["domain"] = domain
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))

Expand Down
3 changes: 2 additions & 1 deletion tests/test_oic_provider_logout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit bac06a4

Please sign in to comment.