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

Make javascript injection easier for things like cookies consent, or others #366

Open
jorgemoralespou opened this issue May 13, 2024 · 11 comments
Labels
enhancement New feature or request

Comments

@jorgemoralespou
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Sometimes it's required to add additional javascript scripts to the Educates Platform to allow for things like cookie consent due to GDPR laws. Currently you can add these to a theme, but it's maybe overly complicated.

Describe the solution you'd like

Investigate what kind of javascripts would need to be injected (also if CSPs would need to be modified) and provide an easy way to set these up. An example of scripts could be https://cookieconsent.orestbida.com/

Describe alternatives you've considered

No response

Additional information

Current Kubernetes slack thread with some information about this: https://kubernetes.slack.com/archives/C05UWT4SKRV/p1715589756629829

@jorgemoralespou jorgemoralespou added the enhancement New feature or request label May 13, 2024
@GrahamDumpleton
Copy link
Contributor

GrahamDumpleton commented May 14, 2024

First possible change suggest is that currently you can specify Secret resource with theme files.

apiVersion: v1
kind: Secret
metadata:
  name: workshops.example.com-theme
  namespace: default
stringData:
  workshop-dashboard.html: ""
  workshop-dashboard.css: ""
  workshop-dashboard.js: ""
  workshop-instructions.html: ""
  workshop-instructions.js: ""
  workshop-instructions.css: ""
  workshop-started.html: ""
  workshop-finished.html: ""
  training-portal.html: ""
  training-portal.js: ""
  training-portal.css: ""

The .html file is embedded directly in head section of pages.

The .css file is included into the head section using a link element.

The .js file is included at the bottom of the HTML body element using a script element.

Right now everything has to be in the one file which is not convenient when needing to include multiple files from third party packages along with your own customisations.

To enable having multiple files, have convention whereby for example in addition to training-portal.html, you can have files training-portal-nnn.html where nnn is integer number. Where such files exist they will be embedded/included at appropriate point in sorted order.

Thus Secret might be defined as:

apiVersion: v1
kind: Secret
metadata:
  name: workshops.example.com-theme
  namespace: default
stringData:
  training-portal-1.html: "..."
  training-portal-2.html: "..."
  training-portal-1.js: ""
  training-portal-2.js: ""
  training-portal-1.css: ""
  training-portal-2.css: ""

If variant with no number is still provided, it would be embedded/included before the numbered ones.

@GrahamDumpleton
Copy link
Contributor

Steps for how to use current theme support to add CookieConsent package.

@GrahamDumpleton
Copy link
Contributor

So the idea of numbered files for stuff needed to be included in head of page is unnecessary since you can use the .html snippet file with link and script elements to include the arbitrary files attached to the secret.

<link rel="stylesheet" href="/static/workshops/theme/cookieconsent.css">
<script type="text/javascript" src="/static/workshops/theme/cookieconsent.umd.js"></script>

The .js file though is still a single entry only which is included using a script tag with src attribute.

The needed for numbered files would not arise for inclusion at the end of the body if option existed for supply arbitrary HTML at the end of the body. That way could use multiple script tags to include separate Javascript files, or just embed Javascript, or even plain HTML if that made sense.

@GrahamDumpleton
Copy link
Contributor

Note that need for including multiple Javascript files at end of body element of page only becomes a requirement where front loading them in the head element is not possible for performance or runtime reasons.

@mocdaniel
Copy link
Contributor

Bit late to the party and not so much to add to the initial description, but I really like the suggestion to add <link>/<script> tags in the .html snippet and reference arbitrary files defined via the theme secret.

Another solution to many situations like this could be to allow modification of CSP rules, as most 3rd party scripts/styles you might want to include are available from public CDNs. E.g. the mentioned cookie-consent script/css combination would be available from a CDN, but the hard-coded self CSPs prevent this way of including them in the portal.

@GrahamDumpleton
Copy link
Contributor

Concern over CSP is that there are different constraint types and need to know which to add them to.

CSP_CONNECT_SRC = (
    "'self'",
    f"*.{INGRESS_DOMAIN}",
    "www.google-analytics.com",
    "*.clarity.ms",
    "c.bing.com",
    "*.amplitude.com",
)

CSP_DEFAULT_SRC = ("'none'",)
CSP_STYLE_SRC = ("'self'",)
CSP_SCRIPT_SRC = ("'self'",)
CSP_SCRIPT_SRC = ("'self'", "www.clarity.ms", "cdn.amplitude.com")
CSP_IMG_SRC = (
    "'self'",
    "data:",
    "www.google-analytics.com",
    "www.googletagmanager.com",
)
CSP_FONT_SRC = ("'self'",)
CSP_FRAME_SRC = ("'self'",)
CSP_INCLUDE_NONCE_IN = ("script-src",)
CSP_FRAME_ANCESTORS = ("'self'",)

@mocdaniel
Copy link
Contributor

Yeah I noticed this when going through the Django code - would it be possible/feasible to expose all CSP_* variables used in Django for configuration in e.g. the TrainingPortal definition?

@GrahamDumpleton
Copy link
Contributor

Am a bit wary of doing that since no guarantee that the training portal will remain a Python/Django application. So any mechanism for specifying extra hostnames needs to be as simple as possible and not expose internal implementation details. As such, it may be better to just accept a list of additional host patterns and internally we make assumptions as to which of all those they should be added to, even if we end up adding hosts to a category they don't need to be in.

@mocdaniel
Copy link
Contributor

I see. This definitely makes sense. I then share your concerns that hostnames passed to the portal's config might end up in the wrong/too many CSPs out of engineering necessity.
I'd say then, an easier way to inject Javascript would be a good start.

In addition, I'll create a PR to document the use-case of 'including multiple additional .js/.css files and referencing them in the .html snippet file of a theme' a bit better, as this looks like the best solution to me right now.

@GrahamDumpleton
Copy link
Contributor

@mocdaniel Even with CookieConsent injected, how were you then enabling/disabling analytics as necessary? Were you still relying on Educates analytics code injection or inserting your own?

In https://cookieconsent.orestbida.com/advanced/manage-scripts.html it talks about using data attributes to mark script sections which should be enabled/disabled based on consent, but that relies on setting script type to text/plain which means wouldn't run if CookieConsent not also be used.

Trying to work out whether anything else needs to be done to make existing analytics code injection work better with any cookie consent libraries.

@mocdaniel
Copy link
Contributor

I inserted the same snippets you insert upon setting up Google Analytics via G-Tag by adding new entries to a custom theme Secret and referencing them from the training-portal.html key within the same secret.
This way I was able to adjust the scripts according to the CookieConsent docs you linked above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

No branches or pull requests

3 participants