-
Notifications
You must be signed in to change notification settings - Fork 729
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
middleware based auth for pages, robots.txt, sitemap.xml #164
base: main
Are you sure you want to change the base?
Changes from all commits
d972a2c
aa0cd25
7e47955
c5b4256
8d33552
a62802f
c937e79
83f4b24
3b5812c
36b17ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -4,11 +4,8 @@ import MemoriesPage from "../../content"; | |||||
import { db } from "@/server/db"; | ||||||
import { and, eq } from "drizzle-orm"; | ||||||
import { spacesAccess } from "@/server/db/schema"; | ||||||
import { auth } from "@/server/auth"; | ||||||
|
||||||
async function Page({ params: { spaceid } }: { params: { spaceid: number } }) { | ||||||
const user = await auth(); | ||||||
|
||||||
const { success, data } = await getMemoriesInsideSpace(spaceid); | ||||||
if (!success ?? !data) return redirect("/home"); | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment: The redirect logic in the Page function does not handle errors properly. Solution: Change the condition to explicitly check for both success and data. Reason For Comment: Using a nullish coalescing operator (??) in the condition might lead to unexpected behavior. It should explicitly check for both success and data.
Suggested change
|
||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import { type MetadataRoute } from 'next' | ||
import { routeGroups, routeTypes } from '@/routes' | ||
|
||
export default function robots(): MetadataRoute.Robots { | ||
return { | ||
rules: { | ||
userAgent: '*', | ||
allow: [...routeGroups.landing], | ||
disallow: [...routeTypes.authed], | ||
}, | ||
sitemap: 'https://supermemory.ai/sitemap.xml', | ||
} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,12 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
import { type MetadataRoute } from 'next' | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
export default function sitemap(): MetadataRoute.Sitemap { | ||||||||||||||||||||||||||||||||||||||||||||||
return [ | ||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||
url: 'https://supermemory.ai/', | ||||||||||||||||||||||||||||||||||||||||||||||
lastModified: new Date(), | ||||||||||||||||||||||||||||||||||||||||||||||
changeFrequency: 'yearly', | ||||||||||||||||||||||||||||||||||||||||||||||
priority: 1, | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+4
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment: The Solution: Add URLs for authenticated pages to the Reason For Comment: The current implementation only includes the root URL, which is insufficient for SEO purposes. Authenticated pages should be included in the sitemap to improve discoverability by search engines.
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,24 +1,42 @@ | ||||||||||||||||||||||||||
import { NextRequest, NextResponse } from "next/server"; | ||||||||||||||||||||||||||
import { type NextRequest, NextResponse } from "next/server"; | ||||||||||||||||||||||||||
import { auth } from "./server/auth"; | ||||||||||||||||||||||||||
import { routeTypes } from "@/routes"; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
const corsHeaders = { | ||||||||||||||||||||||||||
"Access-Control-Allow-Origin": "*", | ||||||||||||||||||||||||||
"Access-Control-Allow-Methods": "GET, POST, PUT, DELETE, OPTIONS", | ||||||||||||||||||||||||||
"Access-Control-Allow-Headers": "Content-Type, Authorization", | ||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
export function middleware(request: NextRequest) { | ||||||||||||||||||||||||||
if (request.method === "OPTIONS") { | ||||||||||||||||||||||||||
return new NextResponse(null, { headers: corsHeaders }); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
export async function middleware(request: NextRequest) { | ||||||||||||||||||||||||||
if (request.nextUrl.pathname.startsWith("/api")) { | ||||||||||||||||||||||||||
if (request.method === "OPTIONS") { | ||||||||||||||||||||||||||
return new NextResponse(null, { headers: corsHeaders }); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
const response = NextResponse.next(); | ||||||||||||||||||||||||||
Object.entries(corsHeaders).forEach(([key, value]) => { | ||||||||||||||||||||||||||
response.headers.set(key, value); | ||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
return response; | ||||||||||||||||||||||||||
const response = NextResponse.next(); | ||||||||||||||||||||||||||
Object.entries(corsHeaders).forEach(([key, value]) => { | ||||||||||||||||||||||||||
response.headers.set(key, value); | ||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||
return response; | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
const info = await auth(); | ||||||||||||||||||||||||||
if (routeTypes.authed.some((route) => request.nextUrl.pathname.startsWith(route))) { | ||||||||||||||||||||||||||
if (!info) { | ||||||||||||||||||||||||||
return NextResponse.redirect(new URL("/signin", request.nextUrl)); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} else if (routeTypes.unAuthedOnly.some((route) => request.nextUrl.pathname.endsWith(route))) { | ||||||||||||||||||||||||||
if (info) { | ||||||||||||||||||||||||||
return NextResponse.redirect(new URL("/home", request.nextUrl)); | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
return NextResponse.next(); | ||||||||||||||||||||||||||
Comment on lines
+23
to
+33
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Comment: The middleware function should handle both authenticated and unauthenticated routes correctly. Solution: Ensure that the authentication check is comprehensive and covers all necessary routes. Reason For Comment: If the authentication logic fails or is misconfigured, users may gain unauthorized access or be incorrectly redirected.
Suggested change
|
||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
export const config = { | ||||||||||||||||||||||||||
matcher: "/api/:path*", | ||||||||||||||||||||||||||
matcher: [ | ||||||||||||||||||||||||||
'/((?!_next/static|_next/image|image|favicon.ico).*)', | ||||||||||||||||||||||||||
'/api/:path*', | ||||||||||||||||||||||||||
'/.:path*', | ||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
export const routeGroups = { | ||
auth: ["/privacy", "/signin", "/tos"], | ||
canvas: ["/canvas"], | ||
dash: ["/memories", "/space", "/chat", "/home", "/note"], | ||
landing: ["/"], | ||
other: ["/ref", "/onboarding"], | ||
}; | ||
|
||
export const routeTypes = { | ||
authed: [...routeGroups.canvas, ...routeGroups.dash, ...routeGroups.other], | ||
unauthed: [...routeGroups.auth, ...routeGroups.landing], | ||
unAuthedOnly: [...routeGroups.landing, "/signin"], | ||
} |
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.
Comment: Redirect logic in Signin function should handle more cases.
Solution: Add error handling to manage failed authentication attempts and provide user feedback.
Reason For Comment: The current implementation only checks if a user exists but does not handle cases where the authentication fails or if the user is not authorized.