Serve inline assets on error pages (fixes #3)

[chrisfrank]

Description of Change

On error pages, serve inline styles and images instead of linking to external files.

Related Issue

See #3: Because asset requests are authenticated, the views that handle auth errors render without any styles or images.

Motivation and Context

Instead of disabling auth for asset requests (#22), this is an attempt to implement @isaacwhite's more security-conscious proposal. I ran into a few complications while working on this, which I'll document below:

1. We've removed JS from error pages, but the user-tools dropdown requires JS to function. I worked around this by hiding .user-tools via CSS, but we could also explore conditionally not rendering it in the template, or inlining whatever JS that dropdown requires. (And actually, error pages are still loading jQuery from Cloudfare, because some of the footer code depends on it.)

2. I'm using custom sass functions to generate base64 data URLs and to read from config/strings.yaml. The node-sass docs warn that custom functions are experimental and should be used with caution, but they seem to be working well here. As an alternative, I explored handling the base64-encoding on boot in the server code, instead of on build via node-sass. This approach worked too, but it seems better to do this work on build.

3. We could probably cut more CSS from the error styles, but I think it would require either splitting up _furniture.scss into separate partials, or not including that partial at all but duplicating a fair bit of the code from it. Is this worth exploring further? I wanted to check in before I tried, because I don't know whether we can split up core partials without breaking people's customizations.

Checklist

• Ran npm run lint and updated code style accordingly
  ESLint passes for all new code, but there was an existing error in server/docs that I haven't tried to fix.
• npm run test passes
• PR has a description and all contributors/stakeholder are noted/cc'ed
• tests are updated and/or added to cover new code
• relevant documentation is changed and/or added

[isaacwhite]

This is great, thanks @chrisfrank!

Makes sense to keep runtime compilation to a minimum; there's just a couple tweaks I'm curious if we could make to the approach you've set up here:

• Checking in compiled css isn't something we do elsewhere in Library and I'd prefer to avoid it. Maybe we could instead write an errors.css file into the public folder and read it off disk and cache inside the errors request? Then we could pass it into the render call as a value and emit it unescaped into the page here.
• When reading in that errors.css file later, would you be comfortable base64 encoding images once and caching that for future requests? It seems nice to avoid the additional script in bin if we can, and running the base64 encode as part of the core Library process (but deferring after boot) would let us write the error build task using just cli options.

On your question about cutting more css but needing to split up _furniture.scss, there's an interesting thread that might provide a solution but isn't available yet. I think it's fine to punt on slimming down the inline css that comes from that file until we have a better approach.

[isaacwhite]

What are you thoughts on updating this function so it returned a promise and used async/promise versions of the file operations so it was non-blocking in other runtime contexts? That could make it easier to decide dynamically later (in the middle of a request, for instance) to base64 encode something without locking up the main Library server.

[chrisfrank]

Will do!

[chrisfrank]

Hi @isaacwhite,

Thanks for getting to this so quickly again. Most of the changes you've suggested sound good to me — just one thing I want to check on before I work on implementing them this week.

When reading in that errors.css file later, would you be comfortable base64 encoding images once and caching that for future requests?

I'd be comfortable changing the EJS templates to render <img src=${base64}"/> instead of using base64-encoded CSS background images -- is that what you mean? If not, does it seem like a reasonable approach to you? I don't see a way to parse custom SCSS functions from the stylesheet at runtime without requiring node-sass in the server process. I'd prefer to avoid that, both because I wouldn't want to rely on "experimental" features in runtime code and because I suspect it will noticeably increase the server's memory footprint.

[isaacwhite]

Hey @chrisfrank,

Thanks for your thoughtful questions. I was envisioning that the inline stylesheet could be written normally to the assets folder with url() declarations for background images. The sass compiler would build that as part of the normal build process for existing styles; then the server could read in the compiled css for the inline template and regex (or something else) the url() declarations to replace them with base64 images of the contents on disk.

That would keep the sass compiler out of the server runtime, but I'd be curious how complicated that regex seems to you. Does that make sense as a path to explore?

[chrisfrank]

I like that idea, @isaacwhite. Switching to a regex means we'd lose the ability to reference assets by name using the stringTemplate function, but all things considered I think that's a good tradeoff. I'll get on it!

[chrisfrank]

I have an async version of this PR open in #56 — if that version seems more promising, I'll go ahead and close this.

[isaacwhite]

@chrisfrank #56 seems promising! I'll leave some additional comments over there. Thanks!