Skip to content

PoC: fastapi-users #363

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 9 commits into from
Closed

PoC: fastapi-users #363

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
587d9ae
requirements.txt: add `fastapi-users` package
Sep 25, 2023
01b6570
api.models: extend models from `fastapi-users`
Sep 25, 2023
c831920
api.db: get database instance
Sep 25, 2023
8c47587
api.main: initialize Beanie on app startup
Sep 25, 2023
b68ee12
api.auth: Implement `UserManager`
Sep 25, 2023
6209bad
api.auth: create authentication backend
Sep 25, 2023
51ed081
api.auth: create an instance of `FastAPIUsers`
Sep 25, 2023
faf2d00
api.models: add models for `fastapi-users` routers
Sep 25, 2023
a51e911
api.main: include routers provided by `fastapi-users`
Sep 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions api/auth.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,13 @@
from pydantic import BaseModel, BaseSettings, Field
from .db import Database
from .models import User
from fastapi_users.db import BeanieUserDatabase, ObjectIDIDMixin
from fastapi_users import BaseUserManager, FastAPIUsers
from typing import Optional, Any, Dict
from .models import TestUser
from beanie import PydanticObjectId
from fastapi import Depends, Request, Response
from fastapi_users.authentication import AuthenticationBackend, BearerTransport, JWTStrategy


class Token(BaseModel):
Expand Down Expand Up @@ -125,3 +132,71 @@ async def validate_scopes(self, requested_scopes):
if scope not in self._user_scopes:
return False, scope
return True, None


SECRET = "SECRET"

class UserManager(ObjectIDIDMixin, BaseUserManager[TestUser, PydanticObjectId]):
reset_password_token_secret = SECRET
verification_token_secret = SECRET

async def on_after_register(self, user: TestUser, request: Optional[Request] = None):
print(f"User {user.id} has registered.")

async def on_after_login(
self,
user: TestUser,
request: Optional[Request] = None,
response: Optional[Response] = None,
):
print(f"User {user.id} logged in.")

async def on_after_forgot_password(
self, user: TestUser, token: str, request: Optional[Request] = None
):
print(f"User {user.id} has forgot their password. Reset token: {token}")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess this is where an email would be sent with the reset token in a URL, and then the user would then come back via the on_after_request_verify method?

We could probably test this on the command line using an API token initially although this shouldn't be allowed in production as any leaked token should not enable changing the user's password imho. One issue of course is it's not possible to invalidate a JWT token, it just has a built-in expiry date. I wonder what's the standard way to deal with this in other APIs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I guess this is where an email would be sent with the reset token in a URL, and then the user would then come back via the on_after_request_verify method?

Yes, that is true.


async def on_after_request_verify(
self, user: TestUser, token: str, request: Optional[Request] = None
):
print(f"Verification requested for user {user.id}. Verification token: {token}")
return {"token": token}

async def on_after_verify(
self, user: TestUser, request: Optional[Request] = None
):
print(f"Verification successful for user {user.id}")

async def on_after_update(
self,
user: TestUser,
update_dict: Dict[str, Any],
request: Optional[Request] = None,
):
print(f"User {user.id} has been updated with {update_dict}.")

async def on_before_delete(self, user: TestUser, request: Optional[Request] = None):
print(f"User {user.id} is going to be deleted")

async def get_user_db():
"""Database adapter for fastapi-users"""
yield BeanieUserDatabase(TestUser)

async def get_user_manager(user_db: BeanieUserDatabase = Depends(get_user_db)):
yield UserManager(user_db)

bearer_transport = BearerTransport(tokenUrl="auth/jwt/login")

def get_jwt_strategy() -> JWTStrategy:
return JWTStrategy(secret=SECRET, lifetime_seconds=3600)

auth_backend = AuthenticationBackend(
name="jwt",
transport=bearer_transport,
get_strategy=get_jwt_strategy,
)

fastapi_users_instance = FastAPIUsers[TestUser, PydanticObjectId](
get_user_manager,
[auth_backend],
)
7 changes: 6 additions & 1 deletion api/db.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
from bson import ObjectId
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, User, Regression, UserGroup, TestUser
from fastapi_users.db import BeanieUserDatabase


class Database:
Expand Down Expand Up @@ -38,6 +39,10 @@ def __init__(self, service='mongodb://db:27017', db_name='kernelci'):
self._motor = motor_asyncio.AsyncIOMotorClient(service)
self._db = self._motor[db_name]

@property
def db(self):
return self._db

def _get_collection(self, model):
col = self.COLLECTIONS[model]
return self._db[col]
Expand Down
49 changes: 49 additions & 0 deletions api/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,18 @@
User,
UserGroup,
UserProfile,
TestUser,
UserCreate,
UserRead,
UserUpdate,
Password,
get_model_from_kind
)
from .paginator_models import PageModel
from .pubsub import PubSub, Subscription
from beanie import init_beanie
from .auth import fastapi_users_instance, auth_backend


app = FastAPI()
db = Database(service=(os.getenv('MONGO_SERVICE') or 'mongodb://db:27017'))
Expand All @@ -63,6 +70,15 @@ async def create_indexes():
"""Startup event handler to create database indexes"""
await db.create_indexes()

@app.on_event('startup')
async def fastapi_users_beanie_init():
"""Startup event handler for beanie"""
await init_beanie(
database=db.db,
document_models=[
TestUser,
],
)
Comment on lines +76 to +81
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe this call could be moved to api.auth with a method in the Database class to avoid exposing the underlying .db object? That way only the Database class would follow the object-oriented principle of accessing its own data. We already know we need to import the beanie modules in api.auth so it wouldn't be adding extra dependencies there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason I kept this call in main.py was that init_beanie needs an actual instantiated database object and not just the class reference. Still, I'll take another look if I can move things around.


@app.exception_handler(ValueError)
async def value_error_exception_handler(request: Request, exc: ValueError):
Expand Down Expand Up @@ -630,6 +646,38 @@ async def put_regression(regression_id: str, regression: Regression,
return obj


app.include_router(
fastapi_users_instance.get_auth_router(auth_backend, requires_verification=True),
prefix="/auth/jwt",
tags=["auth"]
)
app.include_router(
fastapi_users_instance.get_register_router(UserRead, UserCreate),
prefix="/auth",
tags=["auth"],
)
app.include_router(
fastapi_users_instance.get_reset_password_router(),
prefix="/auth",
tags=["auth"],
)
app.include_router(
fastapi_users_instance.get_verify_router(UserRead),
prefix="/auth",
tags=["auth"],
)
app.include_router(
fastapi_users_instance.get_users_router(UserRead, UserUpdate, requires_verification=True),
prefix="/users",
tags=["users"],
)

current_active_user = fastapi_users_instance.current_user(active=True)

@app.get("/authenticated-route")
async def authenticated_route(user: TestUser = Depends(current_active_user)):
return {"message": f"Hello {user.username}!"}

app = VersionedFastAPI(
app,
version_format='{major}',
Expand All @@ -639,6 +687,7 @@ async def put_regression(regression_id: str, regression: Regression,
on_startup=[
pubsub_startup,
create_indexes,
fastapi_users_beanie_init,
]
)

Expand Down
23 changes: 23 additions & 0 deletions api/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@
FileUrl,
SecretStr,
)
from beanie import Indexed, Document
from fastapi_users.db import BeanieBaseUser
from fastapi_users import schemas
from beanie import PydanticObjectId, Indexed
from typing import Optional
Comment on lines +27 to +31
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if these should rather be in a separate module to avoid "polluting" the whole api.models namespace with fastapi_users imports.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, we should have a separate model file just like we have for pagination related models.



class PyObjectId(ObjectId):
Expand Down Expand Up @@ -148,6 +153,24 @@ def create_indexes(cls, collection):
collection.create_index("profile.username", unique=True)


class TestUser(BeanieBaseUser, Document):
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the class called TestUser just because this is a PoC?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, exactly.

"""Test user"""
username: Indexed(str, unique=True)


class UserRead(schemas.BaseUser[PydanticObjectId]):
username: Indexed(str, unique=True)


class UserCreate(schemas.BaseUserCreate):
username: Indexed(str, unique=True)


class UserUpdate(schemas.BaseUserUpdate):
username: Optional[Indexed(str, unique=True)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of interest, why is this one optional whereas it's not in the other models?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's to allow users to only send fields that are updating. If I remove the Optional, the endpoint would expect username field in the request even if the user wants it to be unchanged.
If you look at the definition of the parent class BaseUserUpdate, you'll see all the fields are optional.

class BaseUserUpdate(CreateUpdateDictModel):
    password: Optional[str]
    email: Optional[EmailStr]
    is_active: Optional[bool]
    is_superuser: Optional[bool]
    is_verified: Optional[bool]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I think this brings us back to the idea I mentioned a while ago about having a different model for the API and for the database.




class KernelVersion(BaseModel):
"""Linux kernel version model"""
version: int = Field(
Expand Down
1 change: 1 addition & 0 deletions docker/api/requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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