@recaptime-dev's working patches + fork for Phorge, a community fork of Phabricator. (Upstream dev and stable branches are at upstream/main and upstream/stable respectively.)
hq.recaptime.dev/wiki/Phorge
phorge
phabricator
Actually check CSRF on Password and LDAP forms
Summary: Ref T4339. We didn't previously check `isFormPost()` on these, but now should.
Test Plan: Changed csrf token on login, got kicked out.
Reviewers: btrahan, chad
Reviewed By: chad
CC: aran
Maniphest Tasks: T4339
Differential Revision: https://secure.phabricator.com/D8051
+35
-30
+18
-16
src/applications/auth/provider/PhabricatorAuthProviderLDAP.php
+18
-16
src/applications/auth/provider/PhabricatorAuthProviderLDAP.php
···
150
150
return array($account, $response);
151
151
}
152
152
153
-
try {
154
-
if (strlen($username) && $has_password) {
155
-
$adapter = $this->getAdapter();
156
-
$adapter->setLoginUsername($username);
157
-
$adapter->setLoginPassword($password);
153
+
if ($request->isFormPost()) {
154
+
try {
155
+
if (strlen($username) && $has_password) {
156
+
$adapter = $this->getAdapter();
157
+
$adapter->setLoginUsername($username);
158
+
$adapter->setLoginPassword($password);
158
159
159
-
// TODO: This calls ldap_bind() eventually, which dumps cleartext
160
-
// passwords to the error log. See note in PhutilAuthAdapterLDAP.
161
-
// See T3351.
160
+
// TODO: This calls ldap_bind() eventually, which dumps cleartext
161
+
// passwords to the error log. See note in PhutilAuthAdapterLDAP.
162
+
// See T3351.
162
163
163
-
DarkConsoleErrorLogPluginAPI::enableDiscardMode();
164
-
$account_id = $adapter->getAccountID();
165
-
DarkConsoleErrorLogPluginAPI::disableDiscardMode();
166
-
} else {
167
-
throw new Exception("Username and password are required!");
164
+
DarkConsoleErrorLogPluginAPI::enableDiscardMode();
165
+
$account_id = $adapter->getAccountID();
166
+
DarkConsoleErrorLogPluginAPI::disableDiscardMode();
167
+
} else {
168
+
throw new Exception("Username and password are required!");
169
+
}
170
+
} catch (Exception $ex) {
171
+
// TODO: Make this cleaner.
172
+
throw $ex;
168
173
}
169
-
} catch (Exception $ex) {
170
-
// TODO: Make this cleaner.
171
-
throw $ex;
172
174
}
173
175
174
176
return array($this->loadOrCreateAccount($account_id), $response);
+17
-14
src/applications/auth/provider/PhabricatorAuthProviderPassword.php
+17
-14
src/applications/auth/provider/PhabricatorAuthProviderPassword.php
···
163
163
$account = null;
164
164
$log_user = null;
165
165
166
-
if (!$require_captcha || $captcha_valid) {
167
-
$username_or_email = $request->getStr('username');
168
-
if (strlen($username_or_email)) {
169
-
$user = id(new PhabricatorUser())->loadOneWhere(
170
-
'username = %s',
171
-
$username_or_email);
166
+
if ($request->isFormPost()) {
167
+
if (!$require_captcha || $captcha_valid) {
168
+
$username_or_email = $request->getStr('username');
169
+
if (strlen($username_or_email)) {
170
+
$user = id(new PhabricatorUser())->loadOneWhere(
171
+
'username = %s',
172
+
$username_or_email);
172
173
173
-
if (!$user) {
174
-
$user = PhabricatorUser::loadOneWithEmailAddress($username_or_email);
175
-
}
174
+
if (!$user) {
175
+
$user = PhabricatorUser::loadOneWithEmailAddress(
176
+
$username_or_email);
177
+
}
176
178
177
-
if ($user) {
178
-
$envelope = new PhutilOpaqueEnvelope($request->getStr('password'));
179
-
if ($user->comparePassword($envelope)) {
180
-
$account = $this->loadOrCreateAccount($user->getPHID());
181
-
$log_user = $user;
179
+
if ($user) {
180
+
$envelope = new PhutilOpaqueEnvelope($request->getStr('password'));
181
+
if ($user->comparePassword($envelope)) {
182
+
$account = $this->loadOrCreateAccount($user->getPHID());
183
+
$log_user = $user;
184
+
}
182
185
}
183
186
}
184
187
}