Injected auth models

[infomiho]

Closes #1080

Qucik recap of things that happen in this PR:

1. Instead of asking from users to define the auth related entities, we inject them in the Prisma file

2. The auth entities changed to fit all auth methods into one uniform shape and allow for easier extension later on

   model Auth {
     id         String         @id @default(uuid())
     userId     Int?           @unique
     user       User?          @relation(fields: [userId], references: [id], onDelete: Cascade)
     identities AuthIdentity[]
   }
   
   model AuthIdentity {
     providerName   String
     providerUserId String
     providerData   String @default("{}")
     authId         String
     auth           Auth   @relation(fields: [authId], references: [id], onDelete: Cascade)
   
     @@id([providerName, providerUserId])
   }

3. Since some of the information is now stored in providerData JSON field, a bit of work was done to serialize and deserialize the field properly

4. All of the auth methods were updated to use the new structure

5. Unit, e2e and headless tests were updated

What is left

• Update docs on all of the changes
• Prepare migration tips (I've been successful in using the seed scripts to migrate from existing auth to new auth models)

[Martinsos]

Another round of review done!

[Martinsos]

Ok, I think I get it better now.
Yeah, you need this to be able to say req.user later.
So far we used no types, now you started using express.Request, but that one won't let you do req.user. And we need that in many places.

The thing is, we don't need it in all places. And if I am correct, we know in which places we need it, and in which places we don't right? So I wonder, can we make it more precise? Instead of saying user could be in any request, should we go step further and specify it per request?

We could make handleRejection<T extends Request> and then call it with handleRejection<WithUser<Request>>(... where appropriate.

I am still learning Typescript so I maybe made up some things here or they don't make fully sense, but I think the direction should make sense. Probably the bigger question is, is it worth doing this, I will let you decide. I see 12 different call sites of handleRejection, and I think if there is a single one there that for sure doesn't need user, then it is worth doing it. Even if not, if there is a chance there will be such usage, I think it is worth doing it. But if you think that doesn't make sense let me know! I am also not sure if this is indeed easily doable with Express's current typings, if that could make it harder.

EDIT: there is certainly at least one such place: handleRejection in server/src/core/auth.js, the first one, const auth = handleRejection, that one doesn't have user on it for sure, right?

[Martinsos]

As for the name: if we are using that one Request always instead of Express's Request, then I would probably go with just Request even, since it is virtually a replacement for Express one. Or maybe WaspRequest.
But if we sometimes have user in request, sometimes don't, and similar, then I would instead go for type level operators, something like WithUser and then construct it in place as needed. Maybe have some aliases for common combos.

[Martinsos]

We could though create a template file under templates/universal/src/auth/user.ts, right, and then just inject this very first line with imports into it, couldn't we? At the moment. And then once we have restructuring done, we might even have some other options open, as it being part of SDK means we can more easily share some code maybe?

[infomiho]

I've already commented on that option here: #1583 (comment)

As I said, I've considered it and went with a simpler solution of duplication. This can be the temporary solution if it's going to change with the restructuring.

[Martinsos]

I commented again because you said in the comment in the code (the one I commented on here) that we can't deduplicate this, which is not correct, we can, but you decided we rather shouldn't as it is not worth it. So maybe let's update comment with that info?

[Martinsos]

Btw, if you wanted to do actual nominal typing in Typescript, check this: https://byby.dev/ts-nominal-typing .

[Martinsos]

Aha, good catch!

[infomiho]

We agreed to improve upon the timing attack prevention by improving the fake work mechanism:

• make sure that real and fake flows take the same time
• handle the "try again in 1 minute" case better

[Martinsos]

1. Ah yeah, makes sense -> so we can't pretend sign up was successful if we allow unverified email. How about we just forbid unverified email?
2. Example of an auth system -> well what do they do, how do they handle it? I thought I remember getting such an email but I can't remember for sure. If I go into research, it will take me much longer than just reviewing, but ideally you would do the research here and know what others do, with examples and stuff. Why do you think it is out of scope? You think we are being too pedantic? I don't know but it sounds weird that I try to sign up, and web app says I succeeded and it sent me an email, but then I get no email, I would conclude something is wrong and give up, not that I already maybe have an account. Right?
3. (and 4 and 5) -> yeah ok I brainfarted a bit there, but then what we can do is send them an email saying that somebody already created an account with their email and they have to wait a bit before they can claim it, or they can click on password reset and we give them link. How about that?

[Martinsos]

Yeah, let's reafctor it.

I get the point with nesting, but what you loose is obvious branching -> instead of reader being very clear about all cases being handled, I have to think quite a bit here to figure out these two really cover all the cases. And it can also be easier in the future to introduce a mistake where these conditions don't perfectly match each other and suddenly they don't cover all the cases any more.

[Martinsos]

Maybe I expressed myself clumsily, but what I meant to say is that this type is not useful to describe values, but only to describe other types (type level programming). I don't think that is clear by itself, and would mention that in the comment.