diff --git a/code/authenticator/RESTfulAPI_TokenAuthExtension.php b/code/authenticator/RESTfulAPI_TokenAuthExtension.php index 64af2bd..2d3b39c 100644 --- a/code/authenticator/RESTfulAPI_TokenAuthExtension.php +++ b/code/authenticator/RESTfulAPI_TokenAuthExtension.php @@ -19,6 +19,15 @@ class RESTfulAPI_TokenAuthExtension extends DataExtension 'ApiTokenExpire' => 'Int' ); + // Add a unique index to the API token. + // Should increase lookup performance and prevent duplicate tokens. + private static $indexes = array( + 'ApiToken' => array( + 'type' => 'unique', + 'value' => '"ApiToken"' + ) + ); + function updateCMSFields(FieldList $fields) { $fields->removeByName('ApiToken'); diff --git a/code/authenticator/RESTfulAPI_TokenAuthenticator.php b/code/authenticator/RESTfulAPI_TokenAuthenticator.php index 58c8a01..b32b794 100644 --- a/code/authenticator/RESTfulAPI_TokenAuthenticator.php +++ b/code/authenticator/RESTfulAPI_TokenAuthenticator.php @@ -152,14 +152,7 @@ public function login(SS_HTTPRequest $request) )); if ( $member ) { - $tokenData = $this->generateToken(); - - $tokenDBColumn = $this->tokenConfig['DBColumn']; - $expireDBColumn = $this->tokenConfig['expireDBColumn']; - - $member->{$tokenDBColumn} = $tokenData['token']; - $member->{$expireDBColumn} = $tokenData['expire']; - $member->write(); + $tokenData = $this->updateToken($member); $member->login(); } } @@ -203,16 +196,7 @@ public function logout(SS_HTTPRequest $request) if ( $this->tokenConfig['owner'] === 'Member' ) { - //generate expired token - $tokenData = $this->generateToken( true ); - - //write - $tokenDBColumn = $this->tokenConfig['DBColumn']; - $expireDBColumn = $this->tokenConfig['expireDBColumn']; - - $member->{$tokenDBColumn} = $tokenData['token']; - $member->{$expireDBColumn} = $tokenData['expire']; - $member->write(); + $this->updateToken($member, true); } } } @@ -291,16 +275,7 @@ public function resetToken($id, $expired = false) if ( $owner ) { - //generate token - $tokenData = $this->generateToken( $expired ); - - //write - $tokenDBColumn = $this->tokenConfig['DBColumn']; - $expireDBColumn = $this->tokenConfig['expireDBColumn']; - - $owner->{$tokenDBColumn} = $tokenData['token']; - $owner->{$expireDBColumn} = $tokenData['expire']; - $owner->write(); + $this->updateToken($owner, $expired); } else{ user_error("API Token owner '$ownerClass' not found with ID = $id", E_USER_WARNING); @@ -344,6 +319,40 @@ private function generateToken($expired = false) ); } + /** + * Update the token of a token owner + * @param $owner The token owner instance to update + * @param bool $expired Set to true to generate an outdated token + * @return array|null Token data array('token' => HASH, 'expire' => EXPIRY_DATE) + */ + private function updateToken($owner, $expired = false) + { + $ownerId = null; + $tokenData = null; + + // DB field names + $tokenDBColumn = $this->tokenConfig['DBColumn']; + $expireDBColumn = $this->tokenConfig['expireDBColumn']; + + do { + try { + //generate token + $tokenData = $this->generateToken( $expired ); + + // ensure we never regenerate the same token! + if($owner->{$tokenDBColumn} != $tokenData['token']){ + $owner->{$tokenDBColumn} = $tokenData['token']; + $owner->{$expireDBColumn} = $tokenData['expire']; + $ownerId = $owner->write(); + } + } catch(Exception $e){ + $ownerId = null; + } + } while(!$ownerId); + + return $tokenData; + } + /** * Returns the DataObject related to the token diff --git a/tests/authenticator/RESTfulAPI_TokenAuthenticator_Test.php b/tests/authenticator/RESTfulAPI_TokenAuthenticator_Test.php index 984b949..4ca4293 100644 --- a/tests/authenticator/RESTfulAPI_TokenAuthenticator_Test.php +++ b/tests/authenticator/RESTfulAPI_TokenAuthenticator_Test.php @@ -209,4 +209,35 @@ public function testAuthenticate() "TokenAuth authentication failure should return a RESTfulAPI_Error" ); } + + /** + * Test edge case of a member with the same API token + */ + public function testTokenUniqueness() + { + $writtenCount = 0; + // attempt to write two members with the same API token + for($i = 0; $i < 2; $i++){ + try { + $memberID = Member::create(array( + 'Email' => 'TestMember' . $i, + 'Password' => 'test', + 'ApiToken' => 'Same token' + ))->write(); + + if($memberID){ + $writtenCount++; + } + } catch(Exception $e){ + // catch exception to not raise errors + } + } + + + $this->assertEquals( + 1, + $writtenCount, + 'The API token must be unique' + ); + } } \ No newline at end of file