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
F13239817: D13151.id31788.diff
Wed, May 22, 6:53 AM
F13236615: D13151.id31788.diff
Tue, May 21, 10:15 AM
F13236599: D13151.id31802.diff
Tue, May 21, 10:14 AM
F13236574: D13151.id.diff
Tue, May 21, 10:11 AM
F13232824: D13151.diff
Tue, May 21, 1:30 AM
F13208009: D13151.id31802.diff
Thu, May 16, 7:41 AM
F13200657: D13151.diff
Tue, May 14, 2:22 AM
F13192581: D13151.id31788.diff
Sun, May 12, 8:46 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
Branch
spaces1
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 6543
Build 6565: [Placeholder Plan] Wait for 30 Seconds

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.