Anyone not signed in holds the guest role
Whether somebody with no account may see something was being answered one feature at a time: a securesend_allow_guests setting, an enable flag per kiosk, and very nearly a third for guides - each with its own settings page, its own vocabulary and its own hand-written branch inside a route. Every one of them answered a permissions question, and this app already administers permissions per role. So a caller with no token is not nobody: they hold the seeded `guest` role. require_permission consults it when there is no identity, which is the entire mechanism. The role is seeded holding NOTHING, so an upgrade opens no door; a site opens one by ticking a box in the same grid it uses for every other role, and closes it the same way. Revocation is read on every request rather than cached, because taking guest access away is what somebody does the moment they realise what they opened, and it has to bite immediately. The redundant @jwt_required() above require_permission is gone from 184 routes. It was belt-and-braces before; now it is actively wrong, because it refuses a request with no token before the permission check can have an opinion, so a grant would appear to do nothing. What a guest may hold is bounded. Any .view or .read permission is grantable. A WRITE has to be blessed by whoever owns it, through the new get_guest_grantable_permissions hook (contract 0.21.0), and Secure Send is the only case that exists: sending a password to a vendor is the one write this product means to offer somebody who never signs in. Everything else is refused by the API and greyed out in the grid rather than warned about - a checkbox that quietly makes deleting an asset anonymous is not a warning anybody reads twice. A guest write records no creator: the trail says when and from where, never who, and that is the reason for the bar. GE-Enforce and Slides gained read permissions so they can be opened to guests without handing out the manage right. Their read routes accept either through require_any_permission, so nothing an existing manager could reach has moved. The navigation endpoint now returns only what the caller can actually reach. Without that a signed-out visitor gets a sidebar of the whole app where every link bounces off the login page, which reads as a broken site rather than as a closed door. A nav item may name several permissions; holding any one is enough, which is what GE-Enforce and Slides need now that a viewer and a manager hold different strings. Two things this found on the way: current_user is a LocalProxy, so `current_user is None` is False for a request with nobody behind it - harmless under a required JWT, an AttributeError the moment verification became optional. And an image pasted into a guide before its first save was never attached to it, so it was visible to editors and invisible to every reader; saving now attaches it, and `flask guides fix-images` repairs what is stored. The guides guest setting written earlier today is gone. Granting guides.view to the guest role replaces it, which is the point.
This commit is contained in:
cproudlock
190
tests/test_core/test_guest_role.py
Normal file
190
tests/test_core/test_guest_role.py
Normal file
@@ -0,0 +1,190 @@
|
||||
"""The guest role: what somebody with no account may do.
|
||||
|
||||
Guest access used to be a per-feature boolean - one for Secure Send, one per
|
||||
kiosk, one nearly written for guides - each with its own settings page and its
|
||||
own hand-written branch inside a route. This replaced all of that with a role,
|
||||
so the question is answered where every other permission question is answered.
|
||||
|
||||
The contract these tests hold:
|
||||
|
||||
- a guest holds nothing by default, so an upgrade opens no door
|
||||
- granting the guest role a permission is what opens one
|
||||
- revoking it closes the door on the next request, not the next restart
|
||||
- a guest can never be granted a write unless its owner declared it safe
|
||||
- a signed-in caller is unaffected by any of it
|
||||
"""
|
||||
|
||||
import pytest
|
||||
|
||||
from shopdb.core.models import Permission, Role
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def guestrole(db):
|
||||
"""The seeded guest role, holding nothing."""
|
||||
Permission.seed()
|
||||
Role.seedguest()
|
||||
db.session.commit()
|
||||
return Role.guest()
|
||||
|
||||
|
||||
def grant(db, role, *names):
|
||||
role.permissions = Permission.query.filter(Permission.name.in_(names)).all()
|
||||
db.session.commit()
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The default: nothing
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_a_guest_holds_nothing_by_default(guestrole):
|
||||
"""An upgrade must not open a door a site never asked to open."""
|
||||
assert guestrole.permissions == []
|
||||
|
||||
|
||||
def test_a_guest_is_refused_a_read_nobody_granted(client, guestrole):
|
||||
response = client.get('/api/auditlogs')
|
||||
assert response.status_code == 401
|
||||
|
||||
|
||||
def test_a_guest_is_refused_a_write(client, guestrole):
|
||||
response = client.post('/api/assets', json={'assetnumber': 'GUEST-1'})
|
||||
assert response.status_code == 401
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Granting, and taking it back
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_granting_the_guest_role_a_view_opens_that_read(client, db, guestrole):
|
||||
grant(db, guestrole, 'audit.view')
|
||||
response = client.get('/api/auditlogs')
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
def test_a_grant_does_not_leak_into_neighbouring_permissions(client, db, guestrole):
|
||||
"""audit.view is not apitokens.view. The role holds exactly what it was
|
||||
given."""
|
||||
grant(db, guestrole, 'audit.view')
|
||||
assert client.get('/api/apitokens').status_code == 401
|
||||
|
||||
|
||||
def test_a_write_stays_shut_even_when_a_read_is_open(client, db, guestrole):
|
||||
grant(db, guestrole, 'audit.view')
|
||||
assert client.post('/api/assets', json={'assetnumber': 'X'}).status_code == 401
|
||||
|
||||
|
||||
def test_revoking_closes_the_door_on_the_next_request(client, db, guestrole):
|
||||
"""Not on the next restart. Revoking is what somebody does the moment they
|
||||
realise what they opened up, and it has to take effect then."""
|
||||
grant(db, guestrole, 'audit.view')
|
||||
assert client.get('/api/auditlogs').status_code == 200
|
||||
|
||||
guestrole.permissions = []
|
||||
db.session.commit()
|
||||
assert client.get('/api/auditlogs').status_code == 401
|
||||
|
||||
|
||||
def test_a_site_with_no_guest_role_behaves_as_it_always_did(client, db):
|
||||
"""An older database, seeded before any of this existed."""
|
||||
role = Role.guest()
|
||||
if role is not None:
|
||||
db.session.delete(role)
|
||||
db.session.commit()
|
||||
assert client.get('/api/auditlogs').status_code == 401
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# The guardrail
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_a_view_permission_is_grantable(app):
|
||||
from shopdb.core.models import guest_grantable_permissions
|
||||
with app.app_context():
|
||||
grantable = guest_grantable_permissions()
|
||||
assert 'assets.view' in grantable
|
||||
assert 'reports.view' in grantable
|
||||
|
||||
|
||||
def test_a_write_permission_is_not_grantable(app):
|
||||
from shopdb.core.models import guest_grantable_permissions
|
||||
with app.app_context():
|
||||
grantable = guest_grantable_permissions()
|
||||
for name in ('assets.delete', 'assets.create', 'users.create',
|
||||
'settings.edit'):
|
||||
assert name not in grantable, name
|
||||
|
||||
|
||||
def test_the_api_refuses_to_grant_a_guest_a_write(client, db, auth_headers,
|
||||
guestrole):
|
||||
"""Refused at the door rather than warned about: a checkbox that quietly
|
||||
makes deleting an asset anonymous is not a warning anybody reads twice."""
|
||||
response = client.put('/api/users/roles/{}'.format(guestrole.roleid),
|
||||
headers=auth_headers,
|
||||
json={'permissions': ['assets.delete']})
|
||||
assert response.status_code == 400
|
||||
assert 'assets.delete' in response.get_json()['data']['error']['message']
|
||||
assert guestrole.permissions == []
|
||||
|
||||
|
||||
def test_the_api_allows_granting_a_guest_a_view(client, db, auth_headers,
|
||||
guestrole):
|
||||
response = client.put('/api/users/roles/{}'.format(guestrole.roleid),
|
||||
headers=auth_headers,
|
||||
json={'permissions': ['audit.view']})
|
||||
assert response.status_code == 200
|
||||
assert [p.name for p in guestrole.permissions] == ['audit.view']
|
||||
|
||||
|
||||
def test_the_guest_role_cannot_be_deleted(client, auth_headers, guestrole):
|
||||
"""Deleting it does not close the door, it removes the only place the door
|
||||
is described."""
|
||||
response = client.delete('/api/users/roles/{}'.format(guestrole.roleid))
|
||||
assert response.status_code in (401, 403)
|
||||
|
||||
response = client.delete('/api/users/roles/{}'.format(guestrole.roleid),
|
||||
headers=auth_headers)
|
||||
assert response.status_code == 400
|
||||
assert Role.guest() is not None
|
||||
|
||||
|
||||
def test_the_role_list_says_what_a_guest_may_hold(client, auth_headers,
|
||||
guestrole):
|
||||
"""So the grid can grey the rest out rather than letting somebody tick a
|
||||
box the API will refuse."""
|
||||
rows = client.get('/api/users/roles', headers=auth_headers).get_json()['data']
|
||||
guest = next(r for r in rows if r['rolename'] == 'guest')
|
||||
assert guest['isguest'] is True
|
||||
assert 'assets.view' in guest['grantable']
|
||||
assert 'assets.delete' not in guest['grantable']
|
||||
|
||||
other = next((r for r in rows if r['rolename'] != 'guest'), None)
|
||||
if other is not None:
|
||||
assert other['grantable'] is None, 'only the guest role is bounded'
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Signed-in callers are unaffected
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
def test_a_member_still_needs_their_own_permission(client, member_headers,
|
||||
db, guestrole):
|
||||
"""A grant to guests is not a grant to everybody: a signed-in member
|
||||
without the permission is still refused, with 403 rather than 401 because
|
||||
signing in would not change the answer."""
|
||||
grant(db, guestrole, 'audit.view')
|
||||
response = client.get('/api/auditlogs', headers=member_headers)
|
||||
assert response.status_code == 403
|
||||
|
||||
|
||||
def test_an_admin_is_unaffected(client, auth_headers, guestrole):
|
||||
assert client.get('/api/auditlogs', headers=auth_headers).status_code == 200
|
||||
|
||||
|
||||
def test_a_bad_token_is_still_refused(client, db, guestrole):
|
||||
"""Optional verification tolerates a MISSING token, not a broken one - a
|
||||
forged token must not be quietly downgraded to guest access."""
|
||||
grant(db, guestrole, 'audit.view')
|
||||
response = client.get('/api/auditlogs',
|
||||
headers={'Authorization': 'Bearer not.a.token'})
|
||||
assert response.status_code == 422
|
||||
Reference in New Issue
Block a user