From 3b66565481c2917560bc2f3e6d6e8c33bf184d0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rn-Michael=20Miehe?= <40151420+ldericher@users.noreply.github.com> Date: Wed, 30 Mar 2022 02:02:45 +0000 Subject: [PATCH] remove get_current_user_if_admin --- api/kiwi_vpn_api/routers/_common.py | 22 ++-------------- api/kiwi_vpn_api/routers/admin.py | 8 ++++-- api/kiwi_vpn_api/routers/device.py | 2 +- api/kiwi_vpn_api/routers/user.py | 40 ++++++++++++++++++----------- 4 files changed, 34 insertions(+), 38 deletions(-) diff --git a/api/kiwi_vpn_api/routers/_common.py b/api/kiwi_vpn_api/routers/_common.py index 1d7bff7..875b15e 100644 --- a/api/kiwi_vpn_api/routers/_common.py +++ b/api/kiwi_vpn_api/routers/_common.py @@ -5,7 +5,6 @@ Common dependencies for routers. from fastapi import Depends, HTTPException, status from fastapi.security import OAuth2PasswordBearer -from kiwi_vpn_api.db.tag import TagValue from ..config import Config, Settings from ..db import Device, User @@ -41,8 +40,8 @@ class Responses: "description": "Must be admin", "content": None, } - PERMISSION_ERROR = { - "description": "You're not allowed that action", + NEEDS_PERMISSION = { + "description": "You're not allowed that operation", "content": None, } ENTRY_EXISTS = { @@ -53,10 +52,6 @@ class Responses: "description": "Entry does not exist in database", "content": None, } - CANT_TARGET_SELF = { - "description": "Operation can't target yourself", - "content": None, - } async def get_current_user( @@ -81,19 +76,6 @@ async def get_current_user( return user -async def get_current_user_if_admin( - current_user: User = Depends(get_current_user), -) -> User: - """ - Fail if the currently logged-in user is not an admin. - """ - - if current_user.has_tag(TagValue.admin): - raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) - - return current_user - - async def get_user_by_name( user_name: str, current_config: Config | None = Depends(Config.load), diff --git a/api/kiwi_vpn_api/routers/admin.py b/api/kiwi_vpn_api/routers/admin.py index 7727bad..b337a9a 100644 --- a/api/kiwi_vpn_api/routers/admin.py +++ b/api/kiwi_vpn_api/routers/admin.py @@ -8,7 +8,7 @@ from sqlmodel import select from ..config import Config from ..db import Connection, TagValue, User, UserCreate -from ._common import Responses, get_current_user_if_admin +from ._common import Responses, get_current_user router = APIRouter(prefix="/admin", tags=["admin"]) @@ -79,12 +79,16 @@ async def create_initial_admin( ) async def set_config( config: Config, - _: User = Depends(get_current_user_if_admin), + current_user: User = Depends(get_current_user), ): """ PUT ./config: Edit `kiwi-vpn` main config. """ + # check permissions + if not current_user.has_tag(TagValue.admin): + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) + # update config file, reconnect to database config.save() Connection.connect(config.db.uri) diff --git a/api/kiwi_vpn_api/routers/device.py b/api/kiwi_vpn_api/routers/device.py index 6611cbf..4e2bd16 100644 --- a/api/kiwi_vpn_api/routers/device.py +++ b/api/kiwi_vpn_api/routers/device.py @@ -17,7 +17,7 @@ router = APIRouter(prefix="/device", tags=["device"]) status.HTTP_200_OK: Responses.OK, status.HTTP_400_BAD_REQUEST: Responses.NOT_INSTALLED, status.HTTP_401_UNAUTHORIZED: Responses.NEEDS_USER, - status.HTTP_403_FORBIDDEN: Responses.PERMISSION_ERROR, + status.HTTP_403_FORBIDDEN: Responses.NEEDS_PERMISSION, status.HTTP_404_NOT_FOUND: Responses.ENTRY_DOESNT_EXIST, status.HTTP_409_CONFLICT: Responses.ENTRY_EXISTS, }, diff --git a/api/kiwi_vpn_api/routers/user.py b/api/kiwi_vpn_api/routers/user.py index 0529b16..56a0a35 100644 --- a/api/kiwi_vpn_api/routers/user.py +++ b/api/kiwi_vpn_api/routers/user.py @@ -8,8 +8,7 @@ from pydantic import BaseModel from ..config import Config from ..db import TagValue, User, UserCreate, UserRead -from ._common import (Responses, get_current_user, get_current_user_if_admin, - get_user_by_name) +from ._common import Responses, get_current_user, get_user_by_name router = APIRouter(prefix="/user", tags=["user"]) @@ -54,7 +53,7 @@ async def login( @router.get("/current", response_model=UserRead) -async def get_current_user( +async def get_current_user_route( current_user: User = Depends(get_current_user), ): """ @@ -77,13 +76,17 @@ async def get_current_user( ) async def add_user( user: UserCreate, - _: User = Depends(get_current_user_if_admin), + current_user: User = Depends(get_current_user), ) -> User: """ POST ./: Create a new user in the database. """ - # actually create the new user + # check permissions + if not current_user.can_create(User): + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) + + # create the new user new_user = User.create(user=user) # fail if creation was unsuccessful @@ -103,23 +106,22 @@ async def add_user( status.HTTP_200_OK: Responses.OK, status.HTTP_400_BAD_REQUEST: Responses.NOT_INSTALLED, status.HTTP_401_UNAUTHORIZED: Responses.NEEDS_USER, - status.HTTP_403_FORBIDDEN: Responses.NEEDS_ADMIN, + status.HTTP_403_FORBIDDEN: Responses.NEEDS_PERMISSION, status.HTTP_404_NOT_FOUND: Responses.ENTRY_DOESNT_EXIST, - status.HTTP_406_NOT_ACCEPTABLE: Responses.CANT_TARGET_SELF, }, response_model=User, ) async def remove_user( - current_user: User = Depends(get_current_user_if_admin), + current_user: User = Depends(get_current_user), user: User = Depends(get_user_by_name), ): """ DELETE ./{user_name}: Remove a user from the database. """ - # stop inting - if current_user.name == user.name: - raise HTTPException(status_code=status.HTTP_406_NOT_ACCEPTABLE) + # check permissions + if not current_user.can_admin(user): + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) # delete user user.delete() @@ -136,15 +138,19 @@ async def remove_user( ) async def extend_tags( tags: list[TagValue], - _: User = Depends(get_current_user_if_admin), + current_user: User = Depends(get_current_user), user: User = Depends(get_user_by_name), ): """ POST ./{user_name}/tags: Add tags to a user. """ - user.add_tags(tags) + # check permissions + if not current_user.can_admin(user): + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) + # change user + user.add_tags(tags) user.update() @@ -159,13 +165,17 @@ async def extend_tags( ) async def remove_tags( tags: list[TagValue], - _: User = Depends(get_current_user_if_admin), + current_user: User = Depends(get_current_user), user: User = Depends(get_user_by_name), ): """ DELETE ./{user_name}/tags: Remove tags from a user. """ - user.remove_tags(tags) + # check permissions + if not current_user.can_admin(user): + raise HTTPException(status_code=status.HTTP_403_FORBIDDEN) + # change user + user.remove_tags(tags) user.update()