From 0b4a1db842b78cc67e4ad3bcb83c4c5123d66dde Mon Sep 17 00:00:00 2001 From: Cam Proudlock Date: Fri, 21 Aug 2026 15:53:42 -0400 Subject: [PATCH] Secure send: guests by setting, and a custom view count --- plugins/tools/api/routes.py | 126 ++++++++++++++++-- .../tools/frontend/views/SecureSendCreate.vue | 23 +++- plugins/tools/plugin.py | 39 ++++-- tests/test_plugins/test_tools_securesend.py | 83 ++++++++++++ 4 files changed, 247 insertions(+), 24 deletions(-) diff --git a/plugins/tools/api/routes.py b/plugins/tools/api/routes.py index f55dd620..9dd4231d 100644 --- a/plugins/tools/api/routes.py +++ b/plugins/tools/api/routes.py @@ -6,9 +6,15 @@ on first read needs somewhere to keep the ciphertext until then. TWO DIFFERENT DOORS, on purpose. -CREATE needs a login and a permission. Anonymous creation would be an -untraceable way to move data out of a system that otherwise records who did -what, and the creator is the only person these rows can ever name. +CREATE needs a LOGIN and nothing more. Sharing a password is something anybody +at the site may legitimately need to do, and a permission somebody has to be +granted first turns a two-minute job into a ticket - which is how people end up +pasting credentials into chat instead. + +The login stays, though, and is not a formality: anonymous creation would make +this an unattributable write endpoint that stores arbitrary ciphertext, and the +creator is the only person these rows can ever name. Everybody browsing this app +is already signed in, so requiring it costs nothing. READ needs neither, because the recipient usually has no account here and the link is normally handed over in a call or a chat. The unguessable token IS the @@ -27,12 +33,12 @@ import secrets as pysecrets from datetime import datetime, timedelta, timezone from flask import Blueprint, request -from flask_jwt_extended import jwt_required, get_jwt_identity +from flask import current_app +from flask_jwt_extended import jwt_required, get_jwt_identity, current_user from shopdb.api import ( - db, + db, cache, Setting, success_response, error_response, ErrorCodes, - require_permission, ) from ..models import SecureSecret @@ -50,18 +56,113 @@ MAX_CIPHERTEXT = 4096 DEFAULT_EXPIRY_HOURS = 24 MAX_EXPIRY_HOURS = 168 -MAX_VIEWS = 5 +# Generous, because "how many people need this password" is the site's business +# and not a number worth arguing with. The floor matters more than the ceiling: +# a secret readable an unlimited number of times is a password on a web page. +MAX_VIEWS = 100 + +# Guest creation, when a site enables it, is an unauthenticated write. Bounded +# per address so it cannot be used to fill a table or park data: a person +# sharing passwords does it a handful of times an hour, not hundreds. +GUEST_CREATE_MAX = 10 +GUEST_CREATE_WINDOW_SECONDS = 600 def _utcnow(): return datetime.now(timezone.utc).replace(tzinfo=None) +def _sweep_expired(): + """Delete secrets whose window has passed, whoever asked. + + Redeeming removes a row, and so does touching an expired one - but a secret + nobody ever opens was removed by NOTHING. The recipient goes on holiday, the + link is never clicked, and the ciphertext sits there for good: not a + disclosure risk, since it cannot be read, but it makes "nothing lingers" + untrue and grows the table in exactly the case where the feature failed. + + Swept opportunistically rather than on a schedule: creating a secret is + authenticated and infrequent, and an air-gapped site is not going to have a + cron job anybody remembers to add. + """ + stale = SecureSecret.query.filter(SecureSecret.expiresat < _utcnow()).all() + for secret in stale: + db.session.delete(secret) + return len(stale) + + +def _guests_allowed(): + """May somebody with no account create a secret? Default NO. + + Read straight from the row rather than the settings cache: this decides + whether an unauthenticated caller may write, and being wrong on the + permissive side for five minutes is the wrong way round to be wrong. + """ + try: + row = Setting.query.filter_by(key='securesend_allow_guests').first() + except Exception: + # FAIL CLOSED. If the setting cannot be read - the row is missing, the + # table is not there yet on a half-migrated site, the database is + # unhappy - the answer is no. An error must never be the thing that + # opens an unauthenticated write endpoint. + current_app.logger.exception('could not read securesend_allow_guests') + return False + return bool(row and str(row.value).strip().lower() in ('true', '1', 'yes')) + + +def _guest_ratelimited(): + """Bounded guest creation, per address, in a fixed window. + + Same shape as the login limiter. Not a security boundary - a determined + person has more addresses - but it stops an open endpoint being an easy + place to park data or fill a disk. + """ + address = (request.headers.get('X-Forwarded-For', '').split(',')[0].strip() + or request.remote_addr or 'unknown') + bucket = int(_utcnow().timestamp()) // GUEST_CREATE_WINDOW_SECONDS + key = f'securesendguest:{address}:{bucket}' + used = cache.get(key) or 0 + if used >= GUEST_CREATE_MAX: + return True + cache.set(key, used + 1, timeout=GUEST_CREATE_WINDOW_SECONDS) + return False + + @tools_bp.route('/secrets', methods=['POST']) -@jwt_required() -@require_permission('tools.secrets') +@jwt_required(optional=True) def create_secret(): - """Store an already-encrypted secret and return its token.""" + """Store an already-encrypted secret and return its token. + + OPTIONAL jwt so the SETTING decides rather than the decorator: a guest has + to reach the handler to be told whether this site lets them in. + """ + # WHO MAY CREATE. Signed in: the tools.secrets permission, with the usual + # admin bypass. Not signed in: only when the site has deliberately opened + # it, and then within a rate limit. + # get_jwt_identity(), NOT `current_user is None`: current_user is a + # LocalProxy that is never None itself, so that test silently took the + # signed-in branch for a guest and then blew up dereferencing it. + identity = get_jwt_identity() + if identity is None: + if not _guests_allowed(): + return error_response( + ErrorCodes.UNAUTHORIZED, + 'Sign in to create a secret link.', + http_code=401) + if _guest_ratelimited(): + return error_response( + ErrorCodes.VALIDATION_ERROR, + 'Too many links from this address. Try again shortly.', + http_code=429) + elif not current_user.haspermission('tools.secrets'): + return error_response( + ErrorCodes.FORBIDDEN, + 'You do not have permission to create secret links.', + http_code=403) + # Tidy up on the way past. Cheap, and it keeps the table bounded without a + # scheduler existing anywhere. + _sweep_expired() + data = request.get_json(silent=True) or {} ciphertext = (data.get('ciphertext') or '').strip() iv = (data.get('iv') or '').strip() @@ -173,4 +274,9 @@ def peek_secret(token): """ secret = SecureSecret.query.filter_by(token=(token or '').strip()).first() live = bool(secret and not secret.is_expired() and not secret.is_spent()) + if secret is not None and not live: + # A late click tidies its own row, so a dead secret does not need + # somebody to create a new one before it is cleaned up. + db.session.delete(secret) + db.session.commit() return success_response({'available': live}) diff --git a/plugins/tools/frontend/views/SecureSendCreate.vue b/plugins/tools/frontend/views/SecureSendCreate.vue index 591acd99..7bef0d91 100644 --- a/plugins/tools/frontend/views/SecureSendCreate.vue +++ b/plugins/tools/frontend/views/SecureSendCreate.vue @@ -53,12 +53,17 @@ @@ -99,6 +104,11 @@ import { const supported = cryptoAvailable() +// Matches the server's ceiling. A free number rather than a dropdown because +// "how many people need this password" is the sender's business - a shift +// handover is not the same as one colleague on a call. +const MAX_VIEWS = 100 + const plaintext = ref('') const expiryhours = ref(24) const views = ref(1) @@ -205,6 +215,7 @@ function reset() { .options { display: flex; gap: 1rem; flex-wrap: wrap; } .options .field { flex: 1 1 12rem; } .counter { font-size: 0.8125rem; color: var(--text-light); margin: 0 0 0.5rem; } +.field-hint { display: block; font-size: 0.8125rem; color: var(--text-light); margin-top: 4px; } .counter.over { color: var(--warning); font-weight: 600; } .control-note { color: var(--warning); font-size: 0.875rem; margin: 8px 0; } .control-hint { color: var(--text-light); font-size: 0.8125rem; margin: 8px 0 12px; } diff --git a/plugins/tools/plugin.py b/plugins/tools/plugin.py index a1cbc7fb..6f369519 100644 --- a/plugins/tools/plugin.py +++ b/plugins/tools/plugin.py @@ -54,21 +54,44 @@ class ToolsPlugin(BasePlugin): return [SecureSecret] def get_permissions(self) -> List: - """One permission, on CREATING a secret only. + """Who may CREATE a secret, when guests are not allowed. - There is deliberately no read permission: the recipient usually has no - account here, and the unguessable token is what authorises the read. - Gating that side would make the tool useless for the case it exists for - - handing somebody a credential while you are on a call with them. + The default access model: signed in, and holding this. Reading needs no + permission at all - the recipient usually has no account here and the + unguessable token is the authorisation. - No delete permission either. A secret is spent by being read or by - expiring, and both remove the row; a hand-delete route would only add a - way to destroy the record of a secret that had not been read yet. + A site that turns on securesend_allow_guests bypasses this for people + with no account; it still applies to anyone who IS signed in, so a + member without it cannot create one while a guest can. That is + deliberate: opening the door to guests is a decision about the door, not + about revoking what a role does or does not hold. """ return [ ('tools.secrets', 'Create secure send links', 'tools'), ] + def get_settings_defaults(self) -> List[dict]: + """The one switch this plugin owns. + + Public so the create page can read it BEFORE login and show a guest the + form rather than a sign-in wall. It says nothing sensitive: whether the + door is open is visible to anyone who tries the door. + """ + return [ + { + 'key': 'securesend_allow_guests', + 'value': 'false', + 'valuetype': 'boolean', + 'category': 'tools', + 'public': True, + 'description': ('Let people with no account create secure send ' + 'links. Off by default: a guest secret has no ' + 'creator, so the audit trail can say when and ' + 'from where but never who. Receiving a secret ' + 'never needs an account either way.'), + }, + ] + def get_navigation_items(self) -> List[dict]: return [ { diff --git a/tests/test_plugins/test_tools_securesend.py b/tests/test_plugins/test_tools_securesend.py index 00c31161..4b8b3044 100644 --- a/tests/test_plugins/test_tools_securesend.py +++ b/tests/test_plugins/test_tools_securesend.py @@ -159,3 +159,86 @@ def test_an_unknown_token_answers_the_same_as_a_spent_one(client, auth_headers): assert spent.status_code == never.status_code == 404 assert (spent.get_json()['data']['error']['message'] == never.get_json()['data']['error']['message']) + + +# ------------------------------------------------- guests, when a site allows + +def _allow_guests(db, allowed=True): + from shopdb.core.models import Setting + row = Setting.query.filter_by(key='securesend_allow_guests').first() + if row is None: + row = Setting(key='securesend_allow_guests', valuetype='boolean', + category='tools') + db.session.add(row) + row.value = 'true' if allowed else 'false' + db.session.commit() + + +def test_a_guest_is_refused_by_default(client, db): + """Default OFF. An unauthenticated write endpoint is not something to get + by accident.""" + _allow_guests(db, False) + resp = client.post('/api/tools/secrets', json={'ciphertext': CIPHER, 'iv': IV}) + assert resp.status_code in (401, 403, 422) + + +def test_a_guest_can_create_once_a_site_allows_it(client, db): + _allow_guests(db, True) + resp = client.post('/api/tools/secrets', json={'ciphertext': CIPHER, 'iv': IV}) + assert resp.status_code in (200, 201), resp.get_data(as_text=True)[:300] + assert resp.get_json()['data']['token'] + + +def test_a_guest_secret_has_no_creator(client, db): + """The cost of the setting, pinned: the audit trail can say when and from + where, but never who.""" + _allow_guests(db, True) + token = client.post('/api/tools/secrets', + json={'ciphertext': CIPHER, 'iv': IV} + ).get_json()['data']['token'] + secret = SecureSecret.query.filter_by(token=token).first() + assert secret.createdbyuserid is None + + +def test_guest_creation_is_rate_limited(client, db): + """An open endpoint must not be an easy place to park data.""" + from plugins.tools.api.routes import GUEST_CREATE_MAX + _allow_guests(db, True) + codes = [client.post('/api/tools/secrets', + json={'ciphertext': CIPHER, 'iv': IV}).status_code + for _ in range(GUEST_CREATE_MAX + 2)] + assert 429 in codes, codes + + +def test_a_signed_in_user_still_needs_the_permission(client, db, member_headers): + """Opening the door to guests is a decision about the DOOR, not about + revoking what a role holds.""" + _allow_guests(db, True) + resp = client.post('/api/tools/secrets', + json={'ciphertext': CIPHER, 'iv': IV}, + headers=member_headers) + assert resp.status_code == 403 + + +# ----------------------------------------------------------------- retention + +def test_an_expired_secret_nobody_opened_is_swept(client, db, auth_headers): + """Nothing removed a secret that was never clicked: the recipient goes on + holiday and the ciphertext sits there for good.""" + from datetime import datetime, timedelta, timezone + token = _create(client, auth_headers).get_json()['data']['token'] + secret = SecureSecret.query.filter_by(token=token).first() + secret.expiresat = (datetime.now(timezone.utc).replace(tzinfo=None) + - timedelta(hours=1)) + db.session.commit() + + # Any later create sweeps it, without a scheduler existing anywhere. + _create(client, auth_headers) + assert SecureSecret.query.filter_by(token=token).first() is None + + +def test_a_custom_view_count_is_allowed(client, auth_headers): + from plugins.tools.api.routes import MAX_VIEWS + assert _create(client, auth_headers, views=25).status_code in (200, 201) + assert _create(client, auth_headers, views=MAX_VIEWS).status_code in (200, 201) + assert _create(client, auth_headers, views=MAX_VIEWS + 1).status_code == 400