Page MenuHomePhabricator

Make CSRF salt per-user instead of per-request
ClosedPublic

Authored by epriestley on Jun 4 2015, 6:49 PM.
Tags
None
Referenced Files
F15586498: D13151.id31802.diff
Thu, May 8, 6:52 PM
F15557430: D13151.id31788.diff
Mon, Apr 28, 9:34 PM
F15554193: D13151.id.diff
Mon, Apr 28, 5:57 AM
F15549589: D13151.diff
Sun, Apr 27, 6:43 AM
F15543658: D13151.id31788.diff
Sat, Apr 26, 12:11 AM
F15500826: D13151.id31788.diff
Sun, Apr 13, 7:32 PM
F15464928: D13151.id.diff
Apr 2 2025, 1:07 PM
F15457794: D13151.diff
Mar 30 2025, 7:02 PM
Subscribers

Details

Summary

Fixes T8326. This removes calls to PhabricatorStartup from places that daemons may access.

This salt doesn't need to be global; it's embedded in the token we return. It's fine if we use a different salt every time. In practice, we always use the same viewer, so this change causes little or no behavioral change.

Ref T8424. For Spaces, I need a per-request cache for all spaces, because they have unusual access patterns and require repeated access, in some cases by multiple viewers.

We don't currently have a per-request in-process cache that we, e.g., clear in the daemons.

We do have a weak/theoretical/forward-looking attempt at this in PhabricatorStartup::getGlobal() but I'm going to throw that away (it's kind of junky, partly because of T8326) and replace it with a more formal mechanism.

Test Plan
  • Submitted some forms.
  • Grepped for csrf.salt.
  • Viewed page source, saw nice CSRF tokens with salt.
  • All the salts are still the same on every page I checked, but it doesn't matter if this isn't true everywhere.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

epriestley retitled this revision from to Make CSRF salt per-user instead of per-request.
epriestley updated this object.
epriestley edited the test plan for this revision. (Show Details)
epriestley added a reviewer: btrahan.
btrahan edited edge metadata.
This revision is now accepted and ready to land.Jun 4 2015, 8:27 PM
This revision was automatically updated to reflect the committed changes.