academic-innovation / django-lti

LTI Advantage support for Django projects.
https://academic-innovation.github.io/django-lti/
MIT License
15 stars 7 forks source link

Fix Cookie problems by implementing Client Side postMessages #103

Open yunusdemir opened 7 months ago

yunusdemir commented 7 months ago

First of all, I would like to thank you for your work on this package. We've been using it for a little while at the company I work for, and are very happy with it.

Also, if you just want to read my question, read the last paragraph ;)

Background

The Cookie Problems

Third-party cookies are being blocked by web browsers. By my tests on macOS, at least Safari and Chrome block the cookies. Cookies are being used in the OpenID Connect flow to keep track of the state (in PyLTI1.3), and the fact that there are issues is being acknowledged by the developer of PyLTI1.3. Unfortunately, the mentioned fixes do not work for deep linking, which seems to be needed to be opened in an iframe.

The solutions provided by 1EdTech (still in review and comment status)

As you might know, the solution provided by the proposed specification is to use postMessages. The specifications, copied and pasted from 1EdTech: OIDC Login with LTI Client Site PostMessages (start here) LTI Client Side postMessages LTI postMessage Storage

Maintainer difficulties PyLTI1.3

The current maintainer of PyLTI1.3 indicated that he does not have time for implementing new features, and that also seems to be the case for reviewing PRs, as you can see with #3 and the PR that has been made upstream.

Implementation of the postMessage solution

At my work, we're trying to patch some classes and implement the postMessage solution ourselves. Although I must admit that the implantation is very much incomplete at this moment. We're willing to collaborate on it, and/or write the code and try to push that code upstream, and make sure it's available for the open-source community.

I guess the main question that we have is: Have you already had a look at the postMessage specification, and have you been thinking of implementing it in this repo? Especially, as probably creating a PR in PyLTI1.3 won't be merged.

Another source on this matter: unicon.net

michaelwheeler commented 7 months ago

Glad to hear that this library is helpful @yunusdemir!

Yes, I'm familiar with the postMessage spec, although I haven't implemented it in any tools I work on. I'll admit that it hasn't been a priority for this library for a few reasons:

  1. Although using postMessages allows a launch to proceed through the login initiation flow, Django session cookies and CSRF cookies would still be blocked in third-party contexts. While it would probably be possible to build a Django LTI tool without using sessions, I don't think that it's a use case that we're prepared to directly support at the moment.
  2. With the eventual arrival of CHIPS and the Storage Access API it should be possible to address issues with all third-party cookies in a standards-compliant way.
  3. This library (via PyLTI1.3) does support prompting users to break out of an iframe into a new tab or window when cookies aren't available. I acknowledge that this often results in a diminished user experience, but at least it's something.

With all that said, I'd certainly be interested to learn more about how you're patching classes to implement postMessage support in your tool. Perhaps it may inspire some ideas about how I could make django-lti more extendable.

jonespm commented 1 month ago

There is an issue opened on the library about this https://github.com/dmitry-viskov/pylti1.3/issues/119 but from what I can tell nobody has worked on it. The comment there from March 2024 is that just released Python 3.13 would support partitioned (2 above) then Django could make use of it. But still not sure about Safari and the Safari issue is still opened.

michaelwheeler commented 1 month ago

It seems like CHIPS support from Safari is largely hung up on issues around cookie size.

Support for the partitioned attribute doesn't appear to have made it into Python 3.13 (via https://github.com/python/cpython/pull/112714), as far as I can tell. For reference, the issue tracking Django support for partitioned cookies suggests a few approaches for adding this in without support in http.cookies. There is also this approach building on pylti1.3 that might be worth investigating.

pushyamig commented 1 month ago

Here is the ticket in Django to support Partitioned Cookie: https://code.djangoproject.com/ticket/34613#comment:1. and Pull Request still waiting