diff --git a/plugins/guides/api/routes.py b/plugins/guides/api/routes.py index ac59426c..a4801d7d 100644 --- a/plugins/guides/api/routes.py +++ b/plugins/guides/api/routes.py @@ -11,6 +11,7 @@ serving the working copy from the reader's route would quietly undo it. """ from datetime import datetime +import json import re from flask import Blueprint, request, Response @@ -19,7 +20,7 @@ from flask_jwt_extended import jwt_required, get_jwt_identity, current_user from shopdb.api import ( db, success_response, error_response, ErrorCodes, - require_permission, + require_permission, harden_upload_response, ) from ..models import Guide, GuideRevision, GuideImage @@ -109,27 +110,51 @@ def list_guides(): drafts are invisible to readers - but an author must be able to find their own, or drafting is a trap door. """ - query = Guide.query + caneditall = _has('guides.edit') + mine = _userid() + + # Match against the PUBLISHED text for anything the caller is only allowed + # to read, and against the working copy only for guides they may edit. + # Searching the working copy for everyone leaks draft wording to readers: + # they get a hit on a word that is not in the page when they open it. + published = db.aliased(GuideRevision) + query = Guide.query.outerjoin( + published, Guide.publishedrevisionid == published.guiderevisionid) search = (request.args.get('q') or '').strip() if search: like = '%{}%'.format(search) - query = query.filter(db.or_( - Guide.title.ilike(like), - Guide.summary.ilike(like), - Guide.tags.ilike(like), + + def matches(title, summary, tags, blocks): # The body too: people look for "the one about the drive letter", # not for a title they never read. - Guide.blocksjson.ilike(like), - )) + return db.or_(title.ilike(like), summary.ilike(like), + tags.ilike(like), blocks.ilike(like)) + + working = matches(Guide.title, Guide.summary, Guide.tags, Guide.blocksjson) + readable = matches(published.title, published.summary, + published.tags, published.blocksjson) + if caneditall: + query = query.filter(db.or_(working, readable)) + else: + query = query.filter(db.or_( + readable, + db.and_(Guide.createdby == mine, working), + )) tag = (request.args.get('tag') or '').strip() if tag: - query = query.filter(Guide.tags.ilike('%{}%'.format(tag))) + like = '%{}%'.format(tag) + if caneditall: + query = query.filter(db.or_(Guide.tags.ilike(like), + published.tags.ilike(like))) + else: + query = query.filter(db.or_( + published.tags.ilike(like), + db.and_(Guide.createdby == mine, Guide.tags.ilike(like)), + )) - caneditall = _has('guides.edit') if not caneditall: - mine = _userid() query = query.filter(db.or_( Guide.publishedrevisionid.isnot(None), Guide.createdby == mine, @@ -143,6 +168,18 @@ def list_guides(): data['ispublished'] = bool(guide.publishedrevisionid) data['checklistitems'] = checklistitemcount(guide.blocks) data['canedit'] = _mayedit(guide) + + # A reader's card must describe the guide they will actually open. The + # Guide row carries the working copy, so a retitled draft would list + # under its new name and open under its old one. + if not data['canedit'] and guide.publishedrevisionid: + revision = db.session.get(GuideRevision, guide.publishedrevisionid) + if revision is not None: + data['title'] = revision.title + data['summary'] = revision.summary + data['tags'] = [t.strip() for t in (revision.tags or '').split(',') + if t.strip()] + data['checklistitems'] = checklistitemcount(revision.blocks) out.append(data) return success_response(out) @@ -151,9 +188,24 @@ def list_guides(): @jwt_required() @require_permission('guides.view') def list_tags(): - """Every tag in use, with a count, for the filter chips.""" + """Every tag in use, with a count, for the filter chips. + + Readers count published tags only. A chip carrying a tag that exists solely + on somebody's draft is both a leak and a dead end - filtering by it returns + nothing a reader is allowed to see. + """ counts = {} - for (tags,) in db.session.query(Guide.tags).filter(Guide.tags.isnot(None)).all(): + if _has('guides.edit'): + pairs = db.session.query(Guide.tags).filter(Guide.tags.isnot(None)).all() + else: + published = db.aliased(GuideRevision) + pairs = db.session.query(published.tags).join( + Guide, Guide.publishedrevisionid == published.guiderevisionid + ).filter(published.tags.isnot(None)).all() + mine = _userid() + pairs += db.session.query(Guide.tags).filter( + Guide.createdby == mine, Guide.tags.isnot(None)).all() + for (tags,) in pairs: for tag in [t.strip() for t in tags.split(',') if t.strip()]: counts[tag] = counts.get(tag, 0) + 1 return success_response([ @@ -229,6 +281,13 @@ def list_revisions(guideid): guide = db.session.get(Guide, guideid) if guide is None: return error_response(ErrorCodes.NOT_FOUND, 'Guide not found', http_code=404) + # History is editors' business. guides.view alone is permission to read what + # was PUBLISHED; the revision list is every draft this guide has ever been, + # including the ones deliberately not published yet. + if not _mayedit(guide): + return error_response(ErrorCodes.FORBIDDEN, + 'You do not have permission to see this history', + http_code=403) rows = (GuideRevision.query.filter_by(guideid=guideid) .order_by(GuideRevision.revision.desc()).all()) published = guide.publishedrevisionid @@ -242,10 +301,27 @@ def list_revisions(guideid): @jwt_required() @require_permission('guides.view') def read_revision(revisionid): + """One revision by id. + + Gated on its PARENT, not on guides.view alone. Revision ids are sequential + integers, so an ungated route here is an invitation to walk 1..n and read + every unpublished draft on the site - which is the opposite of what + docs/GUIDES.md promises about drafts. + + A reader may fetch the published revision (it is what the slug route would + serve anyway); everything else takes edit rights on that guide. + """ revision = db.session.get(GuideRevision, revisionid) if revision is None: return error_response(ErrorCodes.NOT_FOUND, 'Revision not found', http_code=404) + guide = db.session.get(Guide, revision.guideid) + ispublished = bool(guide is not None + and guide.publishedrevisionid == revision.guiderevisionid) + if not ispublished and not (guide is not None and _mayedit(guide)): + # 404, not 403: a draft's existence is itself the thing being withheld. + return error_response(ErrorCodes.NOT_FOUND, 'Revision not found', + http_code=404) return success_response(revision.to_dict(includeblocks=True)) @@ -311,7 +387,16 @@ def update_guide(guideid): db.session.rollback() return error_response(ErrorCodes.VALIDATION_ERROR, str(exc)) - if 'title' in data: + # The slug is set once, at create, and does NOT follow the title. + # + # _slugify's own docstring says the slug is stored so that renaming cannot + # break a link somebody pasted into a work order months ago - and then this + # re-slugged on every save that carried a title, which the editor's 3-second + # autosave does on every keystroke. Fixing a typo in a heading silently + # moved the guide's URL and broke exactly the links the stored slug exists + # to protect. A guide with no slug (older row, or a failed create) still + # gets one here. + if not guide.slug: guide.slug = _slugify(guide.title, guide.guideid) db.session.commit() @@ -426,6 +511,47 @@ def upload_image(): return success_response(image.to_dict(), message='Image uploaded') +def _mayseeimage(image): + """Whether the caller may fetch this image. + + An image is content. One pasted into a published guide must be readable by + anyone who can read that guide; one pasted into a draft must not be, or the + draft leaks a screenshot at a time to anyone willing to count upwards + through image ids. + + So: editors of the parent see it, and everyone else sees it only if the + PUBLISHED revision actually references it. An unattached image - pasted + before the guide was first saved - belongs to whoever uploaded it. + """ + if image.guideid is None: + return _has('guides.edit') or image.uploadedby == _userid() + + guide = db.session.get(Guide, image.guideid) + if guide is None: + return _has('guides.edit') + if _mayedit(guide): + return True + if not guide.publishedrevisionid: + return False + + revision = db.session.get(GuideRevision, guide.publishedrevisionid) + if revision is None: + return False + try: + blocks = json.loads(revision.blocksjson or '[]') + except (TypeError, ValueError): + return False + needle = '/guides/images/{}'.format(image.guideimageid) + for block in blocks: + if not isinstance(block, dict) or block.get('type') != 'image': + continue + if str(block.get('imageid') or '') == str(image.guideimageid): + return True + if needle in str(block.get('url') or ''): + return True + return False + + @guides_bp.route('/images/', methods=['GET']) @jwt_required() @require_permission('guides.view') @@ -433,10 +559,15 @@ def serve_image(imageid): image = db.session.get(GuideImage, imageid) if image is None: return error_response(ErrorCodes.NOT_FOUND, 'Image not found', http_code=404) - return Response(image.data, mimetype=image.mimetype, headers={ + if not _mayseeimage(image): + return error_response(ErrorCodes.NOT_FOUND, 'Image not found', http_code=404) + # Same inert-document headers every other upload route serves under: an + # uploaded file is attacker-influenced content and must not be able to run + # as a document in this origin. + return harden_upload_response(Response(image.data, mimetype=image.mimetype, headers={ # Immutable: an image is never edited in place, only replaced by a new # upload with a new id, so it can be cached hard. 'Cache-Control': 'private, max-age=31536000, immutable', 'Content-Disposition': 'inline; filename="{}"'.format( image.filename or 'image{}'.format(imageid)), - }) + })) diff --git a/plugins/guides/frontend/components/GuideBlocks.vue b/plugins/guides/frontend/components/GuideBlocks.vue index ebc24551..d4459517 100644 --- a/plugins/guides/frontend/components/GuideBlocks.vue +++ b/plugins/guides/frontend/components/GuideBlocks.vue @@ -72,8 +72,27 @@
{{ block.text }}
+
- + +

+ Image unavailable{{ block.alt ? ': ' + block.alt : '' }} +

+

Loading image...

{{ block.caption }}
@@ -97,8 +116,9 @@