Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

We might not be correctly clearing OIDC cookies when handling an OIDC callback #12782

Open
DMRobertson opened this issue May 18, 2022 · 0 comments
Labels
A-SSO Single Sign-On (maybe OIDC) T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@DMRobertson
Copy link
Contributor

DMRobertson commented May 18, 2022

Looking at sentry, this one is really weird. It happens when session == b"", which might be a result of the cookie clearing done before. Testing it a bit, on Safari, it looks like the cookie clearing line 201 does not clear the cookie properly, and instead sets it to a blank value, hence the issue.

So, it's a legit exception, and probably a legit bug because we're not clearing the cookie properly.

Originally posted by @sandhose in #12723 (comment)

The lines in question:

# Remove the cookies. There is a good chance that if the callback failed
# once, it will fail next time and the code will already be exchanged.
# Removing the cookies early avoids spamming the provider with token requests.
#
# we have to build the header by hand rather than calling request.addCookie
# because the latter does not support SameSite=None
# (https://twistedmatrix.com/trac/ticket/10088)
for cookie_name, options in _SESSION_COOKIES:
request.cookies.append(
b"%s=; Expires=Thu, Jan 01 1970 00:00:00 UTC; %s"
% (cookie_name, options)
)

And the sentry report: https://sentry.matrix.org/sentry/synapse-matrixorg/issues/219508/?query=is%3Aunresolved

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-SSO Single Sign-On (maybe OIDC) T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

2 participants