Skip to content

Conversation

@heryandjaruma
Copy link
Collaborator

@heryandjaruma heryandjaruma commented May 16, 2025

User description

  • Add portal cors
  • Exempt logout from csrf validation

PR Type

Bug fix, Enhancement


Description

Exempt /auth/logout from CSRF protection
Add portal domains to CORS origin list


Changes walkthrough 📝

Relevant files
Configuration changes
csrf_middleware.ts
Exempt logout from CSRF protection                                             

functions/src/middlewares/csrf_middleware.ts

  • Added /auth/logout to csrfExemptRoutes
+1/-0     
server.ts
Allow portal domains in CORS                                                         

functions/src/server.ts

  • Added https://portal.garudahacks.com to corsOptions.origin
  • Added https://www.portal.garudahacks.com to corsOptions.origin
  • +2/-0     

    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 16, 2025
    @heryandjaruma heryandjaruma merged commit f3241a5 into main May 16, 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: 1 🔵⚪⚪⚪⚪
    🧪 No relevant tests
    🔒 Security concerns

    CSRF exemption:
    The logout endpoint is now bypassing CSRF protection, which could allow malicious sites to force users to log out without consent. Ensure this is acceptable.

    ⚡ Recommended focus areas for review

    CSRF Exemption

    Exempting /auth/logout from CSRF protection could allow cross-site requests to trigger unwanted logouts. Confirm this behavior is intended and safe.

    "/auth/logout",
    Hardcoded Origins

    Adding CORS origins directly in code makes updates harder. Consider loading allowed origins from environment variables or configuration.

    "https://portal.garudahacks.com",
    "https://www.portal.garudahacks.com",

    @github-actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Security
    Reinstate CSRF protection on logout

    Removing the logout route from the CSRF exemption list will ensure that logout
    requests require a valid CSRF token, preventing potential cross‐site logout attacks.
    Keep logout protected by CSRF middleware.

    functions/src/middlewares/csrf_middleware.ts [9]

    -"/auth/logout",
    +const csrfExemptRoutes = [
    +  "/auth/login",
    +  "/auth/register",
    +  "/auth/session-login",
    +  // "/auth/reset-password",
    +];
    Suggestion importance[1-10]: 9

    __

    Why: Removing "/auth/logout" from csrfExemptRoutes prevents unauthorized logout CSRF attacks and restores security protection.

    High
    General
    Use env var for portal origins

    Load portal origins from an environment variable to avoid hardcoding and make it
    easier to update allowed domains across environments.

    functions/src/server.ts [18-19]

    -"https://portal.garudahacks.com",
    -"https://www.portal.garudahacks.com",
    +...(process.env.PORTAL_ORIGINS?.split(',') || [
    +  "https://portal.garudahacks.com",
    +  "https://www.portal.garudahacks.com",
    +]),
    Suggestion importance[1-10]: 6

    __

    Why: Loading portal origins from process.env.PORTAL_ORIGINS removes hardcoding and enhances configurability across environments.

    Low
    Merge portal domains into regex

    Combine both portal domains into a single regular expression entry to reduce
    duplication and simplify origin matching.

    functions/src/server.ts [18-19]

    -"https://portal.garudahacks.com",
    -"https://www.portal.garudahacks.com",
    +/^https:\/\/(www\.)?portal\.garudahacks\.com$/,
    Suggestion importance[1-10]: 5

    __

    Why: Consolidating both portal domains into a single regex reduces duplication and simplifies origin matching with minimal risk.

    Low

    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