Logged in cookies not deleted on logout over HTTPS, not reproducible on edx.org

Hi. This is mainly to vent out, as it took some hours to debug and I’m not sure whose problem is it; and as a heads-up to @regis .

This is an issue not of tutor, but of the edx-platform, so I didn’t see fitting to raise an issue in your repo. However, it is not reproducible on courses.edx.org, which has led to me being 100% sure they are not running the codebase at edx/edx-platform/master, which they report they do.

Long story short. When logging in over HTTPS and using django-cookies-samesite config value DCS_SESSION_COOKIE_SAMESITE = "None" (which tutor is using and edx.org very much seems to be), login cookies (edx-logged-in, edx-jwt-cookie-signature, etc) are set with SameSite=None and Secure, as seen in openedx/core/djangoapps/user_authn/cookies.py:

def standard_cookie_settings(request):
    ...
    cookie_settings['secure'] = request.is_secure()

However, when logging out, those cookies are deleted using Django’s response.delete_cookie(), which does not set the Secure attribute:

for cookie_name in ALL_LOGGED_IN_COOKIE_NAMES:
        response.delete_cookie(
            cookie_name,
            path='/',
            domain=settings.SESSION_COOKIE_DOMAIN
        )

So, at least using a recent version of Chrome, the logged in cookies are not deleted, because they have SameSite=None but not the Secure attribute (including, again, the JWT cookies):

That means that any MFA using the JWT cookies to authenticate (looking at the edx/frontend-platform code, that would mean all of the official ones) will remain logged in when the user logs out from the LMS. That’s not only a functional issue, but potentially a security one.

HOWEVER, trying to reproduce this in edx.org, it’s clear that they set the Secure attribute on the log out cookies, so this issue does not arise:

I’ve reviewed many branches, tags, docs and Jira issues trying to find if/when this was reported/fixed, with no avail.

In the end, this was very easy to fix once found, just changing a few lines in openedx/core/djangoapps/user_authn/cookies.py to match the behaviour at log in:

def delete_logged_in_cookies(response, request):
    for cookie_name in ALL_LOGGED_IN_COOKIE_NAMES:
        response.set_cookie(
            cookie_name,
            '',
            path='/',
            domain=settings.SESSION_COOKIE_DOMAIN,
            secure=request.is_secure(),
            max_age=0,
            expires=datetime.datetime.utcfromtimestamp(0)
        )

    return response

Again, I’m aware this is not an issue with tutor but with edx-platform. However, seeing that it has been shadowly fixed in the production edx.org deployment, and that work is being done with MFAs by @regis (https://github.com/overhangio/tutor-gradebook), I hope at least to save a headache to some poor developer if they face this issue and somehow end up here.

Anyway, if anyone knows what’s the right thing to do in a situation like this, I’m all ears.

Cheers.

Hey @moraleja39, thanks for the thorough investigation and the detailed report, this is much appreciated. I am not going to be in front of a computer for a couple days, so I’m going to ask you to do a few more things:

  1. Report your findings to security@edx.org. Add Ned Batchelder and me in cc and link to this post.
  2. Open a pull request on edx-platform to resolve this issue
  3. Open a pull request on the Tutor repo where you cherry pick your patch.

Can you do that?

1 Like

Hey Regis. Thanks for your guidance. Sure thing, I’ll take care of it tomorrow morning.