@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
Change the "can see remote address?" policy to "is administrator?" everywhere
Summary:
Depends on D18970. Ref T13049. Currently, the policy for viewing remote addresses is:
- In activity logs: administrators.
- In push and pull logs: users who can edit the corresponding repository.
This sort of makes sense, but is also sort of weird. Particularly, I think it's kind of hard to understand and predict, and hard to guess that this is the behavior we implement. The actual implementation is complex, too.
Instead, just use the rule "administrators can see remote addresses" consistently across all applications. This should generally be more strict than the old rule, because administrators could usually have seen everyone's address in the activity logs anyway. It's also simpler and more expected, and I don't really know of any legit use cases for the "repository editor" rule.
Test Plan: Viewed pull/push/activity logs as non-admin. Saw remote addresses as an admin, and none as a non-admin.
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13049
Differential Revision: https://secure.phabricator.com/D18971
+37
-44
+18
-2
src/applications/diffusion/query/DiffusionPullLogSearchEngine.php
+18
-2
src/applications/diffusion/query/DiffusionPullLogSearchEngine.php
···
60
60
}
61
61
62
62
protected function newExportFields() {
63
-
return array(
63
+
$viewer = $this->requireViewer();
64
+
65
+
$fields = array(
64
66
id(new PhabricatorPHIDExportField())
65
67
->setKey('repositoryPHID')
66
68
->setLabel(pht('Repository PHID')),
···
86
88
->setKey('date')
87
89
->setLabel(pht('Date')),
88
90
);
91
+
92
+
if ($viewer->getIsAdmin()) {
93
+
$fields[] = id(new PhabricatorStringExportField())
94
+
->setKey('remoteAddress')
95
+
->setLabel(pht('Remote Address'));
96
+
}
97
+
98
+
return $fields;
89
99
}
90
100
91
101
protected function newExportData(array $events) {
···
117
127
$puller_name = null;
118
128
}
119
129
120
-
$export[] = array(
130
+
$map = array(
121
131
'repositoryPHID' => $repository_phid,
122
132
'repository' => $repository_name,
123
133
'pullerPHID' => $puller_phid,
···
127
137
'code' => $event->getResultCode(),
128
138
'date' => $event->getEpoch(),
129
139
);
140
+
141
+
if ($viewer->getIsAdmin()) {
142
+
$map['remoteAddress'] = $event->getRemoteAddress();
143
+
}
144
+
145
+
$export[] = $map;
130
146
}
131
147
132
148
return $export;
+13
-23
src/applications/diffusion/view/DiffusionPullLogListView.php
+13
-23
src/applications/diffusion/view/DiffusionPullLogListView.php
···
22
22
}
23
23
$handles = $viewer->loadHandles($handle_phids);
24
24
25
-
// Figure out which repositories are editable. We only let you see remote
26
-
// IPs if you have edit capability on a repository.
27
-
$editable_repos = array();
28
-
if ($events) {
29
-
$editable_repos = id(new PhabricatorRepositoryQuery())
30
-
->setViewer($viewer)
31
-
->requireCapabilities(
32
-
array(
33
-
PhabricatorPolicyCapability::CAN_VIEW,
34
-
PhabricatorPolicyCapability::CAN_EDIT,
35
-
))
36
-
->withPHIDs(mpull($events, 'getRepositoryPHID'))
37
-
->execute();
38
-
$editable_repos = mpull($editable_repos, null, 'getPHID');
39
-
}
25
+
// Only administrators can view remote addresses.
26
+
$remotes_visible = $viewer->getIsAdmin();
40
27
41
28
$rows = array();
42
-
$any_host = false;
43
29
foreach ($events as $event) {
44
30
if ($event->getRepositoryPHID()) {
45
31
$repository = $event->getRepository();
···
47
33
$repository = null;
48
34
}
49
35
50
-
// Reveal this if it's valid and the user can edit the repository. For
51
-
// invalid requests you currently have to go fishing in the database.
52
-
$remote_address = '-';
53
-
if ($repository) {
54
-
if (isset($editable_repos[$event->getRepositoryPHID()])) {
55
-
$remote_address = $event->getRemoteAddress();
56
-
}
36
+
if ($remotes_visible) {
37
+
$remote_address = $event->getRemoteAddress();
38
+
} else {
39
+
$remote_address = null;
57
40
}
58
41
59
42
$event_id = $event->getID();
···
107
90
'',
108
91
'n',
109
92
'right',
93
+
))
94
+
->setColumnVisibility(
95
+
array(
96
+
true,
97
+
true,
98
+
true,
99
+
$remotes_visible,
110
100
));
111
101
112
102
return $table;
+6
-19
src/applications/diffusion/view/DiffusionPushLogListView.php
+6
-19
src/applications/diffusion/view/DiffusionPushLogListView.php
···
25
25
26
26
$handles = $viewer->loadHandles($handle_phids);
27
27
28
-
// Figure out which repositories are editable. We only let you see remote
29
-
// IPs if you have edit capability on a repository.
30
-
$editable_repos = array();
31
-
if ($logs) {
32
-
$editable_repos = id(new PhabricatorRepositoryQuery())
33
-
->setViewer($viewer)
34
-
->requireCapabilities(
35
-
array(
36
-
PhabricatorPolicyCapability::CAN_VIEW,
37
-
PhabricatorPolicyCapability::CAN_EDIT,
38
-
))
39
-
->withPHIDs(mpull($logs, 'getRepositoryPHID'))
40
-
->execute();
41
-
$editable_repos = mpull($editable_repos, null, 'getPHID');
42
-
}
28
+
// Only administrators can view remote addresses.
29
+
$remotes_visible = $viewer->getIsAdmin();
43
30
44
31
$rows = array();
45
32
$any_host = false;
46
33
foreach ($logs as $log) {
47
34
$repository = $log->getRepository();
48
35
49
-
// Reveal this if it's valid and the user can edit the repository.
50
-
$remote_address = '-';
51
-
if (isset($editable_repos[$log->getRepositoryPHID()])) {
36
+
if ($remotes_visible) {
52
37
$remote_address = $log->getPushEvent()->getRemoteAddress();
38
+
} else {
39
+
$remote_address = null;
53
40
}
54
41
55
42
$event_id = $log->getPushEvent()->getID();
···
142
129
true,
143
130
true,
144
131
true,
145
-
true,
132
+
$remotes_visible,
146
133
true,
147
134
$any_host,
148
135
));