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.

Sign up for GitHub

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

update auth to better-auth v1 #71

Merged
merged 1 commit into from
Nov 26, 2024
Merged

update auth to better-auth v1 #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 26 additions & 7 deletions apps/www/components/custom/account-switcher.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
"use client";
import { authClient } from "@/lib/auth-client";
import { Session } from "@repo/auth";
import Link from "next/link";
import { useRouter } from "next/navigation";
interface Props {
session: Session[] | null;
activeSession: Session | null;
}
export default function AccountSwitcher({ session, activeSession }: Props) {
const router = useRouter();
const onSelect = async (sessionId: string) => {
console.log(sessionId);
const onSelect = async (token: string) => {
console.log(token);
const active = await authClient.multiSession.setActive({
sessionId: sessionId,
sessionToken: token,
});

console.log(active);
Expand All @@ -29,20 +30,27 @@ export default function AccountSwitcher({ session, activeSession }: Props) {
},
});
};

if (!activeSession || !session) {
return <div>loading sessions</div>;
}
const handleCurrentSignOut = async () => {
await authClient.multiSession.revoke({
sessionToken: activeSession?.session.token
});
window.location.reload();
};
Comment on lines +37 to +42
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling and improve user experience

The current implementation lacks error handling and forces a page reload.

   const handleCurrentSignOut = async () => {
+    try {
+      setIsLoading(true);
       await authClient.multiSession.revoke({
         sessionToken: activeSession?.session.token
       });
-      window.location.reload();
+      router.push('/auth');
+    } catch (error) {
+      console.error('Failed to sign out:', error);
+      // Show error notification to user
+    } finally {
+      setIsLoading(false);
+    }
   };

Committable suggestion skipped: line range outside the PR's diff.

return (
<div className="flex items-center justify-center gap-2 p-4">
<select
onChange={(e) => onSelect(e.target.value)}
value={activeSession.session.id}
value={activeSession.session.token}
className="border-2 border-gray-300[0.3] rounded-md p-2"
>
{session.map((item) => {
return (
<option key={item.session.id} value={item.session.id}>
{item.user.name}{" "}
<option key={item.session.id} value={item.session.token}>
{item.user.name}
</option>
);
})}
Expand All @@ -51,8 +59,19 @@ export default function AccountSwitcher({ session, activeSession }: Props) {
className="flex border border-neutral-900[0.2] bg-neutral-900/60 p-2"
onClick={handleSignOut}
>
logout
Logout from all
</div>
<div
className="flex border border-neutral-900[0.2] bg-neutral-900/60 p-2"
onClick={handleCurrentSignOut}
>
Logout
</div>
<Link href={"/auth"}>
<div className="flex border border-neutral-900[0.2] bg-neutral-900/60 p-2">
Add Account
</div>
</Link>
</div>
);
}
2 changes: 1 addition & 1 deletion apps/www/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@
"@repo/db": "workspace:*",
"@repo/types": "workspace:*",
"@tabler/icons-react": "^3.21.0",
"better-auth": "0.8.1-beta.1",
"better-auth": "1.0.5",
"class-variance-authority": "^0.7.0",
"clsx": "^2.1.1",
"contentlayer2": "^0.5.3",
Expand Down
4 changes: 2 additions & 2 deletions packages/auth/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@
"author": "",
"license": "ISC",
"dependencies": {
"@repo/typescript-config": "workspace:*",
"@repo/db": "workspace:*",
"better-auth": "0.8.6-beta.4",
"@repo/typescript-config": "workspace:*",
"better-auth": "1.0.5",
"better-call": "0.2.14-beta.3",
"oslo": "^1.2.1"
}
Expand Down
2 changes: 1 addition & 1 deletion packages/auth/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"extends": "@repo/typescript-config/base.json",
"include": ["src", "*.ts"],
"exclude": ["node_modules"]
"exclude": ["node_modules"],
}
31 changes: 31 additions & 0 deletions packages/database/prisma/migrations/20241126154111_final_auth/migration.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
Warnings:

- You are about to drop the column `expiresAt` on the `account` table. All the data in the column will be lost.
- A unique constraint covering the columns `[token]` on the table `session` will be added. If there are existing duplicate values, this will fail.
- Added the required column `createdAt` to the `account` table without a default value. This is not possible if the table is not empty.
- Added the required column `updatedAt` to the `account` table without a default value. This is not possible if the table is not empty.
- Added the required column `createdAt` to the `session` table without a default value. This is not possible if the table is not empty.
- Added the required column `token` to the `session` table without a default value. This is not possible if the table is not empty.
- Added the required column `updatedAt` to the `session` table without a default value. This is not possible if the table is not empty.
Comment on lines +4 to +10
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

⚠️ Migration might fail on non-empty tables

The migration adds multiple required columns (createdAt, updatedAt) without default values to both account and session tables. This will fail if these tables contain existing data.

Consider modifying the migration to handle existing data:

- ADD COLUMN     "createdAt" TIMESTAMP(3) NOT NULL,
- ADD COLUMN     "updatedAt" TIMESTAMP(3) NOT NULL
+ ADD COLUMN     "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP,
+ ADD COLUMN     "updatedAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP

Committable suggestion skipped: line range outside the PR's diff.


*/
-- AlterTable
ALTER TABLE "account" DROP COLUMN "expiresAt",
ADD COLUMN "accessTokenExpiresAt" TIMESTAMP(3),
ADD COLUMN "createdAt" TIMESTAMP(3) NOT NULL,
ADD COLUMN "refreshTokenExpiresAt" TIMESTAMP(3),
ADD COLUMN "scope" TEXT,
ADD COLUMN "updatedAt" TIMESTAMP(3) NOT NULL;
Comment on lines +14 to +19
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codebase verification

⚠️ Potential issue

Data loss warning: expiresAt column dropped without migration

The migration script shows that the expiresAt column is being dropped with the warning "All the data in the column will be lost" and there's no data migration logic to transfer existing expiration data to the new accessTokenExpiresAt column. This is concerning because:

  • The account table was created in the previous migration (20241110144439_auth)
  • The current migration (20241126154111_final_auth) drops expiresAt without preserving its data
  • The new columns accessTokenExpiresAt and refreshTokenExpiresAt are nullable, but any existing expiration data will be lost

Suggested actions:

  • Add a data migration step to copy expiresAt values to accessTokenExpiresAt before dropping the column
  • Or confirm that data loss is acceptable if this is a development environment with no production data
🔗 Analysis chain

Verify data migration for removed expiresAt column

The expiresAt column is being dropped and replaced with more specific expiration columns. Ensure any existing expiration data is migrated appropriately.

Let me check the migration history to understand if this is a new table or if there's existing data that needs migration.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if there's existing data in the expiresAt column that needs migration
echo "Checking for existing expiration data..."
ast-grep --pattern 'expiresAt' # Look for usages in the codebase
rg -l "expiresAt" # Find files referencing this column

Length of output: 324

Script:

#!/bin/bash
# Check the previous migration that introduced the expiresAt column
cat packages/database/prisma/migrations/20241110144439_auth/migration.sql

# Check if there are any data migrations in the current migration
cat packages/database/prisma/migrations/20241126154111_final_auth/migration.sql

Length of output: 3275


-- AlterTable
ALTER TABLE "session" ADD COLUMN "createdAt" TIMESTAMP(3) NOT NULL,
ADD COLUMN "token" TEXT NOT NULL,
ADD COLUMN "updatedAt" TIMESTAMP(3) NOT NULL;

-- AlterTable
ALTER TABLE "verification" ADD COLUMN "updatedAt" TIMESTAMP(3),
ALTER COLUMN "createdAt" DROP NOT NULL;

-- CreateIndex
CREATE UNIQUE INDEX "session_token_key" ON "session"("token");
36 changes: 22 additions & 14 deletions packages/database/prisma/schema.prisma
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ model User {
image String?
createdAt DateTime
updatedAt DateTime
accounts Account[]
sessions Session[]
accounts Account[]

@@unique([email])
@@map("user")
Expand All @@ -27,35 +27,44 @@ model User {
model Session {
id String @id
expiresAt DateTime
token String
createdAt DateTime
updatedAt DateTime
ipAddress String?
userAgent String?
userId String
user User @relation(fields: [userId], references: [id], onDelete: Cascade)

@@unique([token])
@@map("session")
}

model Account {
id String @id
accountId String
providerId String
userId String
user User @relation(fields: [userId], references: [id], onDelete: Cascade)
accessToken String?
refreshToken String?
idToken String?
expiresAt DateTime?
password String?
id String @id
accountId String
providerId String
userId String
user User @relation(fields: [userId], references: [id], onDelete: Cascade)
accessToken String?
refreshToken String?
idToken String?
accessTokenExpiresAt DateTime?
refreshTokenExpiresAt DateTime?
scope String?
password String?
createdAt DateTime
updatedAt DateTime

@@map("account")
}

model Verification {
id String @id
id String @id
identifier String
value String
expiresAt DateTime
createdAt DateTime
createdAt DateTime?
updatedAt DateTime?
Comment on lines +66 to +67
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Enforce timestamp consistency

Making createdAt and updatedAt optional deviates from standard practices and could lead to data consistency issues. Consider making these fields required with default values.

 model Verification {
   id         String    @id
   identifier String
   value      String
   expiresAt  DateTime
-  createdAt  DateTime?
-  updatedAt  DateTime?
+  createdAt  DateTime  @default(now())
+  updatedAt  DateTime  @updatedAt

   @@map("verification")
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
createdAt DateTime?
updatedAt DateTime?
createdAt DateTime @default(now())
updatedAt DateTime @updatedAt


@@map("verification")
}
Expand All @@ -65,5 +74,4 @@ model Trigger {
name String
email String
emailVerified Boolean

}
2 changes: 1 addition & 1 deletion packages/typescript-config/base.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,6 @@
"resolveJsonModule": true,
"skipLibCheck": true,
"strict": true,
"target": "ES2022"
"target": "ES2022",
}
}
44 changes: 22 additions & 22 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading