-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Security review, other small improvements / fixes #2
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@@ -0,0 +1,4 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding ESLint
@@ -33,17 +33,11 @@ export default function AccountsPage() { | |||
const router = useRouter() | |||
const debugMode = process.env.NEXT_PUBLIC_DEBUG_MODE === 'true' | |||
|
|||
const fetchAccounts = async () => { | |||
const fetchAccounts = useCallback(async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Next convention
|
||
export async function GET(request: Request, { params }: { params: { slug: string } }) { | ||
const slug = params.slug | ||
|
||
if (!slug) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved lots of code into lib
(see imports above)
@@ -1,117 +0,0 @@ | |||
import { NextRequest, NextResponse } from "next/server" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed this entirely since it's possible debug mode could be set in other ways in the future, and if set, it reveals all env vars. No need for this code IMO so removed.
@@ -0,0 +1,13 @@ | |||
import { NextRequest, NextResponse } from 'next/server'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -1,87 +1,63 @@ | |||
import { NextRequest, NextResponse } from "next/server"; | |||
import { randomUUID } from "crypto"; | |||
import { getAuth } from "@clerk/nextjs/server"; | |||
import { getPipedreamExternalUserId } from "@/lib/clerk"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Replaced some references to the Clerk user ID with UUIDs, since we decided to use those instead. Added additional validation logic for UUIDs / other improved auth checks here
const { isLoaded, userId } = useAuth() | ||
const { user } = useUser() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No longer generating UUIDs client side, only server-side (in /api
)
@@ -71,20 +70,7 @@ export function MainNav() { | |||
Connected Accounts | |||
</Link> | |||
|
|||
{/* Only show Clerk Test link in debug mode */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing more old debugging code
const ChartStyle = ({ id, config }: { id: string; config: ChartConfig }) => { | ||
const colorConfig = Object.entries(config).filter( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Old implementation used https://legacy.reactjs.org/docs/dom-elements.html#dangerouslysetinnerhtml , which opens us up to XSS attacks
@@ -0,0 +1,300 @@ | |||
'use server'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved all Supabase code server-side, to protect against the clients getting our Supabase anon key directly (which does allow some operations that would cost us money if abused)
@@ -0,0 +1 @@ | |||
strict-peer-dependencies=false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This solves the peer dependency issue with react
for anyone who uses this repo
No description provided.