fast-jelly! the Gel auth extension bindings for FastAPI #3
Conversation
Almost the whole thing. Missing some of the event domain for now.
Co-authored-by: Fantix King <[email protected]>
| 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)}") |
There was a problem hiding this comment.
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.
would the example application be able to recover?
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.
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.
all of the work done here
"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.
"All of the work" as in creating the ext::auth::Identity?
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.
that goes against the design of the auth extension
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.
let's just make the amendment and documentation.
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 given ext::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.
I'm still a little confused about the suggestion
Oh sorry I meant something completely different than the current HTTP-based design, where you would do something like this in EdgeQL:
with identity := ext::auth::sign_up()
insert User { ..., identities := identity }
But this is in another parallel universe and comes with its own issues (like where to put the password hashing job, etc.).
Do you mean we should write some guidance
Yeah, I'd say something like this in the documentation:
Accessing the Auth extension APIs through HTTP means, such database
accesses cannot be a part of your transaction; handling the non-atomicity
here is your responsibility. In other words, your application should be ready
to handle the cases when e.g. the HTTP call to `/auth/register` succeeded
but the EdgeQL to create a corresponding `User` (from your own schema) failed.
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.
But this is in another parallel universe
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, I'd say something like this in the documentation:
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.
| ): | ||
| sign_up_response = await auth.handle_sign_up( | ||
| sign_up_body, | ||
| verify_url=str(request.url_for("verify")), |
There was a problem hiding this comment.
This generates the public URL for the "verify" endpoint at runtime.
| ) -> ext.EmailPassword: | ||
| return ext.EmailPassword( | ||
| await core.make_async(client), | ||
| secure_cookie=request.base_url.is_secure, |
There was a problem hiding this comment.
This detects if the app is visited under TLS or not - it's convenient if the cookie follows the security setup for development, but this should be enforced on production environments.
This adds the binding of Gel auth extension for FastAPI, with example application:
Depends on geldata/gel-python#586