Skip to content

Conversation

@heryandjaruma
Copy link
Collaborator

@heryandjaruma heryandjaruma commented May 17, 2025

User description

  • Add bypass

PR Type

Enhancement


Description

  • Add HTTP methods to CORS options

  • Remove explicit preflight handler

  • Bypass auth and CSRF on OPTIONS

  • Retain CORS and session validation


Changes walkthrough 📝

Relevant files
Enhancement
server.ts
Update CORS options and OPTIONS middleware                             

functions/src/server.ts

  • Included HTTP methods in corsOptions
  • Removed app.options("*", cors(...)) preflight handler
  • Added middleware sending 204 for OPTIONS
  • Commented bypass for auth and CSRF
  • +11/-2   

    Need help?
  • Type /help how to ... in the comments thread for any questions about PR-Agent usage.
  • Check out the documentation for more information.
  • @heryandjaruma heryandjaruma self-assigned this May 17, 2025
    @heryandjaruma heryandjaruma merged commit abaabb9 into main May 17, 2025
    1 check passed
    @github-actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Middleware Order

    The bypass middleware for OPTIONS requests is added after cookieParser and JSON parsing, causing unnecessary processing on preflight requests. Consider moving it before these middlewares to short-circuit earlier.

    // Bypass auth and CSRF for OPTIONS requests
    app.use((req: Request, res: Response, next: NextFunction) => {
      if (req.method === "OPTIONS") {
        res.status(204).send(""); // Ensure preflight requests return 204
        return;
      }
      next();
    });
    CORS Methods

    The CORS methods array omits some HTTP verbs (e.g., HEAD, PATCH). Verify that all required methods your API uses are covered.

    methods: ["GET", "POST", "PUT", "DELETE", "OPTIONS"],

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Security
    Include CORS headers on preflight

    Invoke the CORS middleware in your OPTIONS bypass so the response includes proper
    CORS headers. Wrap the handler in cors(corsOptions) and use res.sendStatus(204) for
    clarity.

    functions/src/server.ts [32-38]

     app.use((req: Request, res: Response, next: NextFunction) => {
       if (req.method === "OPTIONS") {
    -    res.status(204).send(""); // Ensure preflight requests return 204
    -    return;
    +    return cors(corsOptions)(req, res, () => res.sendStatus(204));
       }
       next();
     });
    Suggestion importance[1-10]: 8

    __

    Why: Wrapping the OPTIONS handler in the cors(corsOptions) middleware ensures the preflight response includes the proper CORS headers, which is essential for compliant cross-origin requests.

    Medium
    Auto-handle CORS preflight

    Extend the CORS options to let the middleware auto-respond to preflight requests
    with a 204 status, avoiding a custom bypass. Add optionsSuccessStatus and
    preflightContinue so CORS handles OPTIONS before auth/CSRF middleware.

    functions/src/server.ts [11-24]

     const corsOptions: CorsOptions = {
       origin: [
         "http://localhost:3000",
         "http://localhost:3001",
         "http://localhost:5173",
         "https://garudahacks.com",
         "https://www.garudahacks.com",
         "https://portal-ochre-iota.vercel.app",
         "https://portal.garudahacks.com",
       ],
       credentials: true,
       allowedHeaders: ["Content-Type", "Authorization", "X-XSRF-TOKEN"],
       methods: ["GET", "POST", "PUT", "DELETE", "OPTIONS"],
    +  optionsSuccessStatus: 204,
    +  preflightContinue: false,
     };
    Suggestion importance[1-10]: 7

    __

    Why: The suggestion correctly leverages optionsSuccessStatus and preflightContinue on the corsOptions to simplify preflight handling and remove the custom bypass, improving maintainability and security.

    Medium

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    2 participants