pantheon/hestia
2
0
Fork 0
Code Issues 23 Pull requests Projects Releases Packages Wiki Activity Actions

feat: add Clerk Auth #43

Merged
piopi merged 4 commits from please-god-help-us into master 2025-01-04 21:24:47 -05:00
Conversation 0 Commits 4 Files changed 17 +152 -248
piopi commented 2025-01-01 12:31:55 -05:00 (Migrated from github.com)
Copy link

Closes #13

Closes #13
DanMihailescu (Migrated from github.com) reviewed 2025-01-01 12:31:55 -05:00
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 11:18:15 -05:00
.gitignore
@ -12,6 +12,7 @@ node_modules
# OS
.DS_Store
Thumbs.db
.idea
BenjaminPalko (Migrated from github.com) commented 2025-01-02 11:18:14 -05:00
Copy link

😂

😂
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 11:18:52 -05:00
.env
@ -7,1 +6,4 @@
DATABASE_URL="postgres://hestia:test123@localhost:5432/hestia"
# CLERK
BenjaminPalko (Migrated from github.com) commented 2025-01-02 11:18:51 -05:00
Copy link

thats a bit wordy

thats a bit wordy
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 11:20:11 -05:00
package.json
BenjaminPalko (Migrated from github.com) commented 2025-01-02 11:20:11 -05:00
Copy link

Add --watch to generate

Add `--watch` to generate
piopi (Migrated from github.com) reviewed 2025-01-02 11:21:45 -05:00
package.json
piopi (Migrated from github.com) commented 2025-01-02 11:21:45 -05:00
Copy link

If you add watch the next command won't run, will need to use the package concurrency to make it work

If you add watch the next command won't run, will need to use the package concurrency to make it work
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 13:25:14 -05:00
package.json
BenjaminPalko (Migrated from github.com) commented 2025-01-02 13:25:14 -05:00
Copy link

It might make more sense to move prisma:push to database:up since its a one-shot and you need to run it manually when you make DB changes

It might make more sense to move prisma:push to database:up since its a one-shot and you need to run it manually when you make DB changes
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 13:26:24 -05:00
prisma/schema.prisma
BenjaminPalko (Migrated from github.com) commented 2025-01-02 13:26:24 -05:00
Copy link

@unique

`@unique`
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 13:27:20 -05:00
package.json
BenjaminPalko (Migrated from github.com) commented 2025-01-02 13:27:20 -05:00
Copy link

I believe we can drop oslo

I believe we can drop oslo
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 13:52:59 -05:00
src/lib/server/auth/index.ts
BenjaminPalko (Migrated from github.com) commented 2025-01-02 13:52:59 -05:00
Copy link

???

???
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 13:53:19 -05:00
src/lib/server/auth/index.ts
@ -22,1 +52,4 @@
return {
user: { name: user.name },
};
}
BenjaminPalko (Migrated from github.com) commented 2025-01-02 13:53:19 -05:00
Copy link

You can have multiple per user..?

You can have multiple per user..?
piopi (Migrated from github.com) reviewed 2025-01-02 13:54:20 -05:00
src/lib/server/auth/index.ts
@ -22,1 +52,4 @@
return {
user: { name: user.name },
};
}
piopi (Migrated from github.com) commented 2025-01-02 13:54:20 -05:00
Copy link

yeah but at least one

yeah but at least one
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 13:55:37 -05:00
src/routes/+page.server.ts
BenjaminPalko (Migrated from github.com) commented 2025-01-02 13:55:37 -05:00
Copy link

at this point you could just do export const load = validateSession;

at this point you could just do `export const load = validateSession;`
piopi (Migrated from github.com) reviewed 2025-01-02 13:57:24 -05:00
prisma/schema.prisma
piopi (Migrated from github.com) commented 2025-01-02 13:57:24 -05:00
Copy link

I didn't put it to allow multiple users (on different tenant) to have the same clerkId. Like you could be managing multiple orgs and in the db it will correspond to two different users with the same clerk id

I didn't put it to allow multiple users (on different tenant) to have the same clerkId. Like you could be managing multiple orgs and in the db it will correspond to two different users with the same clerk id
piopi (Migrated from github.com) reviewed 2025-01-02 13:57:41 -05:00
src/lib/server/auth/index.ts
piopi (Migrated from github.com) commented 2025-01-02 13:57:40 -05:00
Copy link

don't like that 😉 ?

don't like that 😉 ?
piopi (Migrated from github.com) reviewed 2025-01-02 13:58:30 -05:00
prisma/schema.prisma
piopi (Migrated from github.com) commented 2025-01-02 13:58:30 -05:00
Copy link

when we have a tenant table, I could add unique('clerkId', 'tenantId')

when we have a tenant table, I could add `unique('clerkId', 'tenantId')`
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 20:02:05 -05:00
src/hooks.server.ts
@ -0,0 +1,3 @@
import { withClerkHandler } from 'clerk-sveltekit/server';
export const handle = withClerkHandler();
BenjaminPalko (Migrated from github.com) commented 2025-01-02 16:24:50 -05:00
Copy link

https://github.com/wobsoriano/clerk-sveltekit?tab=readme-ov-file#configure-the-server-hook

https://github.com/wobsoriano/clerk-sveltekit?tab=readme-ov-file#configure-the-server-hook
src/lib/server/auth/index.ts
BenjaminPalko (Migrated from github.com) commented 2025-01-02 16:25:24 -05:00
Copy link

Why are we using clerk/express?

Why are we using clerk/express?
src/routes/app/+layout.svelte
@ -14,0 +25,4 @@
await clerk.signOut();
return;
}
BenjaminPalko (Migrated from github.com) commented 2025-01-02 14:03:24 -05:00
Copy link

Can you explain what this is doing?

Can you explain what this is doing?
piopi (Migrated from github.com) reviewed 2025-01-02 20:03:34 -05:00
src/hooks.server.ts
@ -0,0 +1,3 @@
import { withClerkHandler } from 'clerk-sveltekit/server';
export const handle = withClerkHandler();
piopi (Migrated from github.com) commented 2025-01-02 20:03:34 -05:00
Copy link

I am using UI components from the lib so no headless

I am using UI components from the lib so no headless
piopi (Migrated from github.com) reviewed 2025-01-02 20:04:05 -05:00
src/lib/server/auth/index.ts
piopi (Migrated from github.com) commented 2025-01-02 20:04:05 -05:00
Copy link

Because clerk is deprecating the nodejs package and is recommending to use this one for backend

Because clerk is deprecating the nodejs package and is recommending to use this one for backend
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 20:04:22 -05:00
src/lib/server/auth/index.ts
@ -22,1 +52,4 @@
return {
user: { name: user.name },
};
}
BenjaminPalko (Migrated from github.com) commented 2025-01-02 20:04:22 -05:00
Copy link

Okay. Somewhat of a nitpick, Ive never been a fan of hard-coding an array index like this in the situation the array is null or the element doesn't exist.

Okay. Somewhat of a nitpick, Ive never been a fan of hard-coding an array index like this in the situation the array is null or the element doesn't exist.
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 20:04:47 -05:00
src/lib/server/auth/index.ts
BenjaminPalko (Migrated from github.com) commented 2025-01-02 20:04:47 -05:00
Copy link

WE ARE PROFESSIONALS HERE MOE

WE ARE PROFESSIONALS HERE MOE
piopi (Migrated from github.com) reviewed 2025-01-02 20:05:44 -05:00
src/routes/app/+layout.svelte
@ -14,0 +25,4 @@
await clerk.signOut();
return;
}
piopi (Migrated from github.com) commented 2025-01-02 20:05:44 -05:00
Copy link

I will add some comment, it is automatically selecting the first organization that the user is part of, if you don't do that the user get login without an organization

I will add some comment, it is automatically selecting the first organization that the user is part of, if you don't do that the user get login without an organization
piopi (Migrated from github.com) reviewed 2025-01-02 20:07:38 -05:00
src/lib/server/auth/index.ts
@ -22,1 +52,4 @@
return {
user: { name: user.name },
};
}
piopi (Migrated from github.com) commented 2025-01-02 20:07:38 -05:00
Copy link

I will add a check, I don't think it will ever happen that a user doesn't have an email, you can't register to Clerk without it

I will add a check, I don't think it will ever happen that a user doesn't have an email, you can't register to Clerk without it
piopi (Migrated from github.com) reviewed 2025-01-02 20:08:15 -05:00
.env
@ -7,1 +6,4 @@
DATABASE_URL="postgres://hestia:test123@localhost:5432/hestia"
# CLERK
piopi (Migrated from github.com) commented 2025-01-02 20:08:15 -05:00
Copy link

Making sure no one try to change it

Making sure no one try to change it
piopi (Migrated from github.com) reviewed 2025-01-02 20:09:50 -05:00
src/lib/server/auth/index.ts
piopi (Migrated from github.com) commented 2025-01-02 20:09:50 -05:00
Copy link

I will change it to an empty string, I thought we were fun professionals

I will change it to an empty string, I thought we were fun professionals
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 20:09:57 -05:00
prisma/schema.prisma
BenjaminPalko (Migrated from github.com) commented 2025-01-02 20:09:57 -05:00
Copy link

Yeah, I guess tenants have 'their own' users, but this is the id coming from clerk and shouldn't ever be duplicated, unless we make the tenants bring their own clerk account like with twilio?

Yeah, I guess tenants have 'their own' users, but this is the id coming from clerk and shouldn't ever be duplicated, unless we make the tenants bring their own clerk account like with twilio?
piopi (Migrated from github.com) reviewed 2025-01-02 20:11:02 -05:00
src/hooks.server.ts
@ -0,0 +1,3 @@
import { withClerkHandler } from 'clerk-sveltekit/server';
export const handle = withClerkHandler();
piopi (Migrated from github.com) commented 2025-01-02 20:11:02 -05:00
Copy link

Oh the part your linked, I am using a new version of the lib that using the latest API of Clerk so things changed a lot from that ReadMe

Oh the part your linked, I am using a new version of the lib that using the latest API of Clerk so things changed a lot from that ReadMe
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 21:03:38 -05:00
src/lib/server/auth/index.ts
BenjaminPalko (Migrated from github.com) commented 2025-01-02 21:03:38 -05:00
Copy link

is the user fullName optional...? I'd rather not default to a magic string

is the user fullName optional...? I'd rather not default to a magic string
BenjaminPalko (Migrated from github.com) reviewed 2025-01-02 21:04:40 -05:00
src/lib/server/auth/index.ts
BenjaminPalko (Migrated from github.com) commented 2025-01-02 21:04:40 -05:00
Copy link

If there is a situation where name will be empty/undefined consider changing the db schema

If there is a situation where name will be empty/undefined consider changing the db schema
piopi (Migrated from github.com) reviewed 2025-01-02 23:17:05 -05:00
src/routes/+page.server.ts
piopi (Migrated from github.com) commented 2025-01-02 23:17:05 -05:00
Copy link

for the sake of consistency and a bit readability, I will keep it like this

for the sake of consistency and a bit readability, I will keep it like this
BenjaminPalko (Migrated from github.com) reviewed 2025-01-03 10:57:36 -05:00
src/routes/app/+layout.svelte
@ -14,0 +25,4 @@
await clerk.signOut();
return;
}
BenjaminPalko (Migrated from github.com) commented 2025-01-03 10:57:36 -05:00
Copy link

It would be better to do this check on the server to prevent anything from being leaked to the client side, you can use +layout.server.ts. We could throw an error, 403 - User does not belong to a Tenant

It would be better to do this check on the server to prevent anything from being leaked to the client side, you can use `+layout.server.ts`. We could throw an error, `403 - User does not belong to a Tenant`
piopi (Migrated from github.com) reviewed 2025-01-03 10:59:11 -05:00
src/routes/app/+layout.svelte
@ -14,0 +25,4 @@
await clerk.signOut();
return;
}
piopi (Migrated from github.com) commented 2025-01-03 10:59:11 -05:00
Copy link

I am already logging out the user on the backend, this is only selecting an org for him

I am already logging out the user on the backend, this is only selecting an org for him
BenjaminPalko (Migrated from github.com) reviewed 2025-01-03 10:59:13 -05:00
src/routes/app/+layout.svelte
@ -14,0 +25,4 @@
await clerk.signOut();
return;
}
BenjaminPalko (Migrated from github.com) commented 2025-01-03 10:59:12 -05:00
Copy link

Also, modify the error page to contain a signout button, perhaps under certain http codes (like 403)

Also, modify the error page to contain a signout button, perhaps under certain http codes (like 403)
BenjaminPalko (Migrated from github.com) approved these changes 2025-01-03 11:21:15 -05:00
Sign in to join this conversation.
Reviewers
No reviewers
DanMihailescu
BenjaminPalko
Labels
Clear labels   
bug

Something isn't working

  
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
enhancement

New feature or request

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
question

Further information is requested

  
refactor

Rework something

  
wontfix

This will not be worked on

No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
refactor
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: pantheon/hestia#43
No description provided.
Delete branch "please-god-help-us"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?