Homec4science

Make CSRF salt per-user instead of per-request

Authored by epriestley <git@epriestley.com> on Jun 5 2015, 02:26.

Description

Make CSRF salt per-user instead of per-request

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.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T8326, T8424

Differential Revision: https://secure.phabricator.com/D13151

Details

Committed
epriestley <git@epriestley.com>Jun 5 2015, 02:26
Pushed
aubortJan 31 2017, 17:16
Parents
rPHb9d004e9c41b: Integrate Diviner with global search
Branches
Unknown
Tags
Unknown

Event Timeline

epriestley <git@epriestley.com> committed rPHe5b923743a13: Make CSRF salt per-user instead of per-request (authored by epriestley <git@epriestley.com>).Jun 5 2015, 02:26