Skip to content

Commit 9add9ca

Browse files
committed
add bruteforce protection in OauthApiController
Signed-off-by: Julien Veyssier <julien-nc@posteo.net>
1 parent 7ac0ad9 commit 9add9ca

2 files changed

Lines changed: 23 additions & 5 deletions

File tree

apps/oauth2/lib/Controller/OauthApiController.php

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ public function __construct(string $appName,
8686
/**
8787
* @PublicPage
8888
* @NoCSRFRequired
89+
* @BruteForceProtection(action=oauth2GetToken)
8990
*
9091
* @param string $grant_type
9192
* @param string $code
@@ -98,9 +99,11 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client
9899

99100
// We only handle two types
100101
if ($grant_type !== 'authorization_code' && $grant_type !== 'refresh_token') {
101-
return new JSONResponse([
102+
$response = new JSONResponse([
102103
'error' => 'invalid_grant',
103104
], Http::STATUS_BAD_REQUEST);
105+
$response->throttle(['invalid_grant' => $grant_type]);
106+
return $response;
104107
}
105108

106109
// We handle the initial and refresh tokens the same way
@@ -111,17 +114,21 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client
111114
try {
112115
$accessToken = $this->accessTokenMapper->getByCode($code);
113116
} catch (AccessTokenNotFoundException $e) {
114-
return new JSONResponse([
117+
$response = new JSONResponse([
115118
'error' => 'invalid_request',
116119
], Http::STATUS_BAD_REQUEST);
120+
$response->throttle(['invalid_request' => 'token not found', 'code' => $code]);
121+
return $response;
117122
}
118123

119124
try {
120125
$client = $this->clientMapper->getByUid($accessToken->getClientId());
121126
} catch (ClientNotFoundException $e) {
122-
return new JSONResponse([
127+
$response = new JSONResponse([
123128
'error' => 'invalid_request',
124129
], Http::STATUS_BAD_REQUEST);
130+
$response->throttle(['invalid_request' => 'client not found', 'client_id' => $accessToken->getClientId()]);
131+
return $response;
125132
}
126133

127134
if (isset($this->request->server['PHP_AUTH_USER'])) {
@@ -133,15 +140,18 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client
133140
$storedClientSecret = $this->crypto->decrypt($client->getSecret());
134141
} catch (\Exception $e) {
135142
$this->logger->error('OAuth client secret decryption error', ['exception' => $e]);
143+
// we don't throttle here because it might not be a bruteforce attack
136144
return new JSONResponse([
137145
'error' => 'invalid_client',
138146
], Http::STATUS_BAD_REQUEST);
139147
}
140148
// The client id and secret must match. Else we don't provide an access token!
141149
if ($client->getClientIdentifier() !== $client_id || $storedClientSecret !== $client_secret) {
142-
return new JSONResponse([
150+
$response = new JSONResponse([
143151
'error' => 'invalid_client',
144152
], Http::STATUS_BAD_REQUEST);
153+
$response->throttle(['invalid_client' => 'client ID or secret does not match']);
154+
return $response;
145155
}
146156

147157
$decryptedToken = $this->crypto->decrypt($accessToken->getEncryptedToken(), $code);
@@ -154,9 +164,11 @@ public function getToken($grant_type, $code, $refresh_token, $client_id, $client
154164
} catch (InvalidTokenException $e) {
155165
//We can't do anything...
156166
$this->accessTokenMapper->delete($accessToken);
157-
return new JSONResponse([
167+
$response = new JSONResponse([
158168
'error' => 'invalid_request',
159169
], Http::STATUS_BAD_REQUEST);
170+
$response->throttle(['invalid_request' => 'token is invalid']);
171+
return $response;
160172
}
161173

162174
// Rotate the apptoken (so the old one becomes invalid basically)

apps/oauth2/tests/Controller/OauthApiControllerTest.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,6 +99,7 @@ public function testGetTokenInvalidGrantType() {
9999
$expected = new JSONResponse([
100100
'error' => 'invalid_grant',
101101
], Http::STATUS_BAD_REQUEST);
102+
$expected->throttle(['invalid_grant' => 'foo']);
102103

103104
$this->assertEquals($expected, $this->oauthApiController->getToken('foo', null, null, null, null));
104105
}
@@ -107,6 +108,7 @@ public function testGetTokenInvalidCode() {
107108
$expected = new JSONResponse([
108109
'error' => 'invalid_request',
109110
], Http::STATUS_BAD_REQUEST);
111+
$expected->throttle(['invalid_request' => 'token not found', 'code' => 'invalidcode']);
110112

111113
$this->accessTokenMapper->method('getByCode')
112114
->with('invalidcode')
@@ -119,6 +121,7 @@ public function testGetTokenInvalidRefreshToken() {
119121
$expected = new JSONResponse([
120122
'error' => 'invalid_request',
121123
], Http::STATUS_BAD_REQUEST);
124+
$expected->throttle(['invalid_request' => 'token not found', 'code' => 'invalidrefresh']);
122125

123126
$this->accessTokenMapper->method('getByCode')
124127
->with('invalidrefresh')
@@ -131,6 +134,7 @@ public function testGetTokenClientDoesNotExist() {
131134
$expected = new JSONResponse([
132135
'error' => 'invalid_request',
133136
], Http::STATUS_BAD_REQUEST);
137+
$expected->throttle(['invalid_request' => 'client not found', 'client_id' => 42]);
134138

135139
$accessToken = new AccessToken();
136140
$accessToken->setClientId(42);
@@ -164,6 +168,7 @@ public function testGetTokenInvalidClient($clientId, $clientSecret) {
164168
$expected = new JSONResponse([
165169
'error' => 'invalid_client',
166170
], Http::STATUS_BAD_REQUEST);
171+
$expected->throttle(['invalid_client' => 'client ID or secret does not match']);
167172

168173
$accessToken = new AccessToken();
169174
$accessToken->setClientId(42);
@@ -186,6 +191,7 @@ public function testGetTokenInvalidAppToken() {
186191
$expected = new JSONResponse([
187192
'error' => 'invalid_request',
188193
], Http::STATUS_BAD_REQUEST);
194+
$expected->throttle(['invalid_request' => 'token is invalid']);
189195

190196
$accessToken = new AccessToken();
191197
$accessToken->setClientId(42);

0 commit comments

Comments
 (0)