Skip to content

Commit aa26891

Browse files
bshafferbshaffer
authored
Issue 725 (bshaffer#729)
* ensures unsetAccessToken and unsetRefreshToken return a bool
1 parent 8a68f38 commit aa26891

File tree

8 files changed

+73
-20
lines changed

8 files changed

+73
-20
lines changed

‎src/OAuth2/Storage/AccessTokenInterface.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,7 @@ public function setAccessToken($oauth_token, $client_id, $user_id, $expires, $sc
5555
* @param $access_token
5656
* Access token to be expired.
5757
*
58+
* @return BOOL true if an access token was unset, false if not
5859
* @ingroup oauth2_section_6
5960
*
6061
* @todo v2.0 include this method in interface. Omitted to maintain BC in v1.x

‎src/OAuth2/Storage/Cassandra.php

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -136,14 +136,19 @@ protected function expireValue($key)
136136
unset($this->cache[$key]);
137137

138138
$cf = new ColumnFamily($this->cassandra, $this->config['column_family']);
139-
try {
140-
// __data key set as C* requires a field
141-
$cf->remove($key, array('__data'));
142-
} catch (\Exception $e) {
143-
return false;
139+
140+
if ($cf->get_count($key) > 0) {
141+
try {
142+
// __data key set as C* requires a field
143+
$cf->remove($key, array('__data'));
144+
} catch (\Exception $e) {
145+
return false;
146+
}
147+
148+
return true;
144149
}
145150

146-
return true;
151+
return false;
147152
}
148153

149154
/* AuthorizationCodeInterface */

‎src/OAuth2/Storage/DynamoDB.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -186,10 +186,11 @@ public function unsetAccessToken($access_token)
186186
{
187187
$result = $this->client->deleteItem(array(
188188
'TableName' => $this->config['access_token_table'],
189-
'Key' => $this->client->formatAttributes(array("access_token" => $access_token))
189+
'Key' => $this->client->formatAttributes(array("access_token" => $access_token)),
190+
'ReturnValues' => 'ALL_OLD',
190191
));
191192

192-
return true;
193+
return null !== $result->get('Attributes');
193194
}
194195

195196
/* OAuth2\Storage\AuthorizationCodeInterface */

‎src/OAuth2/Storage/Memory.php

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,7 +236,13 @@ public function setRefreshToken($refresh_token, $client_id, $user_id, $expires,
236236

237237
public function unsetRefreshToken($refresh_token)
238238
{
239-
unset($this->refreshTokens[$refresh_token]);
239+
if (isset($this->refreshTokens[$refresh_token])) {
240+
unset($this->refreshTokens[$refresh_token]);
241+
242+
return true;
243+
}
244+
245+
return false;
240246
}
241247

242248
public function setRefreshTokens($refresh_tokens)
@@ -259,7 +265,13 @@ public function setAccessToken($access_token, $client_id, $user_id, $expires, $s
259265

260266
public function unsetAccessToken($access_token)
261267
{
262-
unset($this->accessTokens[$access_token]);
268+
if (isset($this->accessTokens[$access_token])) {
269+
unset($this->accessTokens[$access_token]);
270+
271+
return true;
272+
}
273+
274+
return false;
263275
}
264276

265277
public function scopeExists($scope)

‎src/OAuth2/Storage/Mongo.php

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,11 @@ public function setAccessToken($access_token, $client_id, $user_id, $expires, $s
161161

162162
public function unsetAccessToken($access_token)
163163
{
164-
$this->collection('access_token_table')->remove(array('access_token' => $access_token));
164+
$result = $this->collection('access_token_table')->remove(array(
165+
'access_token' => $access_token
166+
), array('w' => 1));
167+
168+
return $result['n'] > 0;
165169
}
166170

167171

@@ -254,9 +258,11 @@ public function setRefreshToken($refresh_token, $client_id, $user_id, $expires,
254258

255259
public function unsetRefreshToken($refresh_token)
256260
{
257-
$this->collection('refresh_token_table')->remove(array('refresh_token' => $refresh_token));
261+
$result = $this->collection('refresh_token_table')->remove(array(
262+
'refresh_token' => $refresh_token
263+
), array('w' => 1));
258264

259-
return true;
265+
return $result['n'] > 0;
260266
}
261267

262268
// plaintext passwords are bad! Override this for your application

‎src/OAuth2/Storage/Pdo.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,9 @@ public function unsetAccessToken($access_token)
160160
{
161161
$stmt = $this->db->prepare(sprintf('DELETE FROM %s WHERE access_token = :access_token', $this->config['access_token_table']));
162162

163-
return $stmt->execute(compact('access_token'));
163+
$stmt->execute(compact('access_token'));
164+
165+
return $stmt->rowCount() > 0;
164166
}
165167

166168
/* OAuth2\Storage\AuthorizationCodeInterface */
@@ -301,7 +303,9 @@ public function unsetRefreshToken($refresh_token)
301303
{
302304
$stmt = $this->db->prepare(sprintf('DELETE FROM %s WHERE refresh_token = :refresh_token', $this->config['refresh_token_table']));
303305

304-
return $stmt->execute(compact('refresh_token'));
306+
$stmt->execute(compact('refresh_token'));
307+
308+
return $stmt->rowCount() > 0;
305309
}
306310

307311
// plaintext passwords are bad! Override this for your application

‎src/OAuth2/Storage/Redis.php

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,9 @@ public function setRefreshToken($refresh_token, $client_id, $user_id, $expires,
209209

210210
public function unsetRefreshToken($refresh_token)
211211
{
212-
return $this->expireValue($this->config['refresh_token_key'] . $refresh_token);
212+
$result = $this->expireValue($this->config['refresh_token_key'] . $refresh_token);
213+
214+
return $result > 0;
213215
}
214216

215217
/* AccessTokenInterface */
@@ -229,7 +231,9 @@ public function setAccessToken($access_token, $client_id, $user_id, $expires, $s
229231

230232
public function unsetAccessToken($access_token)
231233
{
232-
return $this->expireValue($this->config['access_token_key'] . $access_token);
234+
$result = $this->expireValue($this->config['access_token_key'] . $access_token);
235+
236+
return $result > 0;
233237
}
234238

235239
/* ScopeInterface */

‎test/OAuth2/Storage/AccessTokenTest.php

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public function testUnsetAccessToken(AccessTokenInterface $storage)
6464
return;
6565
}
6666

67-
// assert token we are unset does not exist
67+
// assert token we are about to unset does not exist
6868
$token = $storage->getAccessToken('revokabletoken');
6969
$this->assertFalse($token);
7070

@@ -73,10 +73,30 @@ public function testUnsetAccessToken(AccessTokenInterface $storage)
7373
$success = $storage->setAccessToken('revokabletoken', 'client ID', 'SOMEUSERID', $expires);
7474
$this->assertTrue($success);
7575

76-
$storage->unsetAccessToken('revokabletoken');
76+
// assert unsetAccessToken returns true
77+
$result = $storage->unsetAccessToken('revokabletoken');
78+
$this->assertTrue($result);
7779

78-
// assert token we are unset does not exist
80+
// assert token we unset does not exist
7981
$token = $storage->getAccessToken('revokabletoken');
8082
$this->assertFalse($token);
8183
}
84+
85+
/** @dataProvider provideStorage */
86+
public function testUnsetAccessTokenReturnsFalse(AccessTokenInterface $storage)
87+
{
88+
if ($storage instanceof NullStorage || !method_exists($storage, 'unsetAccessToken')) {
89+
$this->markTestSkipped('Skipped Storage: ' . $storage->getMessage());
90+
91+
return;
92+
}
93+
94+
// assert token we are about to unset does not exist
95+
$token = $storage->getAccessToken('nonexistanttoken');
96+
$this->assertFalse($token);
97+
98+
// assert unsetAccessToken returns false
99+
$result = $storage->unsetAccessToken('nonexistanttoken');
100+
$this->assertFalse($result);
101+
}
82102
}

0 commit comments

Comments
 (0)