OAuth: Fix rare problem in which request tokens were sometimes being
returned as access tokens.
This commit is contained in:
parent
28d40e5eed
commit
05361bb686
|
@ -84,6 +84,7 @@ class ApiOauthAccessTokenAction extends ApiOauthAction
|
|||
common_debug(var_export($req, true));
|
||||
$code = $e->getCode();
|
||||
$this->clientError($e->getMessage(), empty($code) ? 401 : $code, 'text');
|
||||
return;
|
||||
}
|
||||
|
||||
if (empty($atok)) {
|
||||
|
|
|
@ -152,8 +152,11 @@ class ApiStatusNetOAuthDataStore extends StatusNetOAuthDataStore
|
|||
);
|
||||
}
|
||||
|
||||
// check to see if we have previously issued an access token for this application
|
||||
// and profile
|
||||
// Check to see if we have previously issued an access token for
|
||||
// this application and profile; if so we can just return the
|
||||
// existing access token. That seems to be the best practice. It
|
||||
// makes it so users only have to authorize the app once per
|
||||
// machine.
|
||||
|
||||
$appUser = new Oauth_application_user();
|
||||
|
||||
|
@ -172,6 +175,26 @@ class ApiStatusNetOAuthDataStore extends StatusNetOAuthDataStore
|
|||
)
|
||||
);
|
||||
|
||||
$at = null;
|
||||
|
||||
// Special case: we used to store request tokens in the
|
||||
// Oauth_application_user record, and the access_type would
|
||||
// always be 0 (no access) as a failsafe until an access
|
||||
// token was issued and replaced the request token. There could
|
||||
// be a few old Oauth_application_user records storing request
|
||||
// tokens still around, and we don't want to accidentally
|
||||
// return a useless request token instead of a new access
|
||||
// token. So if we find one, we generate a new access token
|
||||
// and update the existing Oauth_application_user record before
|
||||
// returning the new access token. This should be rare.
|
||||
|
||||
if ($appUser->access_type == 0) {
|
||||
|
||||
$at = $this->generateNewAccessToken($consumer, $rt, $verifier);
|
||||
$this->updateAppUser($appUser, $app, $at);
|
||||
|
||||
} else {
|
||||
|
||||
$at = new Token();
|
||||
|
||||
// fetch the full access token
|
||||
|
@ -186,6 +209,7 @@ class ApiStatusNetOAuthDataStore extends StatusNetOAuthDataStore
|
|||
_('Could not issue access token.')
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// Yay, we can re-issue the access token
|
||||
return new OAuthToken($at->tok, $at->secret);
|
||||
|
@ -200,7 +224,40 @@ class ApiStatusNetOAuthDataStore extends StatusNetOAuthDataStore
|
|||
)
|
||||
);
|
||||
|
||||
// make a brand new access token
|
||||
$at = $this->generateNewAccessToken($consumer, $rt, $verifier);
|
||||
$this->newAppUser($tokenAssoc, $app, $at);
|
||||
|
||||
// Okay, good
|
||||
return new OAuthToken($at->tok, $at->secret);
|
||||
}
|
||||
|
||||
} else {
|
||||
|
||||
// the token was not authorized or not verfied
|
||||
common_log(
|
||||
LOG_INFO,
|
||||
sprintf(
|
||||
"API OAuth - Attempt to exchange unauthorized or unverified request token %s for an access token.",
|
||||
$rt->tok
|
||||
)
|
||||
);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* Generate a new access token and save it to the database
|
||||
*
|
||||
* @param Consumer $consumer the OAuth consumer
|
||||
* @param Token $rt the authorized request token
|
||||
* @param string $verifier the OAuth 1.0a verifier
|
||||
*
|
||||
* @access private
|
||||
*
|
||||
* @return Token $at the new access token
|
||||
*/
|
||||
private function generateNewAccessToken($consumer, $rt, $verifier)
|
||||
{
|
||||
$at = new Token();
|
||||
|
||||
$at->consumer_key = $consumer->key;
|
||||
|
@ -226,7 +283,22 @@ class ApiStatusNetOAuthDataStore extends StatusNetOAuthDataStore
|
|||
common_debug('request token "' . $rt->tok . '" updated', __FILE__);
|
||||
}
|
||||
|
||||
// insert a new Oauth_application_user record w/access token
|
||||
return $at;
|
||||
}
|
||||
|
||||
/*
|
||||
* Add a new app user (Oauth_application_user) record
|
||||
*
|
||||
* @param Oauth_token_association $tokenAssoc token-to-app association
|
||||
* @param Oauth_application $app the OAuth client app
|
||||
* @param Token $at the access token
|
||||
*
|
||||
* @access private
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
private function newAppUser($tokenAssoc, $app, $at)
|
||||
{
|
||||
$appUser = new Oauth_application_user();
|
||||
|
||||
$appUser->profile_id = $tokenAssoc->profile_id;
|
||||
|
@ -239,25 +311,39 @@ class ApiStatusNetOAuthDataStore extends StatusNetOAuthDataStore
|
|||
|
||||
if (!$result) {
|
||||
common_log_db_error($appUser, 'INSERT', __FILE__);
|
||||
|
||||
// TRANS: Server error displayed when a database error occurs.
|
||||
$this->serverError(_('Database error inserting OAuth application user.'));
|
||||
}
|
||||
|
||||
// Okay, good
|
||||
return new OAuthToken($at->tok, $at->secret);
|
||||
}
|
||||
|
||||
} else {
|
||||
|
||||
// the token was not authorized or not verfied
|
||||
common_log(
|
||||
LOG_INFO,
|
||||
sprintf(
|
||||
"API OAuth - Attempt to exchange unauthorized or unverified request token %s for an access token.",
|
||||
$rt->tok
|
||||
)
|
||||
throw new Exception(
|
||||
_('Database error inserting OAuth application user.')
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/*
|
||||
* Update an existing app user (Oauth_application_user) record
|
||||
*
|
||||
* @param Oauth_application_user $appUser existing app user rec
|
||||
* @param Oauth_application $app the OAuth client app
|
||||
* @param Token $at the access token
|
||||
*
|
||||
* @access private
|
||||
*
|
||||
* @return void
|
||||
*/
|
||||
private function updateAppUser($appUser, $app, $at)
|
||||
{
|
||||
$original = clone($appUser);
|
||||
$appUser->access_type = $app->access_type;
|
||||
$appUser->token = $at->tok;
|
||||
|
||||
$result = $appUser->update($original);
|
||||
|
||||
if (!$result) {
|
||||
common_log_db_error($appUser, 'UPDATE', __FILE__);
|
||||
// TRANS: Server error displayed when a database error occurs.
|
||||
throw new Exception(
|
||||
_('Database error updating OAuth application user.')
|
||||
);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
|
|
Loading…
Reference in New Issue
Block a user