recaptime.dev / phorge
@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
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

Provide an auto-refresh mechanism for OAuth providers to deliver fresh tokens

Summary:
Ref T2852. Give OAuth providers a formal method so you can ask them for tokens; they issue a refresh request if necessary.

We could automatically refresh these tokens in daemons as they near expiry to improve performance; refreshes are blocking in-process round trip requests. If we do this for all tokens, it's a lot of requests (say, 20k users * 2 auth mechanisms * 1-hour tokens ~= a million requests a day). We could do it selectively for tokens that are actually in use (i.e., if we refresh a token in response to a user request, we keep refreshing it for 24 hours automatically). For now, I'm not pursuing any of this.

If we fail to refresh a token, we don't have a great way to communicate it to the user right now. The remedy is "log out and log in again", but there's no way for them to figure this out. The major issue is that a lot of OAuth integrations should not throw if they fail, or can't reasonably be rasied to the user (e.g., activity in daemons, loading profile pictures, enriching links, etc). For now, this shouldn't really happen. In future diffs, I plan to make the "External Accounts" settings page provide some information about tokens again, and possibly push some flag to accounts like "you should refresh your X link", but we'll see if issues crop up.

Test Plan: Used `bin/auth refresh` to verify refreshes. I'll wait an hour and reload a page with an Asana link to verify the auto-refresh part.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2852

Differential Revision: https://secure.phabricator.com/D6280

epriestley f8ed6422 c94ef134

+47 -3
+3 -2
src/applications/auth/management/PhabricatorAuthManagementRefreshWorkflow.php
··· 126 126 new PhutilNumber( 127 127 $account->getProperty('oauth.token.access.expires') - time()))); 128 128 129 - $adapter->refreshAccessToken($refresh_token); 129 + $provider->getOAuthAccessToken($account, $force_refresh = true); 130 130 131 131 $console->writeOut( 132 132 "+ %s\n", 133 133 pht( 134 134 "Refreshed token, new token expires in %s seconds.", 135 - new PhutilNumber($adapter->getAccessTokenExpires() - time()))); 135 + new PhutilNumber( 136 + $account->getProperty('oauth.token.access.expires') - time()))); 136 137 137 138 } 138 139
+43
src/applications/auth/provider/PhabricatorAuthProviderOAuth.php
··· 283 283 284 284 protected function willSaveAccount(PhabricatorExternalAccount $account) { 285 285 parent::willSaveAccount($account); 286 + $this->synchronizeOAuthAccount($account); 287 + } 286 288 289 + protected function synchronizeOAuthAccount( 290 + PhabricatorExternalAccount $account) { 287 291 $adapter = $this->getAdapter(); 288 292 289 293 $oauth_token = $adapter->getAccessToken(); ··· 298 302 299 303 $expires = $adapter->getAccessTokenExpires(); 300 304 $account->setProperty('oauth.token.access.expires', $expires); 305 + } 306 + 307 + public function getOAuthAccessToken( 308 + PhabricatorExternalAccount $account, 309 + $force_refresh = false) { 310 + 311 + if ($account->getProviderKey() !== $this->getProviderKey()) { 312 + throw new Exception("Account does not match provider!"); 313 + } 314 + 315 + if (!$force_refresh) { 316 + $access_expires = $account->getProperty('oauth.token.access.expires'); 317 + $access_token = $account->getProperty('oauth.token.access'); 318 + 319 + // Don't return a token with fewer than this many seconds remaining until 320 + // it expires. 321 + $shortest_token = 60; 322 + 323 + if ($access_token) { 324 + if ($access_expires > (time() + $shortest_token)) { 325 + return $access_token; 326 + } 327 + } 328 + } 329 + 330 + $refresh_token = $account->getProperty('oauth.token.refresh'); 331 + if ($refresh_token) { 332 + $adapter = $this->getAdapter(); 333 + if ($adapter->supportsTokenRefresh()) { 334 + $adapter->refreshAccessToken($refresh_token); 335 + 336 + $this->synchronizeOAuthAccount($account); 337 + $unguarded = AphrontWriteGuard::beginScopedUnguardedWrites(); 338 + $account->save(); 339 + unset($unguarded); 340 + } 341 + } 342 + 343 + return null; 301 344 } 302 345 303 346 }
+1 -1
src/applications/doorkeeper/bridge/DoorkeeperBridgeAsana.php
··· 35 35 // right now so this is currently moot. 36 36 $account = head($accounts); 37 37 38 - $token = $account->getProperty('oauth.token.access'); 38 + $token = $provider->getOAuthAccessToken($account); 39 39 if (!$token) { 40 40 return; 41 41 }