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

jinja2_environment_from_request plugin hook #2225

Closed
Tracked by #2251
simonw opened this issue Jan 5, 2024 · 8 comments
Closed
Tracked by #2251

jinja2_environment_from_request plugin hook #2225

simonw opened this issue Jan 5, 2024 · 8 comments

Comments

@simonw
Copy link
Owner

simonw commented Jan 5, 2024

For Datasette Cloud I want the ability to have one Datasette instance serve different templates depending on the host of the incoming request - so I can have a private simon.datasette.cloud instance running default Datasette, but I can let users have a public simon.datasette.site instance which uses their own custom templates.

I don't want people running custom templates on *.datasette.cloud for security reasons - I dont want an XSS hole in a custom template being able to steal cookies or perform actions on behalf of signed-in users.

I tried implementing this at first with a monkeypatch, but ran into problems. I'm going to instead do a research spike to see if a plugin hook that allows plugins to influence the Jinja environment based on the incoming request is a clean and better mechanism for this.

@simonw
Copy link
Owner Author

simonw commented Jan 5, 2024

Potential design:

def jinja2_environment_from_request(request, datasette, env):

Where env is the existing environment, and it's strongly suggested that this return an overlay on it. The plugin documentation could describe overlays in detail with an example.

That name is consistent with actor_from_request(datasette, request) and prepare_jinja2_environment(env, datasette).

Overlays look like this:

            jinja_env = self.jinja_env.overlay(
                loader=ChoiceLoader([FileSystemLoader("/tmp/custom-templates"), self.jinja_env.loader])
            )

@simonw
Copy link
Owner Author

simonw commented Jan 5, 2024

One of the key things this will enable is a single Datasette instance that returns different sites (using different templates) on different hosts - someone with a side-project hobby like mine could run a single instance and host simonwillison.net and www.niche-museums.com from the same Datasette, with custom templates that vary based on the host.

@simonw
Copy link
Owner Author

simonw commented Jan 5, 2024

A catch with that plan:

datasette/datasette/app.py

Lines 1567 to 1580 in 45b88f2

class DatasetteRouter:
def __init__(self, datasette, routes):
self.ds = datasette
self.routes = routes or []
# Build a list of pages/blah/{name}.html matching expressions
pattern_templates = [
filepath
for filepath in self.ds.jinja_env.list_templates()
if "{" in filepath and filepath.startswith("pages/")
]
self.page_routes = [
(route_pattern_from_filepath(filepath[len("pages/") :]), filepath)
for filepath in pattern_templates
]

That code implements the thing where custom templates can define new pages - but note that it doesn't have access to the request object at that point, which means it wouldn't be able to work differently for different hosts.

I can fix that by making it dynamic as opposed to running it statically once when the class is constructed, hope that won't be too much of a performance hit.

I could maybe fix that by having template loaders that cache their own list_templates() calls.

simonw added a commit that referenced this issue Jan 5, 2024
With stub tests and documentation so far.

Refs #2225
@simonw
Copy link
Owner Author

simonw commented Jan 5, 2024

b43d5c3 is the first version that passes the existing Datasette tests, still needs tests and documentation and I should actually use it to make sure it solves the problem.

@simonw
Copy link
Owner Author

simonw commented Jan 5, 2024

I'm going to make this into a documented internal method too:

datasette/datasette/app.py

Lines 439 to 446 in b43d5c3

def get_jinja_environment(self, request: Request = None) -> Environment:
environment = self._jinja_env
if request:
for environment in pm.hook.jinja2_environment_from_request(
datasette=self, request=request, env=environment
):
pass
return environment

@simonw
Copy link
Owner Author

simonw commented Jan 5, 2024

Here's the previous attempt at a plugin for this which used monkeypatching and didn't quite work:

from datasette.app import Datasette
from datasette.views.base import BaseView
from datasette.utils.asgi import Response
from datasette.utils import path_with_format
from jinja2 import ChoiceLoader, FileSystemLoader, Template
from typing import Any, Dict, List, Optional, Union
from datasette import Request, Context
import os


def is_custom_public_site(request):
    return (
        request
        and os.environ.get("DATASETTE_CUSTOM_TEMPLATE_DIR")
        and (
            request.host.endswith(".datasette.site")
            or request.host.endswith(".datasette.net")
        )
    )


def get_env(datasette, request):
    # if request and request.host != 'localhost' and not request.host.endswith('.cloud'):
    #     breakpoint()
    if is_custom_public_site(request):
        print("get_env", request, os.environ["DATASETTE_CUSTOM_TEMPLATE_DIR"])
        return datasette.jinja_env.overlay(
            loader=ChoiceLoader(
                [
                    FileSystemLoader(os.environ["DATASETTE_CUSTOM_TEMPLATE_DIR"]),
                    datasette.jinja_env.loader,
                ]
            ),
            enable_async=True,
        )
    return datasette.jinja_env


async def new_render_template(
    self,
    templates: Union[List[str], str, Template],
    context: Optional[Union[Dict[str, Any], Context]] = None,
    request: Optional[Request] = None,
    view_name: Optional[str] = None,
):
    if isinstance(templates, str):
        templates = [templates]

    # if request and request.host != 'localhost' and not request.host.endswith('.cloud'):
    #     breakpoint()

    # If all templates are strings
    if (
        request
        and isinstance(templates, list)
        and all(isinstance(t, str) for t in templates)
    ):
        # Check if request's host matches .datasette.site
        if is_custom_public_site(request):
            jinja_env = get_env(self, request)
            templates = [jinja_env.select_template(templates)]

    return await original_render_template(
        self=self,
        templates=templates,
        context=context,
        request=request,
        view_name=view_name,
    )


original_render_template = Datasette.render_template
Datasette.render_template = new_render_template


# TOtal copy-paste replacement of original BaseView.render method:
async def new_base_render(self, templates, request, context=None):
    context = context or {}
    jinja_env = get_env(self.ds, request)
    template = jinja_env.select_template(templates)
    
    # if request and request.host != 'localhost' and not request.host.endswith('.cloud'):
    #     breakpoint()

    template_context = {
        **context,
        **{
            "select_templates": [
                f"{'*' if template_name == template.name else ''}{template_name}"
                for template_name in templates
            ],
        },
    }
    headers = {}
    if self.has_json_alternate:
        alternate_url_json = self.ds.absolute_url(
            request,
            self.ds.urls.path(path_with_format(request=request, format="json")),
        )
        template_context["alternate_url_json"] = alternate_url_json
        headers.update(
            {
                "Link": '{}; rel="alternate"; type="application/json+datasette"'.format(
                    alternate_url_json
                )
            }
        )
    return Response.html(
        await self.ds.render_template(
            template,
            template_context,
            request=request,
            view_name=self.name,
        ),
        headers=headers,
    )


BaseView.render = new_base_render

@simonw
Copy link
Owner Author

simonw commented Jan 5, 2024

Got it working! Here's my plugin, plugins/custom_templates_for_host.py:

from datasette import hookimpl
from jinja2 import ChoiceLoader, FileSystemLoader
import os


def is_custom_public_site(request):
    return (
        request
        and os.environ.get("DATASETTE_CUSTOM_TEMPLATE_DIR")
        and (
            request.host.endswith(".datasette.site")
            or request.host.endswith(".datasette.net")
        )
    )


@hookimpl
def jinja2_environment_from_request(request, env):
    if is_custom_public_site(request):
        print("get_env", request, os.environ["DATASETTE_CUSTOM_TEMPLATE_DIR"])
        return env.overlay(
            loader=ChoiceLoader(
                [
                    FileSystemLoader(os.environ["DATASETTE_CUSTOM_TEMPLATE_DIR"]),
                    env.loader,
                ]
            ),
            enable_async=True,
        )
    return env

And the tests, tests/test_custom_templates_for_host.py:

import pathlib
import pytest


@pytest.mark.asyncio
@pytest.mark.parametrize("path", ("/", "/test/dogs", "/test"))
@pytest.mark.parametrize(
    "host,expect_special",
    (
        (None, False),
        ("foo.datasette.cloud", False),
        ("foo.datasette.site", True),
        ("foo.datasette.net", True),
    ),
)
async def test_custom_templates_for_host(
    datasette, path, host, expect_special, tmpdir, monkeypatch
):
    templates = pathlib.Path(tmpdir / "custom-templates")
    templates.mkdir()
    (templates / "base.html").write_text(
        '{% extends "default:base.html" %}{% block footer %}Custom footer!{% endblock %}',
        encoding="utf-8",
    )
    monkeypatch.setenv("DATASETTE_CUSTOM_TEMPLATE_DIR", str(templates))
    headers = {}
    if host:
        headers["host"] = host
    response = await datasette.client.get(path, headers=headers)
    assert response.status_code == 200
    if expect_special:
        assert "Custom footer!" in response.text
    else:
        assert "Custom footer!" not in response.text

@simonw
Copy link
Owner Author

simonw commented Jan 5, 2024

@simonw simonw closed this as completed in c7a4706 Jan 5, 2024
@simonw simonw changed the title Experiment: jinja2_environment_from_request plugin hook jinja2_environment_from_request plugin hook Jan 5, 2024
@simonw simonw added this to the Datasette 1.0a8 milestone Jan 31, 2024
@simonw simonw mentioned this issue Feb 5, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant