Secure users API and handle manual password recovery.

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,

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_id: UUID,
-    user_update: schemas.UserUpdate,
+        user_update: schemas.UserUpdate,
-    users_repo: UsersRepository = Depends(get_repository(UsersRepository)),
+        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:

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")

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

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):

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

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:
+    ) as async_client:
-        yield 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(test_user.username)
-    access_token = auth_service.create_access_token("admin")
+    base_client.headers = {
-    client.headers = {
+        **base_client.headers,
-        **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):