Skip to content

Commit c098465

Browse files
provokateurinprovokateurin
committed
feat(Sharing): Add rate limiting and brute-force protection for ApiV1Controller
Signed-off-by: provokateurin <kate@provokateurin.de>
1 parent aed4fd0 commit c098465

1 file changed

Lines changed: 24 additions & 2 deletions

File tree

‎apps/sharing/lib/Controller/ApiV1Controller.php‎

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,12 @@
3131
use NCU\Sharing\Source\ShareSource;
3232
use OCA\Sharing\ResponseDefinitions;
3333
use OCP\AppFramework\Http;
34+
use OCP\AppFramework\Http\Attribute\AnonRateLimit;
3435
use OCP\AppFramework\Http\Attribute\ApiRoute;
36+
use OCP\AppFramework\Http\Attribute\BruteForceProtection;
3537
use OCP\AppFramework\Http\Attribute\NoAdminRequired;
3638
use OCP\AppFramework\Http\Attribute\PublicPage;
39+
use OCP\AppFramework\Http\Attribute\UserRateLimit;
3740
use OCP\AppFramework\Http\DataResponse;
3841
use OCP\AppFramework\OCSController;
3942
use OCP\IDBConnection;
@@ -46,7 +49,6 @@
4649
use ValueError;
4750

4851
// TODO: Add "recipient suggestions" endpoint
49-
// TODO: Add rate limiting
5052

5153
/**
5254
* @psalm-import-type SharingShare from ResponseDefinitions
@@ -90,6 +92,7 @@ public function __construct(
9092
*/
9193
#[NoAdminRequired]
9294
#[ApiRoute(verb: 'GET', url: '/api/v1/recipients')]
95+
#[UserRateLimit(limit: 5, period: 1)]
9396
public function searchRecipients(?array $filterRecipientTypeClasses, string $query, int $limit = 10, int $offset = 0, ?string $id = null): DataResponse {
9497
/** @psalm-suppress DocblockTypeContradiction */
9598
if ($limit < 1) {
@@ -144,6 +147,7 @@ public function generateSecret(): DataResponse {
144147
*/
145148
#[NoAdminRequired]
146149
#[ApiRoute(verb: 'POST', url: '/api/v1/share')]
150+
#[UserRateLimit(limit: 1, period: 5)]
147151
public function createShare(): DataResponse {
148152
try {
149153
try {
@@ -176,6 +180,7 @@ public function createShare(): DataResponse {
176180
*/
177181
#[NoAdminRequired]
178182
#[ApiRoute(verb: 'PUT', url: '/api/v1/share/{id}/state')]
183+
#[UserRateLimit(limit: 1, period: 5)]
179184
public function updateShareState(string $id, string $state): DataResponse {
180185
try {
181186
$shareState = ShareState::from($state);
@@ -215,6 +220,7 @@ public function updateShareState(string $id, string $state): DataResponse {
215220
*/
216221
#[NoAdminRequired]
217222
#[ApiRoute(verb: 'PUT', url: '/api/v1/share/{id}/user-status')]
223+
#[UserRateLimit(limit: 1, period: 5)]
218224
public function updateShareUserStatus(string $id, string $userStatus): DataResponse {
219225
try {
220226
$shareUserStatus = ShareUserStatus::from($userStatus);
@@ -254,6 +260,7 @@ public function updateShareUserStatus(string $id, string $userStatus): DataRespo
254260
*/
255261
#[NoAdminRequired]
256262
#[ApiRoute(verb: 'POST', url: '/api/v1/share/{id}/source')]
263+
#[UserRateLimit(limit: 1, period: 1)]
257264
public function addShareSource(string $id, string $class, string $value): DataResponse {
258265
try {
259266
try {
@@ -290,6 +297,7 @@ public function addShareSource(string $id, string $class, string $value): DataRe
290297
*/
291298
#[NoAdminRequired]
292299
#[ApiRoute(verb: 'DELETE', url: '/api/v1/share/{id}/source')]
300+
#[UserRateLimit(limit: 1, period: 1)]
293301
public function removeShareSource(string $id, string $class, string $value): DataResponse {
294302
try {
295303
try {
@@ -326,6 +334,7 @@ public function removeShareSource(string $id, string $class, string $value): Dat
326334
*/
327335
#[NoAdminRequired]
328336
#[ApiRoute(verb: 'POST', url: '/api/v1/share/{id}/recipient')]
337+
#[UserRateLimit(limit: 1, period: 1)]
329338
public function addShareRecipient(string $id, string $class, string $value, ?string $instance): DataResponse {
330339
try {
331340
try {
@@ -363,6 +372,7 @@ public function addShareRecipient(string $id, string $class, string $value, ?str
363372
*/
364373
#[NoAdminRequired]
365374
#[ApiRoute(verb: 'DELETE', url: '/api/v1/share/{id}/recipient')]
375+
#[UserRateLimit(limit: 1, period: 1)]
366376
public function removeShareRecipient(string $id, string $class, string $value, ?string $instance): DataResponse {
367377
try {
368378
try {
@@ -400,6 +410,7 @@ public function removeShareRecipient(string $id, string $class, string $value, ?
400410
*/
401411
#[NoAdminRequired]
402412
#[ApiRoute(verb: 'PUT', url: '/api/v1/share/{id}/recipient/secret')]
413+
#[UserRateLimit(limit: 1, period: 5)]
403414
public function updateShareRecipientSecret(string $id, string $class, string $value, ?string $instance, string $secret): DataResponse {
404415
try {
405416
try {
@@ -437,6 +448,7 @@ public function updateShareRecipientSecret(string $id, string $class, string $va
437448
*/
438449
#[NoAdminRequired]
439450
#[ApiRoute(verb: 'PUT', url: '/api/v1/share/{id}/property')]
451+
#[UserRateLimit(limit: 1, period: 1)]
440452
public function updateShareProperty(string $id, string $class, ?string $value): DataResponse {
441453
try {
442454
try {
@@ -474,6 +486,7 @@ public function updateShareProperty(string $id, string $class, ?string $value):
474486
*/
475487
#[NoAdminRequired]
476488
#[ApiRoute(verb: 'PUT', url: '/api/v1/share/{id}/permission')]
489+
#[UserRateLimit(limit: 1, period: 1)]
477490
public function updateSharePermission(string $id, string $class, bool $enabled): DataResponse {
478491
try {
479492
try {
@@ -510,6 +523,7 @@ public function updateSharePermission(string $id, string $class, bool $enabled):
510523
*/
511524
#[NoAdminRequired]
512525
#[ApiRoute(verb: 'PUT', url: '/api/v1/share/{id}/permission/preset')]
526+
#[UserRateLimit(limit: 1, period: 1)]
513527
public function selectSharePermissionPreset(string $id, string $permissionPresetClass): DataResponse {
514528
try {
515529
try {
@@ -542,6 +556,7 @@ public function selectSharePermissionPreset(string $id, string $permissionPreset
542556
*/
543557
#[NoAdminRequired]
544558
#[ApiRoute(verb: 'DELETE', url: '/api/v1/share/{id}')]
559+
#[UserRateLimit(limit: 1, period: 5)]
545560
public function deleteShare(string $id): DataResponse {
546561
try {
547562
try {
@@ -576,6 +591,9 @@ public function deleteShare(string $id): DataResponse {
576591
#[PublicPage]
577592
// This should be a GET, but GET doesn't allow a request body which is required for the $arguments.
578593
#[ApiRoute(verb: 'POST', url: '/api/v1/share/{id}')]
594+
#[UserRateLimit(limit: 1, period: 1)]
595+
#[AnonRateLimit(limit: 1, period: 5)]
596+
#[BruteForceProtection(action: 'getShare')]
579597
public function getShare(string $id, ?string $secret = null, array $arguments = []): DataResponse {
580598
try {
581599
try {
@@ -589,7 +607,10 @@ public function getShare(string $id, ?string $secret = null, array $arguments =
589607
throw $exception;
590608
}
591609
} catch (ShareNotFoundException $shareNotFoundException) {
592-
return new DataResponse($shareNotFoundException->getHint(), Http::STATUS_NOT_FOUND);
610+
$response = new DataResponse($shareNotFoundException->getHint(), Http::STATUS_NOT_FOUND);
611+
// Share might not be found due to the secret being wrong or filtering removing the share due to wrong arguments.
612+
$response->throttle();
613+
return $response;
593614
}
594615
}
595616

@@ -609,6 +630,7 @@ public function getShare(string $id, ?string $secret = null, array $arguments =
609630
*/
610631
#[NoAdminRequired]
611632
#[ApiRoute(verb: 'GET', url: '/api/v1/shares')]
633+
#[UserRateLimit(limit: 1, period: 1)]
612634
public function getShares(?string $filterSourceTypeClass, ?string $filterSourceTypeValue, ?string $filterState, ?string $filterUserStatus, ?string $lastShareID, int $limit = 100): DataResponse {
613635
/** @psalm-suppress DocblockTypeContradiction */
614636
if ($limit < 1) {

0 commit comments

Comments
 (0)