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
F14058243: D13151.diff
Sun, Nov 17, 10:21 AM
F14035334: D13151.id31802.diff
Sun, Nov 10, 5:31 AM
F14035314: D13151.id.diff
Sun, Nov 10, 5:27 AM
F14034727: D13151.id31788.diff
Sun, Nov 10, 1:56 AM
F14034725: D13151.id31802.diff
Sun, Nov 10, 1:55 AM
F14034713: D13151.id31788.diff
Sun, Nov 10, 1:51 AM
F14034652: D13151.id.diff
Sun, Nov 10, 1:38 AM
F14030543: D13151.id31788.diff
Sat, Nov 9, 4:13 AM
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.