From a6ac5f0dbf5b2e8d0d0c55a58e7a41ee7d5ad5dc Mon Sep 17 00:00:00 2001 From: Kevin Morris Date: Mon, 29 Nov 2021 19:44:18 -0800 Subject: [PATCH 1/6] fix(rpc): fix ordering of related records They were being ordered by IDs; they should be ordered by Names. Signed-off-by: Kevin Morris --- aurweb/rpc.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/aurweb/rpc.py b/aurweb/rpc.py index c70ddf1a..7bdae638 100644 --- a/aurweb/rpc.py +++ b/aurweb/rpc.py @@ -193,7 +193,7 @@ class RPC: models.DependencyType.Name.label("Type"), models.PackageDependency.DepName.label("Name"), models.PackageDependency.DepCondition.label("Cond") - ).distinct().order_by("ID"), + ).distinct().order_by("Name"), # PackageRelation db.query( @@ -205,7 +205,7 @@ class RPC: models.RelationType.Name.label("Type"), models.PackageRelation.RelName.label("Name"), models.PackageRelation.RelCondition.label("Cond") - ).distinct().order_by("ID"), + ).distinct().order_by("Name"), # Groups db.query(models.PackageGroup).join( @@ -217,7 +217,7 @@ class RPC: literal("Groups").label("Type"), models.Group.Name.label("Name"), literal(str()).label("Cond") - ).distinct().order_by("ID"), + ).distinct().order_by("Name"), # Licenses db.query(models.PackageLicense).join( @@ -230,7 +230,7 @@ class RPC: literal("License").label("Type"), models.License.Name.label("Name"), literal(str()).label("Cond") - ).distinct().order_by("ID"), + ).distinct().order_by("Name"), # Keywords db.query(models.PackageKeyword).join( @@ -242,7 +242,7 @@ class RPC: literal("Keywords").label("Type"), models.PackageKeyword.Keyword.label("Name"), literal(str()).label("Cond") - ).distinct().order_by("ID") + ).distinct().order_by("Name") ] # Union all subqueries together. From ecbab8546b68574c939b269dac1ac2d77e53766b Mon Sep 17 00:00:00 2001 From: Steven Guikal Date: Thu, 21 Oct 2021 17:48:29 -0400 Subject: [PATCH 2/6] fix(FastAPI): access AccountType ID directly Signed-off-by: Steven Guikal --- aurweb/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aurweb/auth.py b/aurweb/auth.py index 4d6dafc6..1527f0a3 100644 --- a/aurweb/auth.py +++ b/aurweb/auth.py @@ -244,7 +244,7 @@ def account_type_required(one_of: set): def decorator(func): @functools.wraps(func) async def wrapper(request: fastapi.Request, *args, **kwargs): - if request.user.AccountType.ID not in one_of: + if request.user.AccountTypeID not in one_of: return RedirectResponse("/", status_code=int(HTTPStatus.SEE_OTHER)) return await func(request, *args, **kwargs) From 125b244f4478146ae293f76ca4a53a77160feda3 Mon Sep 17 00:00:00 2001 From: Steven Guikal Date: Thu, 21 Oct 2021 17:49:10 -0400 Subject: [PATCH 3/6] fix(FastAPI): use account type vars instead of strings Signed-off-by: Steven Guikal --- aurweb/routers/trusted_user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/aurweb/routers/trusted_user.py b/aurweb/routers/trusted_user.py index 7c0a0404..f0cea61e 100644 --- a/aurweb/routers/trusted_user.py +++ b/aurweb/routers/trusted_user.py @@ -228,7 +228,7 @@ async def trusted_user_proposal_post(request: Request, @router.get("/addvote") @auth_required(True, redirect="/addvote") -@account_type_required({"Trusted User", "Trusted User & Developer"}) +@account_type_required({TRUSTED_USER, TRUSTED_USER_AND_DEV}) async def trusted_user_addvote(request: Request, user: str = str(), type: str = "add_tu", From a10f8663fd9bc4ddb990f1dfba31cb3331f4c9e9 Mon Sep 17 00:00:00 2001 From: Steven Guikal Date: Tue, 30 Nov 2021 15:44:18 -0500 Subject: [PATCH 4/6] fix(FastAPI): reorganize credential checkin into dedicated file Signed-off-by: Steven Guikal --- aurweb/{auth.py => auth/__init__.py} | 98 ------------------------ aurweb/auth/creds.py | 76 ++++++++++++++++++ aurweb/models/user.py | 9 ++- aurweb/routers/accounts.py | 6 +- aurweb/routers/packages.py | 56 +++++++------- aurweb/templates.py | 3 +- templates/partials/account/comment.html | 2 +- templates/partials/account_form.html | 2 +- templates/partials/archdev-navbar.html | 4 +- templates/partials/comment_actions.html | 8 +- templates/partials/packages/actions.html | 8 +- templates/partials/packages/comment.html | 2 +- templates/partials/packages/details.html | 4 +- test/test_auth.py | 12 +-- test/test_user.py | 25 +++--- 15 files changed, 143 insertions(+), 172 deletions(-) rename aurweb/{auth.py => auth/__init__.py} (75%) create mode 100644 aurweb/auth/creds.py diff --git a/aurweb/auth.py b/aurweb/auth/__init__.py similarity index 75% rename from aurweb/auth.py rename to aurweb/auth/__init__.py index 1527f0a3..82192cc2 100644 --- a/aurweb/auth.py +++ b/aurweb/auth/__init__.py @@ -250,101 +250,3 @@ def account_type_required(one_of: set): return await func(request, *args, **kwargs) return wrapper return decorator - - -CRED_ACCOUNT_CHANGE_TYPE = 1 -CRED_ACCOUNT_EDIT = 2 -CRED_ACCOUNT_EDIT_DEV = 3 -CRED_ACCOUNT_LAST_LOGIN = 4 -CRED_ACCOUNT_SEARCH = 5 -CRED_ACCOUNT_LIST_COMMENTS = 28 -CRED_COMMENT_DELETE = 6 -CRED_COMMENT_UNDELETE = 27 -CRED_COMMENT_VIEW_DELETED = 22 -CRED_COMMENT_EDIT = 25 -CRED_COMMENT_PIN = 26 -CRED_PKGBASE_ADOPT = 7 -CRED_PKGBASE_SET_KEYWORDS = 8 -CRED_PKGBASE_DELETE = 9 -CRED_PKGBASE_DISOWN = 10 -CRED_PKGBASE_EDIT_COMAINTAINERS = 24 -CRED_PKGBASE_FLAG = 11 -CRED_PKGBASE_LIST_VOTERS = 12 -CRED_PKGBASE_NOTIFY = 13 -CRED_PKGBASE_UNFLAG = 15 -CRED_PKGBASE_VOTE = 16 -CRED_PKGREQ_FILE = 23 -CRED_PKGREQ_CLOSE = 17 -CRED_PKGREQ_LIST = 18 -CRED_TU_ADD_VOTE = 19 -CRED_TU_LIST_VOTES = 20 -CRED_TU_VOTE = 21 -CRED_PKGBASE_MERGE = 29 - - -def has_any(user, *account_types): - return str(user.AccountType) in set(account_types) - - -def user_developer_or_trusted_user(user): - return True - - -def trusted_user(user): - return has_any(user, "Trusted User", "Trusted User & Developer") - - -def developer(user): - return has_any(user, "Developer", "Trusted User & Developer") - - -def trusted_user_or_dev(user): - return has_any(user, "Trusted User", "Developer", - "Trusted User & Developer") - - -# A mapping of functions that users must pass to have credentials. -cred_filters = { - CRED_PKGBASE_FLAG: user_developer_or_trusted_user, - CRED_PKGBASE_NOTIFY: user_developer_or_trusted_user, - CRED_PKGBASE_VOTE: user_developer_or_trusted_user, - CRED_PKGREQ_FILE: user_developer_or_trusted_user, - CRED_ACCOUNT_CHANGE_TYPE: trusted_user_or_dev, - CRED_ACCOUNT_EDIT: trusted_user_or_dev, - CRED_ACCOUNT_LAST_LOGIN: trusted_user_or_dev, - CRED_ACCOUNT_LIST_COMMENTS: trusted_user_or_dev, - CRED_ACCOUNT_SEARCH: trusted_user_or_dev, - CRED_COMMENT_DELETE: trusted_user_or_dev, - CRED_COMMENT_UNDELETE: trusted_user_or_dev, - CRED_COMMENT_VIEW_DELETED: trusted_user_or_dev, - CRED_COMMENT_EDIT: trusted_user_or_dev, - CRED_COMMENT_PIN: trusted_user_or_dev, - CRED_PKGBASE_ADOPT: trusted_user_or_dev, - CRED_PKGBASE_SET_KEYWORDS: trusted_user_or_dev, - CRED_PKGBASE_DELETE: trusted_user_or_dev, - CRED_PKGBASE_EDIT_COMAINTAINERS: trusted_user_or_dev, - CRED_PKGBASE_DISOWN: trusted_user_or_dev, - CRED_PKGBASE_LIST_VOTERS: trusted_user_or_dev, - CRED_PKGBASE_UNFLAG: trusted_user_or_dev, - CRED_PKGREQ_CLOSE: trusted_user_or_dev, - CRED_PKGREQ_LIST: trusted_user_or_dev, - CRED_TU_ADD_VOTE: trusted_user, - CRED_TU_LIST_VOTES: trusted_user_or_dev, - CRED_TU_VOTE: trusted_user, - CRED_ACCOUNT_EDIT_DEV: developer, - CRED_PKGBASE_MERGE: trusted_user_or_dev, -} - - -def has_credential(user: User, - credential: int, - approved_users: list = tuple()): - - if user in approved_users: - return True - - if credential in cred_filters: - cred_filter = cred_filters.get(credential) - return cred_filter(user) - - return False diff --git a/aurweb/auth/creds.py b/aurweb/auth/creds.py new file mode 100644 index 00000000..100aad8c --- /dev/null +++ b/aurweb/auth/creds.py @@ -0,0 +1,76 @@ +from aurweb.models.account_type import DEVELOPER_ID, TRUSTED_USER_AND_DEV_ID, TRUSTED_USER_ID, USER_ID +from aurweb.models.user import User + +ACCOUNT_CHANGE_TYPE = 1 +ACCOUNT_EDIT = 2 +ACCOUNT_EDIT_DEV = 3 +ACCOUNT_LAST_LOGIN = 4 +ACCOUNT_SEARCH = 5 +ACCOUNT_LIST_COMMENTS = 28 +COMMENT_DELETE = 6 +COMMENT_UNDELETE = 27 +COMMENT_VIEW_DELETED = 22 +COMMENT_EDIT = 25 +COMMENT_PIN = 26 +PKGBASE_ADOPT = 7 +PKGBASE_SET_KEYWORDS = 8 +PKGBASE_DELETE = 9 +PKGBASE_DISOWN = 10 +PKGBASE_EDIT_COMAINTAINERS = 24 +PKGBASE_FLAG = 11 +PKGBASE_LIST_VOTERS = 12 +PKGBASE_NOTIFY = 13 +PKGBASE_UNFLAG = 15 +PKGBASE_VOTE = 16 +PKGREQ_FILE = 23 +PKGREQ_CLOSE = 17 +PKGREQ_LIST = 18 +TU_ADD_VOTE = 19 +TU_LIST_VOTES = 20 +TU_VOTE = 21 +PKGBASE_MERGE = 29 + +user_developer_or_trusted_user = set([USER_ID, TRUSTED_USER_ID, DEVELOPER_ID, TRUSTED_USER_AND_DEV_ID]) +trusted_user_or_dev = set([TRUSTED_USER_ID, DEVELOPER_ID, TRUSTED_USER_AND_DEV_ID]) +developer = set([DEVELOPER_ID, TRUSTED_USER_AND_DEV_ID]) +trusted_user = set([TRUSTED_USER_ID, TRUSTED_USER_AND_DEV_ID]) + +cred_filters = { + PKGBASE_FLAG: user_developer_or_trusted_user, + PKGBASE_NOTIFY: user_developer_or_trusted_user, + PKGBASE_VOTE: user_developer_or_trusted_user, + PKGREQ_FILE: user_developer_or_trusted_user, + ACCOUNT_CHANGE_TYPE: trusted_user_or_dev, + ACCOUNT_EDIT: trusted_user_or_dev, + ACCOUNT_LAST_LOGIN: trusted_user_or_dev, + ACCOUNT_LIST_COMMENTS: trusted_user_or_dev, + ACCOUNT_SEARCH: trusted_user_or_dev, + COMMENT_DELETE: trusted_user_or_dev, + COMMENT_UNDELETE: trusted_user_or_dev, + COMMENT_VIEW_DELETED: trusted_user_or_dev, + COMMENT_EDIT: trusted_user_or_dev, + COMMENT_PIN: trusted_user_or_dev, + PKGBASE_ADOPT: trusted_user_or_dev, + PKGBASE_SET_KEYWORDS: trusted_user_or_dev, + PKGBASE_DELETE: trusted_user_or_dev, + PKGBASE_EDIT_COMAINTAINERS: trusted_user_or_dev, + PKGBASE_DISOWN: trusted_user_or_dev, + PKGBASE_LIST_VOTERS: trusted_user_or_dev, + PKGBASE_UNFLAG: trusted_user_or_dev, + PKGREQ_CLOSE: trusted_user_or_dev, + PKGREQ_LIST: trusted_user_or_dev, + TU_ADD_VOTE: trusted_user, + TU_LIST_VOTES: trusted_user_or_dev, + TU_VOTE: trusted_user, + ACCOUNT_EDIT_DEV: developer, + PKGBASE_MERGE: trusted_user_or_dev, +} + + +def has_credential(user: User, + credential: int, + approved_users: list = tuple()): + + if user in approved_users: + return True + return user.AccountTypeID in cred_filters[credential] diff --git a/aurweb/models/user.py b/aurweb/models/user.py index 03634a36..f0724202 100644 --- a/aurweb/models/user.py +++ b/aurweb/models/user.py @@ -1,6 +1,7 @@ import hashlib from datetime import datetime +from typing import List, Set import bcrypt @@ -136,10 +137,10 @@ class User(Base): request.cookies["AURSID"] = self.session.SessionID return self.session.SessionID - def has_credential(self, credential: str, approved: list = tuple()): - import aurweb.auth - cred = getattr(aurweb.auth, credential) - return aurweb.auth.has_credential(self, cred, approved) + def has_credential(self, credential: Set[int], + approved: List["User"] = list()): + from aurweb.auth.creds import has_credential + return has_credential(self, credential, approved) def logout(self, request): del request.cookies["AURSID"] diff --git a/aurweb/routers/accounts.py b/aurweb/routers/accounts.py index 545811f0..360857e8 100644 --- a/aurweb/routers/accounts.py +++ b/aurweb/routers/accounts.py @@ -10,7 +10,7 @@ from sqlalchemy import and_, or_ import aurweb.config from aurweb import cookies, db, l10n, logging, models, util -from aurweb.auth import account_type_required, auth_required +from aurweb.auth import account_type_required, auth_required, creds from aurweb.captcha import get_captcha_salts from aurweb.exceptions import ValidationError from aurweb.l10n import get_translator_for_request @@ -176,7 +176,7 @@ def make_account_form_context(context: dict, user_account_type_id = context.get("account_types")[0][0] - if request.user.has_credential("CRED_ACCOUNT_EDIT_DEV"): + if request.user.has_credential(creds.ACCOUNT_EDIT_DEV): context["account_types"].append((at.DEVELOPER_ID, at.DEVELOPER)) context["account_types"].append((at.TRUSTED_USER_AND_DEV_ID, at.TRUSTED_USER_AND_DEV)) @@ -332,7 +332,7 @@ async def account_register_post(request: Request, def cannot_edit(request, user): """ Return a 401 HTMLResponse if the request user doesn't have authorization, otherwise None. """ - has_dev_cred = request.user.has_credential("CRED_ACCOUNT_EDIT_DEV", + has_dev_cred = request.user.has_credential(creds.ACCOUNT_EDIT_DEV, approved=[user]) if not has_dev_cred: return HTMLResponse(status_code=HTTPStatus.UNAUTHORIZED) diff --git a/aurweb/routers/packages.py b/aurweb/routers/packages.py index b5f8478e..2bf04949 100644 --- a/aurweb/routers/packages.py +++ b/aurweb/routers/packages.py @@ -10,7 +10,7 @@ import aurweb.filters import aurweb.packages.util from aurweb import db, defaults, l10n, logging, models, util -from aurweb.auth import auth_required +from aurweb.auth import auth_required, creds from aurweb.exceptions import ValidationError from aurweb.models.package_request import ACCEPTED_ID, PENDING_ID, REJECTED_ID from aurweb.models.relation_type import CONFLICTS_ID, PROVIDES_ID, REPLACES_ID @@ -413,7 +413,7 @@ async def pkgbase_comment_delete(request: Request, name: str, id: int, pkgbase = get_pkg_or_base(name, models.PackageBase) comment = get_pkgbase_comment(pkgbase, id) - authorized = request.user.has_credential("CRED_COMMENT_DELETE", + authorized = request.user.has_credential(creds.COMMENT_DELETE, [comment.User]) if not authorized: _ = l10n.get_translator_for_request(request) @@ -439,7 +439,7 @@ async def pkgbase_comment_undelete(request: Request, name: str, id: int, pkgbase = get_pkg_or_base(name, models.PackageBase) comment = get_pkgbase_comment(pkgbase, id) - has_cred = request.user.has_credential("CRED_COMMENT_UNDELETE", + has_cred = request.user.has_credential(creds.COMMENT_UNDELETE, approved=[comment.User]) if not has_cred: _ = l10n.get_translator_for_request(request) @@ -464,7 +464,7 @@ async def pkgbase_comment_pin(request: Request, name: str, id: int, pkgbase = get_pkg_or_base(name, models.PackageBase) comment = get_pkgbase_comment(pkgbase, id) - has_cred = request.user.has_credential("CRED_COMMENT_PIN", + has_cred = request.user.has_credential(creds.COMMENT_PIN, approved=[pkgbase.Maintainer]) if not has_cred: _ = l10n.get_translator_for_request(request) @@ -489,7 +489,7 @@ async def pkgbase_comment_unpin(request: Request, name: str, id: int, pkgbase = get_pkg_or_base(name, models.PackageBase) comment = get_pkgbase_comment(pkgbase, id) - has_cred = request.user.has_credential("CRED_COMMENT_PIN", + has_cred = request.user.has_credential(creds.COMMENT_PIN, approved=[pkgbase.Maintainer]) if not has_cred: _ = l10n.get_translator_for_request(request) @@ -514,7 +514,7 @@ async def package_base_comaintainers(request: Request, name: str) -> Response: # Unauthorized users (Non-TU/Dev and not the pkgbase maintainer) # get redirected to the package base's page. - has_creds = request.user.has_credential("CRED_PKGBASE_EDIT_COMAINTAINERS", + has_creds = request.user.has_credential(creds.PKGBASE_EDIT_COMAINTAINERS, approved=[pkgbase.Maintainer]) if not has_creds: return RedirectResponse(f"/pkgbase/{name}", @@ -541,7 +541,7 @@ async def package_base_comaintainers_post( # Unauthorized users (Non-TU/Dev and not the pkgbase maintainer) # get redirected to the package base's page. - has_creds = request.user.has_credential("CRED_PKGBASE_EDIT_COMAINTAINERS", + has_creds = request.user.has_credential(creds.PKGBASE_EDIT_COMAINTAINERS, approved=[pkgbase.Maintainer]) if not has_creds: return RedirectResponse(f"/pkgbase/{name}", @@ -779,7 +779,7 @@ async def pkgbase_keywords(request: Request, name: str, async def pkgbase_flag_get(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) - has_cred = request.user.has_credential("CRED_PKGBASE_FLAG") + has_cred = request.user.has_credential(creds.PKGBASE_FLAG) if not has_cred or pkgbase.Flagger is not None: return RedirectResponse(f"/pkgbase/{name}", status_code=HTTPStatus.SEE_OTHER) @@ -803,7 +803,7 @@ async def pkgbase_flag_post(request: Request, name: str, return render_template(request, "packages/flag.html", context, status_code=HTTPStatus.BAD_REQUEST) - has_cred = request.user.has_credential("CRED_PKGBASE_FLAG") + has_cred = request.user.has_credential(creds.PKGBASE_FLAG) if has_cred and not pkgbase.Flagger: now = int(datetime.utcnow().timestamp()) with db.begin(): @@ -830,7 +830,7 @@ async def pkgbase_flag_comment(request: Request, name: str): def pkgbase_unflag_instance(request: Request, pkgbase: models.PackageBase): has_cred = request.user.has_credential( - "CRED_PKGBASE_UNFLAG", approved=[pkgbase.Flagger, pkgbase.Maintainer]) + creds.PKGBASE_UNFLAG, approved=[pkgbase.Flagger, pkgbase.Maintainer]) if has_cred: with db.begin(): pkgbase.OutOfDateTS = None @@ -851,7 +851,7 @@ def pkgbase_notify_instance(request: Request, pkgbase: models.PackageBase): notif = db.query(pkgbase.notifications.filter( models.PackageNotification.UserID == request.user.ID ).exists()).scalar() - has_cred = request.user.has_credential("CRED_PKGBASE_NOTIFY") + has_cred = request.user.has_credential(creds.PKGBASE_NOTIFY) if has_cred and not notif: with db.begin(): db.create(models.PackageNotification, @@ -872,7 +872,7 @@ def pkgbase_unnotify_instance(request: Request, pkgbase: models.PackageBase): notif = pkgbase.notifications.filter( models.PackageNotification.UserID == request.user.ID ).first() - has_cred = request.user.has_credential("CRED_PKGBASE_NOTIFY") + has_cred = request.user.has_credential(creds.PKGBASE_NOTIFY) if has_cred and notif: with db.begin(): db.delete(notif) @@ -895,7 +895,7 @@ async def pkgbase_vote(request: Request, name: str): vote = pkgbase.package_votes.filter( models.PackageVote.UsersID == request.user.ID ).first() - has_cred = request.user.has_credential("CRED_PKGBASE_VOTE") + has_cred = request.user.has_credential(creds.PKGBASE_VOTE) if has_cred and not vote: now = int(datetime.utcnow().timestamp()) with db.begin(): @@ -919,7 +919,7 @@ async def pkgbase_unvote(request: Request, name: str): vote = pkgbase.package_votes.filter( models.PackageVote.UsersID == request.user.ID ).first() - has_cred = request.user.has_credential("CRED_PKGBASE_VOTE") + has_cred = request.user.has_credential(creds.PKGBASE_VOTE) if has_cred and vote: with db.begin(): db.delete(vote) @@ -958,7 +958,7 @@ def pkgbase_disown_instance(request: Request, pkgbase: models.PackageBase): async def pkgbase_disown_get(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) - has_cred = request.user.has_credential("CRED_PKGBASE_DISOWN", + has_cred = request.user.has_credential(creds.PKGBASE_DISOWN, approved=[pkgbase.Maintainer]) if not has_cred: return RedirectResponse(f"/pkgbase/{name}", @@ -975,7 +975,7 @@ async def pkgbase_disown_post(request: Request, name: str, confirm: bool = Form(default=False)): pkgbase = get_pkg_or_base(name, models.PackageBase) - has_cred = request.user.has_credential("CRED_PKGBASE_DISOWN", + has_cred = request.user.has_credential(creds.PKGBASE_DISOWN, approved=[pkgbase.Maintainer]) if not has_cred: return RedirectResponse(f"/pkgbase/{name}", @@ -1007,7 +1007,7 @@ def pkgbase_adopt_instance(request: Request, pkgbase: models.PackageBase): async def pkgbase_adopt_post(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) - has_cred = request.user.has_credential("CRED_PKGBASE_ADOPT") + has_cred = request.user.has_credential(creds.PKGBASE_ADOPT) if has_cred or not pkgbase.Maintainer: # If the user has credentials, they'll adopt the package regardless # of maintainership. Otherwise, we'll promote the user to maintainer @@ -1021,7 +1021,7 @@ async def pkgbase_adopt_post(request: Request, name: str): @router.get("/pkgbase/{name}/delete") @auth_required(True, redirect="/pkgbase/{name}/delete") async def pkgbase_delete_get(request: Request, name: str): - if not request.user.has_credential("CRED_PKGBASE_DELETE"): + if not request.user.has_credential(creds.PKGBASE_DELETE): return RedirectResponse(f"/pkgbase/{name}", status_code=HTTPStatus.SEE_OTHER) @@ -1036,7 +1036,7 @@ async def pkgbase_delete_post(request: Request, name: str, confirm: bool = Form(default=False)): pkgbase = get_pkg_or_base(name, models.PackageBase) - if not request.user.has_credential("CRED_PKGBASE_DELETE"): + if not request.user.has_credential(creds.PKGBASE_DELETE): return RedirectResponse(f"/pkgbase/{name}", status_code=HTTPStatus.SEE_OTHER) @@ -1070,7 +1070,7 @@ async def packages_unflag(request: Request, package_ids: List[int] = [], models.Package.ID.in_(package_ids)).all() for pkg in packages: has_cred = request.user.has_credential( - "CRED_PKGBASE_UNFLAG", approved=[pkg.PackageBase.Flagger]) + creds.PKGBASE_UNFLAG, approved=[pkg.PackageBase.Flagger]) if not has_cred: return (False, ["You did not select any packages to unflag."]) @@ -1106,7 +1106,7 @@ async def packages_notify(request: Request, package_ids: List[int] = [], notif = db.query(pkgbase.notifications.filter( models.PackageNotification.UserID == request.user.ID ).exists()).scalar() - has_cred = request.user.has_credential("CRED_PKGBASE_NOTIFY") + has_cred = request.user.has_credential(creds.PKGBASE_NOTIFY) # If the request user either does not have credentials # or the notification already exists: @@ -1178,7 +1178,7 @@ async def packages_adopt(request: Request, package_ids: List[int] = [], # Check that the user has credentials for every package they selected. for pkgbase in bases: - has_cred = request.user.has_credential("CRED_PKGBASE_ADOPT") + has_cred = request.user.has_credential(creds.PKGBASE_ADOPT) if not (has_cred or not pkgbase.Maintainer): # TODO: This error needs to be translated. return (False, ["You are not allowed to adopt one of the " @@ -1211,7 +1211,7 @@ async def packages_disown(request: Request, package_ids: List[int] = [], # Check that the user has credentials for every package they selected. for pkgbase in bases: - has_cred = request.user.has_credential("CRED_PKGBASE_DISOWN", + has_cred = request.user.has_credential(creds.PKGBASE_DISOWN, approved=[pkgbase.Maintainer]) if not has_cred: # TODO: This error needs to be translated. @@ -1235,7 +1235,7 @@ async def packages_delete(request: Request, package_ids: List[int] = [], return (False, ["The selected packages have not been deleted, " "check the confirmation checkbox."]) - if not request.user.has_credential("CRED_PKGBASE_DELETE"): + if not request.user.has_credential(creds.PKGBASE_DELETE): return (False, ["You do not have permission to delete packages."]) # A "memo" used to store names of packages that we delete. @@ -1329,10 +1329,10 @@ async def pkgbase_merge_get(request: Request, name: str, status_code = HTTPStatus.OK # TODO: Lookup errors from credential instead of hardcoding them. - # Idea: Something like credential_errors("CRED_PKGBASE_MERGE"). - # Perhaps additionally: bad_credential_status_code("CRED_PKGBASE_MERGE"). + # Idea: Something like credential_errors(creds.PKGBASE_MERGE). + # Perhaps additionally: bad_credential_status_code(creds.PKGBASE_MERGE). # Don't take these examples verbatim. We should find good naming. - if not request.user.has_credential("CRED_PKGBASE_MERGE"): + if not request.user.has_credential(creds.PKGBASE_MERGE): context["errors"] = [ "Only Trusted Users and Developers can merge packages."] status_code = HTTPStatus.UNAUTHORIZED @@ -1434,7 +1434,7 @@ async def pkgbase_merge_post(request: Request, name: str, context["pkgbase"] = pkgbase # TODO: Lookup errors from credential instead of hardcoding them. - if not request.user.has_credential("CRED_PKGBASE_MERGE"): + if not request.user.has_credential(creds.PKGBASE_MERGE): context["errors"] = [ "Only Trusted Users and Developers can merge packages."] return render_template(request, "pkgbase/merge.html", context, diff --git a/aurweb/templates.py b/aurweb/templates.py index a7102ae1..635b22b4 100644 --- a/aurweb/templates.py +++ b/aurweb/templates.py @@ -16,7 +16,7 @@ from fastapi.responses import HTMLResponse import aurweb.config -from aurweb import captcha, cookies, l10n, time, util +from aurweb import auth, captcha, cookies, l10n, time, util # Prepare jinja2 objects. _loader = jinja2.FileSystemLoader(os.path.join( @@ -107,6 +107,7 @@ def make_context(request: Request, title: str, next: str = None): "now": datetime.now(tz=zoneinfo.ZoneInfo(timezone)), "utcnow": int(datetime.utcnow().timestamp()), "config": aurweb.config, + "creds": auth.creds, "next": next if next else request.url.path } diff --git a/templates/partials/account/comment.html b/templates/partials/account/comment.html index bc167cf7..8c310738 100644 --- a/templates/partials/account/comment.html +++ b/templates/partials/account/comment.html @@ -3,7 +3,7 @@ {% set header_cls = "%s %s" | format(header_cls, "comment-deleted") %} {% endif %} -{% if not comment.Deleter or request.user.has_credential("CRED_COMMENT_VIEW_DELETED", approved=[comment.Deleter]) %} +{% if not comment.Deleter or request.user.has_credential(creds.COMMENT_VIEW_DELETED, approved=[comment.Deleter]) %} {% set commented_at = comment.CommentTS | dt | as_timezone(timezone) %}

diff --git a/templates/partials/account_form.html b/templates/partials/account_form.html index 2e47a932..f3c293d8 100644 --- a/templates/partials/account_form.html +++ b/templates/partials/account_form.html @@ -53,7 +53,7 @@

{% endif %} - {% if request.user.has_credential("CRED_ACCOUNT_CHANGE_TYPE") %} + {% if request.user.has_credential(creds.ACCOUNT_CHANGE_TYPE) %}

  • {% trans %}Accounts{% endtrans %} @@ -37,7 +37,7 @@
    • {# Only CRED_TU_LIST_VOTES privileged users see Trusted User #} - {% if request.user.has_credential("CRED_TU_LIST_VOTES") %} + {% if request.user.has_credential(creds.TU_LIST_VOTES) %}
  • {% trans %}Trusted User{% endtrans %}
    • diff --git a/templates/partials/comment_actions.html b/templates/partials/comment_actions.html index b8ccf945..78c4cc22 100644 --- a/templates/partials/comment_actions.html +++ b/templates/partials/comment_actions.html @@ -1,7 +1,7 @@ {% set pkgbasename = comment.PackageBase.Name %} {% if not comment.Deleter %} - {% if request.user.has_credential('CRED_COMMENT_DELETE', approved=[comment.User]) %} + {% if request.user.has_credential(creds.COMMENT_DELETE, approved=[comment.User]) %}
    {% endif %} - {% if request.user.has_credential('CRED_COMMENT_EDIT', approved=[comment.User]) %} + {% if request.user.has_credential(creds.COMMENT_EDIT, approved=[comment.User]) %} {% endif %} {% endif %} -{% elif request.user.has_credential("CRED_COMMENT_UNDELETE", approved=[comment.User]) %} +{% elif request.user.has_credential(creds.COMMENT_UNDELETE, approved=[comment.User]) %} {% endif %} - {% if request.user.has_credential('CRED_PKGBASE_EDIT_COMAINTAINERS', approved=[pkgbase.Maintainer]) %} + {% if request.user.has_credential(creds.PKGBASE_EDIT_COMAINTAINERS, approved=[pkgbase.Maintainer]) %}
  • {{ "Manage Co-Maintainers" | tr }} @@ -107,14 +107,14 @@ {{ "Submit Request" | tr }}
    • - {% if request.user.has_credential("CRED_PKGBASE_DELETE") %} + {% if request.user.has_credential(creds.PKGBASE_DELETE) %}
  • {{ "Delete Package" | tr }}
    • {% endif %} - {% if request.user.has_credential("CRED_PKGBASE_MERGE") %} + {% if request.user.has_credential(creds.PKGBASE_MERGE) %}
  • {{ "Merge Package" | tr }} @@ -130,7 +130,7 @@ />
    • - {% elif request.user.has_credential("CRED_PKGBASE_DISOWN", approved=[pkgbase.Maintainer]) %} + {% elif request.user.has_credential(creds.PKGBASE_DISOWN, approved=[pkgbase.Maintainer]) %}
  • {{ "Disown Package" | tr }} diff --git a/templates/partials/packages/comment.html b/templates/partials/packages/comment.html index 676a7a73..1427e0a0 100644 --- a/templates/partials/packages/comment.html +++ b/templates/partials/packages/comment.html @@ -5,7 +5,7 @@ {% set article_cls = "%s %s" | format(article_cls, "comment-deleted") %} {% endif %} -{% if not comment.Deleter or request.user.has_credential("CRED_COMMENT_VIEW_DELETED", approved=[comment.Deleter]) %} +{% if not comment.Deleter or request.user.has_credential(creds.COMMENT_VIEW_DELETED, approved=[comment.Deleter]) %}

    {% set commented_at = comment.CommentTS | dt | as_timezone(timezone) %} {% set view_account_info = 'View account information for %s' | tr | format(comment.User.Username) %} diff --git a/templates/partials/packages/details.html b/templates/partials/packages/details.html index dbb81c19..78e0ad1c 100644 --- a/templates/partials/packages/details.html +++ b/templates/partials/packages/details.html @@ -33,10 +33,10 @@ {% endif %} - {% if pkgbase.keywords.count() or request.user.has_credential("CRED_PKGBASE_SET_KEYWORDS", approved=[pkgbase.Maintainer]) %} + {% if pkgbase.keywords.count() or request.user.has_credential(creds.PKGBASE_SET_KEYWORDS, approved=[pkgbase.Maintainer]) %} {{ "Keywords" | tr }}: - {% if request.user.has_credential("CRED_PKGBASE_SET_KEYWORDS", approved=[pkgbase.Maintainer]) %} + {% if request.user.has_credential(creds.PKGBASE_SET_KEYWORDS, approved=[pkgbase.Maintainer]) %}
    Date: Thu, 18 Nov 2021 14:17:46 -0500 Subject: [PATCH 5/6] fix(FastAPI): remove login and redirect parameters from auth_required Signed-off-by: Steven Guikal --- aurweb/auth/__init__.py | 27 ++++++-------- aurweb/routers/accounts.py | 22 ++++++------ aurweb/routers/auth.py | 2 +- aurweb/routers/packages.py | 60 ++++++++++++++++---------------- aurweb/routers/trusted_user.py | 10 +++--- test/test_trusted_user_routes.py | 5 +-- 6 files changed, 61 insertions(+), 65 deletions(-) diff --git a/aurweb/auth/__init__.py b/aurweb/auth/__init__.py index 82192cc2..7aa4b526 100644 --- a/aurweb/auth/__init__.py +++ b/aurweb/auth/__init__.py @@ -1,5 +1,4 @@ import functools -import re from datetime import datetime from http import HTTPStatus @@ -122,17 +121,12 @@ class BasicAuthBackend(AuthenticationBackend): def auth_required(is_required: bool = True, - login: bool = True, - redirect: str = "/", template: tuple = None, status_code: HTTPStatus = HTTPStatus.UNAUTHORIZED): """ Authentication route decorator. - If redirect is given, the user will be redirected if the auth state - does not match is_required. - If template is given, it will be rendered with Unauthorized if - is_required does not match and take priority over redirect. + is_required does not match. A precondition of this function is that, if template is provided, it **must** match the following format: @@ -152,8 +146,6 @@ def auth_required(is_required: bool = True, applying any format operations. :param is_required: A boolean indicating whether the function requires auth - :param login: Redirect to `/login`, passing `next=` - :param redirect: Path to redirect to if is_required isn't True :param template: A three-element template tuple: (path, title_iterable, variable_iterable) :param status_code: An optional status_code for template render. @@ -166,14 +158,17 @@ def auth_required(is_required: bool = True, if request.user.is_authenticated() != is_required: url = "/" - if redirect: - path_params_expr = re.compile(r'\{(\w+)\}') - match = re.findall(path_params_expr, redirect) - args = {k: request.path_params.get(k) for k in match} - url = redirect.format(**args) + if is_required: + if request.method == "GET": + url = request.url.path + elif request.method == "POST" and (referer := request.headers.get("Referer")): + aur = aurweb.config.get("options", "aur_location") + "/" + if not referer.startswith(aur): + raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, + detail=_("Bad Referer header.")) + url = referer[len(aur) - 1:] - if login: - url = "/login?" + util.urlencode({"next": url}) + url = "/login?" + util.urlencode({"next": url}) if template: # template=("template.html", diff --git a/aurweb/routers/accounts.py b/aurweb/routers/accounts.py index 360857e8..dade92bb 100644 --- a/aurweb/routers/accounts.py +++ b/aurweb/routers/accounts.py @@ -27,14 +27,14 @@ logger = logging.get_logger(__name__) @router.get("/passreset", response_class=HTMLResponse) -@auth_required(False, login=False) +@auth_required(False) async def passreset(request: Request): context = await make_variable_context(request, "Password Reset") return render_template(request, "passreset.html", context) @router.post("/passreset", response_class=HTMLResponse) -@auth_required(False, login=False) +@auth_required(False) async def passreset_post(request: Request, user: str = Form(...), resetkey: str = Form(default=None), @@ -226,7 +226,7 @@ def make_account_form_context(context: dict, @router.get("/register", response_class=HTMLResponse) -@auth_required(False, login=False) +@auth_required(False) async def account_register(request: Request, U: str = Form(default=str()), # Username E: str = Form(default=str()), # Email @@ -252,7 +252,7 @@ async def account_register(request: Request, @router.post("/register", response_class=HTMLResponse) -@auth_required(False, login=False) +@auth_required(False) async def account_register_post(request: Request, U: str = Form(default=str()), # Username E: str = Form(default=str()), # Email @@ -340,7 +340,7 @@ def cannot_edit(request, user): @router.get("/account/{username}/edit", response_class=HTMLResponse) -@auth_required(True, redirect="/account/{username}") +@auth_required(True) async def account_edit(request: Request, username: str): user = db.query(models.User, models.User.Username == username).first() @@ -356,7 +356,7 @@ async def account_edit(request: Request, username: str): @router.post("/account/{username}/edit", response_class=HTMLResponse) -@auth_required(True, redirect="/account/{username}") +@auth_required(True) async def account_edit_post(request: Request, username: str, U: str = Form(default=str()), # Username @@ -443,7 +443,7 @@ async def account(request: Request, username: str): @router.get("/account/{username}/comments") -@auth_required(redirect="/account/{username}/comments") +@auth_required() async def account_comments(request: Request, username: str): user = get_user_by_name(username) context = make_context(request, "Accounts") @@ -454,7 +454,7 @@ async def account_comments(request: Request, username: str): @router.get("/accounts") -@auth_required(True, redirect="/accounts") +@auth_required(True) @account_type_required({at.TRUSTED_USER, at.DEVELOPER, at.TRUSTED_USER_AND_DEV}) @@ -464,7 +464,7 @@ async def accounts(request: Request): @router.post("/accounts") -@auth_required(True, redirect="/accounts") +@auth_required(True) @account_type_required({at.TRUSTED_USER, at.DEVELOPER, at.TRUSTED_USER_AND_DEV}) @@ -548,7 +548,7 @@ def render_terms_of_service(request: Request, @router.get("/tos") -@auth_required(True, redirect="/tos") +@auth_required(True) async def terms_of_service(request: Request): # Query the database for terms that were previously accepted, # but now have a bumped Revision that needs to be accepted. @@ -572,7 +572,7 @@ async def terms_of_service(request: Request): @router.post("/tos") -@auth_required(True, redirect="/tos") +@auth_required(True) async def terms_of_service_post(request: Request, accept: bool = Form(default=False)): # Query the database for terms that were previously accepted, diff --git a/aurweb/routers/auth.py b/aurweb/routers/auth.py index 1e0b026a..74763667 100644 --- a/aurweb/routers/auth.py +++ b/aurweb/routers/auth.py @@ -29,7 +29,7 @@ async def login_get(request: Request, next: str = "/"): @router.post("/login", response_class=HTMLResponse) -@auth_required(False, login=False) +@auth_required(False) async def login_post(request: Request, next: str = Form(...), user: str = Form(default=str()), diff --git a/aurweb/routers/packages.py b/aurweb/routers/packages.py index 2bf04949..4a2cdce3 100644 --- a/aurweb/routers/packages.py +++ b/aurweb/routers/packages.py @@ -295,7 +295,7 @@ async def package_base_voters(request: Request, name: str) -> Response: @router.post("/pkgbase/{name}/comments") -@auth_required(True, redirect="/pkgbase/{name}/comments") +@auth_required(True) async def pkgbase_comments_post( request: Request, name: str, comment: str = Form(default=str()), @@ -327,7 +327,7 @@ async def pkgbase_comments_post( @router.get("/pkgbase/{name}/comments/{id}/form") -@auth_required(True, login=False) +@auth_required(True) async def pkgbase_comment_form(request: Request, name: str, id: int, next: str = Query(default=None)): """ Produce a comment form for comment {id}. """ @@ -353,7 +353,7 @@ async def pkgbase_comment_form(request: Request, name: str, id: int, @router.post("/pkgbase/{name}/comments/{id}") -@auth_required(True, redirect="/pkgbase/{name}/comments/{id}") +@auth_required(True) async def pkgbase_comment_post( request: Request, name: str, id: int, comment: str = Form(default=str()), @@ -392,7 +392,7 @@ async def pkgbase_comment_post( @router.get("/pkgbase/{name}/comments/{id}/edit") -@auth_required(True, redirect="/pkgbase/{name}/comments/{id}/edit") +@auth_required(True) async def pkgbase_comment_edit(request: Request, name: str, id: int, next: str = Form(default=None)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -407,7 +407,7 @@ async def pkgbase_comment_edit(request: Request, name: str, id: int, @router.post("/pkgbase/{name}/comments/{id}/delete") -@auth_required(True, redirect="/pkgbase/{name}/comments/{id}/delete") +@auth_required(True) async def pkgbase_comment_delete(request: Request, name: str, id: int, next: str = Form(default=None)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -433,7 +433,7 @@ async def pkgbase_comment_delete(request: Request, name: str, id: int, @router.post("/pkgbase/{name}/comments/{id}/undelete") -@auth_required(True, redirect="/pkgbase/{name}/comments/{id}/undelete") +@auth_required(True) async def pkgbase_comment_undelete(request: Request, name: str, id: int, next: str = Form(default=None)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -458,7 +458,7 @@ async def pkgbase_comment_undelete(request: Request, name: str, id: int, @router.post("/pkgbase/{name}/comments/{id}/pin") -@auth_required(True, redirect="/pkgbase/{name}/comments/{id}/pin") +@auth_required(True) async def pkgbase_comment_pin(request: Request, name: str, id: int, next: str = Form(default=None)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -483,7 +483,7 @@ async def pkgbase_comment_pin(request: Request, name: str, id: int, @router.post("/pkgbase/{name}/comments/{id}/unpin") -@auth_required(True, redirect="/pkgbase/{name}/comments/{id}/unpin") +@auth_required(True) async def pkgbase_comment_unpin(request: Request, name: str, id: int, next: str = Form(default=None)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -507,7 +507,7 @@ async def pkgbase_comment_unpin(request: Request, name: str, id: int, @router.get("/pkgbase/{name}/comaintainers") -@auth_required(True, redirect="/pkgbase/{name}/comaintainers") +@auth_required(True) async def package_base_comaintainers(request: Request, name: str) -> Response: # Get the PackageBase. pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -532,7 +532,7 @@ async def package_base_comaintainers(request: Request, name: str) -> Response: @router.post("/pkgbase/{name}/comaintainers") -@auth_required(True, redirect="/pkgbase/{name}/comaintainers") +@auth_required(True) async def package_base_comaintainers_post( request: Request, name: str, users: str = Form(default=str())) -> Response: @@ -584,7 +584,7 @@ async def package_base_comaintainers_post( @router.get("/requests") -@auth_required(True, redirect="/requests") +@auth_required(True) async def requests(request: Request, O: int = Query(default=defaults.O), PP: int = Query(default=defaults.PP)): @@ -618,7 +618,7 @@ async def requests(request: Request, @router.get("/pkgbase/{name}/request") -@auth_required(True, redirect="/pkgbase/{name}/request") +@auth_required(True) async def package_request(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) context = await make_variable_context(request, "Submit Request") @@ -627,7 +627,7 @@ async def package_request(request: Request, name: str): @router.post("/pkgbase/{name}/request") -@auth_required(True, redirect="/pkgbase/{name}/request") +@auth_required(True) async def pkgbase_request_post(request: Request, name: str, type: str = Form(...), merge_into: str = Form(default=None), @@ -699,7 +699,7 @@ async def pkgbase_request_post(request: Request, name: str, @router.get("/requests/{id}/close") -@auth_required(True, redirect="/requests/{id}/close") +@auth_required(True) async def requests_close(request: Request, id: int): pkgreq = get_pkgreq_by_id(id) if not request.user.is_elevated() and request.user != pkgreq.User: @@ -712,7 +712,7 @@ async def requests_close(request: Request, id: int): @router.post("/requests/{id}/close") -@auth_required(True, redirect="/requests/{id}/close") +@auth_required(True) async def requests_close_post(request: Request, id: int, reason: int = Form(default=0), comments: str = Form(default=str())): @@ -775,7 +775,7 @@ async def pkgbase_keywords(request: Request, name: str, @router.get("/pkgbase/{name}/flag") -@auth_required(True, redirect="/pkgbase/{name}/flag") +@auth_required(True) async def pkgbase_flag_get(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -790,7 +790,7 @@ async def pkgbase_flag_get(request: Request, name: str): @router.post("/pkgbase/{name}/flag") -@auth_required(True, redirect="/pkgbase/{name}/flag") +@auth_required(True) async def pkgbase_flag_post(request: Request, name: str, comments: str = Form(default=str())): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -839,7 +839,7 @@ def pkgbase_unflag_instance(request: Request, pkgbase: models.PackageBase): @router.post("/pkgbase/{name}/unflag") -@auth_required(True, redirect="/pkgbase/{name}") +@auth_required(True) async def pkgbase_unflag(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) pkgbase_unflag_instance(request, pkgbase) @@ -860,7 +860,7 @@ def pkgbase_notify_instance(request: Request, pkgbase: models.PackageBase): @router.post("/pkgbase/{name}/notify") -@auth_required(True, redirect="/pkgbase/{name}") +@auth_required(True) async def pkgbase_notify(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) pkgbase_notify_instance(request, pkgbase) @@ -879,7 +879,7 @@ def pkgbase_unnotify_instance(request: Request, pkgbase: models.PackageBase): @router.post("/pkgbase/{name}/unnotify") -@auth_required(True, redirect="/pkgbase/{name}") +@auth_required(True) async def pkgbase_unnotify(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) pkgbase_unnotify_instance(request, pkgbase) @@ -888,7 +888,7 @@ async def pkgbase_unnotify(request: Request, name: str): @router.post("/pkgbase/{name}/vote") -@auth_required(True, redirect="/pkgbase/{name}") +@auth_required(True) async def pkgbase_vote(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -912,7 +912,7 @@ async def pkgbase_vote(request: Request, name: str): @router.post("/pkgbase/{name}/unvote") -@auth_required(True, redirect="/pkgbase/{name}") +@auth_required(True) async def pkgbase_unvote(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -954,7 +954,7 @@ def pkgbase_disown_instance(request: Request, pkgbase: models.PackageBase): @router.get("/pkgbase/{name}/disown") -@auth_required(True, redirect="/pkgbase/{name}/disown") +@auth_required(True) async def pkgbase_disown_get(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -970,7 +970,7 @@ async def pkgbase_disown_get(request: Request, name: str): @router.post("/pkgbase/{name}/disown") -@auth_required(True, redirect="/pkgbase/{name}/disown") +@auth_required(True) async def pkgbase_disown_post(request: Request, name: str, confirm: bool = Form(default=False)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -1003,7 +1003,7 @@ def pkgbase_adopt_instance(request: Request, pkgbase: models.PackageBase): @router.post("/pkgbase/{name}/adopt") -@auth_required(True, redirect="/pkgbase/{name}") +@auth_required(True) async def pkgbase_adopt_post(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -1019,7 +1019,7 @@ async def pkgbase_adopt_post(request: Request, name: str): @router.get("/pkgbase/{name}/delete") -@auth_required(True, redirect="/pkgbase/{name}/delete") +@auth_required(True) async def pkgbase_delete_get(request: Request, name: str): if not request.user.has_credential(creds.PKGBASE_DELETE): return RedirectResponse(f"/pkgbase/{name}", @@ -1031,7 +1031,7 @@ async def pkgbase_delete_get(request: Request, name: str): @router.post("/pkgbase/{name}/delete") -@auth_required(True, redirect="/pkgbase/{name}/delete") +@auth_required(True) async def pkgbase_delete_post(request: Request, name: str, confirm: bool = Form(default=False)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -1279,7 +1279,7 @@ PACKAGE_ACTIONS = { @router.post("/packages") -@auth_required(redirect="/packages") +@auth_required() async def packages_post(request: Request, IDs: List[int] = Form(default=[]), action: str = Form(default=str()), @@ -1311,7 +1311,7 @@ async def packages_post(request: Request, @router.get("/pkgbase/{name}/merge") -@auth_required(redirect="/pkgbase/{name}/merge") +@auth_required() async def pkgbase_merge_get(request: Request, name: str, into: str = Query(default=str()), next: str = Query(default=str())): @@ -1423,7 +1423,7 @@ def pkgbase_merge_instance(request: Request, pkgbase: models.PackageBase, @router.post("/pkgbase/{name}/merge") -@auth_required(redirect="/pkgbase/{name}/merge") +@auth_required() async def pkgbase_merge_post(request: Request, name: str, into: str = Form(default=str()), confirm: bool = Form(default=False), diff --git a/aurweb/routers/trusted_user.py b/aurweb/routers/trusted_user.py index f0cea61e..09de58fe 100644 --- a/aurweb/routers/trusted_user.py +++ b/aurweb/routers/trusted_user.py @@ -41,7 +41,7 @@ ADDVOTE_SPECIFICS = { @router.get("/tu") -@auth_required(True, redirect="/tu") +@auth_required(True) @account_type_required(REQUIRED_TYPES) async def trusted_user(request: Request, coff: int = 0, # current offset @@ -147,7 +147,7 @@ def render_proposal(request: Request, @router.get("/tu/{proposal}") -@auth_required(True, redirect="/tu/{proposal}") +@auth_required(True) @account_type_required(REQUIRED_TYPES) async def trusted_user_proposal(request: Request, proposal: int): context = await make_variable_context(request, "Trusted User") @@ -176,7 +176,7 @@ async def trusted_user_proposal(request: Request, proposal: int): @router.post("/tu/{proposal}") -@auth_required(True, redirect="/tu/{proposal}") +@auth_required(True) @account_type_required(REQUIRED_TYPES) async def trusted_user_proposal_post(request: Request, proposal: int, @@ -227,7 +227,7 @@ async def trusted_user_proposal_post(request: Request, @router.get("/addvote") -@auth_required(True, redirect="/addvote") +@auth_required(True) @account_type_required({TRUSTED_USER, TRUSTED_USER_AND_DEV}) async def trusted_user_addvote(request: Request, user: str = str(), @@ -247,7 +247,7 @@ async def trusted_user_addvote(request: Request, @router.post("/addvote") -@auth_required(True, redirect="/addvote") +@auth_required(True) @account_type_required({TRUSTED_USER, TRUSTED_USER_AND_DEV}) async def trusted_user_addvote_post(request: Request, user: str = Form(default=str()), diff --git a/test/test_trusted_user_routes.py b/test/test_trusted_user_routes.py index 43a3443b..ac7f82d5 100644 --- a/test/test_trusted_user_routes.py +++ b/test/test_trusted_user_routes.py @@ -9,7 +9,7 @@ import pytest from fastapi.testclient import TestClient -from aurweb import db, util +from aurweb import config, db, util from aurweb.models.account_type import AccountType from aurweb.models.tu_vote import TUVote from aurweb.models.tu_voteinfo import TUVoteInfo @@ -124,8 +124,9 @@ def proposal(user, tu_user): def test_tu_index_guest(client): + headers = {"referer": config.get("options", "aur_location") + "/tu"} with client as request: - response = request.get("/tu", allow_redirects=False) + response = request.get("/tu", allow_redirects=False, headers=headers) assert response.status_code == int(HTTPStatus.SEE_OTHER) params = util.urlencode({"next": "/tu"}) From 0b30216229f561cfdcffd27231f200c3901ce26d Mon Sep 17 00:00:00 2001 From: Steven Guikal Date: Thu, 18 Nov 2021 15:18:17 -0500 Subject: [PATCH 6/6] fix(FastAPI): remove unnecessary arguments to auth_required Signed-off-by: Steven Guikal --- aurweb/auth/__init__.py | 59 ++-------------------------------- aurweb/routers/accounts.py | 24 ++++++-------- aurweb/routers/packages.py | 54 +++++++++++++++---------------- aurweb/routers/trusted_user.py | 10 +++--- 4 files changed, 43 insertions(+), 104 deletions(-) diff --git a/aurweb/auth/__init__.py b/aurweb/auth/__init__.py index 7aa4b526..8ceb136c 100644 --- a/aurweb/auth/__init__.py +++ b/aurweb/auth/__init__.py @@ -5,6 +5,7 @@ from http import HTTPStatus import fastapi +from fastapi import HTTPException from fastapi.responses import RedirectResponse from sqlalchemy import and_ from starlette.authentication import AuthCredentials, AuthenticationBackend @@ -15,7 +16,6 @@ import aurweb.config from aurweb import db, l10n, util from aurweb.models import Session, User from aurweb.models.account_type import ACCOUNT_TYPE_ID -from aurweb.templates import make_variable_context, render_template class StubQuery: @@ -125,29 +125,7 @@ def auth_required(is_required: bool = True, status_code: HTTPStatus = HTTPStatus.UNAUTHORIZED): """ Authentication route decorator. - If template is given, it will be rendered with Unauthorized if - is_required does not match. - - A precondition of this function is that, if template is provided, - it **must** match the following format: - - template=("template.html", ["Some Template For", "{}"], ["username"]) - - Where `username` is a FastAPI request path parameter, fitting - a route like: `/some_route/{username}`. - - If you wish to supply a non-formatted template, just omit any Python - format strings (with the '{}' substring). The third tuple element - will not be used, and so anything can be supplied. - - template=("template.html", ["Some Page"], None) - - All title shards and format parameters will be translated before - applying any format operations. - :param is_required: A boolean indicating whether the function requires auth - :param template: A three-element template tuple: - (path, title_iterable, variable_iterable) :param status_code: An optional status_code for template render. Redirects are always SEE_OTHER. """ @@ -164,45 +142,12 @@ def auth_required(is_required: bool = True, elif request.method == "POST" and (referer := request.headers.get("Referer")): aur = aurweb.config.get("options", "aur_location") + "/" if not referer.startswith(aur): + _ = l10n.get_translator_for_request(request) raise HTTPException(status_code=HTTPStatus.BAD_REQUEST, detail=_("Bad Referer header.")) url = referer[len(aur) - 1:] url = "/login?" + util.urlencode({"next": url}) - - if template: - # template=("template.html", - # ["Some Title", "someFormatted {}"], - # ["variable"]) - # => render template.html with title: - # "Some Title someFormatted variables" - path, title_parts, variables = template - _ = l10n.get_translator_for_request(request) - - # Step through title_parts; for each part which contains - # a '{}' in it, apply .format(var) where var = the current - # iteration of variables. - # - # This implies that len(variables) is equal to - # len([part for part in title_parts if '{}' in part]) - # and this must always be true. - # - sanitized = [] - _variables = iter(variables) - for part in title_parts: - if "{}" in part: # If this part is formattable. - key = next(_variables) - var = request.path_params.get(key) - sanitized.append(_(part.format(var))) - else: # Otherwise, just add the translated part. - sanitized.append(_(part)) - - # Glue all title parts together, separated by spaces. - title = " ".join(sanitized) - - context = await make_variable_context(request, title) - return render_template(request, path, context, - status_code=status_code) return RedirectResponse(url, status_code=int(HTTPStatus.SEE_OTHER)) return await func(request, *args, **kwargs) diff --git a/aurweb/routers/accounts.py b/aurweb/routers/accounts.py index dade92bb..388daf84 100644 --- a/aurweb/routers/accounts.py +++ b/aurweb/routers/accounts.py @@ -340,7 +340,7 @@ def cannot_edit(request, user): @router.get("/account/{username}/edit", response_class=HTMLResponse) -@auth_required(True) +@auth_required() async def account_edit(request: Request, username: str): user = db.query(models.User, models.User.Username == username).first() @@ -356,7 +356,7 @@ async def account_edit(request: Request, username: str): @router.post("/account/{username}/edit", response_class=HTMLResponse) -@auth_required(True) +@auth_required() async def account_edit_post(request: Request, username: str, U: str = Form(default=str()), # Username @@ -424,20 +424,14 @@ async def account_edit_post(request: Request, aurtz=TZ, aurlang=L) -account_template = ( - "account/show.html", - ["Account", "{}"], - ["username"] # Query parameters to replace in the title string. -) - - @router.get("/account/{username}") -@auth_required(True, template=account_template, - status_code=HTTPStatus.UNAUTHORIZED) async def account(request: Request, username: str): _ = l10n.get_translator_for_request(request) context = await make_variable_context( request, _("Account") + " " + username) + if not request.user.is_authenticated(): + return render_template(request, "account/show.html", context, + status_code=HTTPStatus.UNAUTHORIZED) context["user"] = get_user_by_name(username) return render_template(request, "account/show.html", context) @@ -454,7 +448,7 @@ async def account_comments(request: Request, username: str): @router.get("/accounts") -@auth_required(True) +@auth_required() @account_type_required({at.TRUSTED_USER, at.DEVELOPER, at.TRUSTED_USER_AND_DEV}) @@ -464,7 +458,7 @@ async def accounts(request: Request): @router.post("/accounts") -@auth_required(True) +@auth_required() @account_type_required({at.TRUSTED_USER, at.DEVELOPER, at.TRUSTED_USER_AND_DEV}) @@ -548,7 +542,7 @@ def render_terms_of_service(request: Request, @router.get("/tos") -@auth_required(True) +@auth_required() async def terms_of_service(request: Request): # Query the database for terms that were previously accepted, # but now have a bumped Revision that needs to be accepted. @@ -572,7 +566,7 @@ async def terms_of_service(request: Request): @router.post("/tos") -@auth_required(True) +@auth_required() async def terms_of_service_post(request: Request, accept: bool = Form(default=False)): # Query the database for terms that were previously accepted, diff --git a/aurweb/routers/packages.py b/aurweb/routers/packages.py index 4a2cdce3..c06ec51f 100644 --- a/aurweb/routers/packages.py +++ b/aurweb/routers/packages.py @@ -295,7 +295,7 @@ async def package_base_voters(request: Request, name: str) -> Response: @router.post("/pkgbase/{name}/comments") -@auth_required(True) +@auth_required() async def pkgbase_comments_post( request: Request, name: str, comment: str = Form(default=str()), @@ -327,7 +327,7 @@ async def pkgbase_comments_post( @router.get("/pkgbase/{name}/comments/{id}/form") -@auth_required(True) +@auth_required() async def pkgbase_comment_form(request: Request, name: str, id: int, next: str = Query(default=None)): """ Produce a comment form for comment {id}. """ @@ -353,7 +353,7 @@ async def pkgbase_comment_form(request: Request, name: str, id: int, @router.post("/pkgbase/{name}/comments/{id}") -@auth_required(True) +@auth_required() async def pkgbase_comment_post( request: Request, name: str, id: int, comment: str = Form(default=str()), @@ -392,7 +392,7 @@ async def pkgbase_comment_post( @router.get("/pkgbase/{name}/comments/{id}/edit") -@auth_required(True) +@auth_required() async def pkgbase_comment_edit(request: Request, name: str, id: int, next: str = Form(default=None)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -407,7 +407,7 @@ async def pkgbase_comment_edit(request: Request, name: str, id: int, @router.post("/pkgbase/{name}/comments/{id}/delete") -@auth_required(True) +@auth_required() async def pkgbase_comment_delete(request: Request, name: str, id: int, next: str = Form(default=None)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -433,7 +433,7 @@ async def pkgbase_comment_delete(request: Request, name: str, id: int, @router.post("/pkgbase/{name}/comments/{id}/undelete") -@auth_required(True) +@auth_required() async def pkgbase_comment_undelete(request: Request, name: str, id: int, next: str = Form(default=None)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -458,7 +458,7 @@ async def pkgbase_comment_undelete(request: Request, name: str, id: int, @router.post("/pkgbase/{name}/comments/{id}/pin") -@auth_required(True) +@auth_required() async def pkgbase_comment_pin(request: Request, name: str, id: int, next: str = Form(default=None)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -483,7 +483,7 @@ async def pkgbase_comment_pin(request: Request, name: str, id: int, @router.post("/pkgbase/{name}/comments/{id}/unpin") -@auth_required(True) +@auth_required() async def pkgbase_comment_unpin(request: Request, name: str, id: int, next: str = Form(default=None)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -507,7 +507,7 @@ async def pkgbase_comment_unpin(request: Request, name: str, id: int, @router.get("/pkgbase/{name}/comaintainers") -@auth_required(True) +@auth_required() async def package_base_comaintainers(request: Request, name: str) -> Response: # Get the PackageBase. pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -532,7 +532,7 @@ async def package_base_comaintainers(request: Request, name: str) -> Response: @router.post("/pkgbase/{name}/comaintainers") -@auth_required(True) +@auth_required() async def package_base_comaintainers_post( request: Request, name: str, users: str = Form(default=str())) -> Response: @@ -584,7 +584,7 @@ async def package_base_comaintainers_post( @router.get("/requests") -@auth_required(True) +@auth_required() async def requests(request: Request, O: int = Query(default=defaults.O), PP: int = Query(default=defaults.PP)): @@ -618,7 +618,7 @@ async def requests(request: Request, @router.get("/pkgbase/{name}/request") -@auth_required(True) +@auth_required() async def package_request(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) context = await make_variable_context(request, "Submit Request") @@ -627,7 +627,7 @@ async def package_request(request: Request, name: str): @router.post("/pkgbase/{name}/request") -@auth_required(True) +@auth_required() async def pkgbase_request_post(request: Request, name: str, type: str = Form(...), merge_into: str = Form(default=None), @@ -699,7 +699,7 @@ async def pkgbase_request_post(request: Request, name: str, @router.get("/requests/{id}/close") -@auth_required(True) +@auth_required() async def requests_close(request: Request, id: int): pkgreq = get_pkgreq_by_id(id) if not request.user.is_elevated() and request.user != pkgreq.User: @@ -712,7 +712,7 @@ async def requests_close(request: Request, id: int): @router.post("/requests/{id}/close") -@auth_required(True) +@auth_required() async def requests_close_post(request: Request, id: int, reason: int = Form(default=0), comments: str = Form(default=str())): @@ -775,7 +775,7 @@ async def pkgbase_keywords(request: Request, name: str, @router.get("/pkgbase/{name}/flag") -@auth_required(True) +@auth_required() async def pkgbase_flag_get(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -790,7 +790,7 @@ async def pkgbase_flag_get(request: Request, name: str): @router.post("/pkgbase/{name}/flag") -@auth_required(True) +@auth_required() async def pkgbase_flag_post(request: Request, name: str, comments: str = Form(default=str())): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -839,7 +839,7 @@ def pkgbase_unflag_instance(request: Request, pkgbase: models.PackageBase): @router.post("/pkgbase/{name}/unflag") -@auth_required(True) +@auth_required() async def pkgbase_unflag(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) pkgbase_unflag_instance(request, pkgbase) @@ -860,7 +860,7 @@ def pkgbase_notify_instance(request: Request, pkgbase: models.PackageBase): @router.post("/pkgbase/{name}/notify") -@auth_required(True) +@auth_required() async def pkgbase_notify(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) pkgbase_notify_instance(request, pkgbase) @@ -879,7 +879,7 @@ def pkgbase_unnotify_instance(request: Request, pkgbase: models.PackageBase): @router.post("/pkgbase/{name}/unnotify") -@auth_required(True) +@auth_required() async def pkgbase_unnotify(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) pkgbase_unnotify_instance(request, pkgbase) @@ -888,7 +888,7 @@ async def pkgbase_unnotify(request: Request, name: str): @router.post("/pkgbase/{name}/vote") -@auth_required(True) +@auth_required() async def pkgbase_vote(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -912,7 +912,7 @@ async def pkgbase_vote(request: Request, name: str): @router.post("/pkgbase/{name}/unvote") -@auth_required(True) +@auth_required() async def pkgbase_unvote(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -954,7 +954,7 @@ def pkgbase_disown_instance(request: Request, pkgbase: models.PackageBase): @router.get("/pkgbase/{name}/disown") -@auth_required(True) +@auth_required() async def pkgbase_disown_get(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -970,7 +970,7 @@ async def pkgbase_disown_get(request: Request, name: str): @router.post("/pkgbase/{name}/disown") -@auth_required(True) +@auth_required() async def pkgbase_disown_post(request: Request, name: str, confirm: bool = Form(default=False)): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -1003,7 +1003,7 @@ def pkgbase_adopt_instance(request: Request, pkgbase: models.PackageBase): @router.post("/pkgbase/{name}/adopt") -@auth_required(True) +@auth_required() async def pkgbase_adopt_post(request: Request, name: str): pkgbase = get_pkg_or_base(name, models.PackageBase) @@ -1019,7 +1019,7 @@ async def pkgbase_adopt_post(request: Request, name: str): @router.get("/pkgbase/{name}/delete") -@auth_required(True) +@auth_required() async def pkgbase_delete_get(request: Request, name: str): if not request.user.has_credential(creds.PKGBASE_DELETE): return RedirectResponse(f"/pkgbase/{name}", @@ -1031,7 +1031,7 @@ async def pkgbase_delete_get(request: Request, name: str): @router.post("/pkgbase/{name}/delete") -@auth_required(True) +@auth_required() async def pkgbase_delete_post(request: Request, name: str, confirm: bool = Form(default=False)): pkgbase = get_pkg_or_base(name, models.PackageBase) diff --git a/aurweb/routers/trusted_user.py b/aurweb/routers/trusted_user.py index 09de58fe..fac68f04 100644 --- a/aurweb/routers/trusted_user.py +++ b/aurweb/routers/trusted_user.py @@ -41,7 +41,7 @@ ADDVOTE_SPECIFICS = { @router.get("/tu") -@auth_required(True) +@auth_required() @account_type_required(REQUIRED_TYPES) async def trusted_user(request: Request, coff: int = 0, # current offset @@ -147,7 +147,7 @@ def render_proposal(request: Request, @router.get("/tu/{proposal}") -@auth_required(True) +@auth_required() @account_type_required(REQUIRED_TYPES) async def trusted_user_proposal(request: Request, proposal: int): context = await make_variable_context(request, "Trusted User") @@ -176,7 +176,7 @@ async def trusted_user_proposal(request: Request, proposal: int): @router.post("/tu/{proposal}") -@auth_required(True) +@auth_required() @account_type_required(REQUIRED_TYPES) async def trusted_user_proposal_post(request: Request, proposal: int, @@ -227,7 +227,7 @@ async def trusted_user_proposal_post(request: Request, @router.get("/addvote") -@auth_required(True) +@auth_required() @account_type_required({TRUSTED_USER, TRUSTED_USER_AND_DEV}) async def trusted_user_addvote(request: Request, user: str = str(), @@ -247,7 +247,7 @@ async def trusted_user_addvote(request: Request, @router.post("/addvote") -@auth_required(True) +@auth_required() @account_type_required({TRUSTED_USER, TRUSTED_USER_AND_DEV}) async def trusted_user_addvote_post(request: Request, user: str = Form(default=str()),