search

tl-its-umich-edu / canvas-course-manager-next

Canvas Course Manager Next: A redesign of the existing CCM application. It extends Canvas features, makes cumbersome features easier to use, and adds new features.
8 stars 9 forks source link

Re-evaluate `FRAME_DOMAIN` as configuration variable #335

Open ssciolla opened 2 years ago

ssciolla commented 2 years ago

@lsloan and @pushyamig shared some observations and points about FRAME_DOMAIN (see #316 for introduction of it). These changes might be considered in the future, but the current arrangement is functional.

1) FRAME_DOMAIN seems to always be the hostname from CANVAS_INSTANCE_URL. Perhaps the value for FRAME_DOMAIN should be derived from that other value. @lsloan suggested that it perhaps if FRAME_DOMAIN isn't specified, CANVAS_INSTANCE_URL.replace('https://') could be the fallback. I lean slightly towards either keeping them as using the same value, or different values, rather than introducing harder to document/implement fallback logic, but the possibility has some merit.

2) @pushyamig suggested that we may at one point need CSP settings for frame-src (or frame-ancestors) to use multiple domains. There is not yet a use case for this, but we could make the value a JSON array or a CSV if we anted to allow multiple strings. The idea of a multi-tenant CCM would require significant refactoring though, since CanvasService and its related configuration are designed to work with just one Canvas instance (though I could multiple instances of CanvasService potentially solving that problem).

lsloan commented 2 years ago

WRT item № 1, I suggest the default of FRAME_DOMAIN be computed as new URL(CANVAS_INSTANCE_URL).host (or perhaps hostname, if that's the recommended element). That way, it'll get the hostname even if a lot more of the URL is present.