@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
Begin adding more guidance to the "One-Time Login" flow
Summary:
Ref T13244. See PHI774. If an install does not use password auth, the "one-time login" flow (via "Welcome" email or "bin/auth recover") is pretty rough. Current behavior:
- If an install uses passwords, the user is prompted to set a password.
- If an install does not use passwords, you're dumped to `/settings/external/` to link an external account. This is pretty sketchy and this UI does not make it clear what users are expected to do (link an account) or why (so they can log in).
Instead, improve this flow:
- Password reset flow is fine.
- (Future Change) If there are external linkable accounts (like Google) and the user doesn't have any linked, I want to give users a flow like a password reset flow that says "link to an external account".
- (This Change) If you're an administrator and there are no providers at all, go to "/auth/" so you can set something up.
- (This Change) If we don't hit on any other rules, just go home?
This may be tweaked a bit as we go, but basically I want to refine the "/settings/external/" case into a more useful flow which gives users more of a chance of surviving it.
Test Plan: Logged in with passwords enabled (got password reset), with nothing enabled as an admin (got sent to Auth), and with something other than passwords enabled (got sent home).
Reviewers: amckinley
Reviewed By: amckinley
Maniphest Tasks: T13244
Differential Revision: https://secure.phabricator.com/D20094
+71
-73
+55
-31
src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php
+55
-31
src/applications/auth/controller/PhabricatorAuthOneTimeLoginController.php
···
119
119
}
120
120
unset($unguarded);
121
121
122
-
$next = '/';
123
-
if (!PhabricatorPasswordAuthProvider::getPasswordProvider()) {
124
-
$next = '/settings/panel/external/';
125
-
} else {
122
+
$next_uri = $this->getNextStepURI($target_user);
126
123
127
-
// We're going to let the user reset their password without knowing
128
-
// the old one. Generate a one-time token for that.
129
-
$key = Filesystem::readRandomCharacters(16);
130
-
$password_type =
131
-
PhabricatorAuthPasswordResetTemporaryTokenType::TOKENTYPE;
132
-
133
-
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
134
-
id(new PhabricatorAuthTemporaryToken())
135
-
->setTokenResource($target_user->getPHID())
136
-
->setTokenType($password_type)
137
-
->setTokenExpires(time() + phutil_units('1 hour in seconds'))
138
-
->setTokenCode(PhabricatorHash::weakDigest($key))
139
-
->save();
140
-
unset($unguarded);
141
-
142
-
$panel_uri = '/auth/password/';
143
-
144
-
$next = (string)id(new PhutilURI($panel_uri))
145
-
->setQueryParams(
146
-
array(
147
-
'key' => $key,
148
-
));
149
-
150
-
$request->setTemporaryCookie(PhabricatorCookies::COOKIE_HISEC, 'yes');
151
-
}
152
-
153
-
PhabricatorCookies::setNextURICookie($request, $next, $force = true);
124
+
PhabricatorCookies::setNextURICookie($request, $next_uri, $force = true);
154
125
155
126
$force_full_session = false;
156
127
if ($link_type === PhabricatorAuthSessionEngine::ONETIME_RECOVER) {
···
206
177
207
178
return id(new AphrontDialogResponse())->setDialog($dialog);
208
179
}
180
+
181
+
private function getNextStepURI(PhabricatorUser $user) {
182
+
$request = $this->getRequest();
183
+
184
+
// If we have password auth, let the user set or reset their password after
185
+
// login.
186
+
$have_passwords = PhabricatorPasswordAuthProvider::getPasswordProvider();
187
+
if ($have_passwords) {
188
+
// We're going to let the user reset their password without knowing
189
+
// the old one. Generate a one-time token for that.
190
+
$key = Filesystem::readRandomCharacters(16);
191
+
$password_type =
192
+
PhabricatorAuthPasswordResetTemporaryTokenType::TOKENTYPE;
193
+
194
+
$unguarded = AphrontWriteGuard::beginScopedUnguardedWrites();
195
+
id(new PhabricatorAuthTemporaryToken())
196
+
->setTokenResource($user->getPHID())
197
+
->setTokenType($password_type)
198
+
->setTokenExpires(time() + phutil_units('1 hour in seconds'))
199
+
->setTokenCode(PhabricatorHash::weakDigest($key))
200
+
->save();
201
+
unset($unguarded);
202
+
203
+
$panel_uri = '/auth/password/';
204
+
205
+
$request->setTemporaryCookie(PhabricatorCookies::COOKIE_HISEC, 'yes');
206
+
207
+
return (string)id(new PhutilURI($panel_uri))
208
+
->setQueryParams(
209
+
array(
210
+
'key' => $key,
211
+
));
212
+
}
213
+
214
+
$providers = id(new PhabricatorAuthProviderConfigQuery())
215
+
->setViewer($user)
216
+
->withIsEnabled(true)
217
+
->execute();
218
+
219
+
// If there are no configured providers and the user is an administrator,
220
+
// send them to Auth to configure a provider. This is probably where they
221
+
// want to go. You can end up in this state by accidentally losing your
222
+
// first session during initial setup, or after restoring exported data
223
+
// from a hosted instance.
224
+
if (!$providers && $user->getIsAdmin()) {
225
+
return '/auth/';
226
+
}
227
+
228
+
// If we didn't find anywhere better to send them, give up and just send
229
+
// them to the home page.
230
+
return '/';
231
+
}
232
+
209
233
}
+16
-42
src/applications/auth/query/PhabricatorAuthProviderConfigQuery.php
+16
-42
src/applications/auth/query/PhabricatorAuthProviderConfigQuery.php
···
6
6
private $ids;
7
7
private $phids;
8
8
private $providerClasses;
9
-
10
-
const STATUS_ALL = 'status:all';
11
-
const STATUS_ENABLED = 'status:enabled';
12
-
13
-
private $status = self::STATUS_ALL;
9
+
private $isEnabled;
14
10
15
11
public function withPHIDs(array $phids) {
16
12
$this->phids = $phids;
···
22
18
return $this;
23
19
}
24
20
25
-
public function withStatus($status) {
26
-
$this->status = $status;
21
+
public function withProviderClasses(array $classes) {
22
+
$this->providerClasses = $classes;
27
23
return $this;
28
24
}
29
25
30
-
public function withProviderClasses(array $classes) {
31
-
$this->providerClasses = $classes;
26
+
public function withIsEnabled($is_enabled) {
27
+
$this->isEnabled = $is_enabled;
32
28
return $this;
33
29
}
34
30
35
-
public static function getStatusOptions() {
36
-
return array(
37
-
self::STATUS_ALL => pht('All Providers'),
38
-
self::STATUS_ENABLED => pht('Enabled Providers'),
39
-
);
31
+
public function newResultObject() {
32
+
return new PhabricatorAuthProviderConfig();
40
33
}
41
34
42
35
protected function loadPage() {
43
-
$table = new PhabricatorAuthProviderConfig();
44
-
$conn_r = $table->establishConnection('r');
45
-
46
-
$data = queryfx_all(
47
-
$conn_r,
48
-
'SELECT * FROM %T %Q %Q %Q',
49
-
$table->getTableName(),
50
-
$this->buildWhereClause($conn_r),
51
-
$this->buildOrderClause($conn_r),
52
-
$this->buildLimitClause($conn_r));
53
-
54
-
return $table->loadAllFromArray($data);
36
+
return $this->loadStandardPage($this->newResultObject());
55
37
}
56
38
57
-
protected function buildWhereClause(AphrontDatabaseConnection $conn) {
58
-
$where = array();
39
+
protected function buildWhereClauseParts(AphrontDatabaseConnection $conn) {
40
+
$where = parent::buildWhereClauseParts($conn);
59
41
60
42
if ($this->ids !== null) {
61
43
$where[] = qsprintf(
···
78
60
$this->providerClasses);
79
61
}
80
62
81
-
$status = $this->status;
82
-
switch ($status) {
83
-
case self::STATUS_ALL:
84
-
break;
85
-
case self::STATUS_ENABLED:
86
-
$where[] = qsprintf(
87
-
$conn,
88
-
'isEnabled = 1');
89
-
break;
90
-
default:
91
-
throw new Exception(pht("Unknown status '%s'!", $status));
63
+
if ($this->isEnabled !== null) {
64
+
$where[] = qsprintf(
65
+
$conn,
66
+
'isEnabled = %d',
67
+
(int)$this->isEnabled);
92
68
}
93
69
94
-
$where[] = $this->buildPagingClause($conn);
95
-
96
-
return $this->formatWhereClause($conn, $where);
70
+
return $where;
97
71
}
98
72
99
73
public function getQueryApplicationClass() {