From 6f98cba65e957b568f9430305f12ddcaca73374f Mon Sep 17 00:00:00 2001 From: Lephe Date: Fri, 26 Feb 2021 18:29:25 +0100 Subject: [PATCH] review of privileges and forum permissions * Sorted privileges into categories, similar to the v4.3 style Added privilege check utilities: * Forum: is_news(), is_default_accessible() and is_default_postable() * Member: can_access_forum(), can_post_in_forum(), can_edit_post(), and can_delete_post() Unfortunately current_user is not a Guest when logged out, so one cannot usually write current_user.can_*() without checking for authentication first, so the checks are still somewhat verbose. Reviewed forum permissions; the following permission issues have been fixed (I have tested most but not all of them prior to fixing): * app/routes/forum/index.py: Users that were not meant to access a forum could still obtain a listing of the topics * app/routes/forum/topic.py: Users that were not meant to see topics could still read them by browsing the URL * app/routes/forum/topic.py: Authenticated users could post in any topic, including ones that they should not have access to * app/routes/posts/edit.py: Users with edit.posts (eg. mods) could edit and delete messages in forums they can't access (eg. creativecalc) * app/templates/account/user.html: Users with admin panel access would see account editing links they can't use (affects developers) * app/templates/base/navbar/forum.html: The "Forum" tab would list all forums including ones the user doesn't have access to * app/templates/forum/index.html: Users would see every single forum, including ones they can't access * app/template/widgets/thread.html: Anyone would see Edit/Delete links on every message, even though most were unusable Miscellaneous changes: * app/routes/forum/topic.py: Ordered comments by date as intended, which I assume worked by chance until now * Removed the old assets/privs.txt files which is now superseded by the list implemented in app/data/groups.yaml This commit changes group and forum information, run master.py with: @> forums update @> groups update --- app/data/forums.yaml | 12 ++- app/data/groups.yaml | 118 +++++++++++++++++-------- app/models/forum.py | 13 +++ app/models/user.py | 29 ++++++ app/routes/admin/account.py | 4 +- app/routes/admin/attachments.py | 2 +- app/routes/admin/config.py | 2 +- app/routes/admin/forums.py | 2 +- app/routes/admin/groups.py | 2 +- app/routes/admin/index.py | 2 +- app/routes/admin/members.py | 2 +- app/routes/admin/trophies.py | 6 +- app/routes/forum/index.py | 15 ++-- app/routes/forum/topic.py | 14 ++- app/routes/posts/edit.py | 8 +- app/templates/account/user.html | 2 +- app/templates/base/footer.html | 2 +- app/templates/base/navbar/account.html | 2 +- app/templates/base/navbar/forum.html | 5 +- app/templates/forum/forum.html | 6 +- app/templates/forum/index.html | 32 ++++--- app/templates/forum/topic.html | 7 +- app/templates/widgets/thread.html | 9 +- app/utils/priv_required.py | 2 +- app/utils/validators/file.py | 4 +- assets/privs.txt | 42 --------- 26 files changed, 201 insertions(+), 143 deletions(-) delete mode 100644 assets/privs.txt diff --git a/app/data/forums.yaml b/app/data/forums.yaml index 9f6c931..2485a30 100644 --- a/app/data/forums.yaml +++ b/app/data/forums.yaml @@ -1,10 +1,8 @@ # This file is a list of forums to create when setting up Planète Casio. # # * Keys are used as URLs paths and for unique identification. -# * Prefixes represent the privilege category for a forum. Owning privileges -# with this prefix allows the user to post in this forum and all its -# sub-forum regardless of their settings ("forum-root-*" are hyper powerful). -# * For open forums, use the prefix "open". +# * Prefixes are used to identify privileges for each forum, see groups.yaml +# for details. /: name: Forum de Planète Casio @@ -104,8 +102,8 @@ prefix: discussion descr: Sujets hors-sujet et discussion libre. -# Limited-access board -# Prefixes "admin" and "assoc" are reserved for this and require special +# Limited-access boards +# Prefixes "admin" and "creativecalc" are reserved for this and require special # privileges to list, read and edit topics and messages. /admin: @@ -116,5 +114,5 @@ /creativecalc: name: CreativeCalc - prefix: assoc + prefix: creativecalc descr: Forum privé de l'association CreativeCalc, réservé aux membres. diff --git a/app/data/groups.yaml b/app/data/groups.yaml index 97e2b5f..03ffd21 100644 --- a/app/data/groups.yaml +++ b/app/data/groups.yaml @@ -1,63 +1,109 @@ +# LIST OF PRIVILEGES: +# +# Access to specific forums (see forums.yaml for prefix values): +# forum.access. +# forum.post. +# forum.post-news +# forum.post-anywhere +# -> All forums are readable by default except and +# -> All forums are writable by default except , , +# children of , and forums with children ("categories") +# -> Use member.can_access_forum(forum) and member.can_post_in_forum(forum) +# +# Access to extended publication methods: +# publish.schedule-posts +# publish.pin-posts +# publish.shared-files +# +# Moderation: +# edit.posts +# edit.tests +# edit.accounts +# edit.trophies +# delete.posts +# delete.tests +# delete.accounts +# delete.shared-files +# move.posts +# +# Shoutbox: +# shoutbox.kick +# shoutbox.ban +# +# Miscellaneous: +# misc.unlimited-pms +# misc.dev-infos +# misc.community-login +# misc.admin-panel +# misc.no-upload-limits +# +# TODO: PRIVILEGES NOT YET IMPLEMENTED: +# The features that these privileges control are not implemented yet, or the +# privilege checks are missing. +# +# publish.* +# edit.tests +# delete.tests delete.shared-files +# move.posts +# shoutbox.* +# misc.unlimited-pms +# misc.community-login + - name: Administrateur css: "color: #ee0000;" descr: "Vous voyez Chuck Norris ? Pareil." - privs: access-admin-board access-assoc-board write-news write-anywhere - upload-shared-files delete-shared-files - edit-posts delete-posts scheduled-posting - delete-content move-public-content move-private-content showcase-content - edit-static-content extract-posts - delete-notes delete-tests - shoutbox-kick shoutbox-ban - unlimited-pms footer-statistics community-login - access-admin-panel edit-account delete-account edit-trophies - delete_notification no-upload-limits + privs: forum.access.admin forum.access.creativecalc forum.post-news + forum.post-anywhere + publish.schedule-posts publish.pin-posts publish.shared-files + edit.posts edit.tests edit.accounts edit.trophies + delete.posts delete.tests delete.accounts delete.shared-files + move.posts + shoutbox.kick shoutbox.ban + misc.unlimited-pms misc.dev-infos misc.community-login misc.admin-panel + misc.no-upload-limits - name: Modérateur css: "color: green;" descr: "Maîtres du kick, ils sont là pour faire respecter un semblant d'ordre." - privs: access-admin-board - edit-posts delete-posts - move-public-content extract-posts - delete-notes delete-tests - shoutbox-kick shoutbox-ban - unlimited-pms no-upload-limits + privs: forum.access.admin + edit.posts edit.tests + delete.posts delete.tests + move.posts + shoutbox.kick shoutbox.ban + misc.unlimited-pms misc.no-upload-limits - name: Développeur css: "color: #4169e1;" descr: "Les développeurs maintiennent et améliorent le code du site." - privs: access-admin-board - upload-shared-files delete-shared-files - scheduled-posting - edit-static-content - unlimited-pms footer-statistics community-login - access-admin-panel no-upload-limits + privs: forum.access.admin forum.post-anywhere + publish.schedule-posts publish.shared-files + delete.shared-files + misc.unlimited-pms misc.dev-infos misc.community-login misc.admin-panel - name: Rédacteur css: "color: blue;" descr: "Rédigent les meilleurs articles de la page d'accueil, rien que pour vous <3" - privs: access-admin-board write-news - upload-shared-files delete-shared-files - scheduled-posting - showcase-content edit-static-content - no-upload-limits + privs: forum.access.admin forum.post-news + publish.schedule-posts publish.pin-posts publish.shared-files + delete.shared-files + misc.no-upload-limits - name: Responsable communauté css: "color: DarkOrange;" descr: "Anime les pages Twitter et Facebook de Planète Casio et surveille l'évolution du monde autour de nous !" - privs: access-admin-board write-news - upload-shared-files delete-shared-files - scheduled-posting - showcase-content + privs: forum.access.admin forum.post-news + publish.schedule-posts publish.pin-posts publish.shared-files + delete.shared-files - name: Partenaire css: "color: purple;" descr: "Membres de l'équipe d'administration des sites partenaires." - privs: write-news - upload-shared-files delete-shared-files - scheduled-posting + privs: forum.post-news + publish.schedule-posts publish.shared-files + delete.shared-files - name: Compte communautaire css: "background:#d8d8d8; border-radius:4px; color:#303030; padding:1px 2px;" @@ -66,12 +112,12 @@ name: Robot css: "color: #cf25d0;" descr: "♫ Je suis Nono, le petit robot, l'ami d'Ulysse ♫" - privs: shoutbox-post shoutbox-kick shoutbox-ban + privs: shoutbox.kick shoutbox.ban - name: Membre de CreativeCalc css: "color: #222222;" descr: "CreativeCalc est l'association qui gère Planète Casio." - privs: access-assoc-board + privs: forum.access.creativecalc - name: No login css: "color: #888888;" diff --git a/app/models/forum.py b/app/models/forum.py index 5e4208a..231b539 100644 --- a/app/models/forum.py +++ b/app/models/forum.py @@ -37,6 +37,19 @@ class Forum(db.Model): else: self.parent = parent + def is_news(self): + """Whether this forum is a news board.""" + return (self.parent is not None) and (self.parent.prefix == "news") + + def is_default_accessible(self): + """Whether this forum can be read without privileges.""" + return (self.prefix != "admin") and (self.prefix != "creativecalc") + + def is_default_postable(self): + """Whether this forum can be posted to without privileges.""" + return self.is_default_accessible() and (not self.is_news()) and \ + (self.sub_forums == []) + def post_count(self): """Number of posts in every topic of the forum, without subforums.""" # TODO: optimize this with real ORM diff --git a/app/models/user.py b/app/models/user.py index 7590e72..c0df9e3 100644 --- a/app/models/user.py +++ b/app/models/user.py @@ -164,6 +164,35 @@ class Member(User): sp = SpecialPrivilege.query.filter_by(mid=self.id).all() return sorted(row.priv for row in sp) + def can_access_forum(self, forum): + """Whether this member can read the forum's contents.""" + return forum.is_default_accessible() or \ + self.priv(f"forum.access.{forum.prefix}") + + def can_post_in_forum(self, forum): + """Whether this member can post in the forum.""" + return forum.is_default_postable() or \ + (forum.is_news() and self.priv("forum.post-news")) or \ + self.priv("forum.post.{forum.prefix}") or \ + self.priv("forum.post-anywhere") + + def can_access_post(self, post): + """Whether this member can access the post's forum (if any).""" + if post.type == "comment" and post.thread.owner_topic: + return self.can_access_forum(post.thread.owner_post.forum) + # Posts from other types of content are all public + return True + + def can_edit_post(self, post): + """Whether this member can edit the post.""" + return self.can_access_post(post) and \ + ((post.author == self) or self.priv("edit.posts")) + + def can_delete_post(self, post): + """Whether this member can delete the post.""" + return self.can_access_post(post) and \ + ((post.author == self) or self.priv("delete.posts")) + def update(self, **data): """ Update all or part of the user's metadata. The [data] dictionary diff --git a/app/routes/admin/account.py b/app/routes/admin/account.py index 1d4c04c..065a48c 100644 --- a/app/routes/admin/account.py +++ b/app/routes/admin/account.py @@ -14,7 +14,7 @@ from config import V5Config @app.route('/admin/compte//editer', methods=['GET', 'POST']) -@priv_required('access-admin-panel', 'edit-account') +@priv_required('misc.admin-panel', 'edit.accounts') def adm_edit_account(user_id): user = Member.query.filter_by(id=user_id).first_or_404() @@ -119,7 +119,7 @@ def adm_edit_account(user_id): @app.route('/admin/compte//supprimer', methods=['GET', 'POST']) -@priv_required('access-admin-panel', 'delete-account') +@priv_required('misc.admin-panel', 'delete.accounts') def adm_delete_account(user_id): user = Member.query.filter_by(id=user_id).first_or_404() diff --git a/app/routes/admin/attachments.py b/app/routes/admin/attachments.py index b8b3bdf..2439509 100644 --- a/app/routes/admin/attachments.py +++ b/app/routes/admin/attachments.py @@ -6,7 +6,7 @@ from app.utils.render import render # TODO: add pagination & moderation tools (deletion) @app.route('/admin/fichiers', methods=['GET']) -@priv_required('access-admin-panel') +@priv_required('misc.admin-panel') def adm_attachments(): attachments = Attachment.query.all() diff --git a/app/routes/admin/config.py b/app/routes/admin/config.py index 102287d..b950fbd 100644 --- a/app/routes/admin/config.py +++ b/app/routes/admin/config.py @@ -4,7 +4,7 @@ from app import app from config import V5Config @app.route('/admin/config', methods=['GET']) -@priv_required('access-admin-panel') +@priv_required('misc.admin-panel') def adm_config(): config = {k: getattr(V5Config, k) for k in [ "DOMAIN", "DB_NAME", "USE_LDAP", "LDAP_ROOT", "LDAP_ENV", diff --git a/app/routes/admin/forums.py b/app/routes/admin/forums.py index 000d872..c3bfb52 100644 --- a/app/routes/admin/forums.py +++ b/app/routes/admin/forums.py @@ -4,7 +4,7 @@ from app.models.forum import Forum from app import app, db @app.route('/admin/forums', methods=['GET']) -@priv_required('access-admin-panel') +@priv_required('misc.admin-panel') def adm_forums(): main_forum = Forum.query.filter_by(parent=None).first() diff --git a/app/routes/admin/groups.py b/app/routes/admin/groups.py index 6c81bb1..3c2ff3d 100644 --- a/app/routes/admin/groups.py +++ b/app/routes/admin/groups.py @@ -10,7 +10,7 @@ import os @app.route('/admin/groupes', methods=['GET', 'POST']) -@priv_required('access-admin-panel') +@priv_required('misc.admin-panel') def adm_groups(): # Users with either groups or special privileges users_groups = Member.query.join(GroupMember) diff --git a/app/routes/admin/index.py b/app/routes/admin/index.py index bd0c7cc..e5fbe70 100644 --- a/app/routes/admin/index.py +++ b/app/routes/admin/index.py @@ -4,6 +4,6 @@ from app import app @app.route('/admin', methods=['GET']) -@priv_required('access-admin-panel') +@priv_required('misc.admin-panel') def adm(): return render('admin/index.html') diff --git a/app/routes/admin/members.py b/app/routes/admin/members.py index 407d661..b780a38 100644 --- a/app/routes/admin/members.py +++ b/app/routes/admin/members.py @@ -6,7 +6,7 @@ from app import app, db @app.route('/admin/membres', methods=['GET', 'POST']) -@priv_required('access-admin-panel') +@priv_required('misc.admin-panel') def adm_members(): users = Member.query.all() users = sorted(users, key = lambda x: x.name) diff --git a/app/routes/admin/trophies.py b/app/routes/admin/trophies.py index 80265bc..0561660 100644 --- a/app/routes/admin/trophies.py +++ b/app/routes/admin/trophies.py @@ -7,7 +7,7 @@ from app import app, db @app.route('/admin/trophees', methods=['GET', 'POST']) -@priv_required('access-admin-panel', 'edit-trophies') +@priv_required('misc.admin-panel', 'edit.trophies') def adm_trophies(): form = TrophyForm() if request.method == "POST": @@ -31,7 +31,7 @@ def adm_trophies(): @app.route('/admin/trophees//editer', methods=['GET', 'POST']) -@priv_required('access-admin-panel', 'edit-trophies') +@priv_required('misc.admin-panel', 'edit.trophies') def adm_edit_trophy(trophy_id): trophy = Trophy.query.filter_by(id=trophy_id).first_or_404() @@ -59,7 +59,7 @@ def adm_edit_trophy(trophy_id): @app.route('/admin/trophees//supprimer', methods=['GET', 'POST']) -@priv_required('access-admin-panel', 'edit-trophies') +@priv_required('misc.admin-panel', 'edit.trophies') def adm_delete_trophy(trophy_id): trophy = Trophy.query.filter_by(id=trophy_id).first_or_404() diff --git a/app/routes/forum/index.py b/app/routes/forum/index.py index aa0d209..dbdbdb8 100644 --- a/app/routes/forum/index.py +++ b/app/routes/forum/index.py @@ -20,21 +20,18 @@ def forum_index(): @app.route('/forum//', methods=['GET', 'POST']) @app.route('/forum//p/', methods=['GET', 'POST']) def forum_page(f, page=1): + if not f.is_default_accessible() and not ( + current_user.is_authenticated and current_user.can_access_forum(f)): + abort(403) + if current_user.is_authenticated: form = TopicCreationForm() else: form = AnonymousTopicCreationForm() - # TODO: do not hardcode name of news forums if form.validate_on_submit() and ( - # User can write anywhere - (current_user.is_authenticated and current_user.priv('write-anywhere')) - # Forum is news forum TODO: add good condition to check if it's news - or ("/actus" in f.url and current_user.is_authenticated - and current_user.priv('write-news')) - # Forum is not news and is a leaf: - or ("/actus" not in f.url and not f.sub_forums)) and ( - V5Config.ENABLE_GUEST_POST or current_user.is_authenticated): + (V5Config.ENABLE_GUEST_POST and f.is_default_postable()) or \ + (current_user.is_authenticated and current_user.can_post_in_forum(f))): # Manage author if current_user.is_authenticated: diff --git a/app/routes/forum/topic.py b/app/routes/forum/topic.py index c5dab42..ebb2be7 100644 --- a/app/routes/forum/topic.py +++ b/app/routes/forum/topic.py @@ -18,6 +18,10 @@ from datetime import datetime def forum_topic(f, page): t, page = page + if not f.is_default_accessible() and not ( + current_user.is_authenticated and current_user.can_access_forum(f)): + abort(403) + # Quick n' dirty workaround to converters if f != t.forum: abort(404) @@ -27,8 +31,10 @@ def forum_topic(f, page): else: form = AnonymousCommentForm() - if form.validate_on_submit() and \ - (V5Config.ENABLE_GUEST_POST or current_user.is_authenticated): + if form.validate_on_submit() and ( + V5Config.ENABLE_GUEST_POST or \ + (current_user.is_authenticated and current_user.can_post_in_forum(f))): + # Manage author if current_user.is_authenticated: author = current_user @@ -70,8 +76,8 @@ def forum_topic(f, page): if page == -1: page = (t.thread.comments.count() - 1) // Thread.COMMENTS_PER_PAGE + 1 - comments = t.thread.comments.paginate(page, - Thread.COMMENTS_PER_PAGE, True) + comments = t.thread.comments.order_by(Comment.date_created.asc()) \ + .paginate(page, Thread.COMMENTS_PER_PAGE, True) # Anti-necropost last_com = t.thread.comments.order_by(desc(Comment.date_modified)).first() diff --git a/app/routes/posts/edit.py b/app/routes/posts/edit.py index f8de2aa..cb0f610 100644 --- a/app/routes/posts/edit.py +++ b/app/routes/posts/edit.py @@ -8,7 +8,6 @@ from flask import redirect, url_for, abort, request from flask_login import login_required, current_user @app.route('/post/', methods=['GET','POST']) -# TODO: Allow guest edit of posts @login_required def edit_post(postid): # TODO: Maybe not safe @@ -17,8 +16,8 @@ def edit_post(postid): p = Post.query.filter_by(id=postid).first_or_404() - # TODO: Check whether privileged user has access to board - if p.author != current_user and not current_user.priv("edit-posts"): + # Check permissions. TODO: Allow guests to edit their posts + if current_user.is_anonymous or not current_user.can_edit_post(p): abort(403) if p.type == "comment": @@ -44,8 +43,7 @@ def edit_post(postid): def delete_post(postid): p = Post.query.filter_by(id=postid).first_or_404() - # TODO: Check whether privileged user has access to board - if p.author != current_user and not current_user.priv("delete-posts"): + if current_user.is_anonymous or not current_user.can_delete_post(p): abort(403) for a in p.attachments: diff --git a/app/templates/account/user.html b/app/templates/account/user.html index 8bb6ba1..1df1a06 100644 --- a/app/templates/account/user.html +++ b/app/templates/account/user.html @@ -13,7 +13,7 @@ {% if current_user.is_authenticated %} {% if current_user == member %} - {% elif current_user.priv('access-admin-panel') %} + {% elif current_user.priv('edit.accounts') %} {% endif %} {% endif %} diff --git a/app/templates/base/footer.html b/app/templates/base/footer.html index ed56b16..aa30d5a 100644 --- a/app/templates/base/footer.html +++ b/app/templates/base/footer.html @@ -1,7 +1,7 @@