From a4092e90f6743160e279a0961b5dd276053de6da Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Thu, 5 Oct 2023 12:36:38 +0530 Subject: [PATCH 01/22] api/requirements: add `fastapi-users` package Add `fastapi-users` package with `beanie` and `oauth` tools for user management. The specific `10.4.0` version has been selected to make it compatible with `fastapi 0.68.1` package version. Signed-off-by: Jeny Sadadia --- docker/api/requirements.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/docker/api/requirements.txt b/docker/api/requirements.txt index 5ff9a01b..b00e02d3 100644 --- a/docker/api/requirements.txt +++ b/docker/api/requirements.txt @@ -10,3 +10,4 @@ motor==2.5.1 pymongo-migrate==0.11.0 pyyaml==5.3.1 fastapi-versioning==0.10.0 +fastapi-users[beanie, oauth]==10.4.0 From 018157c64f5f02cca73d4527b8b57b5b4ec49be2 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Thu, 5 Oct 2023 16:09:20 +0530 Subject: [PATCH 02/22] api.user_models: extend models from fastapi-users Add `User` model and extend user model provided by `fastapi-users`. Add an extra field `username` and create `unique` index on it. Also add field `groups` for user groups. Also add schema for user CRUD operation. Use `ModelId` as a parent class for `UserRead` to enable json encoder for ObjectId. Use `Settings` class to rename collection name for `User` model from default `User` to `user`. This change is needed to align the collection name with `Database.COLLECTIONS`. Signed-off-by: Jeny Sadadia --- api/user_models.py | 54 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 api/user_models.py diff --git a/api/user_models.py b/api/user_models.py new file mode 100644 index 00000000..53737a0e --- /dev/null +++ b/api/user_models.py @@ -0,0 +1,54 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# Copyright (C) 2023 Collabora Limited +# Author: Jeny Sadadia + +# Disable flag as user models don't require any public methods +# at the moment +# pylint: disable=too-few-public-methods + +"""User model definitions""" + +from typing import Optional +from pydantic import conlist, Field +from fastapi_users.db import BeanieBaseUser +from fastapi_users import schemas +from beanie import ( + Indexed, + Document, + PydanticObjectId, +) +from .models import DatabaseModel, UserGroup, ModelId + + +class User(BeanieBaseUser, Document, # pylint: disable=too-many-ancestors + DatabaseModel): + """API User model""" + username: Indexed(str, unique=True) + groups: conlist(UserGroup, unique_items=True) = Field( + default=[], + description="A list of groups that user belongs to" + ) + + class Settings(BeanieBaseUser.Settings): + """Configurations""" + # MongoDB collection name for model + name = "user" + + +class UserRead(schemas.BaseUser[PydanticObjectId], ModelId): + """Schema for reading a user""" + username: Indexed(str, unique=True) + groups: conlist(UserGroup, unique_items=True) + + +class UserCreate(schemas.BaseUserCreate): + """Schema for creating a user""" + username: Indexed(str, unique=True) + groups: Optional[conlist(str, unique_items=True)] + + +class UserUpdate(schemas.BaseUserUpdate): + """Schema for updating a user""" + username: Optional[Indexed(str, unique=True)] + groups: Optional[conlist(str, unique_items=True)] From 2239da5d45ffe171d0208a78697e19c483983263 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Fri, 6 Oct 2023 12:56:36 +0530 Subject: [PATCH 03/22] api.db: initialize Beanie Implement `Database.initialize_beanie` method to initialize Beanie ODM to use `fastapi-users` tools for MongoDB. Signed-off-by: Jeny Sadadia --- api/db.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/api/db.py b/api/db.py index 6b07ca15..87933e68 100644 --- a/api/db.py +++ b/api/db.py @@ -7,9 +7,11 @@ """Database abstraction""" from bson import ObjectId +from beanie import init_beanie from fastapi_pagination.ext.motor import paginate from motor import motor_asyncio -from .models import Hierarchy, Node, User, Regression, UserGroup +from .models import Hierarchy, Node, Regression, UserGroup +from .user_models import User class Database: @@ -39,6 +41,15 @@ def __init__(self, service='mongodb://db:27017', db_name='kernelci'): self._motor = motor_asyncio.AsyncIOMotorClient(service) self._db = self._motor[db_name] + async def initialize_beanie(self): + """Initialize Beanie ODM to use `fastapi-users` tools for MongoDB""" + await init_beanie( + database=self._db, + document_models=[ + User, + ], + ) + def _get_collection(self, model): col = self.COLLECTIONS[model] return self._db[col] From aade3cb948e20ab8c627db8885a0fdac67ed632c Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Fri, 6 Oct 2023 13:00:22 +0530 Subject: [PATCH 04/22] api.main: run Beanie initializer on startup In order to use `fastapi-users` package for MongoDB, initialize Beanie on app startup as per the package requirements. Signed-off-by: Jeny Sadadia --- api/main.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/api/main.py b/api/main.py index 84452307..faeb250d 100644 --- a/api/main.py +++ b/api/main.py @@ -70,6 +70,12 @@ async def create_indexes(): await db.create_indexes() +@app.on_event('startup') +async def initialize_beanie(): + """Startup event handler to initialize Beanie""" + await db.initialize_beanie() + + @app.exception_handler(ValueError) async def value_error_exception_handler(request: Request, exc: ValueError): """Global exception handler for 'ValueError'""" @@ -691,6 +697,7 @@ async def put_regression(regression_id: str, regression: Regression, on_startup=[ pubsub_startup, create_indexes, + initialize_beanie, ] ) From b53127e62ae11e251d46df5b05f8ee1974280b79 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Fri, 6 Oct 2023 14:46:25 +0530 Subject: [PATCH 05/22] api.auth: Authentication backend for user management Implement `Authentication.get_user_authentication_backend` to get authentication backend for user management. Authentication backend for `fastapi-users` is composed of two parts: Transaport and Strategy. Transport is a mechanism for token transmisson i.e. bearer or cookie. Strategy is a method to generate and secure tokens. It can be JWT, database or Redis. To accommodate user management with current API design, `Bearer` is selected as a transport method and `JWT` is selected as strategy. Signed-off-by: Jeny Sadadia --- api/auth.py | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) diff --git a/api/auth.py b/api/auth.py index c3e176d0..18c3e9c1 100644 --- a/api/auth.py +++ b/api/auth.py @@ -1,7 +1,8 @@ # SPDX-License-Identifier: LGPL-2.1-or-later # -# Copyright (C) 2021 Collabora Limited +# Copyright (C) 2021-2023 Collabora Limited # Author: Guillaume Tucker +# Author: Jeny Sadadia """User authentication utilities""" @@ -10,6 +11,11 @@ from jose import JWTError, jwt from passlib.context import CryptContext from pydantic import BaseModel, BaseSettings, Field +from fastapi_users.authentication import ( + AuthenticationBackend, + BearerTransport, + JWTStrategy, +) from .db import Database from .models import User @@ -125,3 +131,26 @@ async def validate_scopes(self, requested_scopes): if scope not in self._user_scopes: return False, scope return True, None + + def get_jwt_strategy(self) -> JWTStrategy: + """Get JWT strategy for authentication backend""" + return JWTStrategy( + secret=self._settings.secret_key, + lifetime_seconds=self._settings.access_token_expire_minutes + ) + + def get_user_authentication_backend(self): + """Authentication backend for user management + + Authentication backend for `fastapi-users` is composed of two + parts: Transaport and Strategy. + Transport is a mechanism for token transmisson i.e. bearer or cookie. + Strategy is a method to generate and secure tokens. It can be JWT, + database or Redis. + """ + bearer_transport = BearerTransport(tokenUrl="user/login") + return AuthenticationBackend( + name="jwt", + transport=bearer_transport, + get_strategy=self.get_jwt_strategy, + ) From a4228535de421a13e14a74c310489d3facf29d4f Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Fri, 6 Oct 2023 14:50:52 +0530 Subject: [PATCH 06/22] api/user_manager.py: user management logic Add `user_manager.py` to implement custom user management logic. Extend `BaseUserManager` provided by `fastapi-users` to have customized core logic for user management. Add database adapter `get_user_db` to create a link between user model from `fastapi-users` and API database configuration. Inject `UserManager` at a runtime in a database session. Add `email_sender` module to send verification and reset password token for user accounts via email. Add email templates. Signed-off-by: Jeny Sadadia --- api/email_sender.py | 69 +++++++++++ api/user_manager.py | 116 ++++++++++++++++++ docker-compose.yaml | 1 + docker/api/requirements.txt | 1 + .../email-verification-successful.jinja2 | 8 ++ templates/email-verification.jinja2 | 10 ++ templates/reset-password-successful.jinja2 | 8 ++ templates/reset-password.jinja2 | 10 ++ 8 files changed, 223 insertions(+) create mode 100644 api/email_sender.py create mode 100644 api/user_manager.py create mode 100644 templates/email-verification-successful.jinja2 create mode 100644 templates/email-verification.jinja2 create mode 100644 templates/reset-password-successful.jinja2 create mode 100644 templates/reset-password.jinja2 diff --git a/api/email_sender.py b/api/email_sender.py new file mode 100644 index 00000000..ed0bc6c7 --- /dev/null +++ b/api/email_sender.py @@ -0,0 +1,69 @@ +#!/usr/bin/env python3 +# +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# Copyright (C) 2023 Jeny Sadadia +# Author: Jeny Sadadia + +"""SMTP Email Sender module""" + +from email.mime.multipart import MIMEMultipart +import email +import email.mime.text +import smtplib +from pydantic import BaseSettings, EmailStr + + +class Settings(BaseSettings): + """Email settings""" + smtp_host: str + smtp_port: int + email_sender: EmailStr + email_password: str + + +class EmailSender: + """Class to send email report using SMTP""" + def __init__(self): + self._settings = Settings() + + def _smtp_connect(self): + """Method to create a connection with SMTP server""" + if self._settings.smtp_port == 465: + smtp = smtplib.SMTP_SSL(self._settings.smtp_host, + self._settings.smtp_port) + else: + smtp = smtplib.SMTP(self._settings.smtp_host, + self._settings.smtp_port) + smtp.starttls() + smtp.login(self._settings.email_sender, + self._settings.email_password) + return smtp + + def _create_email(self, email_subject, email_content, email_recipient): + """Method to create an email message from email subject, contect, + sender, and receiver""" + email_msg = MIMEMultipart() + email_text = email.mime.text.MIMEText(email_content, "plain", "utf-8") + email_text.replace_header('Content-Transfer-Encoding', 'quopri') + email_text.set_payload(email_content, 'utf-8') + email_msg.attach(email_text) + email_msg['To'] = email_recipient + email_msg['From'] = self._settings.email_sender + email_msg['Subject'] = email_subject + return email_msg + + def _send_email(self, email_msg): + """Method to send an email message using SMTP""" + smtp = self._smtp_connect() + if smtp: + smtp.send_message(email_msg) + smtp.quit() + + def create_and_send_email(self, email_subject, email_content, + email_recipient): + """Method to create and send email""" + email_msg = self._create_email( + email_subject, email_content, email_recipient + ) + self._send_email(email_msg) diff --git a/api/user_manager.py b/api/user_manager.py new file mode 100644 index 00000000..fbea3adb --- /dev/null +++ b/api/user_manager.py @@ -0,0 +1,116 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# Copyright (C) 2023 Collabora Limited +# Author: Jeny Sadadia + +"""User Manager""" + +from typing import Optional, Any, Dict +from fastapi_users import BaseUserManager +from fastapi_users.db import ( + BaseUserDatabase, + BeanieUserDatabase, + ObjectIDIDMixin, +) +from fastapi_users.password import PasswordHelperProtocol +from beanie import PydanticObjectId +from fastapi import Depends, Request +import jinja2 +from .user_models import User +from .auth import Settings +from .email_sender import EmailSender + + +class UserManager(ObjectIDIDMixin, BaseUserManager[User, PydanticObjectId]): + """User management logic""" + settings = Settings() + reset_password_token_secret = settings.secret_key + verification_token_secret = settings.secret_key + + def __init__(self, user_db: BaseUserDatabase[User, PydanticObjectId], + password_helper: PasswordHelperProtocol | None = None): + self._email_sender = EmailSender() + self._template_env = jinja2.Environment( + loader=jinja2.FileSystemLoader( + "./templates/") + ) + super().__init__(user_db, password_helper) + + async def on_after_register(self, user: User, + request: Optional[Request] = None): + """Handler to execute after successful user registration""" + print(f"User {user.id} has registered.") + + async def on_after_request_verify(self, user: User, token: str, + request: Optional[Request] = None): + """Handler to execute after successful verification request""" + template = self._template_env.get_template("email-verification.jinja2") + subject = "Email verification Token for KernelCI API account" + content = template.render( + username=user.username, token=token + ) + self._email_sender.create_and_send_email(subject, content, user.email) + + async def on_after_verify(self, user: User, + request: Optional[Request] = None): + """Handler to execute after successful user verification""" + print(f"Verification successful for user {user.id}") + template = self._template_env.get_template( + "email-verification-successful.jinja2") + subject = "Email verification successful for KernelCI API account" + content = template.render( + username=user.username, + ) + self._email_sender.create_and_send_email(subject, content, user.email) + + async def on_after_login(self, user: User, + request: Optional[Request] = None): + """Handler to execute after successful user login""" + print(f"User {user.id} logged in.") + + async def on_after_forgot_password(self, user: User, token: str, + request: Optional[Request] = None): + """Handler to execute after successful forgot password request""" + template = self._template_env.get_template("reset-password.jinja2") + subject = "Reset Password Token for KernelCI API account" + content = template.render( + username=user.username, token=token + ) + self._email_sender.create_and_send_email(subject, content, user.email) + + async def on_after_reset_password(self, user: User, + request: Optional[Request] = None): + """Handler to execute after successful password reset""" + print(f"User {user.id} has reset their password.") + template = self._template_env.get_template( + "reset-password-successful.jinja2") + subject = "Password reset successful for KernelCI API account" + content = template.render( + username=user.username, + ) + self._email_sender.create_and_send_email(subject, content, user.email) + + async def on_after_update(self, user: User, update_dict: Dict[str, Any], + request: Optional[Request] = None): + """Handler to execute after successful user update""" + print(f"User {user.id} has been updated with {update_dict}.") + + async def on_before_delete(self, user: User, + request: Optional[Request] = None): + """Handler to execute before user delete.""" + print(f"User {user.id} is going to be deleted.") + + async def on_after_delete(self, user: User, + request: Optional[Request] = None): + """Handler to execute after user delete.""" + print(f"User {user.id} is successfully deleted") + + +async def get_user_db(): + """Database adapter for fastapi-users""" + yield BeanieUserDatabase(User) + + +async def get_user_manager(user_db: BeanieUserDatabase = Depends(get_user_db)): + """Get user manager""" + yield UserManager(user_db) diff --git a/docker-compose.yaml b/docker-compose.yaml index 76c50614..ab825480 100644 --- a/docker-compose.yaml +++ b/docker-compose.yaml @@ -16,6 +16,7 @@ services: - './api:/home/kernelci/api' - './tests:/home/kernelci/tests' - './migrations:/home/kernelci/migrations' + - './templates:/home/kernelci/templates' ports: - '${API_HOST_PORT:-8001}:8000' env_file: diff --git a/docker/api/requirements.txt b/docker/api/requirements.txt index b00e02d3..2ef5c66c 100644 --- a/docker/api/requirements.txt +++ b/docker/api/requirements.txt @@ -11,3 +11,4 @@ pymongo-migrate==0.11.0 pyyaml==5.3.1 fastapi-versioning==0.10.0 fastapi-users[beanie, oauth]==10.4.0 +MarkupSafe==2.0.1 diff --git a/templates/email-verification-successful.jinja2 b/templates/email-verification-successful.jinja2 new file mode 100644 index 00000000..01a04e3f --- /dev/null +++ b/templates/email-verification-successful.jinja2 @@ -0,0 +1,8 @@ +{# SPDX-License-Identifier: LGPL-2.1-or-later -#} + +Hello {{ username }}, + +Your email address has been verified successfully. + +Thanks, +KernelCI SysAdmin diff --git a/templates/email-verification.jinja2 b/templates/email-verification.jinja2 new file mode 100644 index 00000000..053a1f5c --- /dev/null +++ b/templates/email-verification.jinja2 @@ -0,0 +1,10 @@ +{# SPDX-License-Identifier: LGPL-2.1-or-later -#} + +Hello {{ username }}, + +The token for verifying your email address for KernelCI API account is: + + {{ token }} + +Thanks, +KernelCI SysAdmin diff --git a/templates/reset-password-successful.jinja2 b/templates/reset-password-successful.jinja2 new file mode 100644 index 00000000..71efe84d --- /dev/null +++ b/templates/reset-password-successful.jinja2 @@ -0,0 +1,8 @@ +{# SPDX-License-Identifier: LGPL-2.1-or-later -#} + +Hello {{ username }}, + +Your password reset is successful. + +Thanks, +KernelCI SysAdmin diff --git a/templates/reset-password.jinja2 b/templates/reset-password.jinja2 new file mode 100644 index 00000000..04cd35c4 --- /dev/null +++ b/templates/reset-password.jinja2 @@ -0,0 +1,10 @@ +{# SPDX-License-Identifier: LGPL-2.1-or-later -#} + +Hello {{ username }}, + +The token to reset your password for KernelCI API account is: + + {{ token }} + +Thanks, +KernelCI SysAdmin From c738197df632297afca6dbf31f63265e5204ad72 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Fri, 6 Oct 2023 15:39:56 +0530 Subject: [PATCH 07/22] api.main: create an instance of FastAPIUsers Create an instance of `FastAPIUsers` to bind `UserManager` with authentication backend. Signed-off-by: Jeny Sadadia --- api/main.py | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/api/main.py b/api/main.py index faeb250d..86655b51 100644 --- a/api/main.py +++ b/api/main.py @@ -29,13 +29,14 @@ from fastapi_versioning import VersionedFastAPI from bson import ObjectId, errors from pymongo.errors import DuplicateKeyError +from fastapi_users import FastAPIUsers +from beanie import PydanticObjectId from .auth import Authentication, Token from .db import Database from .models import ( Node, Hierarchy, Regression, - User, UserGroup, UserProfile, Password, @@ -43,6 +44,10 @@ ) from .paginator_models import PageModel from .pubsub import PubSub, Subscription +from .user_manager import get_user_manager +from .user_models import ( + User, +) # List of all the supported API versions. This is a placeholder until the API # actually supports multiple versions with different sets of endpoints and @@ -56,6 +61,12 @@ "users": "Regular users"}) pubsub = None # pylint: disable=invalid-name +auth_backend = auth.get_user_authentication_backend() +fastapi_users_instance = FastAPIUsers[User, PydanticObjectId]( + get_user_manager, + [auth_backend], +) + @app.on_event('startup') async def pubsub_startup(): From ffbf21259efda6c786b8a8f2d39b64228e2216b6 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Fri, 6 Oct 2023 15:44:01 +0530 Subject: [PATCH 08/22] api.main: include routers provided by `fastapi-users` Register different routers provided by the package to our application. Signed-off-by: Jeny Sadadia --- api/main.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/api/main.py b/api/main.py index 86655b51..e7ff035d 100644 --- a/api/main.py +++ b/api/main.py @@ -47,6 +47,9 @@ from .user_manager import get_user_manager from .user_models import ( User, + UserRead, + UserCreate, + UserUpdate, ) # List of all the supported API versions. This is a placeholder until the API @@ -699,6 +702,38 @@ async def put_regression(regression_id: str, regression: Regression, return obj +# ----------------------------------------------------------------------------- +# Users + +app.include_router( + fastapi_users_instance.get_auth_router(auth_backend, + requires_verification=True), + prefix="/user", + tags=["user"] +) +app.include_router( + fastapi_users_instance.get_register_router(UserRead, UserCreate), + prefix="/user", + tags=["user"], +) +app.include_router( + fastapi_users_instance.get_reset_password_router(), + prefix="/user", + tags=["user"], +) +app.include_router( + fastapi_users_instance.get_verify_router(UserRead), + prefix="/user", + tags=["user"], +) +app.include_router( + fastapi_users_instance.get_users_router(UserRead, UserUpdate, + requires_verification=True), + prefix="/user", + tags=["user"], +) + + app = VersionedFastAPI( app, version_format='{major}', From c7f2721a0757204c1e36441d6f7f11195051242e Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Fri, 6 Oct 2023 21:52:47 +0530 Subject: [PATCH 09/22] workflows/main.yaml: run pylint on user related files Run pylint on newly created user related files i.e. `api.user_manager` and `api.user_models` in Github workflows. Signed-off-by: Jeny Sadadia --- .github/workflows/main.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 6d8e751d..ef256e64 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -50,6 +50,8 @@ jobs: pylint api.main pylint api.models pylint api.pubsub + pylint api.user_manager + pylint api.user_models pylint tests/unit_tests - name: Export environment variables From e6110b94d478a5b3aca5dfb2a33f43f1e36d7753 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Sat, 7 Oct 2023 12:32:37 +0530 Subject: [PATCH 10/22] api.main: update GET `/users` endpoint Update GET users endpoint as per `User` model defined in `users_model.py`. Also drop `/users/profile` endpoint as the updated user schema doesn't have `profile` field. Signed-off-by: Jeny Sadadia --- api/main.py | 28 +++------------------------- 1 file changed, 3 insertions(+), 25 deletions(-) diff --git a/api/main.py b/api/main.py index e7ff035d..c42ac696 100644 --- a/api/main.py +++ b/api/main.py @@ -204,32 +204,10 @@ async def post_user( return obj -@app.get('/users/profile', response_model=PageModel, - response_model_include={"items": {"__all__": {"profile": { - "username", "groups", "email"}}}, - "total": {"__all__"}, - "limit": {"__all__"}, - "offset": {"__all__"}, - }) -async def get_users_profile(request: Request): - """Get profile of all the users if no request parameters have passed. - Get the matching user profile otherwise.""" - query_params = dict(request.query_params) - # Drop pagination parameters from query as they're already in arguments - for pg_key in ['limit', 'offset']: - query_params.pop(pg_key, None) - paginated_resp = await db.find_by_attributes(User, query_params) - paginated_resp.items = serialize_paginated_data( - User, paginated_resp.items) - return paginated_resp - - @app.get('/users', response_model=PageModel, - response_model_exclude={"items": {"__all__": {"profile": { - "hashed_password"}}}}) -async def get_users( - request: Request, - current_user: User = Security(get_user, scopes=["admin"])): + response_model_exclude={"items": {"__all__": { + "hashed_password"}}}) +async def get_users(request: Request): """Get all the users if no request parameters have passed. Get the matching users otherwise.""" query_params = dict(request.query_params) From 70961f71aa500d3c83a8d48832e698ab638d8c87 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Sat, 7 Oct 2023 12:36:29 +0530 Subject: [PATCH 11/22] api.main: drop endpoint to get user by id `fastapi-users` provides inbuilt router `/users/` for getting user matching ID. Hence, deleting `get_user_by_id` handler that serves the similar purpose. Signed-off-by: Jeny Sadadia --- api/main.py | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/api/main.py b/api/main.py index c42ac696..715429da 100644 --- a/api/main.py +++ b/api/main.py @@ -220,16 +220,6 @@ async def get_users(request: Request): return paginated_resp -@app.get('/user/{user_id}', response_model=User, - response_model_by_alias=False, - response_model_exclude={"profile": {"hashed_password"}}) -async def get_user_by_id( - user_id: str, - current_user: User = Security(get_user, scopes=["admin"])): - """Get user matching provided user id""" - return await db.find_by_id(User, user_id) - - @app.put('/user/profile/{username}', response_model=User, response_model_include={"profile"}, response_model_by_alias=False) From ead8264b71e8b9fd16a67f34ee347f62677834b3 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Sat, 7 Oct 2023 18:15:55 +0530 Subject: [PATCH 12/22] api.main: custom user registration router Implement custom route for user registration to ensure unique username. The route will also convert user group names to `UserGroup` objects. Then the router will call endpoint of `register_router` provided by `fastapi-users` to create a user account. Drop old POST `/user/` handler for user account creation to use this route instead. Signed-off-by: Jeny Sadadia --- api/main.py | 87 ++++++++++++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 41 deletions(-) diff --git a/api/main.py b/api/main.py index 715429da..887eee7b 100644 --- a/api/main.py +++ b/api/main.py @@ -30,6 +30,7 @@ from bson import ObjectId, errors from pymongo.errors import DuplicateKeyError from fastapi_users import FastAPIUsers +from fastapi_users.db import BeanieUserDatabase from beanie import PydanticObjectId from .auth import Authentication, Token from .db import Database @@ -51,6 +52,8 @@ UserCreate, UserUpdate, ) +from .user_manager import UserManager + # List of all the supported API versions. This is a placeholder until the API # actually supports multiple versions with different sets of endpoints and @@ -168,42 +171,6 @@ async def authorize_user(node_id: str, user: User = Depends(get_current_user)): return user -@app.post('/user/{username}', response_model=User, - response_model_by_alias=False) -async def post_user( - username: str, password: Password, email: str, - groups: List[str] = Query([]), - current_user: User = Security(get_user, scopes=["admin"])): - """Create new user""" - try: - hashed_password = auth.get_password_hash( - password.password.get_secret_value()) - group_obj = [] - if groups: - for group_name in groups: - group = await db.find_one(UserGroup, name=group_name) - if not group: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"User group does not exist with name: \ -{group_name}") - group_obj.append(group) - profile = UserProfile( - username=username, - hashed_password=hashed_password, - groups=group_obj, - email=email) - obj = await db.create(User(profile=profile)) - except DuplicateKeyError as error: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"{username} is already taken. Try with different username." - ) from error - await pubsub.publish_cloudevent('user', {'op': 'created', - 'id': str(obj.id)}) - return obj - - @app.get('/users', response_model=PageModel, response_model_exclude={"items": {"__all__": { "hashed_password"}}}) @@ -679,11 +646,49 @@ async def put_regression(regression_id: str, regression: Regression, prefix="/user", tags=["user"] ) -app.include_router( - fastapi_users_instance.get_register_router(UserRead, UserCreate), - prefix="/user", - tags=["user"], -) + +register_router = fastapi_users_instance.get_register_router( + UserRead, UserCreate) + + +@app.post("/user/register", response_model=UserRead, tags=["user"], + response_model_by_alias=False) +async def register(request: Request, user: UserCreate): + """User registration route + + Custom user registration router to ensure unique username. + `user` from request has a list of user group name strings. + This handler will convert them to `UserGroup` objects and + insert user object to database. + """ + existing_user = await db.find_one(User, username=user.username) + if existing_user: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Username already exists", + ) + groups = [] + if user.groups: + for group_name in user.groups: + group = await db.find_one(UserGroup, name=group_name) + if not group: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"User group does not exist with name: \ + {group_name}") + groups.append(group) + user.groups = groups + created_user = await register_router.routes[0].endpoint( + request, user, UserManager(BeanieUserDatabase(User))) + # Update user to be an admin user explicitly if requested as + # `fastapi-users` register route does not allow it + if user.is_superuser: + user_from_id = await db.find_by_id(User, created_user.id) + user_from_id.is_superuser = True + created_user = await db.update(user_from_id) + return created_user + + app.include_router( fastapi_users_instance.get_reset_password_router(), prefix="/user", From 553823a2efefbf91ebb740b0aaf7ecbe3ac7cd7d Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Mon, 9 Oct 2023 18:16:56 +0530 Subject: [PATCH 13/22] api/user_manager: overload `BaseUserManager.authenticate` `/login` endpoint has a field `username` that actually accepts user email address for login. The reason is `fastapi-users` uses `OAuth2PasswordRequestForm` to get login information under the hood that has `username` field. `login` handler uses `BaseUserManager.authenticate` method to authenticate user. Overload the authenticate method to use `username` for authentication instead of `email`. Signed-off-by: Jeny Sadadia --- api/user_manager.py | 34 ++++++++++++++++++++++++++++++++-- 1 file changed, 32 insertions(+), 2 deletions(-) diff --git a/api/user_manager.py b/api/user_manager.py index fbea3adb..19f0766e 100644 --- a/api/user_manager.py +++ b/api/user_manager.py @@ -6,7 +6,9 @@ """User Manager""" from typing import Optional, Any, Dict -from fastapi_users import BaseUserManager +from fastapi import Depends, Request +from fastapi.security import OAuth2PasswordRequestForm +from fastapi_users import BaseUserManager, exceptions from fastapi_users.db import ( BaseUserDatabase, BeanieUserDatabase, @@ -14,7 +16,6 @@ ) from fastapi_users.password import PasswordHelperProtocol from beanie import PydanticObjectId -from fastapi import Depends, Request import jinja2 from .user_models import User from .auth import Settings @@ -105,6 +106,35 @@ async def on_after_delete(self, user: User, """Handler to execute after user delete.""" print(f"User {user.id} is successfully deleted") + async def authenticate( + self, credentials: OAuth2PasswordRequestForm + ) -> User | None: + """ + Overload user authentication method `BaseUserManager.authenticate`. + This is to fix login endpoint to receive `username` instead of `email`. + """ + try: + user = await User.find_one(User.username == credentials.username) + except exceptions.UserNotExists: + # Run the hasher to mitigate timing attack + # Inspired from Django: https://code.djangoproject.com/ticket/20760 + self.password_helper.hash(credentials.password) + return None + + verified, updated_password_hash = \ + self.password_helper.verify_and_update( + credentials.password, user.hashed_password + ) + if not verified: + return None + # Update password hash to a more robust one if needed + if updated_password_hash is not None: + await self.user_db.update( + user, {"hashed_password": updated_password_hash} + ) + + return user + async def get_user_db(): """Database adapter for fastapi-users""" From b77c0e97f4a6e799b47088ed825f92301347cf5e Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Tue, 10 Oct 2023 12:11:41 +0530 Subject: [PATCH 14/22] api.main: get current active user using `FastAPIUsers` instance Use dependency callable `FastAPIUsers.current_user` for getting current active user for authenticated routes. Use it for `/whoami`, POST `/node`, pubsub, and regression related endpoints. Signed-off-by: Jeny Sadadia --- api/main.py | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/api/main.py b/api/main.py index 887eee7b..f081b528 100644 --- a/api/main.py +++ b/api/main.py @@ -113,6 +113,8 @@ async def invalid_id_exception_handler( content={"detail": str(exc)}, ) +current_active_user = fastapi_users_instance.current_user(active=True) + async def get_current_user( security_scopes: SecurityScopes, @@ -354,7 +356,7 @@ async def login_for_access_token( @app.get('/whoami', response_model=User, response_model_by_alias=False) -async def whoami(current_user: User = Depends(get_user)): +async def whoami(current_user: User = Depends(current_active_user)): """Get current user information""" return current_user @@ -499,7 +501,8 @@ async def _verify_user_group_existence(user_groups: List[str]): @app.post('/node', response_model=Node, response_model_by_alias=False) -async def post_node(node: Node, current_user: str = Depends(get_user)): +async def post_node(node: Node, + current_user: str = Depends(current_active_user)): """Create a new node""" if node.parent: parent = await db.find_by_id(Node, node.parent) @@ -510,7 +513,7 @@ async def post_node(node: Node, current_user: str = Depends(get_user)): ) await _verify_user_group_existence(node.user_groups) - node.owner = current_user.profile.username + node.owner = current_user.username obj = await db.create(node) data = _get_node_event_data('created', obj) await pubsub.publish_cloudevent('node', data) @@ -572,13 +575,13 @@ async def put_nodes( # Pub/Sub @app.post('/subscribe/{channel}', response_model=Subscription) -async def subscribe(channel: str, user: User = Depends(get_user)): +async def subscribe(channel: str, user: User = Depends(current_active_user)): """Subscribe handler for Pub/Sub channel""" return await pubsub.subscribe(channel) @app.post('/unsubscribe/{sub_id}') -async def unsubscribe(sub_id: int, user: User = Depends(get_user)): +async def unsubscribe(sub_id: int, user: User = Depends(current_active_user)): """Unsubscribe handler for Pub/Sub channel""" try: await pubsub.unsubscribe(sub_id) @@ -590,7 +593,7 @@ async def unsubscribe(sub_id: int, user: User = Depends(get_user)): @app.get('/listen/{sub_id}') -async def listen(sub_id: int, user: User = Depends(get_user)): +async def listen(sub_id: int, user: User = Depends(current_active_user)): """Listen messages from a subscribed Pub/Sub channel""" try: return await pubsub.listen(sub_id) @@ -602,7 +605,8 @@ async def listen(sub_id: int, user: User = Depends(get_user)): @app.post('/publish/{channel}') -async def publish(raw: dict, channel: str, user: User = Depends(get_user)): +async def publish(raw: dict, channel: str, + user: User = Depends(current_active_user)): """Publish a message on the provided Pub/Sub channel""" attributes = dict(raw) data = attributes.pop('data') @@ -615,7 +619,7 @@ async def publish(raw: dict, channel: str, user: User = Depends(get_user)): @app.post('/regression', response_model=Regression, response_model_by_alias=False) async def post_regression(regression: Regression, - token: str = Depends(get_user)): + user: str = Depends(current_active_user)): """Create a new regression""" obj = await db.create(regression) operation = 'created' @@ -627,7 +631,7 @@ async def post_regression(regression: Regression, @app.put('/regression/{regression_id}', response_model=Regression, response_model_by_alias=False) async def put_regression(regression_id: str, regression: Regression, - token: str = Depends(get_user)): + user: str = Depends(current_active_user)): """Update an already added regression""" regression.id = ObjectId(regression_id) obj = await db.update(regression) From 966fbeadd5b1874921f16e0fca479846d6baee7d Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Tue, 10 Oct 2023 15:49:03 +0530 Subject: [PATCH 15/22] api.main: get current super user using `FastAPIUsers` instance Use dependency callable `FastAPIUsers.current_user` for getting current active super user for authenticated routes. Use it for POST `/user/register` endpoint which needs a superuser for creating other regular users. Signed-off-by: Jeny Sadadia --- api/main.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/api/main.py b/api/main.py index f081b528..0ce2e93d 100644 --- a/api/main.py +++ b/api/main.py @@ -114,6 +114,8 @@ async def invalid_id_exception_handler( ) current_active_user = fastapi_users_instance.current_user(active=True) +current_active_superuser = fastapi_users_instance.current_user(active=True, + superuser=True) async def get_current_user( @@ -277,7 +279,7 @@ async def put_user( @app.post('/group', response_model=UserGroup, response_model_by_alias=False) async def post_user_group( group: UserGroup, - current_user: User = Security(get_user, scopes=["admin"])): + current_user: User = Depends(current_active_superuser)): """Create new user group""" try: obj = await db.create(group) @@ -657,7 +659,8 @@ async def put_regression(regression_id: str, regression: Regression, @app.post("/user/register", response_model=UserRead, tags=["user"], response_model_by_alias=False) -async def register(request: Request, user: UserCreate): +async def register(request: Request, user: UserCreate, + current_user: str = Depends(current_active_superuser)): """User registration route Custom user registration router to ensure unique username. From 04432bf062c1ad698060527d4884e9988aacb62d Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Tue, 10 Oct 2023 15:54:38 +0530 Subject: [PATCH 16/22] api.admin: use `user_models.User` Use latest `User` model schema implemented using `fastapi-users` package. Initialize Beanie as per the package requirement before inserting document to database. Signed-off-by: Jeny Sadadia --- api/admin.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/api/admin.py b/api/admin.py index fbfd0262..5a9b2b50 100644 --- a/api/admin.py +++ b/api/admin.py @@ -17,7 +17,8 @@ from .auth import Authentication from .db import Database -from .models import User, UserGroup, UserProfile +from .models import UserGroup +from .user_models import User async def setup_admin_group(db, admin_group): @@ -42,19 +43,18 @@ async def setup_admin_user(db, username, email, admin_group): return None hashed_password = Authentication.get_password_hash(password) print(f"Creating {username} user...") - profile = UserProfile( + return await db.create(User( username=username, hashed_password=hashed_password, email=email, - groups=[admin_group] - ) - return await db.create(User( - profile=profile + groups=[admin_group], + is_superuser=1 )) async def main(args): db = Database(args.mongo, args.database) + await db.initialize_beanie() group = await setup_admin_group(db, args.admin_group) user = await setup_admin_user(db, args.username, args.email, group) return True From 0c69ff6a9aa55639e8c645a7176c63d82c78c604 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Fri, 13 Oct 2023 18:11:16 +0530 Subject: [PATCH 17/22] api.main: update/add user management routes Overload routes for updating users to ensure unique usernames. It is also used to get user group names as a list of strings and convert it to `UserGroup` object to insert it into DB. Removed all former user routers and update handlers to use `fastapi-users` instead. Drop `models.User` and `models.UserProfile` to use latest user model from `user_models.py`. Signed-off-by: Jeny Sadadia --- api/main.py | 468 ++++++++++++++++++++------------------------------ api/models.py | 32 ---- 2 files changed, 190 insertions(+), 310 deletions(-) diff --git a/api/main.py b/api/main.py index 0ce2e93d..01860ac2 100644 --- a/api/main.py +++ b/api/main.py @@ -17,14 +17,8 @@ HTTPException, status, Request, - Security, - Query, ) from fastapi.responses import JSONResponse, PlainTextResponse -from fastapi.security import ( - OAuth2PasswordRequestForm, - SecurityScopes -) from fastapi_pagination import add_pagination from fastapi_versioning import VersionedFastAPI from bson import ObjectId, errors @@ -32,15 +26,13 @@ from fastapi_users import FastAPIUsers from fastapi_users.db import BeanieUserDatabase from beanie import PydanticObjectId -from .auth import Authentication, Token +from .auth import Authentication from .db import Database from .models import ( Node, Hierarchy, Regression, UserGroup, - UserProfile, - Password, get_model_from_kind ) from .paginator_models import PageModel @@ -113,49 +105,195 @@ async def invalid_id_exception_handler( content={"detail": str(exc)}, ) -current_active_user = fastapi_users_instance.current_user(active=True) -current_active_superuser = fastapi_users_instance.current_user(active=True, - superuser=True) +@app.get('/') +async def root(): + """Root endpoint handler""" + return {"message": "KernelCI API"} + +# ----------------------------------------------------------------------------- +# Users + + +def get_current_user(user: User = Depends( + fastapi_users_instance.current_user(active=True))): + """Get current active user""" + return user + + +def get_current_superuser(user: User = Depends( + fastapi_users_instance.current_user(active=True, superuser=True))): + """Get current active superuser""" + return user + + +app.include_router( + fastapi_users_instance.get_auth_router(auth_backend, + requires_verification=True), + prefix="/user", + tags=["user"] +) + +register_router = fastapi_users_instance.get_register_router( + UserRead, UserCreate) -async def get_current_user( - security_scopes: SecurityScopes, - token: str = Depends(auth.oauth2_scheme)): - """Return the user if authenticated successfully based on the provided - token""" - if security_scopes.scopes: - authenticate_value = f'Bearer scope="{security_scopes.scope_str}"' - else: - authenticate_value = "Bearer" +@app.post("/user/register", response_model=UserRead, tags=["user"], + response_model_by_alias=False) +async def register(request: Request, user: UserCreate, + current_user: User = Depends(get_current_superuser)): + """User registration route - if token == 'None': + Custom user registration router to ensure unique username. + `user` from request has a list of user group name strings. + This handler will convert them to `UserGroup` objects and + insert user object to database. + """ + existing_user = await db.find_one(User, username=user.username) + if existing_user: raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Missing credentials", - headers={"WWW-Authenticate": authenticate_value}, + status_code=status.HTTP_400_BAD_REQUEST, + detail="Username already exists", ) - user, message = await auth.get_current_user(token, security_scopes.scopes) - if user is None: + groups = [] + if user.groups: + for group_name in user.groups: + group = await db.find_one(UserGroup, name=group_name) + if not group: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"User group does not exist with name: \ + {group_name}") + groups.append(group) + user.groups = groups + created_user = await register_router.routes[0].endpoint( + request, user, UserManager(BeanieUserDatabase(User))) + # Update user to be an admin user explicitly if requested as + # `fastapi-users` register route does not allow it + if user.is_superuser: + user_from_id = await db.find_by_id(User, created_user.id) + user_from_id.is_superuser = True + created_user = await db.update(user_from_id) + return created_user + + +app.include_router( + fastapi_users_instance.get_reset_password_router(), + prefix="/user", + tags=["user"], +) +app.include_router( + fastapi_users_instance.get_verify_router(UserRead), + prefix="/user", + tags=["user"], +) + +users_router = fastapi_users_instance.get_users_router( + UserRead, UserUpdate, requires_verification=True) + +app.add_api_route( + path="/whoami", + tags=["user"], + methods=["GET"], + description="Get current user information", + endpoint=users_router.routes[0].endpoint) +app.add_api_route( + path="/user/{id}", + tags=["user"], + methods=["GET"], + description="Get user information by ID", + endpoint=users_router.routes[2].endpoint) +app.add_api_route( + path="/user/{id}", + tags=["user"], + methods=["DELETE"], + description="Delete user by ID", + dependencies=[Depends(get_current_superuser)], + endpoint=users_router.routes[4].endpoint) + + +@app.patch("/user/me", response_model=UserRead, tags=["user"], + response_model_by_alias=False) +async def update_me(request: Request, user: UserUpdate, + current_user: User = Depends(get_current_user)): + """User update route + + Custom user update router handler will only allow users to update + its own profile. Adding itself to 'admin' group is not allowed. + """ + existing_user = await db.find_one(User, username=user.username) + if existing_user: raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail=message, - headers={"WWW-Authenticate": authenticate_value}, + status_code=status.HTTP_400_BAD_REQUEST, + detail="Username already exists", ) - return user + groups = [] + if user.groups: + for group_name in user.groups: + if group_name == "admin": + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Unauthorized to add user to 'admin' group") + group = await db.find_one(UserGroup, name=group_name) + if not group: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"User group does not exist with name: \ + {group_name}") + groups.append(group) + if groups: + user.groups = groups + return await users_router.routes[1].endpoint( + request, user, current_user, UserManager(BeanieUserDatabase(User))) -async def get_user(user: User = Depends(get_current_user)): - """Return the user if active and authenticated""" - if not user.active: - raise HTTPException(status_code=400, detail="Inactive user") - return user +@app.patch("/user/{user_id}", response_model=UserRead, tags=["user"], + response_model_by_alias=False) +async def update_user(user_id: str, request: Request, user: UserUpdate, + current_user: User = Depends(get_current_superuser)): + """Router to allow admin users to update other user account""" + + user_from_id = await db.find_by_id(User, user_id) + if not user_from_id: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"User not found with id: {user_id}", + ) + existing_user = await db.find_one(User, username=user.username) + if existing_user: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Username already exists", + ) -async def authorize_user(node_id: str, user: User = Depends(get_current_user)): + groups = [] + if user.groups: + for group_name in user.groups: + group = await db.find_one(UserGroup, name=group_name) + if not group: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail=f"User group does not exist with name: \ + {group_name}") + groups.append(group) + if groups: + user.groups = groups + + updated_user = await users_router.routes[3].endpoint( + user, request, user_from_id, UserManager(BeanieUserDatabase(User))) + # Update user to be an admin user explicitly if requested as + # `fastapi-users` user update route does not allow it + if user.is_superuser: + user_from_id = await db.find_by_id(User, updated_user.id) + user_from_id.is_superuser = True + updated_user = await db.update(user_from_id) + return updated_user + + +async def authorize_user(node_id: str, + user: User = Depends(get_current_user)): """Return the user if active, authenticated, and authorized""" - if not user.active: - raise HTTPException(status_code=400, detail="Inactive user") # Only the user that created the node or any other user from the permitted # user groups will be allowed to update the node @@ -165,9 +303,9 @@ async def authorize_user(node_id: str, user: User = Depends(get_current_user)): status_code=status.HTTP_404_NOT_FOUND, detail=f"Node not found with id: {node_id}" ) - if not user.profile.username == node_from_id.owner: + if not user.username == node_from_id.owner: if not any(group.name in node_from_id.user_groups - for group in user.profile.groups): + for group in user.groups): raise HTTPException( status_code=status.HTTP_401_UNAUTHORIZED, detail="Unauthorized to complete the operation" @@ -175,7 +313,7 @@ async def authorize_user(node_id: str, user: User = Depends(get_current_user)): return user -@app.get('/users', response_model=PageModel, +@app.get('/users', response_model=PageModel, tags=["user"], response_model_exclude={"items": {"__all__": { "hashed_password"}}}) async def get_users(request: Request): @@ -191,95 +329,14 @@ async def get_users(request: Request): return paginated_resp -@app.put('/user/profile/{username}', response_model=User, - response_model_include={"profile"}, - response_model_by_alias=False) -async def put_user_profile( - username: str, - password: Password, - email: str = None, - groups: List[str] = Query([]), - current_user: User = Depends(get_user)): - """Update own user profile - The handler will only allow users to update its own profile. - Adding itself to 'admin' group is not allowed.""" - if str(current_user.profile.username) != username: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="Unauthorized to update user with provided username") - - hashed_password = auth.get_password_hash( - password.password.get_secret_value()) - group_obj = [] - if groups: - for group_name in groups: - group = await db.find_one(UserGroup, name=group_name) - if not group: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"User group does not exist with name: \ -{group_name}") - if group_name == 'admin': - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="Unauthorized to add user to 'admin' group") - group_obj.append(group) - obj = await db.update(User( - id=current_user.id, - profile=UserProfile( - username=username, - hashed_password=hashed_password, - email=email if email else current_user.profile.email, - groups=group_obj if group_obj else current_user.profile.groups - ))) - await pubsub.publish_cloudevent('user', {'op': 'updated', - 'id': str(obj.id)}) - return obj - - -@app.put('/user/{username}', response_model=User, - response_model_by_alias=False) -async def put_user( - username: str, - email: str = None, - groups: List[str] = Query([]), - current_user: User = Security(get_user, scopes=["admin"])): - """Update user model - Allow admin users to update all user fields except password""" - user = await db.find_one_by_attributes( - User, {'profile.username': username}) - if not user: - raise HTTPException( - status_code=status.HTTP_404_NOT_FOUND, - detail=f"User not found with username: {username}" - ) - group_obj = [] - if groups: - for group_name in groups: - group = await db.find_one(UserGroup, name=group_name) - if not group: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"User group does not exist with name: \ -{group_name}") - group_obj.append(group) - obj = await db.update(User( - id=user.id, - profile=UserProfile( - username=username, - hashed_password=user.profile.hashed_password, - email=email if email else user.profile.email, - groups=group_obj if group_obj else user.profile.groups - ))) - await pubsub.publish_cloudevent('user', {'op': 'updated', - 'id': str(obj.id)}) - return obj +# ----------------------------------------------------------------------------- +# User groups @app.post('/group', response_model=UserGroup, response_model_by_alias=False) async def post_user_group( group: UserGroup, - current_user: User = Depends(current_active_superuser)): + current_user: User = Depends(get_current_superuser)): """Create new user group""" try: obj = await db.create(group) @@ -317,80 +374,6 @@ async def get_group(group_id: str): return await db.find_by_id(UserGroup, group_id) -@app.get('/') -async def root(): - """Root endpoint handler""" - return {"message": "KernelCI API"} - - -@app.post('/token', response_model=Token) -async def login_for_access_token( - form_data: OAuth2PasswordRequestForm = Depends()): - """Get a bearer token for an authenticated user""" - user = await auth.authenticate_user(form_data.username, form_data.password) - if not user: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Incorrect username or password", - headers={"WWW-Authenticate": "Bearer"}, - ) - - is_valid, scope = await auth.validate_scopes(form_data.scopes) - if not is_valid: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Invalid scope: {scope}" - ) - - if 'admin' in form_data.scopes: - if not user.groups or not any( - group.name == 'admin' for group in user.groups): - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="Not allowed to use admin scope" - ) - - access_token = auth.create_access_token(data={ - "sub": user.username, - "scopes": form_data.scopes} - ) - return {"access_token": access_token, "token_type": "bearer"} - - -@app.get('/whoami', response_model=User, response_model_by_alias=False) -async def whoami(current_user: User = Depends(current_active_user)): - """Get current user information""" - return current_user - - -@app.post('/password') -async def reset_password( - username: str, current_password: Password, new_password: Password): - """Set a new password for an authenticated user""" - authenticated = await auth.authenticate_user( - username, current_password.password.get_secret_value()) - if not authenticated: - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail="Incorrect username or password", - ) - - user = await db.find_one_by_attributes( - User, {'profile.username': username}) - user.profile.hashed_password = auth.get_password_hash( - new_password.password.get_secret_value()) - obj = await db.update(user) - await pubsub.publish_cloudevent('user', {'op': 'updated', - 'id': str(obj.id)}) - return obj - - -@app.post('/hash') -def get_password_hash(password: Password): - """Get a password hash from the provided string password""" - return auth.get_password_hash(password.password.get_secret_value()) - - # ----------------------------------------------------------------------------- # Nodes @@ -504,7 +487,7 @@ async def _verify_user_group_existence(user_groups: List[str]): @app.post('/node', response_model=Node, response_model_by_alias=False) async def post_node(node: Node, - current_user: str = Depends(current_active_user)): + current_user: User = Depends(get_current_user)): """Create a new node""" if node.parent: parent = await db.find_by_id(Node, node.parent) @@ -577,13 +560,13 @@ async def put_nodes( # Pub/Sub @app.post('/subscribe/{channel}', response_model=Subscription) -async def subscribe(channel: str, user: User = Depends(current_active_user)): +async def subscribe(channel: str, user: User = Depends(get_current_user)): """Subscribe handler for Pub/Sub channel""" return await pubsub.subscribe(channel) @app.post('/unsubscribe/{sub_id}') -async def unsubscribe(sub_id: int, user: User = Depends(current_active_user)): +async def unsubscribe(sub_id: int, user: User = Depends(get_current_user)): """Unsubscribe handler for Pub/Sub channel""" try: await pubsub.unsubscribe(sub_id) @@ -595,7 +578,7 @@ async def unsubscribe(sub_id: int, user: User = Depends(current_active_user)): @app.get('/listen/{sub_id}') -async def listen(sub_id: int, user: User = Depends(current_active_user)): +async def listen(sub_id: int, user: User = Depends(get_current_user)): """Listen messages from a subscribed Pub/Sub channel""" try: return await pubsub.listen(sub_id) @@ -608,7 +591,7 @@ async def listen(sub_id: int, user: User = Depends(current_active_user)): @app.post('/publish/{channel}') async def publish(raw: dict, channel: str, - user: User = Depends(current_active_user)): + user: User = Depends(get_current_user)): """Publish a message on the provided Pub/Sub channel""" attributes = dict(raw) data = attributes.pop('data') @@ -621,7 +604,7 @@ async def publish(raw: dict, channel: str, @app.post('/regression', response_model=Regression, response_model_by_alias=False) async def post_regression(regression: Regression, - user: str = Depends(current_active_user)): + user: str = Depends(get_current_user)): """Create a new regression""" obj = await db.create(regression) operation = 'created' @@ -633,7 +616,7 @@ async def post_regression(regression: Regression, @app.put('/regression/{regression_id}', response_model=Regression, response_model_by_alias=False) async def put_regression(regression_id: str, regression: Regression, - user: str = Depends(current_active_user)): + user: str = Depends(get_current_user)): """Update an already added regression""" regression.id = ObjectId(regression_id) obj = await db.update(regression) @@ -643,77 +626,6 @@ async def put_regression(regression_id: str, regression: Regression, return obj -# ----------------------------------------------------------------------------- -# Users - -app.include_router( - fastapi_users_instance.get_auth_router(auth_backend, - requires_verification=True), - prefix="/user", - tags=["user"] -) - -register_router = fastapi_users_instance.get_register_router( - UserRead, UserCreate) - - -@app.post("/user/register", response_model=UserRead, tags=["user"], - response_model_by_alias=False) -async def register(request: Request, user: UserCreate, - current_user: str = Depends(current_active_superuser)): - """User registration route - - Custom user registration router to ensure unique username. - `user` from request has a list of user group name strings. - This handler will convert them to `UserGroup` objects and - insert user object to database. - """ - existing_user = await db.find_one(User, username=user.username) - if existing_user: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="Username already exists", - ) - groups = [] - if user.groups: - for group_name in user.groups: - group = await db.find_one(UserGroup, name=group_name) - if not group: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=f"User group does not exist with name: \ - {group_name}") - groups.append(group) - user.groups = groups - created_user = await register_router.routes[0].endpoint( - request, user, UserManager(BeanieUserDatabase(User))) - # Update user to be an admin user explicitly if requested as - # `fastapi-users` register route does not allow it - if user.is_superuser: - user_from_id = await db.find_by_id(User, created_user.id) - user_from_id.is_superuser = True - created_user = await db.update(user_from_id) - return created_user - - -app.include_router( - fastapi_users_instance.get_reset_password_router(), - prefix="/user", - tags=["user"], -) -app.include_router( - fastapi_users_instance.get_verify_router(UserRead), - prefix="/user", - tags=["user"], -) -app.include_router( - fastapi_users_instance.get_users_router(UserRead, UserUpdate, - requires_verification=True), - prefix="/user", - tags=["user"], -) - - app = VersionedFastAPI( app, version_format='{major}', diff --git a/api/models.py b/api/models.py index f7712673..118a37a1 100644 --- a/api/models.py +++ b/api/models.py @@ -18,8 +18,6 @@ AnyHttpUrl, AnyUrl, BaseModel, - conlist, - EmailStr, Field, FileUrl, SecretStr, @@ -118,36 +116,6 @@ def create_indexes(cls, collection): collection.create_index("name", unique=True) -class UserProfile(BaseModel): - """API user profile model""" - username: str - hashed_password: str = Field(description="Hash of the plaintext password") - groups: conlist(UserGroup, unique_items=True) = Field( - default=[], - description="A list of groups that user belongs to" - ) - email: EmailStr = Field( - description="User email address" - ) - - -class User(DatabaseModel): - """API user model - The model will be accessible by admin users only""" - active: bool = Field( - default=True, - description="To check if user is active or not" - ) - profile: UserProfile = Field( - description="User profile details accessible by all users" - ) - - @classmethod - def create_indexes(cls, collection): - """Create an index to bind unique constraint to username""" - collection.create_index("profile.username", unique=True) - - class KernelVersion(BaseModel): """Linux kernel version model""" version: int = Field( From 4e7894ab178e4eb1635b97b26c75814cf56093ad Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Sat, 14 Oct 2023 11:03:32 +0530 Subject: [PATCH 18/22] api.auth: optimize `Authentication` class Now the authentication methods have been used from `fastapi-users` package. Hence, remove all the methods from `Authentication` class related to access tokens, scopes, passwords, and getting current user. Also update signature of the class constructor to drop unused variables. Rename variable `Settings.access_token_expire_minutes` to `Settings.access_token_expire_seconds` as `JWTStrategy` class takes token expiry time in seconds. Update logic to create an instance of `Authentication` class in `api.main` module. Signed-off-by: Jeny Sadadia --- api/auth.py | 110 ++++------------------------------------------------ api/main.py | 4 +- 2 files changed, 9 insertions(+), 105 deletions(-) diff --git a/api/auth.py b/api/auth.py index 18c3e9c1..ad616cb7 100644 --- a/api/auth.py +++ b/api/auth.py @@ -6,28 +6,13 @@ """User authentication utilities""" -from datetime import datetime, timedelta -from fastapi.security import OAuth2PasswordBearer -from jose import JWTError, jwt from passlib.context import CryptContext -from pydantic import BaseModel, BaseSettings, Field +from pydantic import BaseSettings from fastapi_users.authentication import ( AuthenticationBackend, BearerTransport, JWTStrategy, ) -from .db import Database -from .models import User - - -class Token(BaseModel): - """Authentication token model""" - access_token: str = Field( - description='Authentication access token' - ) - token_type: str = Field( - description='Access token type e.g. Bearer' - ) class Settings(BaseSettings): @@ -35,108 +20,29 @@ class Settings(BaseSettings): secret_key: str algorithm: str = "HS256" # Set to None so tokens don't expire - access_token_expire_minutes: float = None + access_token_expire_seconds: int = None class Authentication: - """Authentication utility class - - This class accepts a single argument `database` in its constructor, which - should be a db.Database object. - """ + """Authentication utility class""" CRYPT_CTX = CryptContext(schemes=["bcrypt"], deprecated="auto") - def __init__(self, database: Database, token_url: str, user_scopes: dict): - self._db = database + def __init__(self, token_url: str): self._settings = Settings() - self._user_scopes = user_scopes - self._oauth2_scheme = OAuth2PasswordBearer( - tokenUrl=token_url, - scopes=self._user_scopes - ) - - @property - def oauth2_scheme(self): - """Get authentication scheme""" - return self._oauth2_scheme + self._token_url = token_url @classmethod def get_password_hash(cls, password): """Get a password hash for a given clear text password string""" return cls.CRYPT_CTX.hash(password) - @classmethod - def verify_password(cls, password_hash, user): - """Verify that the password hash matches the user's password""" - return cls.CRYPT_CTX.verify(password_hash, user.hashed_password) - - async def authenticate_user(self, username: str, password: str): - """Authenticate a username / password pair - - Look up a `User` in the database with the provided `username` - and check whether the provided clear text `password` matches the hash - associated with it. - """ - user = await self._db.find_one_by_attributes( - User, {'profile.username': username}) - if not user: - return False - if not self.verify_password(password, user.profile): - return False - return user.profile - - def create_access_token(self, data: dict): - """Create a JWT access token using the provided arbitrary `data`""" - to_encode = data.copy() - if self._settings.access_token_expire_minutes: - expires_delta = timedelta( - minutes=self._settings.access_token_expire_minutes - ) - expire = datetime.utcnow() + expires_delta - to_encode.update({"exp": expire}) - encoded_jwt = jwt.encode( - to_encode, - self._settings.secret_key, algorithm=self._settings.algorithm - ) - return encoded_jwt - - async def get_current_user(self, token, security_scopes): - """Decode the given JWT `token` and look up a matching `User`""" - try: - payload = jwt.decode( - token, - self._settings.secret_key, - algorithms=[self._settings.algorithm] - ) - username: str = payload.get("sub") - token_scopes = payload.get("scopes", []) - if username is None: - return None, "Could not validate credentials" - - for scope in security_scopes: - if scope not in token_scopes: - return None, "Access denied" - - except JWTError as error: - return None, str(error) - - user = await self._db.find_one_by_attributes( - User, {'profile.username': username}) - return user, None - - async def validate_scopes(self, requested_scopes): - """Check if requested scopes are valid user scopes""" - for scope in requested_scopes: - if scope not in self._user_scopes: - return False, scope - return True, None - def get_jwt_strategy(self) -> JWTStrategy: """Get JWT strategy for authentication backend""" return JWTStrategy( secret=self._settings.secret_key, - lifetime_seconds=self._settings.access_token_expire_minutes + algorithm=self._settings.algorithm, + lifetime_seconds=self._settings.access_token_expire_seconds ) def get_user_authentication_backend(self): @@ -148,7 +54,7 @@ def get_user_authentication_backend(self): Strategy is a method to generate and secure tokens. It can be JWT, database or Redis. """ - bearer_transport = BearerTransport(tokenUrl="user/login") + bearer_transport = BearerTransport(tokenUrl=self._token_url) return AuthenticationBackend( name="jwt", transport=bearer_transport, diff --git a/api/main.py b/api/main.py index 01860ac2..a772d591 100644 --- a/api/main.py +++ b/api/main.py @@ -54,9 +54,7 @@ app = FastAPI() db = Database(service=(os.getenv('MONGO_SERVICE') or 'mongodb://db:27017')) -auth = Authentication(db, token_url='token', - user_scopes={"admin": "Superusers", - "users": "Regular users"}) +auth = Authentication(token_url="user/login") pubsub = None # pylint: disable=invalid-name auth_backend = auth.get_user_authentication_backend() From b5018ce582edf187b64835a8c433d35a98994b77 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Wed, 18 Oct 2023 15:30:42 +0530 Subject: [PATCH 19/22] api.main: renamed versioned app instance At the moment, original `FastAPI` app instance and versioned app instance both has same name `app`. Renamed the versioned app instance in order to use original app instace for mocking dependency callable with `app.dependency_overrides`. This is required to mock calls to `get_current_user` and `get_current_superuser` for user related tests. Update `Dockerfile` to use `versioned_app` instance for running API. Signed-off-by: Jeny Sadadia --- api/main.py | 4 ++-- docker/api/Dockerfile | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/api/main.py b/api/main.py index a772d591..0c01d2c1 100644 --- a/api/main.py +++ b/api/main.py @@ -624,7 +624,7 @@ async def put_regression(regression_id: str, regression: Regression, return obj -app = VersionedFastAPI( +versioned_app = VersionedFastAPI( app, version_format='{major}', prefix_format='/v{major}', @@ -642,7 +642,7 @@ async def put_regression(regression_id: str, regression: Regression, The issue has already been reported here: https://github.com/DeanWay/fastapi-versioning/issues/30 """ -for sub_app in app.routes: +for sub_app in versioned_app.routes: if hasattr(sub_app.app, "add_exception_handler"): sub_app.app.add_exception_handler( ValueError, value_error_exception_handler diff --git a/docker/api/Dockerfile b/docker/api/Dockerfile index d3f3a704..1e0c2c3f 100644 --- a/docker/api/Dockerfile +++ b/docker/api/Dockerfile @@ -22,4 +22,4 @@ COPY requirements*.txt /home/kernelci/ ARG REQUIREMENTS=requirements.txt RUN pip install --requirement ${REQUIREMENTS} -CMD ["uvicorn", "api.main:app", "--host", "0.0.0.0", "--reload"] +CMD ["uvicorn", "api.main:versioned_app", "--host", "0.0.0.0", "--reload"] From e23d30028bbc9735c9567260136a62f96e164219 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Tue, 24 Oct 2023 15:09:08 +0530 Subject: [PATCH 20/22] tests/e2e_tests: update user related e2e tests Use `versioned_app` instance for creating test client instance. Update all user related tests to use latest endpoints and user schema from `fastapi-users` integration. Add environment variables related to email settings to Github actions workflow as `UserManager` needs them to initialize `EmailSender` instance. Signed-off-by: Jeny Sadadia --- .github/workflows/test.yml | 4 + tests/e2e_tests/conftest.py | 10 +-- tests/e2e_tests/test_password_handler.py | 16 ++-- tests/e2e_tests/test_user_creation.py | 110 +++++++++++++---------- 4 files changed, 81 insertions(+), 59 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index d8b5ed8d..f453263e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -30,6 +30,10 @@ jobs: - name: Export environment variables run: | echo "SECRET_KEY=$(openssl rand -hex 32)" > .env + echo "SMTP_HOST=smtp.gmail.com" >> .env + echo "SMTP_PORT=465" >> .env + echo "EMAIL_SENDER=test@kernelci.org" >> .env + echo "EMAIL_PASSWORD=random" >> .env - name: Build docker images run: docker-compose -f test-docker-compose.yaml build diff --git a/tests/e2e_tests/conftest.py b/tests/e2e_tests/conftest.py index b040837a..4437adee 100644 --- a/tests/e2e_tests/conftest.py +++ b/tests/e2e_tests/conftest.py @@ -12,7 +12,7 @@ from fastapi.testclient import TestClient -from api.main import app +from api.main import versioned_app BASE_URL = 'http://api:8000/latest/' DB_URL = 'mongodb://db:27017' @@ -25,17 +25,17 @@ @pytest.fixture(scope='session') def test_client(): """Fixture to get FastAPI Test client instance""" - with TestClient(app=app, base_url=BASE_URL) as client: + with TestClient(app=versioned_app, base_url=BASE_URL) as client: yield client @pytest.fixture(scope='session') async def test_async_client(): """Fixture to get Test client for asynchronous tests""" - async with AsyncClient(app=app, base_url=BASE_URL) as client: - await app.router.startup() + async with AsyncClient(app=versioned_app, base_url=BASE_URL) as client: + await versioned_app.router.startup() yield client - await app.router.shutdown() + await versioned_app.router.shutdown() async def db_create(collection, obj): diff --git a/tests/e2e_tests/test_password_handler.py b/tests/e2e_tests/test_password_handler.py index c239b7d7..19679b7a 100644 --- a/tests/e2e_tests/test_password_handler.py +++ b/tests/e2e_tests/test_password_handler.py @@ -2,6 +2,7 @@ # # Copyright (C) 2023 Collabora Limited # Author: Paweł Wieczorek +# Author: Jeny Sadadia """End-to-end test functions for KernelCI API password reset handler""" @@ -17,17 +18,20 @@ @pytest.mark.asyncio async def test_password_endpoint(test_async_client): """ - Test Case : Test KernelCI API /password endpoint to set a new password - when requested with current user's password + Test Case : Test KernelCI API /user/me endpoint to set a new password + when requested with current user's access token Expected Result : HTTP Response Code 200 OK """ - response = await test_async_client.post( - "password?username=test_user", + response = await test_async_client.patch( + "user/me", + headers={ + "Accept": "application/json", + "Authorization": f"Bearer {pytest.BEARER_TOKEN}" + }, data=json.dumps( { - "current_password": {"password": "test"}, - "new_password": {"password": "foo"}, + "password": "foo" } ), ) diff --git a/tests/e2e_tests/test_user_creation.py b/tests/e2e_tests/test_user_creation.py index 0d6b7f6f..9e281c1c 100644 --- a/tests/e2e_tests/test_user_creation.py +++ b/tests/e2e_tests/test_user_creation.py @@ -8,9 +8,11 @@ import json import pytest -from api.models import User, UserGroup, UserProfile -from api.db import Database from e2e_tests.conftest import db_create +from api.models import UserGroup +from api.user_models import User +from api.db import Database +from api.auth import Authentication @pytest.mark.dependency( @@ -21,42 +23,36 @@ @pytest.mark.asyncio async def test_create_admin_user(test_async_client): """ - Test Case : Get hashed password using '/hash' endpoint to create an admin + Test Case : Get hashed password using authentication method to create an admin user. Create the admin user using database create method. - Request authentication token using '/token' endpoint for the user and + Request authentication token using '/user/login' endpoint for the user and store it in pytest global variable 'ADMIN_BEARER_TOKEN'. """ username = 'admin' password = 'test' - response = await test_async_client.post( - "hash", - data=json.dumps({'password': password}) - ) - hashed_password = response.json() - assert response.status_code == 200 + hashed_password = Authentication.get_password_hash(password) - profile = UserProfile( - username=username, - hashed_password=hashed_password, - email='test-admin@kernelci.org', - groups=[UserGroup(name="admin")] - ) obj = await db_create( - Database.COLLECTIONS[User], - User(profile=profile) - ) + Database.COLLECTIONS[User], + User( + username=username, + hashed_password=hashed_password, + email='test-admin@kernelci.org', + groups=[UserGroup(name="admin")], + is_superuser=1, + is_verified=1 + )) assert obj is not None response = await test_async_client.post( - "token", + "user/login", headers={ "Accept": "application/json", "Content-Type": "application/x-www-form-urlencoded" }, - data={ - 'username': username, 'password': password, 'scope': 'admin users' - } + data=f"username={username}&password={password}" ) + print("response.json()", response.json()) assert response.status_code == 200 assert response.json().keys() == { 'access_token', @@ -70,34 +66,50 @@ async def test_create_admin_user(test_async_client): @pytest.mark.asyncio async def test_create_regular_user(test_async_client): """ - Test Case : Test KernelCI API '/user' endpoint to create regular user - when requested with admin user's bearer token. Request '/token' endpoint - for the user and store it in pytest global variable 'BEARER_TOKEN'. + Test Case : Test KernelCI API '/user/register' endpoint to create regular + user when requested with admin user's bearer token. Request '/user/login' + endpoint for the user and store it in pytest global variable 'BEARER_TOKEN'. """ username = 'test_user' password = 'test' email = 'test@kernelci.org' response = await test_async_client.post( - f"user/{username}?email={email}", + "user/register", headers={ "Accept": "application/json", "Authorization": f"Bearer {pytest.ADMIN_BEARER_TOKEN}" }, - data=json.dumps({'password': password}) + data=json.dumps({ + 'username': username, + 'password': password, + 'email': email + }) + ) + assert response.status_code == 200 + assert ('id', 'email', 'is_active', 'is_superuser', 'is_verified', + 'username', 'groups') == tuple(response.json().keys()) + + # User needs to verified before getting access token + # Directly updating user to by pass user verification via email + user_id = response.json()['id'] + response = await test_async_client.patch( + f"user/{user_id}", + headers={ + "Accept": "application/json", + "Content-Type": "application/json", + "Authorization": f"Bearer {pytest.ADMIN_BEARER_TOKEN}" + }, + data=json.dumps({"is_verified": True}) ) assert response.status_code == 200 - assert ('id', 'active', - 'profile') == tuple(response.json().keys()) - assert ('username', 'hashed_password', - 'groups', 'email') == tuple(response.json()['profile'].keys()) response = await test_async_client.post( - "token", + "user/login", headers={ "Accept": "application/json", "Content-Type": "application/x-www-form-urlencoded" }, - data={'username': username, 'password': password} + data=f"username={username}&password={password}" ) assert response.status_code == 200 assert response.json().keys() == { @@ -113,8 +125,8 @@ def test_whoami(test_client): Test Case : Test KernelCI API /whoami endpoint Expected Result : HTTP Response Code 200 OK - JSON with 'id', 'username', 'hashed_password' - and 'active' keys + JSON with 'id', 'email', username', 'groups', 'is_superuser' + 'is_verified' and 'is_active' keys """ response = test_client.get( "whoami", @@ -124,29 +136,31 @@ def test_whoami(test_client): }, ) assert response.status_code == 200 - assert ('id', 'active', - 'profile') == tuple(response.json().keys()) - assert ('username', 'hashed_password', - 'groups', 'email') == tuple(response.json()['profile'].keys()) - assert response.json()['profile']['username'] == 'test_user' + assert ('id', 'email', 'is_active', 'is_superuser', 'is_verified', + 'username', 'groups') == tuple(response.json().keys()) + assert response.json()['username'] == 'test_user' @pytest.mark.dependency(depends=["test_create_regular_user"]) def test_create_user_negative(test_client): """ - Test Case : Test KernelCI API /user endpoint when requested + Test Case : Test KernelCI API /user/register endpoint when requested with regular user's bearer token. Expected Result : - HTTP Response Code 401 Unauthorized - JSON with 'detail' key denoting 'Access denied' error + HTTP Response Code 403 Forbidden + JSON with 'detail' key denoting 'Forbidden' error """ response = test_client.post( - "user/test", + "user/register", headers={ "Accept": "application/json", "Authorization": f"Bearer {pytest.BEARER_TOKEN}" }, - data=json.dumps({'password': 'test'}) + data=json.dumps({ + 'username': 'test', + 'password': 'test', + 'email': 'test@kernelci.org' + }) ) - assert response.status_code == 401 - assert response.json() == {'detail': 'Access denied'} + assert response.status_code == 403 + assert response.json() == {'detail': 'Forbidden'} From 5a11229628b37a578eda5c25e665e3c7ea9b4b34 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Sat, 14 Oct 2023 17:58:12 +0530 Subject: [PATCH 21/22] tests/unit_tests: update user related tests Integrate `mongomock_motor` package to mock async mongo client for Beanie initialization. Add fixture to initialize Beanie on app startup. Remove fixtures and add methods to get current active user. Update `app.dependency_overrides` dictionary to mock dependency callable from `fastapi-users` to get users while creating `TestClient`. Update all the user related tests. Add environment variable to Github actions workflow file. Signed-off-by: Jeny Sadadia --- .github/workflows/main.yml | 4 + docker/api/requirements-tests.txt | 1 + tests/unit_tests/conftest.py | 190 ++++++++++++------ tests/unit_tests/test_listen_handler.py | 9 +- tests/unit_tests/test_node_handler.py | 17 +- tests/unit_tests/test_subscribe_handler.py | 3 +- tests/unit_tests/test_token_handler.py | 85 +++----- tests/unit_tests/test_unsubscribe_handler.py | 6 +- tests/unit_tests/test_user_group_handler.py | 16 +- tests/unit_tests/test_user_handler.py | 193 +++++++++++-------- tests/unit_tests/test_whoami_handler.py | 22 ++- 11 files changed, 310 insertions(+), 236 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index ef256e64..7d0694ef 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -57,6 +57,10 @@ jobs: - name: Export environment variables run: | echo "SECRET_KEY=$(openssl rand -hex 32)" >> $GITHUB_ENV + echo "SMTP_HOST=smtp.gmail.com" >> $GITHUB_ENV + echo "SMTP_PORT=465" >> $GITHUB_ENV + echo "EMAIL_SENDER=test@kernelci.org" >> $GITHUB_ENV + echo "EMAIL_PASSWORD=random" >> $GITHUB_ENV - name: Run pytest run: | diff --git a/docker/api/requirements-tests.txt b/docker/api/requirements-tests.txt index 3d630787..04390e81 100644 --- a/docker/api/requirements-tests.txt +++ b/docker/api/requirements-tests.txt @@ -6,3 +6,4 @@ pytest-dependency==0.5.1 pytest-mock==3.6.1 pytest-order==1.0.1 httpx==0.23.3 +mongomock_motor==0.0.21 diff --git a/tests/unit_tests/conftest.py b/tests/unit_tests/conftest.py index 1df31ee7..7b2e5690 100644 --- a/tests/unit_tests/conftest.py +++ b/tests/unit_tests/conftest.py @@ -14,10 +14,20 @@ import asyncio import fakeredis.aioredis from fastapi.testclient import TestClient +from fastapi import Request, HTTPException, status import pytest - -from api.main import app -from api.models import User, UserGroup, UserProfile +from mongomock_motor import AsyncMongoMockClient +from beanie import init_beanie +from httpx import AsyncClient + +from api.main import ( + app, + versioned_app, + get_current_user, + get_current_superuser, +) +from api.models import UserGroup +from api.user_models import User from api.pubsub import PubSub BEARER_TOKEN = "Bearer \ @@ -33,13 +43,77 @@ BASE_URL = f'http://testserver/{API_VERSION}/' +def mock_get_current_user(request: Request): + """ + Get current active user + """ + token = request.headers.get('authorization') + if not token: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Missing token", + ) + return User( + id='65265305c74695807499037f', + username='bob', + hashed_password='$2b$12$CpJZx5ooxM11bCFXT76/z.o6HWs2sPJy4iP8.' + 'xCZGmM8jWXUXJZ4L', + email='bob@kernelci.org', + is_active=True, + is_superuser=False, + is_verified=True + ) + + +def mock_get_current_admin_user(request: Request): + """ + Get current active admin user + """ + token = request.headers.get('authorization') + if not token: + raise HTTPException( + status_code=status.HTTP_401_UNAUTHORIZED, + detail="Missing token", + ) + if token != ADMIN_BEARER_TOKEN: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, + detail="Forbidden", + ) + return User( + id='653a5e1a7e9312c86f8f86e1', + username='admin', + hashed_password='$2b$12$CpJZx5ooxM11bCFXT76/z.o6HWs2sPJy4iP8.' + 'xCZGmM8jWXUXJZ4K', + email='admin@kernelci.org', + groups=[UserGroup(name='admin')], + is_active=True, + is_superuser=True, + is_verified=True + ) + + +app.dependency_overrides[get_current_user] = mock_get_current_user +app.dependency_overrides[get_current_superuser] = mock_get_current_admin_user + + @pytest.fixture def test_client(): """Fixture to get FastAPI Test client instance""" - with TestClient(app=app, base_url=BASE_URL) as client: + # Mock dependency callables for getting current user + with TestClient(app=versioned_app, base_url=BASE_URL) as client: return client +@pytest.fixture +async def test_async_client(): + """Fixture to get Test client for asynchronous tests""" + async with AsyncClient(app=versioned_app, base_url=BASE_URL) as client: + await versioned_app.router.startup() + yield client + await versioned_app.router.shutdown() + + @pytest.fixture def event_loop(): """Create an instance of the default event loop for each test case. @@ -105,60 +179,6 @@ def mock_db_find_one(mocker): return async_mock -@pytest.fixture -def mock_db_find_one_by_attributes(mocker): - """ - Mocks async call to Database class method - used to find an object with matching attributes - """ - async_mock = AsyncMock() - mocker.patch('api.db.Database.find_one_by_attributes', - side_effect=async_mock) - return async_mock - - -@pytest.fixture -def mock_get_current_user(mocker): - """ - Mocks async call to Authentication class method - used to get current user - """ - async_mock = AsyncMock() - profile = UserProfile( - username='bob', - hashed_password='$2b$12$CpJZx5ooxM11bCFXT76/z.o6HWs2sPJy4iP8.' - 'xCZGmM8jWXUXJZ4K', - email='bob@kernelci.org' - ) - user = User(profile=profile, active=True) - mocker.patch('api.auth.Authentication.get_current_user', - side_effect=async_mock) - async_mock.return_value = user, None - return async_mock - - -@pytest.fixture -def mock_get_current_admin_user(mocker): - """ - Mocks async call to Authentication class method - used to get current user - """ - async_mock = AsyncMock() - profile = UserProfile( - username='admin', - hashed_password='$2b$12$CpJZx5ooxM11bCFXT76/z.o6HWs2sPJy4iP8.' - 'xCZGmM8jWXUXJZ4K', - email='admin@kernelci.org', - groups=[UserGroup(name='admin')]) - user = User( - profile=profile, - active=True) - mocker.patch('api.auth.Authentication.get_current_user', - side_effect=async_mock) - async_mock.return_value = user, None - return async_mock - - @pytest.fixture(autouse=True) def mock_init_sub_id(mocker): """Mocks async call to PubSub method to initialize subscription id""" @@ -238,3 +258,57 @@ def mock_unsubscribe(mocker): mocker.patch('api.pubsub.PubSub.unsubscribe', side_effect=async_mock) return async_mock + + +@pytest.fixture(autouse=True) +async def mock_init_beanie(mocker): + """Mocks async call to Database method to initialize Beanie""" + async_mock = AsyncMock() + client = AsyncMongoMockClient() + init = await init_beanie( + document_models=[User], database=client.get_database(name="db")) + mocker.patch('api.db.Database.initialize_beanie', + side_effect=async_mock, return_value=init) + return async_mock + + +@pytest.fixture +def mock_db_update(mocker): + """ + Mocks async call to Database class method used to update object + """ + async_mock = AsyncMock() + mocker.patch('api.db.Database.update', + side_effect=async_mock) + return async_mock + + +@pytest.fixture +async def mock_beanie_get_user_by_id(mocker): + """Mocks async call to external method to get model by id""" + async_mock = AsyncMock() + mocker.patch('fastapi_users_db_beanie.BeanieUserDatabase.get', + side_effect=async_mock) + return async_mock + + +@pytest.fixture +def mock_auth_current_user(mocker): + """ + Mocks async call to external method to get authenticated user + """ + async_mock = AsyncMock() + mocker.patch('fastapi_users.authentication.Authenticator._authenticate', + side_effect=async_mock) + return async_mock + + +@pytest.fixture +def mock_user_find(mocker): + """ + Mocks async call to external method to find user model using Beanie + """ + async_mock = AsyncMock() + mocker.patch('api.user_models.User.find_one', + side_effect=async_mock) + return async_mock diff --git a/tests/unit_tests/test_listen_handler.py b/tests/unit_tests/test_listen_handler.py index 723813e3..2a493564 100644 --- a/tests/unit_tests/test_listen_handler.py +++ b/tests/unit_tests/test_listen_handler.py @@ -13,8 +13,7 @@ from tests.unit_tests.conftest import BEARER_TOKEN -def test_listen_endpoint(mock_get_current_user, - mock_listen, test_client): +def test_listen_endpoint(mock_listen, test_client): """ Test Case : Test KernelCI API GET /listen endpoint for the positive path @@ -33,8 +32,7 @@ def test_listen_endpoint(mock_get_current_user, assert response.status_code == 200 -def test_listen_endpoint_not_found(mock_get_current_user, - test_client): +def test_listen_endpoint_not_found(test_client): """ Test Case : Test KernelCI API GET /listen endpoint for the negative path @@ -53,8 +51,7 @@ def test_listen_endpoint_not_found(mock_get_current_user, assert 'detail' in response.json() -def test_listen_endpoint_without_token(mock_get_current_user, - test_client): +def test_listen_endpoint_without_token(test_client): """ Test Case : Test KernelCI API GET /listen endpoint for the negative path diff --git a/tests/unit_tests/test_node_handler.py b/tests/unit_tests/test_node_handler.py index baa35e60..f4049ca7 100644 --- a/tests/unit_tests/test_node_handler.py +++ b/tests/unit_tests/test_node_handler.py @@ -17,8 +17,7 @@ from api.paginator_models import PageModel -def test_create_node_endpoint(mock_get_current_user, - mock_db_create, mock_publish_cloudevent, +def test_create_node_endpoint(mock_db_create, mock_publish_cloudevent, test_client): """ Test Case : Test KernelCI API /node endpoint @@ -91,8 +90,7 @@ def test_create_node_endpoint(mock_get_current_user, } -def test_get_nodes_by_attributes_endpoint(mock_get_current_user, - mock_db_find_by_attributes, +def test_get_nodes_by_attributes_endpoint(mock_db_find_by_attributes, test_client): """ Test Case : Test KernelCI API GET /nodes?attribute_name=attribute_value @@ -159,7 +157,6 @@ def test_get_nodes_by_attributes_endpoint(mock_get_current_user, def test_get_nodes_by_attributes_endpoint_node_not_found( - mock_get_current_user, mock_db_find_by_attributes, test_client): """ @@ -190,7 +187,7 @@ def test_get_nodes_by_attributes_endpoint_node_not_found( assert response.json().get('total') == 0 -def test_get_node_by_id_endpoint(mock_get_current_user, mock_db_find_by_id, +def test_get_node_by_id_endpoint(mock_db_find_by_id, test_client): """ Test Case : Test KernelCI API GET /node/{node_id} endpoint @@ -244,8 +241,7 @@ def test_get_node_by_id_endpoint(mock_get_current_user, mock_db_find_by_id, } -def test_get_node_by_id_endpoint_empty_response(mock_get_current_user, - mock_db_find_by_id, +def test_get_node_by_id_endpoint_empty_response(mock_db_find_by_id, test_client): """ Test Case : Test KernelCI API GET /node/{node_id} endpoint @@ -262,7 +258,7 @@ def test_get_node_by_id_endpoint_empty_response(mock_get_current_user, assert response.json() is None -def test_get_all_nodes(mock_get_current_user, mock_db_find_by_attributes, +def test_get_all_nodes(mock_db_find_by_attributes, test_client): """ Test Case : Test KernelCI API GET /nodes endpoint for the @@ -343,8 +339,7 @@ def test_get_all_nodes(mock_get_current_user, mock_db_find_by_attributes, assert len(response.json()) > 0 -def test_get_all_nodes_empty_response(mock_get_current_user, - mock_db_find_by_attributes, +def test_get_all_nodes_empty_response(mock_db_find_by_attributes, test_client): """ Test Case : Test KernelCI API GET /nodes endpoint for the diff --git a/tests/unit_tests/test_subscribe_handler.py b/tests/unit_tests/test_subscribe_handler.py index 269d8e6b..555cea8a 100644 --- a/tests/unit_tests/test_subscribe_handler.py +++ b/tests/unit_tests/test_subscribe_handler.py @@ -11,8 +11,7 @@ from api.pubsub import Subscription -def test_subscribe_endpoint(mock_get_current_user, - mock_subscribe, test_client): +def test_subscribe_endpoint(mock_subscribe, test_client): """ Test Case : Test KernelCI API /subscribe endpoint Expected Result : diff --git a/tests/unit_tests/test_token_handler.py b/tests/unit_tests/test_token_handler.py index 6acf930b..ddd3ef86 100644 --- a/tests/unit_tests/test_token_handler.py +++ b/tests/unit_tests/test_token_handler.py @@ -10,96 +10,65 @@ """Unit test function for KernelCI API token handler""" -from api.models import User, UserGroup, UserProfile +import pytest +from fastapi_users.exceptions import UserNotExists +from api.user_models import User -def test_token_endpoint(mock_db_find_one_by_attributes, - test_client): +@pytest.mark.asyncio +async def test_token_endpoint(test_async_client, mock_user_find): """ - Test Case : Test KernelCI API token endpoint + Test Case : Test KernelCI API /user/login endpoint Expected Result : HTTP Response Code 200 OK JSON with 'access_token' and 'token_type' key """ - profile = UserProfile( + user = User( + id='65265305c74695807499037f', username='bob', hashed_password='$2b$12$CpJZx5ooxM11bCFXT76/z.o6HWs2sPJy4iP8.' 'xCZGmM8jWXUXJZ4K', email='bob@kernelci.org', - groups=[] + groups=[], + is_active=True, + is_superuser=False, + is_verified=True ) - user = User(profile=profile, active=True) - mock_db_find_one_by_attributes.return_value = user - response = test_client.post( - "token", + mock_user_find.return_value = user + response = await test_async_client.post( + "user/login", headers={ "Accept": "application/json", "Content-Type": "application/x-www-form-urlencoded" }, - data={'username': 'bob', 'password': 'hello'} + data="username=bob&password=hello" ) - print("json", response.json()) assert response.status_code == 200 assert ('access_token', 'token_type') == tuple(response.json().keys()) -def test_token_endpoint_incorrect_password(mock_db_find_one_by_attributes, - test_client): +@pytest.mark.asyncio +async def test_token_endpoint_incorrect_password(test_async_client, + mock_user_find): """ - Test Case : Test KernelCI API token endpoint for negative path + Test Case : Test KernelCI API /user/login endpoint for negative path Incorrect password should be passed to the endpoint Expected Result : - HTTP Response Code 401 Unauthorized + HTTP Response Code 400 Bad Request JSON with 'detail' key """ - profile = UserProfile( - username='bob', - hashed_password='$2b$12$CpJZx5ooxM11bCFXT76/z.o6HWs2sPJy4iP8.' - 'xCZGmM8jWXUXJZ4K', - email='bob@kernelci.org', - groups=[]) - user = User(profile=profile, active=True) - mock_db_find_one_by_attributes.return_value = user + mock_user_find.side_effect = UserNotExists # Pass incorrect password - response = test_client.post( - "token", + response = await test_async_client.post( + "user/login", headers={ "Accept": "application/json", "Content-Type": "application/x-www-form-urlencoded" }, - data={'username': 'bob', 'password': 'hi'} + data="username=bob&password=hello1" ) print("response json", response.json()) - assert response.status_code == 401 - assert response.json() == {'detail': 'Incorrect username or password'} - - -def test_token_endpoint_admin_user(mock_db_find_one_by_attributes, - test_client): - """ - Test Case : Test KernelCI API token endpoint for admin user - Expected Result : - HTTP Response Code 200 OK - JSON with 'access_token' and 'token_type' key - """ - profile = UserProfile( - username='test_admin', - hashed_password='$2b$12$CpJZx5ooxM11bCFXT76/z.o6HWs2sPJy4iP8.' - 'xCZGmM8jWXUXJZ4K', - email='test-admin@kernelci.org', - groups=[UserGroup(name='admin')]) - user = User(profile=profile, active=True) - mock_db_find_one_by_attributes.return_value = user - response = test_client.post( - "token", - headers={ - "Accept": "application/json", - "Content-Type": "application/x-www-form-urlencoded" - }, - data={'username': 'test_admin', 'password': 'hello', 'scope': 'admin'} - ) - print("json", response.json()) - assert response.status_code == 200 - assert ('access_token', 'token_type') == tuple(response.json().keys()) + assert response.status_code == 400 + assert response.json() == {'detail': 'LOGIN_BAD_CREDENTIALS'} diff --git a/tests/unit_tests/test_unsubscribe_handler.py b/tests/unit_tests/test_unsubscribe_handler.py index c50f3399..947dce6e 100644 --- a/tests/unit_tests/test_unsubscribe_handler.py +++ b/tests/unit_tests/test_unsubscribe_handler.py @@ -10,8 +10,7 @@ from tests.unit_tests.conftest import BEARER_TOKEN -def test_unsubscribe_endpoint(mock_get_current_user, - mock_unsubscribe, test_client): +def test_unsubscribe_endpoint(mock_unsubscribe, test_client): """ Test Case : Test KernelCI API /unsubscribe endpoint positive path Expected Result : @@ -26,8 +25,7 @@ def test_unsubscribe_endpoint(mock_get_current_user, assert response.status_code == 200 -def test_unsubscribe_endpoint_empty_response(mock_get_current_user, - test_client): +def test_unsubscribe_endpoint_empty_response(test_client): """ Test Case : Test KernelCI API /unsubscribe endpoint negative path Expected Result : diff --git a/tests/unit_tests/test_user_group_handler.py b/tests/unit_tests/test_user_group_handler.py index 169b4773..d500372c 100644 --- a/tests/unit_tests/test_user_group_handler.py +++ b/tests/unit_tests/test_user_group_handler.py @@ -17,8 +17,7 @@ from api.paginator_models import PageModel -def test_create_user_group(mock_get_current_admin_user, - mock_db_create, mock_publish_cloudevent, +def test_create_user_group(mock_db_create, mock_publish_cloudevent, test_client): """ Test Case : Test KernelCI API /group endpoint to create user group @@ -44,18 +43,15 @@ def test_create_user_group(mock_get_current_admin_user, assert ('id', 'name') == tuple(response.json().keys()) -def test_create_group_endpoint_negative(mock_get_current_user, - mock_publish_cloudevent, +def test_create_group_endpoint_negative(mock_publish_cloudevent, test_client): """ Test Case : Test KernelCI API /group endpoint when requested with regular user's bearer token Expected Result : - HTTP Response Code 401 Unauthorized - JSON with 'detail' key denoting 'Access denied' error + HTTP Response Code 403 Forbidden + JSON with 'detail' key denoting 'Forbidden' error """ - mock_get_current_user.return_value = None, "Access denied" - response = test_client.post( "group", headers={ @@ -65,8 +61,8 @@ def test_create_group_endpoint_negative(mock_get_current_user, data=json.dumps({"name": "kernelci"}) ) print(response.json()) - assert response.status_code == 401 - assert response.json() == {'detail': 'Access denied'} + assert response.status_code == 403 + assert response.json() == {'detail': 'Forbidden'} def test_get_groups(mock_db_find_by_attributes, diff --git a/tests/unit_tests/test_user_handler.py b/tests/unit_tests/test_user_handler.py index a75752c4..f7a964da 100644 --- a/tests/unit_tests/test_user_handler.py +++ b/tests/unit_tests/test_user_handler.py @@ -8,145 +8,172 @@ """Unit test function for KernelCI API user handler""" import json +import pytest from tests.unit_tests.conftest import ( ADMIN_BEARER_TOKEN, BEARER_TOKEN, ) -from api.models import User, UserGroup, UserProfile +from api.models import UserGroup +from api.user_models import UserRead -def test_create_regular_user(mock_get_current_admin_user, - mock_db_create, mock_publish_cloudevent, - test_client): +@pytest.mark.asyncio +async def test_create_regular_user(mock_db_find_one, mock_db_create, + test_async_client): """ - Test Case : Test KernelCI API /user endpoint to create regular user - when requested with admin user's bearer token + Test Case : Test KernelCI API /user/register endpoint to create regular + user when requested with admin user's bearer token Expected Result : HTTP Response Code 200 OK - JSON with 'id', 'username', 'hashed_password', and - 'active' keys + JSON with 'id', 'username', 'email', 'groups', 'is_active' + 'is_verified' and 'is_superuser' keys """ - profile = UserProfile(username='test', hashed_password="$2b$12$Whi.dpTC.\ -HR5UHMdMFQeOe1eD4oXaP08oW7ogYqyiNziZYNdUHs8i", email='test@kernelci.org') - user = User(profile=profile, active=True) + mock_db_find_one.return_value = None + user = UserRead( + id='65265305c74695807499037f', + username='test', + email='test@kernelci.org', + groups=[], + is_active=True, + is_verified=False, + is_superuser=False + ) mock_db_create.return_value = user - response = test_client.post( - "user/test?email=test@kernelci.org", + response = await test_async_client.post( + "user/register", headers={ "Accept": "application/json", "Authorization": ADMIN_BEARER_TOKEN }, - data=json.dumps({"password": "test"}) + data=json.dumps({ + 'username': 'test', + 'password': 'test', + 'email': 'test@kernelci.org' + }) ) print(response.json()) assert response.status_code == 200 - assert ('id', 'active', 'profile') == tuple(response.json().keys()) - assert ('username', 'hashed_password', - 'groups', 'email') == tuple(response.json()['profile'].keys()) + assert ('id', 'email', 'is_active', 'is_superuser', 'is_verified', + 'username', 'groups') == tuple(response.json().keys()) -def test_create_admin_user( # pylint: disable=too-many-arguments - mock_get_current_admin_user, - mock_db_create, mock_publish_cloudevent, - test_client, mock_db_find_one): +@pytest.mark.asyncio +async def test_create_admin_user(test_async_client, mock_db_find_one, + mock_db_find_by_id, mock_db_update): """ - Test Case : Test KernelCI API /user endpoint to create admin user + Test Case : Test KernelCI API /user/register endpoint to create admin user when requested with admin user's bearer token Expected Result : HTTP Response Code 200 OK - JSON with 'id', 'username', 'hashed_password', and - 'active' keys + JSON with 'id', 'username', 'email', 'groups', 'is_active' + 'is_verified' and 'is_superuser' keys """ - profile = UserProfile( + user = UserRead( + id='61bda8f2eb1a63d2b7152419', username='test_admin', - hashed_password="$2b$12$Whi.dpTC.HR5UHMdMFQeOe1eD4oXaP08o\ -W7ogYqyiNziZYNdUHs8i", email='test-admin@kernelci.org', - groups=[UserGroup(name='admin')]) - user = User(profile=profile, active=True) - mock_db_create.return_value = user - mock_db_find_one.return_value = UserGroup(name='admin') + groups=[UserGroup(name='admin')], + is_active=True, + is_verified=False, + is_superuser=True + ) + mock_db_find_one.return_value = None + mock_db_find_by_id.return_value = user + mock_db_update.return_value = user - response = test_client.post( - "user/test_admin?groups=admin&email=test-admin@kernelci.org", + response = await test_async_client.post( + "user/register", headers={ "Accept": "application/json", "Authorization": ADMIN_BEARER_TOKEN }, - data=json.dumps({"password": "test"}) + data=json.dumps({ + 'username': 'test_admin', + 'password': 'test', + 'email': 'test-admin@kernelci.org', + 'is_superuser': True + }) ) print(response.json()) assert response.status_code == 200 - assert ('id', 'active', 'profile') == tuple(response.json().keys()) - assert ('username', 'hashed_password', - 'groups', 'email') == tuple(response.json()['profile'].keys()) + assert ('id', 'email', 'is_active', 'is_superuser', 'is_verified', + 'username', 'groups') == tuple(response.json().keys()) -def test_create_user_endpoint_negative(mock_get_current_user, - mock_publish_cloudevent, test_client): +@pytest.mark.asyncio +async def test_create_user_endpoint_negative(test_async_client): """ - Test Case : Test KernelCI API /user endpoint when requested + Test Case : Test KernelCI API /user/register endpoint when requested with regular user's bearer token Expected Result : HTTP Response Code 401 Unauthorized - JSON with 'detail' key denoting 'Access denied' error + JSON with 'detail' key denoting 'Forbidden' error """ - mock_get_current_user.return_value = None, "Access denied" - - response = test_client.post( - "user/test", + response = await test_async_client.post( + "user/register", headers={ "Accept": "application/json", "Authorization": BEARER_TOKEN }, - data=json.dumps({"password": "test"}) + data=json.dumps({ + 'username': 'test', + 'password': 'test', + 'email': 'test@kernelci.org' + }) ) print(response.json()) - assert response.status_code == 401 - assert response.json() == {'detail': 'Access denied'} + assert response.status_code == 403 + assert response.json() == {'detail': 'Forbidden'} -def test_create_user_with_group( # pylint: disable=too-many-arguments - mock_get_current_admin_user, - mock_db_create, mock_publish_cloudevent, - test_client, mock_db_find_one): +@pytest.mark.asyncio +async def test_create_user_with_group(test_async_client, mock_db_find_one, + mock_db_update,mock_db_find_by_id): """ - Test Case : Test KernelCI API /user endpoint to create a user with a - user group + Test Case : Test KernelCI API /user/register endpoint to create a user + with a user group Expected Result : HTTP Response Code 200 OK - JSON with 'id', 'username', 'hashed_password' - 'active', and 'groups' keys + JSON with 'id', 'username', 'email', 'groups', 'is_active' + 'is_verified' and 'is_superuser' keys """ - profile = UserProfile( + user = UserRead( + id='61bda8f2eb1a63d2b7152419', username='test_admin', - hashed_password="$2b$12$Whi.dpTC.HR5UHMdMFQeOe1eD4oXaP08oW7\ -ogYqyiNziZYNdUHs8i", email='test-admin@kernelci.org', - groups=[UserGroup(name='kernelci')]) - user = User(profile=profile, active=True) - mock_db_create.return_value = user - mock_db_find_one.return_value = UserGroup(name='kernelci') + groups=[UserGroup(name='kernelci')], + is_active=True, + is_verified=False, + is_superuser=False + ) + mock_db_find_by_id.return_value = user + mock_db_update.return_value = user + mock_db_find_one.side_effect = [None, UserGroup(name='kernelci')] - response = test_client.post( - "user/test_admin?groups=kernelci&email=test-admin@kernelci.org", + response = await test_async_client.post( + "user/register", headers={ "Accept": "application/json", "Authorization": ADMIN_BEARER_TOKEN }, - data=json.dumps({"password": "test"}) + data=json.dumps({ + 'username': 'test', + 'password': 'test', + 'email': 'test-admin@kernelci.org', + 'groups': ['kernelci'] + }) ) print(response.json()) assert response.status_code == 200 - assert ('id', 'active', 'profile') == tuple(response.json().keys()) - assert ('username', 'hashed_password', - 'groups', 'email') == tuple(response.json()['profile'].keys()) + assert ('id', 'email', 'is_active', 'is_superuser', 'is_verified', + 'username', 'groups') == tuple(response.json().keys()) -def test_get_user_by_id_endpoint(mock_get_current_admin_user, mock_db_find_by_id, - test_client): +@pytest.mark.asyncio +async def test_get_user_by_id_endpoint(test_async_client, + mock_beanie_get_user_by_id): """ Test Case : Test KernelCI API GET /user/{user_id} endpoint with admin token @@ -154,15 +181,18 @@ def test_get_user_by_id_endpoint(mock_get_current_admin_user, mock_db_find_by_id HTTP Response Code 200 OK JSON with User object attributes """ - user_obj = User( + user_obj = UserRead( id='61bda8f2eb1a63d2b7152418', - profile=UserProfile(username='test', hashed_password="$2b$12$Whi.dpTC.\ -HR5UHMdMFQeOe1eD4oXaP08oW7ogYqyiNziZYNdUHs8i", email='test@kernelci.org'), - active=True + username='test', + email='test@kernelci.org', + groups=[], + is_active=True, + is_verified=False, + is_superuser=False ) - mock_db_find_by_id.return_value = user_obj + mock_beanie_get_user_by_id.return_value = user_obj - response = test_client.get( + response = await test_async_client.get( "user/61bda8f2eb1a63d2b7152418", headers={ "Accept": "application/json", @@ -170,10 +200,5 @@ def test_get_user_by_id_endpoint(mock_get_current_admin_user, mock_db_find_by_id }) print("response.json()", response.json()) assert response.status_code == 200 - assert response.json().keys() == { - 'id', - 'active', - 'profile' - } - assert ('username', 'groups', - 'email') == tuple(response.json()['profile'].keys()) + assert ('id', 'email', 'is_active', 'is_superuser', 'is_verified', + 'username', 'groups') == tuple(response.json().keys()) diff --git a/tests/unit_tests/test_whoami_handler.py b/tests/unit_tests/test_whoami_handler.py index 91582645..2f88b649 100644 --- a/tests/unit_tests/test_whoami_handler.py +++ b/tests/unit_tests/test_whoami_handler.py @@ -10,10 +10,13 @@ """Unit test function for KernelCI API whoami handler""" +import pytest from tests.unit_tests.conftest import BEARER_TOKEN +from api.user_models import UserRead -def test_whoami_endpoint(mock_get_current_user, test_client): +@pytest.mark.asyncio +async def test_whoami_endpoint(test_async_client, mock_auth_current_user): """ Test Case : Test KernelCI API /whoami endpoint Expected Result : @@ -21,12 +24,25 @@ def test_whoami_endpoint(mock_get_current_user, test_client): JSON with 'id', 'username', 'hashed_password' and 'active' keys """ - response = test_client.get( + user = UserRead( + id='61bda8f2eb1a63d2b7152420', + username='test-user', + email='test-user@kernelci.org', + groups=[], + is_active=True, + is_verified=False, + is_superuser=False + ) + mock_auth_current_user.return_value = user, BEARER_TOKEN + response = await test_async_client.get( "whoami", headers={ "Accept": "application/json", "Authorization": BEARER_TOKEN }, ) + print(response.json(), response.status_code) assert response.status_code == 200 - assert ('id', 'active', 'profile') == tuple(response.json().keys()) + assert ('id', 'email', 'is_active', 'is_superuser', + 'is_verified', 'username', + 'groups') == tuple(response.json().keys()) From e90f8902a2aa5468fea117af6c6a2462cfbb9258 Mon Sep 17 00:00:00 2001 From: Jeny Sadadia Date: Mon, 30 Oct 2023 16:44:13 +0530 Subject: [PATCH 22/22] api: add `api.config` to define various module settings Instead of defining multiple settings class in different API modules like `pubsub`, `auth`, `email_sender`, add `api.config` module to store all different settings classes. Signed-off-by: Jeny Sadadia --- api/auth.py | 12 ++---------- api/config.py | 32 ++++++++++++++++++++++++++++++++ api/email_sender.py | 12 ++---------- api/pubsub.py | 14 ++++---------- api/user_manager.py | 4 ++-- 5 files changed, 42 insertions(+), 32 deletions(-) create mode 100644 api/config.py diff --git a/api/auth.py b/api/auth.py index ad616cb7..2b9fa02d 100644 --- a/api/auth.py +++ b/api/auth.py @@ -7,20 +7,12 @@ """User authentication utilities""" from passlib.context import CryptContext -from pydantic import BaseSettings from fastapi_users.authentication import ( AuthenticationBackend, BearerTransport, JWTStrategy, ) - - -class Settings(BaseSettings): - """Authentication settings""" - secret_key: str - algorithm: str = "HS256" - # Set to None so tokens don't expire - access_token_expire_seconds: int = None +from .config import AuthSettings class Authentication: @@ -29,7 +21,7 @@ class Authentication: CRYPT_CTX = CryptContext(schemes=["bcrypt"], deprecated="auto") def __init__(self, token_url: str): - self._settings = Settings() + self._settings = AuthSettings() self._token_url = token_url @classmethod diff --git a/api/config.py b/api/config.py new file mode 100644 index 00000000..3bbaa9fa --- /dev/null +++ b/api/config.py @@ -0,0 +1,32 @@ +# SPDX-License-Identifier: LGPL-2.1-or-later +# +# Copyright (C) 2023 Collabora Limited +# Author: Jeny Sadadia + +"""Module settings""" + +from pydantic import BaseSettings, EmailStr + + +class AuthSettings(BaseSettings): + """Authentication settings""" + secret_key: str + algorithm: str = "HS256" + # Set to None so tokens don't expire + access_token_expire_seconds: float = None + + +class PubSubSettings(BaseSettings): + """Pub/Sub settings loaded from the environment""" + cloud_events_source: str = "https://api.kernelci.org/" + redis_host: str = "redis" + redis_db_number: int = 1 + keep_alive_period: int = 45 + + +class EmailSettings(BaseSettings): + """Email settings""" + smtp_host: str + smtp_port: int + email_sender: EmailStr + email_password: str diff --git a/api/email_sender.py b/api/email_sender.py index ed0bc6c7..29857a38 100644 --- a/api/email_sender.py +++ b/api/email_sender.py @@ -11,21 +11,13 @@ import email import email.mime.text import smtplib -from pydantic import BaseSettings, EmailStr - - -class Settings(BaseSettings): - """Email settings""" - smtp_host: str - smtp_port: int - email_sender: EmailStr - email_password: str +from .config import EmailSettings class EmailSender: """Class to send email report using SMTP""" def __init__(self): - self._settings = Settings() + self._settings = EmailSettings() def _smtp_connect(self): """Method to create a connection with SMTP server""" diff --git a/api/pubsub.py b/api/pubsub.py index 7818e2f1..26016243 100644 --- a/api/pubsub.py +++ b/api/pubsub.py @@ -9,15 +9,8 @@ import aioredis from cloudevents.http import CloudEvent, to_json -from pydantic import BaseModel, BaseSettings, Field - - -class Settings(BaseSettings): - """Pub/Sub settings loaded from the environment""" - cloud_events_source: str = "https://api.kernelci.org/" - redis_host: str = "redis" - redis_db_number: int = 1 - keep_alive_period: int = 45 +from pydantic import BaseModel, Field +from .config import PubSubSettings class Subscription(BaseModel): @@ -49,7 +42,8 @@ async def create(cls, *args, **kwargs): return pubsub def __init__(self, host=None, db_number=None): - self._settings = Settings() + # self._settings = Settings() + self._settings = PubSubSettings() if host is None: host = self._settings.redis_host if db_number is None: diff --git a/api/user_manager.py b/api/user_manager.py index 19f0766e..8e7aab83 100644 --- a/api/user_manager.py +++ b/api/user_manager.py @@ -18,13 +18,13 @@ from beanie import PydanticObjectId import jinja2 from .user_models import User -from .auth import Settings +from .config import AuthSettings from .email_sender import EmailSender class UserManager(ObjectIDIDMixin, BaseUserManager[User, PydanticObjectId]): """User management logic""" - settings = Settings() + settings = AuthSettings() reset_password_token_secret = settings.secret_key verification_token_secret = settings.secret_key