If user can see table but NOT database/instance nav links should not display

[simonw]

Spotted this bug while building this plugin:

• https://github.com/simonw/datasette-public

This is a public table, but the two links in the nav go to forbidden pages:

Those nav links shouldn't be shown at all.

[simonw]

Here are all the places that display that navigation bar at the moment:

datasette/datasette/templates/row.html

Lines 18 to 25 in b7fec7f

	{% block nav %}
	<p class="crumbs">
	<a href="{{ urls.instance() }}">home</a> /
	<a href="{{ urls.database(database) }}">{{ database }}</a> /
	<a href="{{ urls.table(database, table) }}">{{ table }}</a>
	</p>
	{{ super() }}
	{% endblock %}

datasette/datasette/templates/table.html

Lines 19 to 25 in b7fec7f

	{% block nav %}
	<p class="crumbs">
	<a href="{{ urls.instance() }}">home</a> /
	<a href="{{ urls.database(database) }}">{{ database }}</a>
	</p>
	{{ super() }}
	{% endblock %}

datasette/datasette/templates/query.html

Lines 21 to 27 in b7fec7f

	{% block nav %}
	<p class="crumbs">
	<a href="{{ urls.instance() }}">home</a> /
	<a href="{{ urls.database(database) }}">{{ database }}</a>
	</p>
	{{ super() }}
	{% endblock %}

datasette/datasette/templates/error.html

Lines 5 to 10 in b7fec7f

	{% block nav %}
	<p class="crumbs">
	<a href="{{ urls.instance() }}">home</a>
	</p>
	{{ super() }}
	{% endblock %}

datasette/datasette/templates/database.html

Lines 12 to 17 in b7fec7f

	{% block nav %}
	<p class="crumbs">
	<a href="{{ urls.instance() }}">home</a>
	</p>
	{{ super() }}
	{% endblock %}

datasette/datasette/templates/show_json.html

Lines 7 to 12 in b7fec7f

	{% block nav %}
	<p class="crumbs">
	<a href="{{ urls.instance() }}">home</a>
	</p>
	{{ super() }}
	{% endblock %}

datasette/datasette/templates/permissions_debug.html

Lines 25 to 30 in b7fec7f

	{% block nav %}
	<p class="crumbs">
	<a href="{{ base_url }}">home</a>
	</p>
	{{ super() }}
	{% endblock %}

https://github.com/simonw/datasette/blob/b7fec7f9020b79c1fe60cc5a2def86b50eeb5af9/datasette/templates/permissions_debug.html#L25-L30t

Interesting to note that failing to include that super() call means the nav menu itself will not be rendered:

datasette/datasette/templates/base.html

Lines 20 to 49 in b7fec7f

	<header><nav>{% block nav %}
	{% set links = menu_links() %}{% if links or show_logout %}
	<details class="nav-menu">
	<summary><svg aria-labelledby="nav-menu-svg-title" role="img"
	fill="currentColor" stroke="currentColor" xmlns="http://www.w3.org/2000/svg"
	viewBox="0 0 16 16" width="16" height="16">
	<title id="nav-menu-svg-title">Menu</title>
	<path fill-rule="evenodd" d="M1 2.75A.75.75 0 011.75 2h12.5a.75.75 0 110 1.5H1.75A.75.75 0 011 2.75zm0 5A.75.75 0 011.75 7h12.5a.75.75 0 110 1.5H1.75A.75.75 0 011 7.75zM1.75 12a.75.75 0 100 1.5h12.5a.75.75 0 100-1.5H1.75z"></path>
	</svg></summary>
	<div class="nav-menu-inner">
	{% if links %}
	<ul>
	{% for link in links %}
	<li><a href="{{ link.href }}">{{ link.label }}</a></li>
	{% endfor %}
	</ul>
	{% endif %}
	{% if show_logout %}
	<form action="{{ urls.logout() }}" method="post">
	<input type="hidden" name="csrftoken" value="{{ csrftoken() }}">
	<button class="button-as-link">Log out</button>
	</form>{% endif %}
	</div>
	</details>{% endif %}
	{% if actor %}
	<div class="actor">
	<strong>{{ display_actor(actor) }}</strong>
	</div>
	{% endif %}
	{% endblock %}</nav></header>

[simonw]

The fact that this code is inconsistent already (urls.instance() v.s. base_url for example) suggests that properly refactoring this is well overdue already.

[simonw]

Related issue:

• Table/database that is private due to inherited permissions does not show padlock #1829

I'd like a code design for this that helps solve both these issues at once.

[simonw]

Maybe new template functions: table_crumbs(table, database) and database_crumbs(database) and instance_crumbs() - which know how to both check the permissions and display the <ul> full of links.

Or just crumbs(table=table, database=database) where both of those are optional. Maybe accept actor= or request= too.

[simonw]

Made some notes on the Jinja documentation while thinking about the best way to do this:

• Full read-through of the Jinja documentation public-notes#4

[simonw]

As part of this I think I want request to always be available in the template context, which will remove the need for https://datasette.io/plugins/datasette-template-request

[simonw]

It's important that, however this works, it supports custom templates changing how the breadcrumbs are displayed. Probably needs a _crumbs.html template.

[simonw]

This patch to default_permissions.py made debugging easier: https://gist.github.com/simonw/daddf022e75a98ea6246ac1e12dc8759

[simonw]

This has turned into a full refactor of how breadcrumbs work.

I'm using my first ever Jinja macro for this - I import that at the top of base.html so that it will be available everywhere else:

{% import "_crumbs.html" as crumbs with context %}<!DOCTYPE html>

The with context bit is needed so the macro can see the new crumb_items() function that I'm adding to the global template rendering scope.

Here's the full content of _crumbs.html:

{% macro nav(request, database=None, table=None) -%}
{% set items=crumb_items(request=request, database=database, table=table) %}
{% if items %}
  <p class="crumbs">
    {% for item in items %}
      <a href="{{ item.href }}">{{ item.label }}</a>
      {% if not loop.last %}
        /
      {% endif %}
    {% endfor %}
  </p>
{% endif %}
{%- endmacro %}

This means custom template authors can use their own _crumbs.html template to do something different with the breadcrumbs.

In the actual templates I display breadcrumbs like this:

{% block crumbs %}
{{ crumbs.nav(request=request, database=database) }}
{% endblock %}

Pass database= to get home / database_name - pass table= as well to get home / database_name / table_name - if you just send request= you just get home.

I've also made the default base template show the home breadcrumbs - other pages such as table.html and row.html can then over-ride {% block crumbs %} to get different breadcrumbs.

[simonw]

I'm going to commit the code now, but then I need to add some extra tests to ensure the breadcrumb permission display logic works correctly.