From eb5ce1bd5c3ff9baeebcbbb1641c12b05b252362 Mon Sep 17 00:00:00 2001 From: Darks Date: Tue, 26 Apr 2022 23:29:11 +0200 Subject: [PATCH] attachement: switch to uuid + check permission in dl widget (#109) Also added is_default_accessible() to Thread class as its owner may be a Topic with forum access restrictions or public main content (like Program) [MIGRATION] This commit contains a new version of the schema. /!\ This migration breaks all attachments --- app/models/attachment.py | 9 +++-- app/models/thread.py | 7 ++++ app/models/user.py | 4 +++ app/utils/markdown_extensions/pclinks.py | 16 +++++++-- ...33816b21_switched_attachment_id_to_uuid.py | 34 +++++++++++++++++++ 5 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 migrations/versions/72df33816b21_switched_attachment_id_to_uuid.py diff --git a/app/models/attachment.py b/app/models/attachment.py index 0bc5856..b710298 100644 --- a/app/models/attachment.py +++ b/app/models/attachment.py @@ -1,13 +1,16 @@ from werkzeug.utils import secure_filename +from sqlalchemy.dialects.postgresql import UUID from sqlalchemy.orm import backref from app import db from app.utils.filesize import filesize from config import V5Config import os +import uuid + class Attachment(db.Model): __tablename__ = 'attachment' - id = db.Column(db.Integer, primary_key=True) + id = db.Column(UUID(as_uuid=True), primary_key=True, default=uuid.uuid4) # Original name of the file name = db.Column(db.Unicode(64)) @@ -24,11 +27,11 @@ class Attachment(db.Model): @property def path(self): return os.path.join(V5Config.DATA_FOLDER, "attachments", - f"{self.id:05}", self.name) + f"{self.id}", self.name) @property def url(self): - return f"/fichiers/{self.id:05}/{self.name}" + return f"/fichiers/{self.id}/{self.name}" def __init__(self, file, comment): diff --git a/app/models/thread.py b/app/models/thread.py index 8dfaa52..cedb4d6 100644 --- a/app/models/thread.py +++ b/app/models/thread.py @@ -53,6 +53,13 @@ class Thread(db.Model): return self.owner_program[0] return None + def is_default_accessible(self): + if self.owner_program != []: + return True + if self.owner_topic != []: + return self.owner_topic[0].forum.is_default_accessible() + return False + def delete(self): """Recursively delete thread and all associated contents.""" # Remove reference to top comment diff --git a/app/models/user.py b/app/models/user.py index 325040d..e1a006d 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -250,6 +250,10 @@ class Member(User): post = comment.thread.owner_post return self.can_edit_post(post) and (comment.author == post.author) + def can_access_file(self, file): + """Whether this member can access the file.""" + return self.can_access_post(file.comment) + def update(self, **data): """ Update all or part of the user's metadata. The [data] dictionary diff --git a/app/utils/markdown_extensions/pclinks.py b/app/utils/markdown_extensions/pclinks.py index f674f60..0e94793 100644 --- a/app/utils/markdown_extensions/pclinks.py +++ b/app/utils/markdown_extensions/pclinks.py @@ -13,8 +13,10 @@ License: [BSD](https://opensource.org/licenses/bsd-license.php) from markdown.extensions import Extension from markdown.inlinepatterns import InlineProcessor +from uuid import UUID import xml.etree.ElementTree as etree from flask import url_for, render_template +from flask_login import current_user from app.utils.unicode_names import normalize from app.utils.filters.humanize import humanize from app.models.poll import Poll @@ -36,7 +38,7 @@ class PCLinkExtension(Extension): self.md = md # append to end of inline patterns - PCLINK_RE = r'<([a-z]+): ?(\w+)>' + PCLINK_RE = r'<([a-z]+): ?([\w-]+)>' pclinkPattern = PCLinksInlineProcessor(PCLINK_RE, self.getConfigs()) pclinkPattern.md = md md.inlinePatterns.register(pclinkPattern, 'pclink', 135) @@ -125,7 +127,7 @@ def handleUser(content_id, context): def handleFile(content_id, context): try: - content_id = int(content_id) + UUID(content_id) except ValueError: return "[ID de fichier invalide]" @@ -134,6 +136,16 @@ def handleFile(content_id, context): if file is None: return "[Fichier non trouvé]" + # Manage permissions + # TODO: use Guest methods when implemented + if current_user.is_authenticated: + if not current_user.can_access_file(file): + return "[Accès au fichier refusé]" + else: + if not file.comment.thread.is_default_accessible(): + return "[Accès au fichier refusé]" + + html = render_template('widgets/download_button.html', file=file) html = html.replace('\n', '') # Needed to avoid lots of
due to etree return etree.fromstring(html) diff --git a/migrations/versions/72df33816b21_switched_attachment_id_to_uuid.py b/migrations/versions/72df33816b21_switched_attachment_id_to_uuid.py new file mode 100644 index 0000000..f330989 --- /dev/null +++ b/migrations/versions/72df33816b21_switched_attachment_id_to_uuid.py @@ -0,0 +1,34 @@ +"""Switched attachment id to UUID + +Revision ID: 72df33816b21 +Revises: d2227d2479e2 +Create Date: 2022-04-26 21:50:05.466388 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = '72df33816b21' +down_revision = 'd2227d2479e2' +branch_labels = None +depends_on = None + + +def upgrade(): + # Create uuid-ossp extension, required to use uuid_generate_v4() + op.execute('CREATE EXTENSION IF NOT EXISTS "uuid-ossp";') + # /!\ This operation will break all attachments /!\ + op.execute("""ALTER TABLE attachment ALTER COLUMN id DROP DEFAULT, + ALTER COLUMN id SET DATA TYPE UUID USING (uuid_generate_v4()), + ALTER COLUMN id SET DEFAULT uuid_generate_v4();""") + # ### end Alembic commands ### + + +def downgrade(): + # /!\ This operation will break all attachments /!\ + op.execute("""ALTER TABLE attachment ALTER COLUMN id DROP DEFAULT, + ALTER COLUMN id SET DATA TYPE integer USING nextval('attachment_id_seq'::regclass), + ALTER COLUMN id SET DEFAULT nextval('attachment_id_seq'::regclass);""") + # ### end Alembic commands ###