Skip to content

DO NOT MERGE ,Modify response of not allowed pages,logo and image ko … #62

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

Open
mr-raj12 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

DO NOT MERGE ,Modify response of not allowed pages,logo and image ko … #62

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
50 changes: 28 additions & 22 deletions src/middleware.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,38 @@
import { auth } from "@/auth"
import { NextResponse } from 'next/server'
import { auth } from "@/auth";
import { NextResponse } from "next/server";

export default auth((req) => {
const isAuthenticated = !!req.auth
const { pathname } = req.nextUrl
const isAuthenticated = !!req.auth;
const { pathname } = req.nextUrl;

if (!isAuthenticated && pathname === "/dashboard") {
return NextResponse.redirect(new URL("/login", req.url))
// Allow all static files and API requests
if (pathname.startsWith("/_next/") || pathname.startsWith("/api/") || pathname.startsWith("/static/")) {
return NextResponse.next();
}
Comment on lines +8 to +11
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The static file handling logic at lines 9-11 is redundant because the matcher configuration at line 37 already excludes these paths from middleware execution. The checks for paths starting with "/_next/", "/api/", and "/static/" will never be reached since the matcher pattern excludes "_next/static", "_next/image", "favicon.ico", and "api". This redundant code should be removed to avoid confusion.

Copilot uses AI. Check for mistakes.

// Redirect root ("/") to "/login"
if (pathname === "/") {
return NextResponse.redirect(new URL("/login", req.url));
}

// Restrict routes
if (!["/login", "/dashboard", "/scanner", "/logout", "/dashboard/non-bit"].includes(pathname)) {
return new NextResponse("Only /login, /dashboard, /scanner, /logout, and /dashboard/non-bit will work", { status: 404 });
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The error message text should not be embedded in the response body with a 404 status code. This creates a poor user experience as it returns a plain text message instead of rendering a proper 404 page. Consider either redirecting to a proper 404 error page or removing the whitelist approach entirely to allow Next.js to handle non-existent routes naturally with its built-in 404 handling.

Suggested change
return new NextResponse("Only /login, /dashboard, /scanner, /logout, and /dashboard/non-bit will work", { status: 404 });
return NextResponse.redirect(new URL("/404", req.url));

Copilot uses AI. Check for mistakes.
Comment on lines +19 to +20
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The /scanner/stats route is missing from the whitelist but exists in the application. Based on the scanner page code which imports and uses the Stats component, this route should be accessible but will be blocked by the whitelist, returning a 404 error instead.

Suggested change
if (!["/login", "/dashboard", "/scanner", "/logout", "/dashboard/non-bit"].includes(pathname)) {
return new NextResponse("Only /login, /dashboard, /scanner, /logout, and /dashboard/non-bit will work", { status: 404 });
if (!["/login", "/dashboard", "/scanner", "/scanner/stats", "/logout", "/dashboard/non-bit"].includes(pathname)) {
return new NextResponse("Only /login, /dashboard, /scanner, /scanner/stats, /logout, and /dashboard/non-bit will work", { status: 404 });

Copilot uses AI. Check for mistakes.
}
Comment on lines +9 to +21
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The matcher configuration does not exclude the public folder, and Next.js serves static files from the public directory at the root path (e.g., /logo.png maps to public/logo.png). With the current whitelist implementation, all public assets like /logo.png, /bitotsav-logo.svg, /manifest.json, and image files will be blocked and return a 404 error. The matcher should exclude public assets, or the whitelist should not be applied to direct file requests. Consider adding a check for file extensions (e.g., .png, .svg, .jpg, .json) or removing the restrictive whitelist approach.

Copilot uses AI. Check for mistakes.
Comment on lines +8 to +21
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The PR title mentions modifying responses for "not allowed pages, logo and image" but the implementation blocks access to valid application routes rather than just handling static assets. The title suggests this change is about fixing logo and image handling, but the actual implementation introduces a restrictive whitelist that breaks most of the application. This discrepancy between the PR description and implementation makes it unclear what the intended goal is. Clarification is needed on whether the goal is to (1) fix static asset handling, or (2) intentionally restrict the application to only a few routes for testing purposes.

Copilot uses AI. Check for mistakes.

Comment on lines +18 to +22
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The hardcoded route whitelist approach creates a maintenance burden and is fragile. Every time a new route is added to the application, the middleware must be updated manually. This is error-prone and doesn't scale well. Consider using a more maintainable approach such as: (1) defining protected routes instead of allowed routes, (2) using route metadata or conventions to determine which routes require authentication, or (3) removing the whitelist entirely and relying on Next.js's natural 404 behavior for non-existent routes while only adding authentication checks for routes that need protection.

Suggested change
// Restrict routes
if (!["/login", "/dashboard", "/scanner", "/logout", "/dashboard/non-bit"].includes(pathname)) {
return new NextResponse("Only /login, /dashboard, /scanner, /logout, and /dashboard/non-bit will work", { status: 404 });
}

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +22
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The route whitelist severely restricts access to most of the application's pages. The whitelist only allows access to /login, /dashboard, /scanner, /logout, and /dashboard/non-bit, but the application has many public routes that should remain accessible including: /events, /tickets, /team, /developers, /sponsors, /about, /merch, /gallery, /alumni, /signature, /virtual-helpdesk, /adminpanel, /concerts, /coming-soon, /leaderboard, /notice, /privacy, /terms, and event detail pages at /events/[id]. All these routes will return a 404 error with this change, breaking the majority of the application's functionality.

Suggested change
// Restrict routes
if (!["/login", "/dashboard", "/scanner", "/logout", "/dashboard/non-bit"].includes(pathname)) {
return new NextResponse("Only /login, /dashboard, /scanner, /logout, and /dashboard/non-bit will work", { status: 404 });
}

Copilot uses AI. Check for mistakes.
// Redirect users based on authentication
if (!isAuthenticated && ["/dashboard", "/dashboard/non-bit"].includes(pathname)) {
Comment on lines +19 to +24
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The /dashboard/day-scholar and /dashboard/bit routes exist in the application but are not included in the whitelist. These routes will return a 404 error even though they are valid application routes. Only /dashboard and /dashboard/non-bit are whitelisted, creating an inconsistency in dashboard sub-route access.

Suggested change
if (!["/login", "/dashboard", "/scanner", "/logout", "/dashboard/non-bit"].includes(pathname)) {
return new NextResponse("Only /login, /dashboard, /scanner, /logout, and /dashboard/non-bit will work", { status: 404 });
}
// Redirect users based on authentication
if (!isAuthenticated && ["/dashboard", "/dashboard/non-bit"].includes(pathname)) {
if (
![
"/login",
"/dashboard",
"/scanner",
"/logout",
"/dashboard/non-bit",
"/dashboard/day-scholar",
"/dashboard/bit",
].includes(pathname)
) {
return new NextResponse(
"Only /login, /dashboard, /scanner, /logout, /dashboard/non-bit, /dashboard/day-scholar, and /dashboard/bit will work",
{ status: 404 },
);
}
// Redirect users based on authentication
if (
!isAuthenticated &&
["/dashboard", "/dashboard/non-bit", "/dashboard/day-scholar", "/dashboard/bit"].includes(pathname)
) {

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The /scanner route is included in the whitelist but has no authentication check in the middleware. However, the scanner page implements its own passcode-based authentication (hardcoded passcode: '192020'). This creates an inconsistency where authenticated users are not redirected from /login but /scanner has no middleware-level authentication check. Consider whether /scanner should require authentication at the middleware level or if it should remain accessible to all users with its own internal authentication mechanism.

Suggested change
if (!isAuthenticated && ["/dashboard", "/dashboard/non-bit"].includes(pathname)) {
if (!isAuthenticated && ["/dashboard", "/dashboard/non-bit", "/scanner"].includes(pathname)) {

Copilot uses AI. Check for mistakes.
return NextResponse.redirect(new URL("/login", req.url));
}

if (isAuthenticated && pathname === "/login") {
return NextResponse.redirect(new URL("/dashboard", req.url))
return NextResponse.redirect(new URL("/dashboard", req.url));
}

return NextResponse.next()
})
return NextResponse.next();
});

// Protect all routes except public ones
// Updated matcher to exclude static assets and API routes
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The original matcher comment provided more context about what was being excluded and why. The new comment is less informative. While this is a minor issue, more descriptive comments help future maintainers understand the intent. Consider expanding the comment to explain why static assets and API routes are excluded from middleware processing.

Copilot uses AI. Check for mistakes.
export const config = {
matcher: [
/*
* Match all request paths except for the ones starting with:
* - api (API routes)
* - _next/static (static files)
* - _next/image (image optimization files)
* - favicon.ico (favicon file)
* - public folder
*/
'/((?!api|_next/static|_next/image|favicon.ico|public).*)',
],
}
matcher: ["/((?!_next/static|_next/image|favicon.ico|api).*)"], // Ignore static files and API routes
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

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

The matcher pattern excludes "_next/image" but the middleware checks for "/_next/" (line 9), which would include Next.js image optimization requests if they weren't already excluded by the matcher. Similarly, the pattern excludes "api" but checks for "/api/" with leading slash. This inconsistency between matcher patterns and runtime checks makes the code harder to understand and maintain.

Suggested change
matcher: ["/((?!_next/static|_next/image|favicon.ico|api).*)"], // Ignore static files and API routes
matcher: ["/((?!_next/|static/|api/|favicon\\.ico).*)"], // Ignore static files and API routes

Copilot uses AI. Check for mistakes.
};