From 0de14ea5db6ed9aece8d27d3b7d307e83bfee680 Mon Sep 17 00:00:00 2001 From: Patrick Jentsch Date: Wed, 12 Apr 2023 12:45:41 +0200 Subject: [PATCH] cleanup CorpusFollowerPermissions --- app/corpora/decorators.py | 19 +++++++++---------- app/corpora/files/json_routes.py | 2 +- app/corpora/followers/json_routes.py | 23 ++++++++++------------- app/corpora/json_routes.py | 10 +++++----- app/corpora/routes.py | 5 +++-- app/models.py | 12 ++++++------ 6 files changed, 34 insertions(+), 37 deletions(-) diff --git a/app/corpora/decorators.py b/app/corpora/decorators.py index 96d4ac69..1d6eb2e1 100644 --- a/app/corpora/decorators.py +++ b/app/corpora/decorators.py @@ -9,14 +9,13 @@ def corpus_follower_permission_required(*permissions): @wraps(f) def decorated_function(*args, **kwargs): corpus_id = kwargs.get('corpus_id') - corpus = Corpus.query.get_or_404(corpus_id) - if current_user == corpus.user or current_user.is_administrator(): - return f(*args, **kwargs) - if not current_user.is_following_corpus(corpus): - abort(403) - corpus_follower_association = CorpusFollowerAssociation.query.filter_by(corpus_id=corpus_id, follower_id=current_user.id).first_or_404() - if not all([corpus_follower_association.role.has_permission(p) for p in permissions]): + cfa = CorpusFollowerAssociation.query.filter_by(corpus_id=corpus_id, follower_id=current_user.id).first() + if cfa is None: abort(403) + corpus = cfa.corpus + if not (corpus.user == current_user or current_user.is_administrator()): + if not all([cfa.role.has_permission(p) for p in permissions]): + abort(403) return f(*args, **kwargs) return decorated_function return decorator @@ -27,8 +26,8 @@ def corpus_owner_or_admin_required(f): def decorated_function(*args, **kwargs): corpus_id = kwargs.get('corpus_id') corpus = Corpus.query.get_or_404(corpus_id) - if current_user == corpus.user or current_user.is_administrator(): - return f(*args, **kwargs) - abort(403) + if not (corpus.user == current_user or current_user.is_administrator()): + abort(403) + return f(*args, **kwargs) return decorated_function diff --git a/app/corpora/files/json_routes.py b/app/corpora/files/json_routes.py index 1794c4d1..e4f06084 100644 --- a/app/corpora/files/json_routes.py +++ b/app/corpora/files/json_routes.py @@ -1,4 +1,4 @@ -from flask import current_app +from flask import abort, current_app from threading import Thread from app import db from app.decorators import content_negotiation diff --git a/app/corpora/followers/json_routes.py b/app/corpora/followers/json_routes.py index b5c3c990..f8517359 100644 --- a/app/corpora/followers/json_routes.py +++ b/app/corpora/followers/json_routes.py @@ -1,5 +1,4 @@ from flask import abort, request -from flask_login import current_user from app import db from app.decorators import content_negotiation from app.models import ( @@ -8,12 +7,12 @@ from app.models import ( CorpusFollowerRole, User ) -from ..decorators import corpus_owner_or_admin_required +from ..decorators import corpus_follower_permission_required from . import bp @bp.route('//followers', methods=['POST']) -@corpus_owner_or_admin_required +@corpus_follower_permission_required('ADD_FOLLOWER') @content_negotiation(consumes='application/json', produces='application/json') def create_corpus_followers(corpus_id): usernames = request.json @@ -32,7 +31,7 @@ def create_corpus_followers(corpus_id): @bp.route('//followers//role', methods=['PUT']) -@corpus_owner_or_admin_required +@corpus_follower_permission_required('UPDATE_FOLLOWER') @content_negotiation(consumes='application/json', produces='application/json') def update_corpus_follower_role(corpus_id, follower_id): role_name = request.json @@ -52,19 +51,17 @@ def update_corpus_follower_role(corpus_id, follower_id): @bp.route('//followers/', methods=['DELETE']) +@corpus_follower_permission_required('REMOVE_FOLLOWER') @content_negotiation(produces='application/json') def delete_corpus_follower(corpus_id, follower_id): - corpus = Corpus.query.get_or_404(corpus_id) - follower = User.query.get_or_404(follower_id) - if not (corpus.user == current_user or follower == current_user or current_user.is_administrator()): - abort(403) - if not follower.is_following_corpus(corpus): - abort(409) - follower.unfollow_corpus(corpus) + cfa = CorpusFollowerAssociation.query.filter_by(corpus_id=corpus_id, follower_id=follower_id).first_or_404() + cfa.follower.unfollow_corpus(cfa.corpus) db.session.commit() response_data = { - 'message': \ - f'"{follower.username}" is not following "{corpus.title}" anymore', + 'message': ( + f'"{cfa.follower.username}" is not following ' + f'"{cfa.corpus.title}" anymore' + ), 'category': 'corpus' } return response_data, 200 diff --git a/app/corpora/json_routes.py b/app/corpora/json_routes.py index 4da60341..e73281ed 100644 --- a/app/corpora/json_routes.py +++ b/app/corpora/json_routes.py @@ -2,11 +2,11 @@ from datetime import datetime from flask import abort, current_app, request, url_for from flask_login import current_user from threading import Thread -from .decorators import corpus_follower_permission_required, corpus_owner_or_admin_required -from app import db, hashids +from app import db from app.decorators import content_negotiation from app.models import Corpus, CorpusFollowerRole from . import bp +from .decorators import corpus_follower_permission_required, corpus_owner_or_admin_required @bp.route('/', methods=['DELETE']) @@ -58,10 +58,9 @@ def build_corpus(corpus_id): @bp.route('//generate-share-link', methods=['POST']) -@corpus_follower_permission_required('GENERATE_SHARE_LINK') +@corpus_follower_permission_required('ADD_FOLLOWER') @content_negotiation(consumes='application/json', produces='application/json') def generate_corpus_share_link(corpus_id): - corpus_hashid = hashids.encode(corpus_id) data = request.json if not isinstance(data, dict): abort(400) @@ -75,7 +74,8 @@ def generate_corpus_share_link(corpus_id): cfr = CorpusFollowerRole.query.filter_by(name=role_name).first() if cfr is None: abort(400) - token = current_user.generate_follow_corpus_token(corpus_hashid, role_name, expiration_date) + corpus = Corpus.query.get_or_404(corpus_id) + token = current_user.generate_follow_corpus_token(corpus.hashid, role_name, expiration_date) corpus_share_link = url_for( 'corpora.follow_corpus', corpus_id=corpus_id, diff --git a/app/corpora/routes.py b/app/corpora/routes.py index ccb70760..c42165db 100644 --- a/app/corpora/routes.py +++ b/app/corpora/routes.py @@ -1,7 +1,6 @@ from flask import abort, flash, redirect, render_template, url_for from flask_breadcrumbs import register_breadcrumb from flask_login import current_user -from .decorators import corpus_follower_permission_required from app import db from app.models import ( Corpus, @@ -10,6 +9,7 @@ from app.models import ( User ) from . import bp +from .decorators import corpus_follower_permission_required from .forms import CreateCorpusForm from .utils import ( corpus_endpoint_arguments_constructor as corpus_eac, @@ -73,8 +73,8 @@ def corpus(corpus_id): @bp.route('//analyse') -@register_breadcrumb(bp, '.entity.analyse', 'Analyse', endpoint_arguments_constructor=corpus_eac) @corpus_follower_permission_required('VIEW') +@register_breadcrumb(bp, '.entity.analyse', 'Analyse', endpoint_arguments_constructor=corpus_eac) def analyse_corpus(corpus_id): corpus = Corpus.query.get_or_404(corpus_id) return render_template( @@ -101,6 +101,7 @@ def import_corpus(): @bp.route('//export') +@corpus_follower_permission_required('VIEW') @register_breadcrumb(bp, '.entity.export', 'Export', endpoint_arguments_constructor=corpus_eac) def export_corpus(corpus_id): abort(503) diff --git a/app/models.py b/app/models.py index 57658074..07d648ad 100644 --- a/app/models.py +++ b/app/models.py @@ -116,9 +116,9 @@ class CorpusFollowerPermission(IntEnum): ADD_CORPUS_FILE = 2 UPDATE_CORPUS_FILE = 4 REMOVE_CORPUS_FILE = 8 - GENERATE_SHARE_LINK = 16 - REMOVE_FOLLOWER = 32 - UPDATE_FOLLOWER = 64 + ADD_FOLLOWER = 16 + UPDATE_FOLLOWER = 32 + REMOVE_FOLLOWER = 64 @staticmethod def get(corpus_follower_permission: Union['CorpusFollowerPermission', int, str]) -> 'CorpusFollowerPermission': @@ -448,9 +448,9 @@ class CorpusFollowerRole(HashidMixin, db.Model): CorpusFollowerPermission.ADD_CORPUS_FILE, CorpusFollowerPermission.UPDATE_CORPUS_FILE, CorpusFollowerPermission.REMOVE_CORPUS_FILE, - CorpusFollowerPermission.GENERATE_SHARE_LINK, - CorpusFollowerPermission.REMOVE_FOLLOWER, - CorpusFollowerPermission.UPDATE_FOLLOWER + CorpusFollowerPermission.ADD_FOLLOWER, + CorpusFollowerPermission.UPDATE_FOLLOWER, + CorpusFollowerPermission.REMOVE_FOLLOWER ] } default_role_name = 'Viewer'