1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-29 10:12:41 +01:00

Make the remote address rules for Settings > Activity Logs more consistent

Summary:
Depends on D18971. Ref T13049. The rule is currently "you can see IP addresses for actions which affect your account".

There's some legitimate motivation for this, since it's good if you can see that someone you don't recognize has been trying to log into your account.

However, this includes cases where an administrator disables/enables your account, or promotes/demotes you to administrator. In these cases, //their// IP is shown!

Make the rule:

  - Administrators can see it (consistent with everything else).
  - You can see your own actions.
  - You can see actions which affected you that have no actor (these are things like login attempts).
  - You can't see other stuff: usually, administrators changing your account settings.

Test Plan: Viewed activity log as a non-admin, no longer saw administrator's IP address disclosed in "Demote from Admin" log.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13049

Differential Revision: https://secure.phabricator.com/D18972
This commit is contained in:
epriestley 2018-01-30 12:13:28 -08:00
parent 8a2863e3f7
commit ff98f6f522
2 changed files with 46 additions and 27 deletions

View file

@ -30,11 +30,32 @@ final class PhabricatorUserLogView extends AphrontView {
$action_map = PhabricatorUserLog::getActionTypeMap();
$base_uri = $this->searchBaseURI;
$viewer_phid = $viewer->getPHID();
$rows = array();
foreach ($logs as $log) {
$ip = $log->getRemoteAddr();
$session = substr($log->getSession(), 0, 6);
$actor_phid = $log->getActorPHID();
$user_phid = $log->getUserPHID();
if ($viewer->getIsAdmin()) {
$can_see_ip = true;
} else if ($viewer_phid == $actor_phid) {
// You can see the address if you took the action.
$can_see_ip = true;
} else if (!$actor_phid && ($viewer_phid == $user_phid)) {
// You can see the address if it wasn't authenticated and applied
// to you (partial login).
$can_see_ip = true;
} else {
// You can't see the address when an administrator disables your
// account, since it's their address.
$can_see_ip = false;
}
if ($can_see_ip) {
$ip = $log->getRemoteAddr();
if ($base_uri) {
$ip = phutil_tag(
'a',
@ -43,18 +64,31 @@ final class PhabricatorUserLogView extends AphrontView {
),
$ip);
}
} else {
$ip = null;
}
$action = $log->getAction();
$action_name = idx($action_map, $action, $action);
if ($actor_phid) {
$actor_name = $handles[$actor_phid]->renderLink();
} else {
$actor_name = null;
}
if ($user_phid) {
$user_name = $handles[$user_phid]->renderLink();
} else {
$user_name = null;
}
$rows[] = array(
phabricator_date($log->getDateCreated(), $viewer),
phabricator_time($log->getDateCreated(), $viewer),
$action_name,
$log->getActorPHID()
? $handles[$log->getActorPHID()]->getName()
: null,
$username = $handles[$log->getUserPHID()]->renderLink(),
$actor_name,
$user_name,
$ip,
$session,
);

View file

@ -26,21 +26,6 @@ final class PhabricatorActivitySettingsPanel extends PhabricatorSettingsPanel {
->withRelatedPHIDs(array($user->getPHID()))
->executeWithCursorPager($pager);
$phids = array();
foreach ($logs as $log) {
$phids[] = $log->getUserPHID();
$phids[] = $log->getActorPHID();
}
if ($phids) {
$handles = id(new PhabricatorHandleQuery())
->setViewer($viewer)
->withPHIDs($phids)
->execute();
} else {
$handles = array();
}
$table = id(new PhabricatorUserLogView())
->setUser($viewer)
->setLogs($logs);