Skip to content
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

User and Identity #8

Open
juni0r opened this issue Nov 24, 2023 · 5 comments
Open

User and Identity #8

juni0r opened this issue Nov 24, 2023 · 5 comments

Comments

@juni0r
Copy link
Contributor

juni0r commented Nov 24, 2023

Currently the authenticated User isn't exposed by the API directly but has to be retrieved separately using ext::auth::Identity. EdgeDB doesn't have the notion of a User, it knows only identities and a User is just another type that happens to be associated with an Identity.

There is a lot of flexibility in this approach as the User type can be defined by the application and carry any additional information. However, it is more intuitive and practical to have the User as the primary object exposed by auth. Most applications will want to associate a User with other types (like author of a Post).

Currently the User is created as a side-effect when the Identity is retrieved:

if (!user && token) {
user = await client.query(`
insert User {
name := '',
identity := global ext::auth::ClientTokenIdentity
}
`)
}

That doesn't feel right, especially if you want to collect other data (like name) as part of the signup process. It's more intuitive to create the User in the signup handler instead of merely a side-effect in the identity handler. The email and password fields can be used to create the identity and all other fields are assigned to the User.

It might be good to allow for customizing the User type etc. but for now, I'd just require that if auth is enabled, there has to be a type named User which has to have at least an identity field.

Just an idea so far. Let me know what you think!

@Tahul
Copy link
Owner

Tahul commented Nov 24, 2023

I would love to have this.

I built built the User integration very briefly, just to showcase how anyone could build what you described on top of it.

One question I have though is how should we ship that to the user of our module.

Right now, applying changes on the schema of the user feels out-of-scope for me as I would like that module to stay close to the bare minimal integration of EdgeDB features offering.

Maybe we could provide a toggleable feature that enables that User feature once it is properly shaped, and use hooks to add additional treatment inside the auth server routes?

What are your thoughts on the User model?

@juni0r
Copy link
Contributor Author

juni0r commented Nov 27, 2023

Ok, so it turns out I was mistaken about Identities. I thought that every auth method (email/pwd as well as oauth) represents a different Identity that needs to be linked to a User.

If I'm not mistaken though, there is only one Identity, which is the conceptual equivalent of a User in other frameworks. This one Identity can be authenticated by different means such as EmailPasswordFactor or via OAuth. Do you know how this works exactly? The EdgeDB docs are quite sparse in that regard.

If an Identity is all you need to manage authentication and authorization, I think we shouldn't provide a User model since that's not the scope of a module.

@tdolsen
Copy link

tdolsen commented Feb 17, 2024

I would say this is a fairly blocking issue, as it's forcing a dbschema that doesn't match in many cases.

  • It's not given that User always will be in the default EdgeDB module.
  • It's not given that the property name is a str.
  • It's very likely that User will have some required properties and constraints.

I would rather have to write a few hooks and queries to control the creation of the the user myself, than to be forced into a paradigm that doesn't fit my project.

@Tahul
Copy link
Owner

Tahul commented Feb 23, 2024

@tdolsen ; could you let me know how you would envision the perfect implementation for auth in your case?

That would help me reflect on the usecases, as I'm not using auth myself on my project using this module. 😅

@tdolsen
Copy link

tdolsen commented Feb 24, 2024

I'll give a little input, absolutely. 🙂

Note: I'm using EdgeDB auth and Nuxt for the first time as an experiment on a side project, and my experience with both are fairly new. (Long time Vue and EdgeDB user though.) This is just to mention that I'm still learning the internals, and that I might not know all possibilities in terms of lifecycle hooks and such.

dbschema definition

The biggest concern I have is that the creation of a User is hardcoded, forcing a specific dbschema implementation. As I mention above, there are several use cases that will cause problems with the module implementation:

  • The user type may reside in a module, or be named something other than User - e.g. account::Client.
  • Same goes for the current_user global.
  • The user type may have multiple EdgeDB auth identities - one for each provider.
  • The user may not have a name, or have a name of a different type.
  • The user may have required properties needed to be set upon creation.

Some of these can be solved with simple configuration, and maybe simplifying the module implementation/take account

Only reflecting EdgeDB auth

EdgeDB's auth module has no concept of User - it only knows about Identity. This means the consumer is completely free to define their own implementation, and to some extent that's all that I expect from a module like nuxt-edgedb.

Basically, if api/auth/identity only returned the current identity, instead of trying to also find a user, that would be OK.

Server API overloading

There are cases where I'd want to do some additional computing for the auth API endpoints, and being able to overload any specific endpoint without having to rewrite the module functionality would be desirable.

Whether this would best be done with some lifecycle hooks, or by providing the API endpoint functionality as server composables, am I really not sure about.

Sane defaults

With the notes above said, am I still not opposed to sane defaults for a module like this. Many projects and developers are probably very content with using (more or less) what you've started on, and as long as it's possible to override the defaults then all are happy.

Concrete suggestions

To summarize my thoughts, here are a some suggestions I think would be an improvement. My thoughts are not complete on all of this, and some of these suggestions might be somewhat redundant.

1. Make select global current_user query configurable

Simply making the query for fetching the user configurable (with your current implementation as default) in the module options.

edgeDb.auth.currentUserQuery = "select global account::ClientUser;"

2. Remove dependency on name property in User type

Since the default implementation of User just inserts an empty name, I suggest removing all references to name. If an empty name is acceptable in the dbschema the default should be defined there, and not in the code.

3. Change identity to identities in User type

Making it possible to have multiple identities for a user by default would raise the glass ceiling quite a bit, and the additional complexity is minimal.

global current_user := assert_single((
  select User
  filter global ext::auth::ClientTokenIdentity in .identities
));

type User {
  required multi identities: ext::auth::Identity;
}

4. Make insert User query configurable

Making the query to insert a User configurable would make it possible to account for some deviations from the default.

edgeDb.auth.insertUserQuery = `insert account::ClientUser {
  someProp := "...",
  identities := ($identityToken)
};`

Maybe there's something to supporting a callback function as well.

5. Make api/auth/signup and/or user creation hookable

Adding a way to hook into the user creation flow, giving access to the full request event, allowing collection of additional data, such that a custom user creation flow can be implemented, would be very nice.

As I see it, this should ideally happen upon signup, as that would be the place to collect initial data for the user.

6. Provide API composables or some way to overload/re-implement API endpoints

This is a little vague for me at the moment, but it would be very desirable to be able to add my own sugar on top of the auth API endpoints.

Next step

I'll await your response to this, and elaborate further if needed. Once we have something we're confident about, am I happy to help out with implementing and test some of the changes myself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants