diff --git a/gns3server/api/routes/controller/dependencies/authentication.py b/gns3server/api/routes/controller/dependencies/authentication.py index c66b4b62..819daec9 100644 --- a/gns3server/api/routes/controller/dependencies/authentication.py +++ b/gns3server/api/routes/controller/dependencies/authentication.py @@ -44,6 +44,10 @@ async def get_user_from_token( async def get_current_active_user(current_user: schemas.User = Depends(get_user_from_token)) -> schemas.User: + # Super admin is always authorized + if current_user.is_superadmin: + return current_user + if not current_user.is_active: raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, diff --git a/gns3server/api/routes/controller/users.py b/gns3server/api/routes/controller/users.py index 2092b251..9649e911 100644 --- a/gns3server/api/routes/controller/users.py +++ b/gns3server/api/routes/controller/users.py @@ -45,7 +45,10 @@ router = APIRouter() @router.get("", response_model=List[schemas.User]) -async def get_users(users_repo: UsersRepository = Depends(get_repository(UsersRepository))) -> List[schemas.User]: +async def get_users( + users_repo: UsersRepository = Depends(get_repository(UsersRepository)), + current_user: schemas.User = Depends(get_current_active_user) +) -> List[schemas.User]: """ Get all users. """ @@ -55,7 +58,9 @@ async def get_users(users_repo: UsersRepository = Depends(get_repository(UsersRe @router.post("", response_model=schemas.User, status_code=status.HTTP_201_CREATED) async def create_user( - user_create: schemas.UserCreate, users_repo: UsersRepository = Depends(get_repository(UsersRepository)) + user_create: schemas.UserCreate, + users_repo: UsersRepository = Depends(get_repository(UsersRepository)), + current_user: schemas.User = Depends(get_current_active_user) ) -> schemas.User: """ Create a new user. @@ -70,9 +75,11 @@ async def create_user( return await users_repo.create_user(user_create) -@router.get("/{user_id}", response_model=schemas.User) +@router.get("/{user_id}",response_model=schemas.User) async def get_user( - user_id: UUID, users_repo: UsersRepository = Depends(get_repository(UsersRepository)) + user_id: UUID, + users_repo: UsersRepository = Depends(get_repository(UsersRepository)), + current_user: schemas.User = Depends(get_current_active_user) ) -> schemas.User: """ Get an user. @@ -86,9 +93,10 @@ async def get_user( @router.put("/{user_id}", response_model=schemas.User) async def update_user( - user_id: UUID, - user_update: schemas.UserUpdate, - users_repo: UsersRepository = Depends(get_repository(UsersRepository)), + user_id: UUID, + user_update: schemas.UserUpdate, + users_repo: UsersRepository = Depends(get_repository(UsersRepository)), + current_user: schemas.User = Depends(get_current_active_user) ) -> schemas.User: """ Update an user. @@ -111,7 +119,7 @@ async def delete_user( """ if current_user.is_superadmin: - raise ControllerForbiddenError("The super user cannot be deleted") + raise ControllerForbiddenError("The super admin cannot be deleted") success = await users_repo.delete_user(user_id) if not success: diff --git a/gns3server/db/models/users.py b/gns3server/db/models/users.py index 99108975..6f9400ba 100644 --- a/gns3server/db/models/users.py +++ b/gns3server/db/models/users.py @@ -37,6 +37,7 @@ class User(BaseTable): is_active = Column(Boolean, default=True) is_superadmin = Column(Boolean, default=False) + @event.listens_for(User.__table__, 'after_create') def create_default_super_admin(target, connection, **kw): @@ -49,4 +50,4 @@ def create_default_super_admin(target, connection, **kw): ) connection.execute(stmt) connection.commit() - log.info("Default super admin account added") + log.info("The default super admin account has been created in the database") diff --git a/gns3server/db/repositories/users.py b/gns3server/db/repositories/users.py index 4b531588..96576cfc 100644 --- a/gns3server/db/repositories/users.py +++ b/gns3server/db/repositories/users.py @@ -26,6 +26,10 @@ import gns3server.db.models as models from gns3server import schemas from gns3server.services import auth_service +import logging + +log = logging.getLogger(__name__) + class UsersRepository(BaseRepository): def __init__(self, db_session: AsyncSession) -> None: @@ -93,6 +97,13 @@ class UsersRepository(BaseRepository): user = await self.get_user_by_username(username) if not user: return None + # Allow user to be authenticated if hashed password in the db is null + # this is useful for manual password recovery like: + # sqlite3 gns3_controller.db "UPDATE users SET hashed_password = null WHERE username = 'admin';" + if user.hashed_password is None: + log.warning(f"User '{username}' has been authenticated without a password " + f"configured. Please set a new password.") + return user if not self._auth_service.verify_password(password, user.hashed_password): return None return user diff --git a/gns3server/schemas/controller/users.py b/gns3server/schemas/controller/users.py index 11aa100e..62421179 100644 --- a/gns3server/schemas/controller/users.py +++ b/gns3server/schemas/controller/users.py @@ -37,7 +37,7 @@ class UserCreate(UserBase): """ username: str = Field(..., min_length=3, regex="[a-zA-Z0-9_-]+$") - password: SecretStr = Field(..., min_length=7, max_length=100) + password: SecretStr = Field(..., min_length=6, max_length=100) class UserUpdate(UserBase): @@ -45,7 +45,7 @@ class UserUpdate(UserBase): Properties to update an user. """ - password: Optional[SecretStr] = Field(None, min_length=7, max_length=100) + password: Optional[SecretStr] = Field(None, min_length=6, max_length=100) class User(DateTimeModelMixin, UserBase): diff --git a/tests/api/routes/controller/test_users.py b/tests/api/routes/controller/test_users.py index 6fbfc7cf..f02d77e5 100644 --- a/tests/api/routes/controller/test_users.py +++ b/tests/api/routes/controller/test_users.py @@ -19,15 +19,16 @@ import pytest from typing import Optional from fastapi import FastAPI, HTTPException, status +from sqlalchemy import update from httpx import AsyncClient from jose import jwt from sqlalchemy.ext.asyncio import AsyncSession from gns3server.db.repositories.users import UsersRepository from gns3server.services import auth_service -from gns3server.services.authentication import DEFAULT_JWT_SECRET_KEY from gns3server.config import Config from gns3server.schemas.controller.users import User +import gns3server.db.models as models pytestmark = pytest.mark.asyncio @@ -38,7 +39,7 @@ class TestUserRoutes: new_user = {"username": "user1", "email": "user1@email.com", "password": "test_password"} response = await client.post(app.url_path_for("create_user"), json=new_user) - assert response.status_code != status.HTTP_404_NOT_FOUND + assert response.status_code == status.HTTP_201_CREATED async def test_users_can_register_successfully( self, @@ -188,18 +189,18 @@ class TestUserLogin: async def test_user_can_login_successfully_and_receives_valid_token( self, app: FastAPI, - client: AsyncClient, + unauthorized_client: AsyncClient, test_user: User, config: Config ) -> None: jwt_secret = config.settings.Controller.jwt_secret_key - client.headers["content-type"] = "application/x-www-form-urlencoded" + unauthorized_client.headers["content-type"] = "application/x-www-form-urlencoded" login_data = { "username": test_user.username, "password": "user1_password", } - res = await client.post(app.url_path_for("login"), data=login_data) + res = await unauthorized_client.post(app.url_path_for("login"), data=login_data) assert res.status_code == status.HTTP_200_OK # check that token exists in response and has user encoded within it @@ -224,19 +225,19 @@ class TestUserLogin: async def test_user_with_wrong_creds_doesnt_receive_token( self, app: FastAPI, - client: AsyncClient, + unauthorized_client: AsyncClient, test_user: User, username: str, password: str, status_code: int, ) -> None: - client.headers["content-type"] = "application/x-www-form-urlencoded" + unauthorized_client.headers["content-type"] = "application/x-www-form-urlencoded" login_data = { "username": username, "password": password, } - res = await client.post(app.url_path_for("login"), data=login_data) + res = await unauthorized_client.post(app.url_path_for("login"), data=login_data) assert res.status_code == status_code assert "access_token" not in res.json() @@ -259,11 +260,11 @@ class TestUserMe: async def test_user_cannot_access_own_data_if_not_authenticated( self, app: FastAPI, - client: AsyncClient, + unauthorized_client: AsyncClient, test_user: User, ) -> None: - res = await client.get(app.url_path_for("get_current_active_user")) + res = await unauthorized_client.get(app.url_path_for("get_current_active_user")) assert res.status_code == status.HTTP_401_UNAUTHORIZED @@ -284,11 +285,31 @@ class TestSuperAdmin: async def test_cannot_delete_super_admin( self, app: FastAPI, - admin_client: AsyncClient, + client: AsyncClient, db_session: AsyncSession ) -> None: user_repo = UsersRepository(db_session) admin_in_db = await user_repo.get_user_by_username("admin") - res = await admin_client.delete(app.url_path_for("delete_user", user_id=admin_in_db.user_id)) + res = await client.delete(app.url_path_for("delete_user", user_id=admin_in_db.user_id)) assert res.status_code == status.HTTP_403_FORBIDDEN + + async def test_admin_can_login_after_password_recovery( + self, + app: FastAPI, + unauthorized_client: AsyncClient, + db_session: AsyncSession + ) -> None: + + # set the admin password to null in the database + query = update(models.User).where(models.User.username == "admin").values(hashed_password=None) + await db_session.execute(query) + await db_session.commit() + + unauthorized_client.headers["content-type"] = "application/x-www-form-urlencoded" + login_data = { + "username": "admin", + "password": "whatever", + } + res = await unauthorized_client.post(app.url_path_for("login"), data=login_data) + assert res.status_code == status.HTTP_200_OK diff --git a/tests/conftest.py b/tests/conftest.py index b75cd8b2..9645c85f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -94,7 +94,7 @@ async def db_session(db_engine): @pytest.fixture -async def client(app: FastAPI, db_session: AsyncSession) -> AsyncClient: +async def base_client(app: FastAPI, db_session: AsyncSession) -> AsyncClient: async def _get_test_db(): try: @@ -108,8 +108,8 @@ async def client(app: FastAPI, db_session: AsyncSession) -> AsyncClient: app=app, base_url="http://test-api", headers={"Content-Type": "application/json"} - ) as client: - yield client + ) as async_client: + yield async_client @pytest.fixture @@ -147,25 +147,33 @@ async def test_compute(db_session: AsyncSession) -> Compute: @pytest.fixture -def authorized_client(client: AsyncClient, test_user: User) -> AsyncClient: +def unauthorized_client(base_client: AsyncClient, test_user: User) -> AsyncClient: + return base_client - access_token = auth_service.create_access_token(test_user.username) - client.headers = { - **client.headers, - "Authorization": f"Bearer {access_token}", - } - return client @pytest.fixture -async def admin_client(client: AsyncClient) -> AsyncClient: +def authorized_client(base_client: AsyncClient, test_user: User) -> AsyncClient: - # user "admin" is automatically created when the users table is created - access_token = auth_service.create_access_token("admin") - client.headers = { - **client.headers, + access_token = auth_service.create_access_token(test_user.username) + base_client.headers = { + **base_client.headers, "Authorization": f"Bearer {access_token}", } - return client + return base_client + + +@pytest.fixture +async def client(base_client: AsyncClient) -> AsyncClient: + + # The super admin is automatically created when the users table is created + # this account that can access all endpoints without restrictions. + access_token = auth_service.create_access_token("admin") + base_client.headers = { + **base_client.headers, + "Authorization": f"Bearer {access_token}", + } + return base_client + @pytest.fixture def controller_config_path(tmpdir):