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
Unknown Object (File)
Tue, Apr 16, 11:15 PM
Unknown Object (File)
Wed, Apr 10, 7:40 PM
Unknown Object (File)
Fri, Apr 5, 1:11 PM
Unknown Object (File)
Sun, Mar 31, 6:45 PM
Unknown Object (File)
Sun, Mar 31, 9:30 AM
Unknown Object (File)
Sat, Mar 23, 8:23 PM
Unknown Object (File)
Fri, Mar 22, 9:10 PM
Unknown Object (File)
Fri, Mar 22, 8:20 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.