From 78be936297b7c80fc1b47f9fce356565e3745113 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fran=C3=A7ois=20Voron?= Date: Fri, 6 May 2022 13:17:22 +0200 Subject: [PATCH] Put exceptions in a dedicated module to avoid circular imports --- .../cookbook_create_user_programmatically.py | 2 +- fastapi_users/__init__.py | 4 +- .../authentication/strategy/db/strategy.py | 6 +- fastapi_users/authentication/strategy/jwt.py | 6 +- .../authentication/strategy/redis.py | 6 +- fastapi_users/exceptions.py | 38 ++++++++ fastapi_users/manager.py | 95 ++++++------------- fastapi_users/router/register.py | 13 +-- fastapi_users/router/reset.py | 23 ++--- fastapi_users/router/users.py | 21 ++-- fastapi_users/router/verify.py | 21 ++-- tests/conftest.py | 14 +-- tests/test_manager.py | 6 +- tests/test_router_reset.py | 2 +- tests/test_router_verify.py | 2 +- 15 files changed, 118 insertions(+), 141 deletions(-) create mode 100644 fastapi_users/exceptions.py diff --git a/docs/src/cookbook_create_user_programmatically.py b/docs/src/cookbook_create_user_programmatically.py index 70724c06..01455d97 100644 --- a/docs/src/cookbook_create_user_programmatically.py +++ b/docs/src/cookbook_create_user_programmatically.py @@ -3,7 +3,7 @@ import contextlib from app.db import get_async_session, get_user_db from app.schemas import UserCreate from app.users import get_user_manager -from fastapi_users.manager import UserAlreadyExists +from fastapi_users.exceptions import UserAlreadyExists get_async_session_context = contextlib.asynccontextmanager(get_async_session) get_user_db_context = contextlib.asynccontextmanager(get_user_db) diff --git a/fastapi_users/__init__.py b/fastapi_users/__init__.py index 1096133c..8cb1021d 100644 --- a/fastapi_users/__init__.py +++ b/fastapi_users/__init__.py @@ -3,16 +3,16 @@ __version__ = "10.0.0" from fastapi_users import models, schemas # noqa: F401 +from fastapi_users.exceptions import InvalidID, InvalidPasswordException from fastapi_users.fastapi_users import FastAPIUsers # noqa: F401 from fastapi_users.manager import ( # noqa: F401 BaseUserManager, IntegerIDMixin, - InvalidID, - InvalidPasswordException, UUIDIDMixin, ) __all__ = [ + "models", "schemas", "FastAPIUsers", "BaseUserManager", diff --git a/fastapi_users/authentication/strategy/db/strategy.py b/fastapi_users/authentication/strategy/db/strategy.py index c9e40ebc..d7c3c7a3 100644 --- a/fastapi_users/authentication/strategy/db/strategy.py +++ b/fastapi_users/authentication/strategy/db/strategy.py @@ -2,11 +2,11 @@ import secrets from datetime import datetime, timedelta, timezone from typing import Any, Dict, Generic, Optional -from fastapi_users import models +from fastapi_users import exceptions, models from fastapi_users.authentication.strategy.base import Strategy from fastapi_users.authentication.strategy.db.adapter import AccessTokenDatabase from fastapi_users.authentication.strategy.db.models import AP -from fastapi_users.manager import BaseUserManager, InvalidID, UserNotExists +from fastapi_users.manager import BaseUserManager class DatabaseStrategy( @@ -37,7 +37,7 @@ class DatabaseStrategy( try: parsed_id = user_manager.parse_id(access_token.user_id) return await user_manager.get(parsed_id) - except (UserNotExists, InvalidID): + except (exceptions.UserNotExists, exceptions.InvalidID): return None async def write_token(self, user: models.UP) -> str: diff --git a/fastapi_users/authentication/strategy/jwt.py b/fastapi_users/authentication/strategy/jwt.py index 4bad548d..39866062 100644 --- a/fastapi_users/authentication/strategy/jwt.py +++ b/fastapi_users/authentication/strategy/jwt.py @@ -2,13 +2,13 @@ from typing import Generic, List, Optional import jwt -from fastapi_users import models +from fastapi_users import exceptions, models from fastapi_users.authentication.strategy.base import ( Strategy, StrategyDestroyNotSupportedError, ) from fastapi_users.jwt import SecretType, decode_jwt, generate_jwt -from fastapi_users.manager import BaseUserManager, InvalidID, UserNotExists +from fastapi_users.manager import BaseUserManager class JWTStrategy(Strategy[models.UP, models.ID], Generic[models.UP, models.ID]): @@ -53,7 +53,7 @@ class JWTStrategy(Strategy[models.UP, models.ID], Generic[models.UP, models.ID]) try: parsed_id = user_manager.parse_id(user_id) return await user_manager.get(parsed_id) - except (UserNotExists, InvalidID): + except (exceptions.UserNotExists, exceptions.InvalidID): return None async def write_token(self, user: models.UP) -> str: diff --git a/fastapi_users/authentication/strategy/redis.py b/fastapi_users/authentication/strategy/redis.py index 616ac7e5..3c3bff42 100644 --- a/fastapi_users/authentication/strategy/redis.py +++ b/fastapi_users/authentication/strategy/redis.py @@ -3,9 +3,9 @@ from typing import Generic, Optional import aioredis -from fastapi_users import models +from fastapi_users import exceptions, models from fastapi_users.authentication.strategy.base import Strategy -from fastapi_users.manager import BaseUserManager, InvalidID, UserNotExists +from fastapi_users.manager import BaseUserManager class RedisStrategy(Strategy[models.UP, models.ID], Generic[models.UP, models.ID]): @@ -26,7 +26,7 @@ class RedisStrategy(Strategy[models.UP, models.ID], Generic[models.UP, models.ID try: parsed_id = user_manager.parse_id(user_id) return await user_manager.get(parsed_id) - except (UserNotExists, InvalidID): + except (exceptions.UserNotExists, exceptions.InvalidID): return None async def write_token(self, user: models.UP) -> str: diff --git a/fastapi_users/exceptions.py b/fastapi_users/exceptions.py new file mode 100644 index 00000000..c073eb69 --- /dev/null +++ b/fastapi_users/exceptions.py @@ -0,0 +1,38 @@ +from typing import Any + + +class FastAPIUsersException(Exception): + pass + + +class InvalidID(FastAPIUsersException): + pass + + +class UserAlreadyExists(FastAPIUsersException): + pass + + +class UserNotExists(FastAPIUsersException): + pass + + +class UserInactive(FastAPIUsersException): + pass + + +class UserAlreadyVerified(FastAPIUsersException): + pass + + +class InvalidVerifyToken(FastAPIUsersException): + pass + + +class InvalidResetPasswordToken(FastAPIUsersException): + pass + + +class InvalidPasswordException(FastAPIUsersException): + def __init__(self, reason: Any) -> None: + self.reason = reason diff --git a/fastapi_users/manager.py b/fastapi_users/manager.py index a4d1c1cd..8b63dfa3 100644 --- a/fastapi_users/manager.py +++ b/fastapi_users/manager.py @@ -5,7 +5,7 @@ import jwt from fastapi import Request from fastapi.security import OAuth2PasswordRequestForm -from fastapi_users import models, schemas +from fastapi_users import exceptions, models, schemas from fastapi_users.db import BaseUserDatabase from fastapi_users.jwt import SecretType, decode_jwt, generate_jwt from fastapi_users.password import PasswordHelper, PasswordHelperProtocol @@ -15,43 +15,6 @@ RESET_PASSWORD_TOKEN_AUDIENCE = "fastapi-users:reset" VERIFY_USER_TOKEN_AUDIENCE = "fastapi-users:verify" -class FastAPIUsersException(Exception): - pass - - -class InvalidID(FastAPIUsersException): - pass - - -class UserAlreadyExists(FastAPIUsersException): - pass - - -class UserNotExists(FastAPIUsersException): - pass - - -class UserInactive(FastAPIUsersException): - pass - - -class UserAlreadyVerified(FastAPIUsersException): - pass - - -class InvalidVerifyToken(FastAPIUsersException): - pass - - -class InvalidResetPasswordToken(FastAPIUsersException): - pass - - -class InvalidPasswordException(FastAPIUsersException): - def __init__(self, reason: Any) -> None: - self.reason = reason - - class BaseUserManager(Generic[models.UP, models.ID]): """ User management logic. @@ -109,7 +72,7 @@ class BaseUserManager(Generic[models.UP, models.ID]): user = await self.user_db.get(id) if user is None: - raise UserNotExists() + raise exceptions.UserNotExists() return user @@ -124,7 +87,7 @@ class BaseUserManager(Generic[models.UP, models.ID]): user = await self.user_db.get_by_email(user_email) if user is None: - raise UserNotExists() + raise exceptions.UserNotExists() return user @@ -140,7 +103,7 @@ class BaseUserManager(Generic[models.UP, models.ID]): user = await self.user_db.get_by_oauth_account(oauth, account_id) if user is None: - raise UserNotExists() + raise exceptions.UserNotExists() return user @@ -167,7 +130,7 @@ class BaseUserManager(Generic[models.UP, models.ID]): existing_user = await self.user_db.get_by_email(user_create.email) if existing_user is not None: - raise UserAlreadyExists() + raise exceptions.UserAlreadyExists() user_dict = ( user_create.create_update_dict() @@ -226,12 +189,12 @@ class BaseUserManager(Generic[models.UP, models.ID]): try: user = await self.get_by_oauth_account(oauth_name, account_id) - except UserNotExists: + except exceptions.UserNotExists: try: # Link account user = await self.get_by_email(account_email) user = await self.user_db.add_oauth_account(user, oauth_account_dict) - except UserNotExists: + except exceptions.UserNotExists: # Create account password = self.password_helper.generate() user_dict = { @@ -269,9 +232,9 @@ class BaseUserManager(Generic[models.UP, models.ID]): :raises UserAlreadyVerified: The user is already verified. """ if not user.is_active: - raise UserInactive() + raise exceptions.UserInactive() if user.is_verified: - raise UserAlreadyVerified() + raise exceptions.UserAlreadyVerified() token_data = { "user_id": str(user.id), @@ -307,29 +270,29 @@ class BaseUserManager(Generic[models.UP, models.ID]): [self.verification_token_audience], ) except jwt.PyJWTError: - raise InvalidVerifyToken() + raise exceptions.InvalidVerifyToken() try: user_id = data["user_id"] email = data["email"] except KeyError: - raise InvalidVerifyToken() + raise exceptions.InvalidVerifyToken() try: user = await self.get_by_email(email) - except UserNotExists: - raise InvalidVerifyToken() + except exceptions.UserNotExists: + raise exceptions.InvalidVerifyToken() try: parsed_id = self.parse_id(user_id) - except InvalidID: - raise InvalidVerifyToken() + except exceptions.InvalidID: + raise exceptions.InvalidVerifyToken() if parsed_id != user.id: - raise InvalidVerifyToken() + raise exceptions.InvalidVerifyToken() if user.is_verified: - raise UserAlreadyVerified() + raise exceptions.UserAlreadyVerified() verified_user = await self._update(user, {"is_verified": True}) @@ -351,7 +314,7 @@ class BaseUserManager(Generic[models.UP, models.ID]): :raises UserInactive: The user is inactive. """ if not user.is_active: - raise UserInactive() + raise exceptions.UserInactive() token_data = { "user_id": str(user.id), @@ -388,22 +351,22 @@ class BaseUserManager(Generic[models.UP, models.ID]): [self.reset_password_token_audience], ) except jwt.PyJWTError: - raise InvalidResetPasswordToken() + raise exceptions.InvalidResetPasswordToken() try: user_id = data["user_id"] except KeyError: - raise InvalidResetPasswordToken() + raise exceptions.InvalidResetPasswordToken() try: parsed_id = self.parse_id(user_id) - except InvalidID: - raise InvalidResetPasswordToken() + except exceptions.InvalidID: + raise exceptions.InvalidResetPasswordToken() user = await self.get(parsed_id) if not user.is_active: - raise UserInactive() + raise exceptions.UserInactive() updated_user = await self._update(user, {"password": password}) @@ -565,7 +528,7 @@ class BaseUserManager(Generic[models.UP, models.ID]): """ try: user = await self.get_by_email(credentials.username) - except UserNotExists: + 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) @@ -588,8 +551,8 @@ class BaseUserManager(Generic[models.UP, models.ID]): if field == "email" and value != user.email: try: await self.get_by_email(value) - raise UserAlreadyExists() - except UserNotExists: + raise exceptions.UserAlreadyExists() + except exceptions.UserNotExists: validated_update_dict["email"] = value validated_update_dict["is_verified"] = False elif field == "password": @@ -609,17 +572,17 @@ class UUIDIDMixin: try: return uuid.UUID(value) except ValueError as e: - raise InvalidID() from e + raise exceptions.InvalidID() from e class IntegerIDMixin: def parse_id(self, value: Any) -> int: if isinstance(value, float): - raise InvalidID() + raise exceptions.InvalidID() try: return int(value) except ValueError as e: - raise InvalidID() from e + raise exceptions.InvalidID() from e UserManagerDependency = DependencyCallable[BaseUserManager[models.UP, models.ID]] diff --git a/fastapi_users/router/register.py b/fastapi_users/router/register.py index 3eab139e..cbd0867f 100644 --- a/fastapi_users/router/register.py +++ b/fastapi_users/router/register.py @@ -2,13 +2,8 @@ from typing import Type from fastapi import APIRouter, Depends, HTTPException, Request, status -from fastapi_users import models, schemas -from fastapi_users.manager import ( - BaseUserManager, - InvalidPasswordException, - UserAlreadyExists, - UserManagerDependency, -) +from fastapi_users import exceptions, models, schemas +from fastapi_users.manager import BaseUserManager, UserManagerDependency from fastapi_users.router.common import ErrorCode, ErrorModel @@ -62,12 +57,12 @@ def get_register_router( created_user = await user_manager.create( user_create, safe=True, request=request ) - except UserAlreadyExists: + except exceptions.UserAlreadyExists: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=ErrorCode.REGISTER_USER_ALREADY_EXISTS, ) - except InvalidPasswordException as e: + except exceptions.InvalidPasswordException as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail={ diff --git a/fastapi_users/router/reset.py b/fastapi_users/router/reset.py index 95e2e93d..1e3db5f7 100644 --- a/fastapi_users/router/reset.py +++ b/fastapi_users/router/reset.py @@ -1,15 +1,8 @@ from fastapi import APIRouter, Body, Depends, HTTPException, Request, status from pydantic import EmailStr -from fastapi_users import models -from fastapi_users.manager import ( - BaseUserManager, - InvalidPasswordException, - InvalidResetPasswordToken, - UserInactive, - UserManagerDependency, - UserNotExists, -) +from fastapi_users import exceptions, models +from fastapi_users.manager import BaseUserManager, UserManagerDependency from fastapi_users.openapi import OpenAPIResponseType from fastapi_users.router.common import ErrorCode, ErrorModel @@ -57,12 +50,12 @@ def get_reset_password_router( ): try: user = await user_manager.get_by_email(email) - except UserNotExists: + except exceptions.UserNotExists: return None try: await user_manager.forgot_password(user, request) - except UserInactive: + except exceptions.UserInactive: pass return None @@ -80,12 +73,16 @@ def get_reset_password_router( ): try: await user_manager.reset_password(token, password, request) - except (InvalidResetPasswordToken, UserNotExists, UserInactive): + except ( + exceptions.InvalidResetPasswordToken, + exceptions.UserNotExists, + exceptions.UserInactive, + ): raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=ErrorCode.RESET_PASSWORD_BAD_TOKEN, ) - except InvalidPasswordException as e: + except exceptions.InvalidPasswordException as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail={ diff --git a/fastapi_users/router/users.py b/fastapi_users/router/users.py index b6a48caf..87ea17de 100644 --- a/fastapi_users/router/users.py +++ b/fastapi_users/router/users.py @@ -2,16 +2,9 @@ from typing import Any, Type from fastapi import APIRouter, Depends, HTTPException, Request, Response, status -from fastapi_users import models, schemas +from fastapi_users import exceptions, models, schemas from fastapi_users.authentication import Authenticator -from fastapi_users.manager import ( - BaseUserManager, - InvalidID, - InvalidPasswordException, - UserAlreadyExists, - UserManagerDependency, - UserNotExists, -) +from fastapi_users.manager import BaseUserManager, UserManagerDependency from fastapi_users.router.common import ErrorCode, ErrorModel @@ -39,7 +32,7 @@ def get_users_router( try: parsed_id = user_manager.parse_id(id) return await user_manager.get(parsed_id) - except (UserNotExists, InvalidID) as e: + except (exceptions.UserNotExists, exceptions.InvalidID) as e: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND) from e @router.get( @@ -103,7 +96,7 @@ def get_users_router( return await user_manager.update( user_update, user, safe=True, request=request ) - except InvalidPasswordException as e: + except exceptions.InvalidPasswordException as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail={ @@ -111,7 +104,7 @@ def get_users_router( "reason": e.reason, }, ) - except UserAlreadyExists: + except exceptions.UserAlreadyExists: raise HTTPException( status.HTTP_400_BAD_REQUEST, detail=ErrorCode.UPDATE_USER_EMAIL_ALREADY_EXISTS, @@ -189,7 +182,7 @@ def get_users_router( return await user_manager.update( user_update, user, safe=False, request=request ) - except InvalidPasswordException as e: + except exceptions.InvalidPasswordException as e: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail={ @@ -197,7 +190,7 @@ def get_users_router( "reason": e.reason, }, ) - except UserAlreadyExists: + except exceptions.UserAlreadyExists: raise HTTPException( status.HTTP_400_BAD_REQUEST, detail=ErrorCode.UPDATE_USER_EMAIL_ALREADY_EXISTS, diff --git a/fastapi_users/router/verify.py b/fastapi_users/router/verify.py index af6578f5..76e084d8 100644 --- a/fastapi_users/router/verify.py +++ b/fastapi_users/router/verify.py @@ -3,15 +3,8 @@ from typing import Type from fastapi import APIRouter, Body, Depends, HTTPException, Request, status from pydantic import EmailStr -from fastapi_users import models, schemas -from fastapi_users.manager import ( - BaseUserManager, - InvalidVerifyToken, - UserAlreadyVerified, - UserInactive, - UserManagerDependency, - UserNotExists, -) +from fastapi_users import exceptions, models, schemas +from fastapi_users.manager import BaseUserManager, UserManagerDependency from fastapi_users.router.common import ErrorCode, ErrorModel @@ -34,7 +27,11 @@ def get_verify_router( try: user = await user_manager.get_by_email(email) await user_manager.request_verify(user, request) - except (UserNotExists, UserInactive, UserAlreadyVerified): + except ( + exceptions.UserNotExists, + exceptions.UserInactive, + exceptions.UserAlreadyVerified, + ): pass return None @@ -73,12 +70,12 @@ def get_verify_router( ): try: return await user_manager.verify(token, request) - except (InvalidVerifyToken, UserNotExists): + except (exceptions.InvalidVerifyToken, exceptions.UserNotExists): raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=ErrorCode.VERIFY_USER_BAD_TOKEN, ) - except UserAlreadyVerified: + except exceptions.UserAlreadyVerified: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, detail=ErrorCode.VERIFY_USER_ALREADY_VERIFIED, diff --git a/tests/conftest.py b/tests/conftest.py index 3a6ddd48..7ad46060 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -22,18 +22,12 @@ from httpx_oauth.oauth2 import OAuth2 from pydantic import UUID4, SecretStr from pytest_mock import MockerFixture -from fastapi_users import models, schemas +from fastapi_users import exceptions, models, schemas from fastapi_users.authentication import AuthenticationBackend, BearerTransport from fastapi_users.authentication.strategy import Strategy from fastapi_users.db import BaseUserDatabase from fastapi_users.jwt import SecretType -from fastapi_users.manager import ( - BaseUserManager, - InvalidID, - InvalidPasswordException, - UserNotExists, - UUIDIDMixin, -) +from fastapi_users.manager import BaseUserManager, UUIDIDMixin from fastapi_users.openapi import OpenAPIResponseType from fastapi_users.password import PasswordHelper @@ -101,7 +95,7 @@ class BaseTestUserManager( self, password: str, user: Union[schemas.UC, models.UP] ) -> None: if len(password) < 3: - raise InvalidPasswordException( + raise exceptions.InvalidPasswordException( reason="Password should be at least 3 characters" ) @@ -533,7 +527,7 @@ class MockStrategy(Strategy[UserModel, IDType]): try: parsed_id = user_manager.parse_id(token) return await user_manager.get(parsed_id) - except (InvalidID, UserNotExists): + except (exceptions.InvalidID, exceptions.UserNotExists): return None return None diff --git a/tests/test_manager.py b/tests/test_manager.py index 634668cf..0ab995f7 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -5,9 +5,7 @@ from fastapi.security import OAuth2PasswordRequestForm from pydantic import UUID4 from pytest_mock import MockerFixture -from fastapi_users.jwt import decode_jwt, generate_jwt -from fastapi_users.manager import ( - IntegerIDMixin, +from fastapi_users.exceptions import ( InvalidID, InvalidPasswordException, InvalidResetPasswordToken, @@ -17,6 +15,8 @@ from fastapi_users.manager import ( UserInactive, UserNotExists, ) +from fastapi_users.jwt import decode_jwt, generate_jwt +from fastapi_users.manager import IntegerIDMixin from tests.conftest import ( UserCreate, UserManagerMock, diff --git a/tests/test_router_reset.py b/tests/test_router_reset.py index 2193f80f..fdfbb378 100644 --- a/tests/test_router_reset.py +++ b/tests/test_router_reset.py @@ -4,7 +4,7 @@ import httpx import pytest from fastapi import FastAPI, status -from fastapi_users.manager import ( +from fastapi_users.exceptions import ( InvalidPasswordException, InvalidResetPasswordToken, UserInactive, diff --git a/tests/test_router_verify.py b/tests/test_router_verify.py index f7619c45..488135ba 100644 --- a/tests/test_router_verify.py +++ b/tests/test_router_verify.py @@ -4,7 +4,7 @@ import httpx import pytest from fastapi import FastAPI, status -from fastapi_users.manager import ( +from fastapi_users.exceptions import ( InvalidVerifyToken, UserAlreadyVerified, UserInactive,