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

Fix csrf token generation in LoginController::loginAction #437

Open
wants to merge 1 commit into
base: 1.4
Choose a base branch
from

Conversation

melissakittl
Copy link
Contributor

Problem

While working in admin interface in one browser tab, then opening another one via /admin/login or a deeplink admin/login/deeplink?object_29_object, the csrf token gets regenerated with $force = true. If you switch back to the first browser tab, the csrf token is not valid anymore, you get an Access denied error and have to reload the admin interface completely.

Expected behavior

Working on multiple tabs is possible without reloading the admin interface (some changes might get lost otherwise)

Actual behavior

In the first browser tab you see the following error, when saving an element (or doing something else)
image
In the logs you see: [2024-02-23T11:03:43.679029+01:00] security.ERROR: Detected CSRF attack on /admin/object/save {"request":"/admin/object/save"} []

Changes in this pull request

First check, if there is already a csrf token set in the session. If no csrf token can be found, then regenerate with $force = truein LoginController::loginAction

first check, if csrf token is still, otherwise regnerate with force = true
Copy link

sonarcloud bot commented Feb 23, 2024

Quality Gate Passed Quality Gate passed

Issues
6 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@kingjia90 kingjia90 added this to the 1.4.1 milestone Apr 5, 2024
@fashxp fashxp modified the milestones: 1.4.1, 1.4.2 Apr 23, 2024
@robertSt7 robertSt7 modified the milestones: 1.4.2, 1.4.3 Apr 24, 2024
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.

None yet

5 participants