Index: src/__phutil_library_map__.php =================================================================== --- src/__phutil_library_map__.php +++ src/__phutil_library_map__.php @@ -1319,6 +1319,7 @@ 'PhabricatorContentSource' => 'applications/metamta/contentsource/PhabricatorContentSource.php', 'PhabricatorContentSourceView' => 'applications/metamta/contentsource/PhabricatorContentSourceView.php', 'PhabricatorController' => 'applications/base/controller/PhabricatorController.php', + 'PhabricatorCookies' => 'applications/auth/constants/PhabricatorCookies.php', 'PhabricatorCoreConfigOptions' => 'applications/config/option/PhabricatorCoreConfigOptions.php', 'PhabricatorCountdown' => 'applications/countdown/storage/PhabricatorCountdown.php', 'PhabricatorCountdownCapabilityDefaultView' => 'applications/countdown/capability/PhabricatorCountdownCapabilityDefaultView.php', @@ -3934,6 +3935,7 @@ 'PhabricatorConpherencePHIDTypeThread' => 'PhabricatorPHIDType', 'PhabricatorContentSourceView' => 'AphrontView', 'PhabricatorController' => 'AphrontController', + 'PhabricatorCookies' => 'Phobject', 'PhabricatorCoreConfigOptions' => 'PhabricatorApplicationConfigOptions', 'PhabricatorCountdown' => array( Index: src/aphront/console/DarkConsoleDataController.php =================================================================== --- src/aphront/console/DarkConsoleDataController.php +++ src/aphront/console/DarkConsoleDataController.php @@ -58,12 +58,17 @@ $panel = $obj->renderPanel(); - if (!empty($_COOKIE['phsid'])) { - $panel = PhutilSafeHTML::applyFunction( - 'str_replace', - $_COOKIE['phsid'], - '(session-key)', - $panel); + // Because cookie names can now be prefixed, wipe out any cookie value + // with the session cookie name anywhere in its name. + $pattern = '('.preg_quote(PhabricatorCookies::COOKIE_SESSION).')'; + foreach ($_COOKIE as $cookie_name => $cookie_value) { + if (preg_match($pattern, $cookie_name)) { + $panel = PhutilSafeHTML::applyFunction( + 'str_replace', + $cookie_value, + '(session-key)', + $panel); + } } $output['panel'][$class] = $panel; Index: src/applications/auth/constants/PhabricatorCookies.php =================================================================== --- /dev/null +++ src/applications/auth/constants/PhabricatorCookies.php @@ -0,0 +1,48 @@ +getRequest(); - $request->setCookie('phusr', $user->getUsername()); - $request->setCookie('phsid', $session_key); + $request->setCookie( + PhabricatorCookies::COOKIE_USERNAME, + $user->getUsername()); + $request->setCookie( + PhabricatorCookies::COOKIE_SESSION, + $session_key); $this->clearRegistrationCookies(); } @@ -101,15 +105,15 @@ $request = $this->getRequest(); // Clear the registration key. - $request->clearCookie('phreg'); + $request->clearCookie(PhabricatorCookies::COOKIE_REGISTRATION); // Clear the client ID / OAuth state key. - $request->clearCookie('phcid'); + $request->clearCookie(PhabricatorCookies::COOKIE_CLIENTID); } private function buildLoginValidateResponse(PhabricatorUser $user) { $validate_uri = new PhutilURI($this->getApplicationURI('validate/')); - $validate_uri->setQueryParam('phusr', $user->getUsername()); + $validate_uri->setQueryParam('expect', $user->getUsername()); return id(new AphrontRedirectResponse())->setURI((string)$validate_uri); } @@ -168,7 +172,8 @@ return array($account, $provider, $response); } - $registration_key = $request->getCookie('phreg'); + $registration_key = $request->getCookie( + PhabricatorCookies::COOKIE_REGISTRATION); // NOTE: This registration key check is not strictly necessary, because // we're only creating new accounts, not linking existing accounts. It @@ -181,7 +186,7 @@ // since you could have simply completed the process yourself. if (!$registration_key) { - $response = $this->renderError( + $response = $this->renderError( pht( 'Your browser did not submit a registration key with the request. '. 'You must use the same browser to begin and complete registration. '. Index: src/applications/auth/controller/PhabricatorAuthLinkController.php =================================================================== --- src/applications/auth/controller/PhabricatorAuthLinkController.php +++ src/applications/auth/controller/PhabricatorAuthLinkController.php @@ -79,7 +79,10 @@ $panel_uri = '/settings/panel/external/'; - $request->setCookie('phcid', Filesystem::readRandomCharacters(16)); + $request->setCookie( + PhabricatorCookies::COOKIE_CLIENTID, + Filesystem::readRandomCharacters(16)); + switch ($this->action) { case 'link': $form = $provider->buildLinkForm($this); Index: src/applications/auth/controller/PhabricatorAuthLoginController.php =================================================================== --- src/applications/auth/controller/PhabricatorAuthLoginController.php +++ src/applications/auth/controller/PhabricatorAuthLoginController.php @@ -166,7 +166,9 @@ $account->save(); unset($unguarded); - $this->getRequest()->setCookie('phreg', $registration_key); + $this->getRequest()->setCookie( + PhabricatorCookies::COOKIE_REGISTRATION, + $registration_key); return id(new AphrontRedirectResponse())->setURI($next_uri); } Index: src/applications/auth/controller/PhabricatorAuthStartController.php =================================================================== --- src/applications/auth/controller/PhabricatorAuthStartController.php +++ src/applications/auth/controller/PhabricatorAuthStartController.php @@ -24,10 +24,10 @@ return $this->processConduitRequest(); } - if ($request->getCookie('phusr') && $request->getCookie('phsid')) { + if (strlen($request->getCookie(PhabricatorCookies::COOKIE_SESSION))) { // The session cookie is invalid, so clear it. - $request->clearCookie('phusr'); - $request->clearCookie('phsid'); + $request->clearCookie(PhabricatorCookies::COOKIE_USERNAME); + $request->clearCookie(PhabricatorCookies::COOKIE_SESSION); return $this->renderError( pht( @@ -71,8 +71,12 @@ } if (!$request->isFormPost()) { - $request->setCookie('next_uri', $next_uri); - $request->setCookie('phcid', Filesystem::readRandomCharacters(16)); + $request->setCookie( + PhabricatorCookies::COOKIE_NEXTURI, + $next_uri); + $request->setCookie( + PhabricatorCookies::COOKIE_CLIENTID, + Filesystem::readRandomCharacters(16)); } $not_buttons = array(); Index: src/applications/auth/controller/PhabricatorAuthValidateController.php =================================================================== --- src/applications/auth/controller/PhabricatorAuthValidateController.php +++ src/applications/auth/controller/PhabricatorAuthValidateController.php @@ -13,7 +13,7 @@ $failures = array(); - if (!strlen($request->getStr('phusr'))) { + if (!strlen($request->getStr('expect'))) { return $this->renderErrors( array( pht( @@ -21,8 +21,8 @@ 'phusr'))); } - $expect_phusr = $request->getStr('phusr'); - $actual_phusr = $request->getCookie('phusr'); + $expect_phusr = $request->getStr('expect'); + $actual_phusr = $request->getCookie(PhabricatorCookies::COOKIE_USERNAME); if ($actual_phusr != $expect_phusr) { if ($actual_phusr) { $failures[] = pht( @@ -54,8 +54,8 @@ return $this->renderErrors($failures); } - $next = $request->getCookie('next_uri'); - $request->clearCookie('next_uri'); + $next = $request->getCookie(PhabricatorCookies::COOKIE_NEXTURI); + $request->clearCookie(PhabricatorCookies::COOKIE_NEXTURI); if (!PhabricatorEnv::isValidLocalWebResource($next)) { $next = '/'; Index: src/applications/auth/controller/PhabricatorEmailTokenController.php =================================================================== --- src/applications/auth/controller/PhabricatorEmailTokenController.php +++ src/applications/auth/controller/PhabricatorEmailTokenController.php @@ -86,7 +86,7 @@ )); } - $request->setCookie('next_uri', $next); + $request->setCookie(PhabricatorCookies::COOKIE_NEXTURI, $next); return $this->loginUser($target_user); } Index: src/applications/auth/controller/PhabricatorLogoutController.php =================================================================== --- src/applications/auth/controller/PhabricatorLogoutController.php +++ src/applications/auth/controller/PhabricatorLogoutController.php @@ -32,8 +32,8 @@ // Destroy the user's session in the database so logout works even if // their cookies have some issues. We'll detect cookie issues when they // try to login again and tell them to clear any junk. - $phsid = $request->getCookie('phsid'); - if ($phsid) { + $phsid = $request->getCookie(PhabricatorCookies::COOKIE_SESSION); + if (strlen($phsid)) { $session = id(new PhabricatorAuthSessionQuery()) ->setViewer($user) ->withSessionKeys(array($phsid)) @@ -42,7 +42,7 @@ $session->delete(); } } - $request->clearCookie('phsid'); + $request->clearCookie(PhabricatorCookies::COOKIE_SESSION); return id(new AphrontRedirectResponse()) ->setURI('/login/'); @@ -52,8 +52,7 @@ $dialog = id(new AphrontDialogView()) ->setUser($user) ->setTitle(pht('Log out of Phabricator?')) - ->appendChild(phutil_tag('p', array(), pht( - 'Are you sure you want to log out?'))) + ->appendChild(pht('Are you sure you want to log out?')) ->addSubmitButton(pht('Logout')) ->addCancelButton('/'); Index: src/applications/auth/provider/PhabricatorAuthProviderOAuth.php =================================================================== --- src/applications/auth/provider/PhabricatorAuthProviderOAuth.php +++ src/applications/auth/provider/PhabricatorAuthProviderOAuth.php @@ -35,9 +35,11 @@ protected function renderLoginForm(AphrontRequest $request, $mode) { $adapter = $this->getAdapter(); - $adapter->setState(PhabricatorHash::digest($request->getCookie('phcid'))); + $adapter->setState( + PhabricatorHash::digest( + $request->getCookie(PhabricatorCookies::COOKIE_CLIENTID))); - $scope = $request->getStr("scope"); + $scope = $request->getStr('scope'); if ($scope) { $adapter->setScope($scope); } @@ -81,14 +83,15 @@ } if ($adapter->supportsStateParameter()) { - $phcid = $request->getCookie('phcid'); + $phcid = $request->getCookie(PhabricatorCookies::COOKIE_CLIENTID); if (!strlen($phcid)) { $response = $controller->buildProviderErrorResponse( $this, pht( - 'Your browser did not submit a "phcid" cookie with OAuth state '. + 'Your browser did not submit a "%s" cookie with OAuth state '. 'information in the request. Check that cookies are enabled. '. - 'If this problem persists, you may need to clear your cookies.')); + 'If this problem persists, you may need to clear your cookies.', + PhabricatorCookies::COOKIE_CLIENTID)); } $state = $request->getStr('state'); Index: src/applications/auth/provider/PhabricatorAuthProviderPassword.php =================================================================== --- src/applications/auth/provider/PhabricatorAuthProviderPassword.php +++ src/applications/auth/provider/PhabricatorAuthProviderPassword.php @@ -87,7 +87,7 @@ $v_user = nonempty( $request->getStr('username'), - $request->getCookie('phusr')); + $request->getCookie(PhabricatorCookies::COOKIE_USERNAME)); $e_user = null; $e_pass = null; @@ -191,8 +191,8 @@ PhabricatorUserLog::ACTION_LOGIN_FAILURE); $log->save(); - $request->clearCookie('phusr'); - $request->clearCookie('phsid'); + $request->clearCookie(PhabricatorCookies::COOKIE_USERNAME); + $request->clearCookie(PhabricatorCookies::COOKIE_SESSION); $response = $controller->buildProviderPageResponse( $this, Index: src/applications/base/controller/PhabricatorController.php =================================================================== --- src/applications/base/controller/PhabricatorController.php +++ src/applications/base/controller/PhabricatorController.php @@ -35,7 +35,7 @@ } else { $user = new PhabricatorUser(); - $phsid = $request->getCookie('phsid'); + $phsid = $request->getCookie(PhabricatorCookies::COOKIE_SESSION); if ($phsid) { $session_user = id(new PhabricatorAuthSessionEngine()) ->loadUserForSession(PhabricatorAuthSession::TYPE_WEB, $phsid); Index: src/applications/people/storage/PhabricatorUserLog.php =================================================================== --- src/applications/people/storage/PhabricatorUserLog.php +++ src/applications/people/storage/PhabricatorUserLog.php @@ -66,7 +66,9 @@ $this->remoteAddr = idx($_SERVER, 'REMOTE_ADDR', ''); } if (!$this->session) { - $this->setSession(idx($_COOKIE, 'phsid')); + // TODO: This is not correct if there's a cookie prefix. This object + // should take an AphrontRequest. + $this->setSession(idx($_COOKIE, PhabricatorCookies::COOKIE_SESSION)); } $this->details['host'] = php_uname('n'); $this->details['user_agent'] = AphrontRequest::getHTTPHeader('User-Agent'); Index: src/applications/settings/panel/PhabricatorSettingsPanelSessions.php =================================================================== --- src/applications/settings/panel/PhabricatorSettingsPanelSessions.php +++ src/applications/settings/panel/PhabricatorSettingsPanelSessions.php @@ -40,11 +40,8 @@ ->withPHIDs($identity_phids) ->execute(); - // TODO: Once this has a real ID column, use that instead. - $sessions = msort($sessions, 'getSessionStart'); - $sessions = array_reverse($sessions); - - $current_key = PhabricatorHash::digest($request->getCookie('phsid')); + $current_key = PhabricatorHash::digest( + $request->getCookie(PhabricatorCookies::COOKIE_SESSION)); $rows = array(); $rowc = array();