Page MenuHomePhabricator

Support CSRF for logged-out users
ClosedPublic

Authored by epriestley on Jan 23 2014, 7:25 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 17, 2:30 PM
Unknown Object (File)
Tue, Apr 16, 12:13 AM
Unknown Object (File)
Sun, Apr 14, 10:53 AM
Unknown Object (File)
Thu, Apr 11, 7:04 AM
Unknown Object (File)
Sun, Mar 31, 3:58 PM
Unknown Object (File)
Sun, Mar 31, 5:42 AM
Unknown Object (File)
Mar 12 2024, 12:43 AM
Unknown Object (File)
Feb 16 2024, 4:50 PM
Subscribers

Details

Reviewers
btrahan
Maniphest Tasks
T4339: Support CSRF for logged-out users
Commits
Restricted Diffusion Commit
rPf9ac534f255d: Support CSRF for logged-out users
Summary

Fixes T4339. If you're anonymous, we use a digest of your session key to generate a CSRF token. Otherwise, everything works normally.

Test Plan

Logged out, logged in, tweaked CSRF in forms -- I'll add some inlines.

Diff Detail

Lint
Lint Skipped
Unit
Tests Skipped

Event Timeline

epriestley updated this revision to Unknown Object (????).Jan 23 2014, 7:27 PM
  • Simplify things slightly.
src/aphront/AphrontRequest.php
215–218

All these changes are just pht() / usability. I've made the error messages a little more user-focused.

src/applications/auth/provider/PhabricatorAuthProviderPassword.php
189

This was incorrect before, but didn't get caught by CSRF because the user is always logged out.

195

This is overly aggressive now that we have anonymous sessions.

src/applications/base/controller/PhabricatorController.php
47–54

I moved this from StandardPageView because we need the token to exist before we can render forms, so it has to happen on every page. That's fine, and probably cleaner, since we have session code in fewer places now.

56–58

"If the user isn't logged in, hash their anonymous session and use it to generate CSRF tokens."

src/applications/people/storage/PhabricatorUser.php
287–291

"If the user is logged in, use normal stuff. Otherwise, use the alternate base string."

src/view/page/PhabricatorStandardPageView.php
166–175

This moved to user stuff.

Nice! Thanks again for the comments.