Paths

Table of Contentst

Differential D8537

Tune cookie behaviors for 'phcid', 'phreg', etc
ClosedPublic
Actions

Authored by epriestley on Mar 14 2014, 7:46 PM.

Tags

None

Referenced Files

	F13166735: D8537.diff
	Tue, May 7, 6:32 AM

	Unknown Object (File)
	Sun, May 5, 6:29 AM

	Unknown Object (File)
	Fri, May 3, 5:49 AM

	Unknown Object (File)
	Mon, Apr 29, 4:27 PM

	Unknown Object (File)
	Wed, Apr 24, 10:32 PM

	Unknown Object (File)
	Sat, Apr 20, 10:05 PM

	Unknown Object (File)
	Sat, Apr 20, 6:39 PM

	Unknown Object (File)
	Thu, Apr 11, 6:43 PM

Subscribers

Details

Reviewers

Maniphest Tasks

T3471: Be more conservative in overwriting `phcid`

Commits

Restricted Diffusion Commit
rP559c0fe88663: Tune cookie behaviors for 'phcid', 'phreg', etc

Summary

Fixes T3471. Specific issues:

Add the ability to set a temporary cookie (expires when the browser closes).
We overwrote 'phcid' on every page load. This creates some issues with browser extensions. Instead, only write it if isn't set. To counterbalance this, make it temporary.
Make the 'next_uri' cookie temporary.
Make the 'phreg' cookie temporary.
Fix an issue where deleted cookies would persist after 302 (?) in some cases (this is/was 100% for me locally).

Test Plan

Closed my browser, reopned it, verified temporary cookies were gone.
Logged in, authed, linked, logged out.

Diff Detail

Repository

Lint

Lint Skipped

Unit

Tests Skipped

Event Timeline

epriestley updated this revision to Diff 20258.Mar 14 2014, 7:46 PM

epriestley retitled this revision from to Tune cookie behaviors for 'phcid', 'phreg', etc.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: btrahan.

epriestley added a task: T3471: Be more conservative in overwriting `phcid`.

Herald added a subscriber: epriestley.Mar 14 2014, 7:46 PM

First I can't have $_GET / $_POST parameters called 'code', and now I can't have $_COOKIE values of 'deleted' ?

Macro suchwow: much sparta such writ

This revision is now accepted and ready to land.Mar 14 2014, 8:06 PM

Yeah, this technically burns users with the username deleted, since it won't prefill in the "Username: ..." field anymore.

We could put some kind of escaping on that cookie if anyone ever complains.

Closed by commit rP559c0fe88663 (authored by @epriestley).

Revision Contents
Changeset List

Path

Size

src/

aphront/

AphrontRequest.php

68 lines

applications/

auth/

constants/

PhabricatorCookies.php

32 lines

controller/

PhabricatorAuthLinkController.php

4 lines

PhabricatorAuthLoginController.php

2 lines

PhabricatorAuthStartController.php

6 lines

Diff 20264

src/aphront/AphrontRequest.php

Loading...

src/applications/auth/constants/PhabricatorCookies.php

Loading...

src/applications/auth/controller/PhabricatorAuthLinkController.php

Loading...

src/applications/auth/controller/PhabricatorAuthLoginController.php

Loading...

src/applications/auth/controller/PhabricatorAuthStartController.php

Loading...