Skip to content

Conversation

@LukasReschke
Copy link
Member

@LukasReschke LukasReschke commented Sep 6, 2021

In case no distributed memory cache is specified this adds a database backend for ratelimiting purposes. Previously the ratelimit check was just skipped.

This also fixes an issue where we passed always the current time instead of the storage period to the backend. And simplified the API to not pass $period where not necessary.

This doesn't aim to be a high performance solution, but just a fallback for instances without any kind of memory cache configured.

Test plan

  • Disable the memory cache in config.php
  • Enable the "Q&A Testing App" in the app settings
  • Go to /index.php/apps/testing/userAndAnonProtected and click a few times. Verify that the requests fails due to hitting the ratelimit.
  • Verify that expired ratelimit entries are removed on requests
  • Repeat the same with memory cache enabled

TODO

  • Revert unnecessary composer changes
  • Cleanup unneeded entries on check
  • PHP CS fixup

In case no distributed memory cache is specified this adds
a database backend for ratelimit purposes.

Signed-off-by: Lukas Reschke <[email protected]>
Signed-off-by: Lukas Reschke <[email protected]>
Signed-off-by: Lukas Reschke <[email protected]>
Signed-off-by: Lukas Reschke <[email protected]>
@LukasReschke LukasReschke marked this pull request as ready for review September 6, 2021 15:51
@LukasReschke
Copy link
Member Author

@skjnldsv This needs quite a careful review as it is quite a big change to backport 🙈

@LukasReschke LukasReschke requested review from a team, ArtificialOwl and PVince81 and removed request for a team September 6, 2021 19:32
Signed-off-by: Lukas Reschke <[email protected]>
@LukasReschke
Copy link
Member Author

/backport to stable22

@LukasReschke
Copy link
Member Author

/backport to stable21

@LukasReschke
Copy link
Member Author

/backport to stable20

@LukasReschke LukasReschke added the 3. to review Waiting for reviews label Sep 7, 2021
@LukasReschke
Copy link
Member Author

/backport to stable19

@nickvergessen
Copy link
Member

Looks good in general, just some minor changes

Signed-off-by: Lukas Reschke <[email protected]>

Co-authored-by: Joas Schilling <[email protected]>
@LukasReschke
Copy link
Member Author

Will run php-cs. Applied @nickvergessen suggestion and tested locally.

@LukasReschke
Copy link
Member Author

Missed to set the remote. Retesting.

Signed-off-by: Lukas Reschke <[email protected]>
@LukasReschke LukasReschke merged commit 0dcc5c0 into master Sep 13, 2021
@LukasReschke LukasReschke deleted the add-database-backend-limiter branch September 13, 2021 11:07
@backportbot-nextcloud
Copy link

The backport to stable21 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable20 failed. Please do this backport manually.

@backportbot-nextcloud
Copy link

The backport to stable19 failed. Please do this backport manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants