@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
OAuthServer - default "whoami" scope and refine scope-asking workflow
Summary: Ref T7153. The "whoami" scope should be default and always on, because otherwise we can't do anything at all. Also, if a client doesn't want a certain scope, don't bother asking the user for it. To get there, had to add "scope" to the definition of a client.
Test Plan: applied the patch to a phabricator "client" and a phabricator "server" as far as oauth shenanigans go. Then I tried to login / register with oauth. If the "client" was configured to ask for "always on" access I got that in the dialogue, and otherwise no additional scope questions were present. Verified scope was properly granted in either case.
Reviewers: epriestley
Reviewed By: epriestley
Subscribers: Korvin, epriestley
Maniphest Tasks: T7153
Differential Revision: https://secure.phabricator.com/D11705
+41
-23
+24
-3
src/applications/oauthserver/PhabricatorOAuthServerScope.php
+24
-3
src/applications/oauthserver/PhabricatorOAuthServerScope.php
···
18
18
);
19
19
}
20
20
21
-
static public function getCheckboxControl($current_scopes) {
21
+
static public function getDefaultScope() {
22
+
return self::SCOPE_WHOAMI;
23
+
}
24
+
25
+
static public function getCheckboxControl(
26
+
array $current_scopes) {
27
+
28
+
$have_options = false;
22
29
$scopes = self::getScopesDict();
23
30
$scope_keys = array_keys($scopes);
24
31
sort($scope_keys);
32
+
$default_scope = self::getDefaultScope();
25
33
26
34
$checkboxes = new AphrontFormCheckboxControl();
27
35
foreach ($scope_keys as $scope) {
36
+
if ($scope == $default_scope) {
37
+
continue;
38
+
}
39
+
if (!isset($current_scopes[$scope])) {
40
+
continue;
41
+
}
42
+
28
43
$checkboxes->addCheckbox(
29
44
$name = $scope,
30
45
$value = 1,
31
46
$label = self::getCheckboxLabel($scope),
32
47
$checked = isset($current_scopes[$scope]));
48
+
$have_options = true;
33
49
}
34
-
$checkboxes->setLabel('Scope');
35
50
36
-
return $checkboxes;
51
+
if ($have_options) {
52
+
$checkboxes->setLabel(pht('Scope'));
53
+
return $checkboxes;
54
+
}
55
+
56
+
return null;
37
57
}
38
58
39
59
static private function getCheckboxLabel($scope) {
···
58
78
$requested_scopes[$scope] = 1;
59
79
}
60
80
}
81
+
$requested_scopes[self::getDefaultScope()] = 1;
61
82
return $requested_scopes;
62
83
}
63
84
+17
-20
src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php
+17
-20
src/applications/oauthserver/controller/PhabricatorOAuthServerAuthController.php
···
13
13
14
14
$server = new PhabricatorOAuthServer();
15
15
$client_phid = $request->getStr('client_id');
16
-
$scope = $request->getStr('scope', array());
16
+
$scope = $request->getStr('scope');
17
17
$redirect_uri = $request->getStr('redirect_uri');
18
18
$response_type = $request->getStr('response_type');
19
19
···
119
119
phutil_tag('strong', array(), 'scope')));
120
120
}
121
121
$scope = PhabricatorOAuthServerScope::scopesListToDict($scope);
122
+
} else {
123
+
return $this->buildErrorResponse(
124
+
'invalid_request',
125
+
pht('Malformed Request'),
126
+
pht(
127
+
'Required parameter %s was not present in the request.',
128
+
phutil_tag('strong', array(), 'scope')));
122
129
}
123
130
124
131
// NOTE: We're always requiring a confirmation dialog to redirect.
···
130
137
list($is_authorized, $authorization) = $auth_info;
131
138
132
139
if ($request->isFormPost()) {
133
-
// TODO: We should probably validate this more? It feels a little funky.
134
140
$scope = PhabricatorOAuthServerScope::getScopesFromRequest($request);
135
141
136
142
if ($authorization) {
···
194
200
}
195
201
196
202
// Here, we're confirming authorization for the application.
197
-
198
-
if ($scope) {
199
-
if ($authorization) {
200
-
$desired_scopes = array_merge($scope,
201
-
$authorization->getScope());
202
-
} else {
203
-
$desired_scopes = $scope;
204
-
}
205
-
if (!PhabricatorOAuthServerScope::validateScopesDict($desired_scopes)) {
206
-
return $this->buildErrorResponse(
207
-
'invalid_scope',
208
-
pht('Invalid Scope'),
209
-
pht('The requested scope is invalid, unknown, or malformed.'));
210
-
}
203
+
if ($authorization) {
204
+
$desired_scopes = array_merge($scope, $authorization->getScope());
211
205
} else {
212
-
$desired_scopes = array(
213
-
PhabricatorOAuthServerScope::SCOPE_WHOAMI => 1,
214
-
PhabricatorOAuthServerScope::SCOPE_OFFLINE_ACCESS => 1,
215
-
);
206
+
$desired_scopes = $scope;
207
+
}
208
+
if (!PhabricatorOAuthServerScope::validateScopesDict($desired_scopes)) {
209
+
return $this->buildErrorResponse(
210
+
'invalid_scope',
211
+
pht('Invalid Scope'),
212
+
pht('The requested scope is invalid, unknown, or malformed.'));
216
213
}
217
214
218
215
$form = id(new AphrontFormView())