-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
fast-jelly! the Gel auth extension bindings for FastAPI #3
Draft
fantix
wants to merge
8
commits into
main
Choose a base branch
from
fast-jelly
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
796a45c
Initial database setup
scotttrinh 61d652e
Implement EdgeDB+FastAPI guide application
scotttrinh 3db8e72
Add auth extension handling
scotttrinh d353276
Support Python 3.9
fantix e713265
Fix logging
fantix e3aa9e4
black
fantix e98541f
Refactor
fantix fada474
Add .gitattributes to fold generated code
fantix File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,7 @@ | ||
# Basic Async Example | ||
|
||
```console | ||
$ uv run main.py | ||
$ uv run uvicorn app.main:fast_api | ||
``` | ||
|
||
## Development | ||
|
Empty file.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,147 @@ | ||
from __future__ import annotations | ||
from typing import Annotated | ||
|
||
import json | ||
import logging | ||
from http import HTTPStatus | ||
|
||
from fastapi import APIRouter, Depends, Form | ||
from fastapi.responses import RedirectResponse | ||
from gel_auth_fastapi import ( | ||
make_email_password, | ||
email_password as core_email_password, | ||
) | ||
|
||
from .config import BASE_URL | ||
from .gel_client import client | ||
from .queries import create_user_async_edgeql as create_user_qry | ||
|
||
logger = logging.getLogger("fast_jelly") | ||
router = APIRouter() | ||
email_password = make_email_password( | ||
client, | ||
verify_url=f"{BASE_URL}/auth/verify", | ||
reset_url=f"{BASE_URL}/ui/reset-password", | ||
) | ||
|
||
|
||
@router.post( | ||
"/auth/register", | ||
response_class=RedirectResponse, | ||
status_code=HTTPStatus.SEE_OTHER, | ||
) | ||
async def register( | ||
email: Annotated[str, Form()], | ||
sign_up_response: Annotated[ | ||
core_email_password.SignUpResponse, Depends(email_password.handle_sign_up) | ||
], | ||
): | ||
if not isinstance(sign_up_response, core_email_password.SignUpFailedResponse): | ||
user = await create_user_qry.create_user( | ||
client, | ||
name=email, | ||
identity_id=sign_up_response.identity_id, | ||
) | ||
print(f"Created user: {json.dumps(user, default=str)}") | ||
|
||
match sign_up_response: | ||
case core_email_password.SignUpCompleteResponse(): | ||
return "/" | ||
case core_email_password.SignUpVerificationRequiredResponse(): | ||
return "/signin?incomplete=verification_required" | ||
case core_email_password.SignUpFailedResponse(): | ||
logger.error("sign up failed: %s", sign_up_response) | ||
return "/signin?error=failure" | ||
case _: | ||
raise Exception("Invalid sign up response") | ||
|
||
|
||
@router.post( | ||
"/auth/authenticate", | ||
response_class=RedirectResponse, | ||
status_code=HTTPStatus.SEE_OTHER, | ||
) | ||
async def authenticate( | ||
sign_in_response: Annotated[ | ||
core_email_password.SignInResponse, Depends(email_password.handle_sign_in) | ||
], | ||
): | ||
match sign_in_response: | ||
case core_email_password.SignInCompleteResponse(): | ||
return "/" | ||
case core_email_password.SignInVerificationRequiredResponse(): | ||
return "/signin?incomplete=verification_required" | ||
case core_email_password.SignInFailedResponse(): | ||
logger.error("sign in failed: %s", sign_in_response) | ||
return "/signin?error=failure" | ||
case _: | ||
raise Exception("Invalid sign in response") | ||
|
||
|
||
@router.get( | ||
"/auth/verify", | ||
response_class=RedirectResponse, | ||
status_code=HTTPStatus.SEE_OTHER, | ||
) | ||
async def verify( | ||
verify_response: Annotated[ | ||
core_email_password.EmailVerificationResponse, | ||
Depends(email_password.handle_verify_email), | ||
], | ||
): | ||
match verify_response: | ||
case core_email_password.EmailVerificationCompleteResponse(): | ||
return "/" | ||
case core_email_password.EmailVerificationMissingProofResponse(): | ||
return "/signin?incomplete=verify" | ||
case core_email_password.EmailVerificationFailedResponse(): | ||
logger.error("verify email failed: %s", verify_response) | ||
return "/signin?error=failure" | ||
case _: | ||
raise Exception("Invalid verify email response") | ||
|
||
|
||
@router.post( | ||
"/auth/send-password-reset", | ||
response_class=RedirectResponse, | ||
status_code=HTTPStatus.SEE_OTHER, | ||
) | ||
async def send_password_reset( | ||
send_password_reset_response: Annotated[ | ||
core_email_password.SendPasswordResetEmailResponse, | ||
Depends(email_password.handle_send_password_reset), | ||
], | ||
): | ||
match send_password_reset_response: | ||
case core_email_password.SendPasswordResetEmailCompleteResponse(): | ||
return "/signin?incomplete=password_reset_sent" | ||
case core_email_password.SendPasswordResetEmailFailedResponse(): | ||
logger.error( | ||
"send password reset failed: %s", send_password_reset_response | ||
) | ||
return "/signin?error=failure" | ||
case _: | ||
raise Exception("Invalid send password reset response") | ||
|
||
|
||
@router.post( | ||
"/auth/reset-password", | ||
response_class=RedirectResponse, | ||
status_code=HTTPStatus.SEE_OTHER, | ||
) | ||
async def reset_password( | ||
reset_password_response: Annotated[ | ||
core_email_password.PasswordResetResponse, | ||
Depends(email_password.handle_reset_password), | ||
], | ||
): | ||
match reset_password_response: | ||
case core_email_password.PasswordResetCompleteResponse(): | ||
return "/" | ||
case core_email_password.PasswordResetMissingProofResponse(): | ||
return "/signin?incomplete=reset_password" | ||
case core_email_password.PasswordResetFailedResponse(): | ||
logger.error("reset password failed: %s", reset_password_response) | ||
return "/signin?error=failure" | ||
case _: | ||
raise Exception("Invalid reset password response") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
import os | ||
|
||
|
||
APP_HOST = os.getenv("APP_HOST", default="localhost") | ||
APP_PORT = os.getenv("APP_PORT", default="8000") | ||
BASE_URL = f"http://{APP_HOST}:{APP_PORT}" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
import asyncio | ||
import os | ||
import secrets | ||
|
||
from gel import create_async_client | ||
|
||
|
||
async def main(): | ||
client = create_async_client() | ||
|
||
auth_signing_key = os.getenv("GEL_AUTH_SIGNING_KEY", secrets.token_urlsafe(32)) | ||
|
||
await client.execute( | ||
f""" | ||
configure current branch reset ext::auth::AuthConfig; | ||
configure current branch reset ext::auth::ProviderConfig; | ||
configure current branch reset ext::auth::EmailPasswordProviderConfig; | ||
configure current branch reset cfg::EmailProviderConfig; | ||
|
||
configure current branch set | ||
ext::auth::AuthConfig::auth_signing_key := "{auth_signing_key}"; | ||
|
||
configure current branch set | ||
ext::auth::AuthConfig::app_name := "Fast Jelly"; | ||
|
||
configure current branch set | ||
ext::auth::AuthConfig::allowed_redirect_urls := {{"http://localhost:8000"}}; | ||
|
||
configure current branch insert | ||
ext::auth::EmailPasswordProviderConfig {{ | ||
require_verification := true, | ||
}}; | ||
|
||
configure current branch insert | ||
cfg::SMTPProviderConfig {{ | ||
name := "mailpit", | ||
host := "localhost", | ||
port := 1025, | ||
username := "smtpuser", | ||
password := "smtppassword", | ||
sender := "[email protected]", | ||
validate_certs := false, | ||
}}; | ||
|
||
configure current branch set | ||
cfg::current_email_provider_name := "mailpit"; | ||
""" | ||
) | ||
|
||
|
||
if __name__ == "__main__": | ||
asyncio.run(main()) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
from __future__ import annotations | ||
|
||
import datetime | ||
from http import HTTPStatus | ||
|
||
import gel | ||
from fastapi import APIRouter, HTTPException | ||
from gel_auth_fastapi import SessionDep | ||
from pydantic import BaseModel | ||
|
||
from .queries import ( | ||
create_event_async_edgeql as create_event_qry, | ||
) | ||
|
||
router = APIRouter() | ||
|
||
|
||
class RequestData(BaseModel): | ||
name: str | ||
address: str | ||
schedule: datetime.datetime | ||
host_name: str | ||
|
||
|
||
@router.post("/events", status_code=HTTPStatus.CREATED) | ||
async def post_event( | ||
event: RequestData, session: SessionDep | ||
) -> create_event_qry.CreateEventResult: | ||
client = session.client | ||
try: | ||
created_event = await create_event_qry.create_event( | ||
client, | ||
name=event.name, | ||
address=event.address, | ||
schedule=event.schedule, | ||
host_name=event.host_name, | ||
) | ||
except gel.errors.InvalidValueError as ex: | ||
raise HTTPException( | ||
status_code=HTTPStatus.BAD_REQUEST, | ||
detail={"error": str(ex)}, | ||
) | ||
|
||
except gel.errors.ConstraintViolationError: | ||
raise HTTPException( | ||
status_code=HTTPStatus.BAD_REQUEST, | ||
detail={"error": "Event '{event.name}' already exists"}, | ||
) | ||
|
||
return created_event |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
import gel | ||
|
||
|
||
client = gel.create_async_client() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
from __future__ import annotations | ||
|
||
import logging | ||
import sys | ||
|
||
from fastapi import FastAPI, APIRouter | ||
|
||
from app import auth, users, events, ui | ||
|
||
|
||
formatter = logging.Formatter("%(asctime)s - %(levelname)s - %(message)s") | ||
stream_handler = logging.StreamHandler(sys.stdout) | ||
stream_handler.setFormatter(formatter) | ||
|
||
logger = logging.getLogger("fast_jelly") | ||
logger.setLevel(logging.DEBUG) | ||
logger.addHandler(stream_handler) | ||
|
||
auth_core_logger = logging.getLogger("gel.auth") | ||
auth_core_logger.setLevel(logging.DEBUG) | ||
auth_core_logger.addHandler(stream_handler) | ||
|
||
fast_api = FastAPI() | ||
fast_api.include_router(ui.router) | ||
fast_api.include_router(auth.router) | ||
|
||
api_router = APIRouter() | ||
api_router.include_router(users.router) | ||
api_router.include_router(events.router) | ||
|
||
fast_api.include_router(api_router, prefix="/api") |
16 changes: 16 additions & 0 deletions
16
python-fastapi/examples/basic-async/app/queries/create_event.edgeql
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
with | ||
name := <str>$name, | ||
address := <str>$address, | ||
schedule := <datetime>$schedule, | ||
host_name := <str>$host_name, | ||
|
||
HOST := (select default::User filter .name = host_name), | ||
CREATED := ( | ||
insert default::Event { | ||
name := name, | ||
address := address, | ||
schedule := schedule, | ||
host := HOST, | ||
} | ||
), | ||
select CREATED { *, host: { * } }; |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something I already mentioned with @1st1 - the creation of the identity (extension domain) and user (application domain) are not done in a single transaction because they are applied each under HTTP and binary protocol. If the identity is created while the user isn't, would the example application be able to recover?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not automatically, but I suppose you could write a query to find all "orphan" identities and try to create the relevant user. FWIW, there is nothing that says users have to be created after the Identity either: if you're implementing an invite flow, for instance, you might already have a
User
in which case you'll be linking to an existing user.But, sure, this is definitely a classic distributed systems problem, no doubt about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, if we use EdgeQL - if possible - for all of the work done here, it won't be a problem for at least this particular use case (quite a common one, I think?).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"All of the work" as in creating the
ext::auth::Identity
? I think that goes against the design of the auth extension which is that it does everything for you via the HTTP API and gives you hooks into the lifecycle for you to do whatever kind of application-level modeling related to that you want to do. Or maybe I'm misunderstanding the suggestion?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly.
I just verified: if
create_user()
fails, as a user, I have no way to sign in, even if I received the verification email and activated my account. In production, the developer has to "write a query to find all "orphan" identities and try to create the relevant user", or alternatlvely, verify and create the user on verification/signin/etc. I think we should also do this in this example, as developers may just fork this example into their production systems.Yeah, I know, that was exactly my question. ;) but now that we're here, let's just make the amendment and documentation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is "the amendment and documentation" that you're suggesting? Do you mean we should write some guidance on what to do if your application is unable to create the matching
User
object for a givenext::auth::Identity
? Like a kind of troubleshooting guide? That seems like something we can add to the documentation, sure! Is there something actionable about that in this PR?I'm still a little confused about the suggestion that we somehow try to create the
ext::auth::Identity
here in the callback: that just won't work since all of the other side effects of the auth extension in the Gel server require that an identity is created and cannot be delayed.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh sorry I meant something completely different than the current HTTP-based design, where you would do something like this in EdgeQL:
But this is in another parallel universe and comes with its own issues (like where to put the password hashing job, etc.).
Yeah, I'd say something like this in the documentation:
And for this example application here, I'll try to add some comments as well as the extra handling for the missing user object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh, yeah I get it now 👍 Yeah, I think even in that universe there is so much HTTP in the mix for things like OAuth that it's frankly not worth it, but the email password flow is close to being implementable completely in EdgeQL 😂
Yeah, great suggestion. I think it's heavily dependent on your in-app authentication data model, but we can provide some good examples both in our guides and in example apps which try to handle these complications. I haven't seen many other Auth products put a lot of guidance into these synchronization issues, so this could be a nice stand-out for us. To be fair, most other auth products own the
User
model too, but they all will suffer from this if they have anything tying app data to auth data.