Skip to content
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

Revert "[PLA-1864] Remove csrf cookie meta and use cookie instead" #130

Merged
merged 1 commit into from
Jun 26, 2024

Conversation

enjinabner
Copy link
Contributor

@enjinabner enjinabner commented Jun 26, 2024

User description

Reverts #127


PR Type

Bug fix, Enhancement


Description

  • Added import and usage of ApiService.reloadCsrf() in various components to ensure CSRF token is reloaded after critical actions like account deletion, login, and logout.
  • Modified API service to include CSRF token in headers for multi-tenant requests and to handle 419 status by reloading the CSRF token and retrying the request.
  • Added meta tag for CSRF token in the HTML head to facilitate CSRF protection.

Changes walkthrough 📝

Relevant files
Enhancement
Settings.vue
Add CSRF token reload after account deletion                         

resources/js/components/pages/Settings.vue

  • Added import for ApiService.
  • Added call to ApiService.reloadCsrf() after account deletion.
  • +2/-0     
    index.ts
    Reload CSRF token after login and logout                                 

    resources/js/store/index.ts

    • Added calls to ApiService.reloadCsrf() after login and logout.
    +2/-0     
    app.blade.php
    Add CSRF token meta tag in HTML head                                         

    resources/views/app.blade.php

    • Added meta tag for CSRF token in the HTML head.
    +2/-1     
    Bug fix
    index.ts
    Include and reload CSRF token in API requests                       

    resources/js/api/index.ts

  • Added retrieval of CSRF token from meta tag.
  • Included CSRF token in headers for multi-tenant requests.
  • Added logic to reload CSRF token on 419 status and retry request.
  • +6/-1     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review [1-5] 3
    🧪 Relevant tests No
    🔒 Security concerns - CSRF Protection:
    The addition of a meta tag for CSRF token and its usage in headers for multi-tenant requests is a critical change. Reviewers should ensure that the implementation does not expose the application to CSRF attacks by verifying that tokens are properly validated on the server side for every state-changing request.
    ⚡ Key issues to review CSRF Token Handling:
    The changes include reloading the CSRF token in various parts of the application. Ensure that the ApiService.reloadCsrf() method properly handles the CSRF token across different states and user actions. It's crucial that this method is robust to prevent security issues.
    Error Handling:
    The modification in ApiService to handle a 419 status and retry the request needs careful review to ensure that it does not create an infinite loop or degrade performance under certain conditions.

    Copy link

    Failed to generate code suggestions for PR

    @enjinabner enjinabner merged commit 8bbb209 into master Jun 26, 2024
    3 checks passed
    @enjinabner enjinabner deleted the revert-127-feature/pla-1864/improve-csrf-cookie branch June 26, 2024 08:31
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Development

    Successfully merging this pull request may close these issues.

    2 participants