Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-evaluate FRAME_DOMAIN as configuration variable #335

Open
ssciolla opened this issue Feb 8, 2022 · 1 comment
Open

Re-evaluate FRAME_DOMAIN as configuration variable #335

ssciolla opened this issue Feb 8, 2022 · 1 comment
Labels
back end Involves changes to the Express application or other server-related files config change Involves a change to the configuration file/format enhancement New feature or request

Comments

@ssciolla
Copy link
Contributor

ssciolla commented Feb 8, 2022

@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).

@ssciolla ssciolla added enhancement New feature or request back end Involves changes to the Express application or other server-related files config change Involves a change to the configuration file/format labels Feb 8, 2022
@lsloan
Copy link
Member

lsloan commented Feb 8, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back end Involves changes to the Express application or other server-related files config change Involves a change to the configuration file/format enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants