Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add database ratelimiting backend #28728

Merged
merged 8 commits into from Sep 13, 2021
Merged

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
LukasReschke added 5 commits Sep 6, 2021
In case no distributed memory cache is specified this adds
a database backend for ratelimit purposes.

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
@LukasReschke LukasReschke marked this pull request as ready for review Sep 6, 2021
@LukasReschke LukasReschke requested review from ChristophWurst and skjnldsv Sep 6, 2021
@LukasReschke
Copy link
Member Author

@LukasReschke LukasReschke commented Sep 6, 2021

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

@LukasReschke LukasReschke requested review from nextcloud/server-backend, PVince81 and ArtificialOwl and removed request for nextcloud/server-backend Sep 6, 2021
Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
@LukasReschke
Copy link
Member Author

@LukasReschke LukasReschke commented Sep 7, 2021

/backport to stable22

@LukasReschke
Copy link
Member Author

@LukasReschke LukasReschke commented Sep 7, 2021

/backport to stable21

@LukasReschke
Copy link
Member Author

@LukasReschke LukasReschke commented Sep 7, 2021

/backport to stable20

@LukasReschke
Copy link
Member Author

@LukasReschke LukasReschke commented Sep 8, 2021

/backport to stable19

@nickvergessen
Copy link
Member

@nickvergessen nickvergessen commented Sep 13, 2021

Looks good in general, just some minor changes

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>

Co-authored-by: Joas Schilling <213943+nickvergessen@users.noreply.github.com>
@LukasReschke LukasReschke requested a review from nickvergessen Sep 13, 2021
@LukasReschke
Copy link
Member Author

@LukasReschke LukasReschke commented Sep 13, 2021

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

@LukasReschke
Copy link
Member Author

@LukasReschke LukasReschke commented Sep 13, 2021

Missed to set the remote. Retesting.

Signed-off-by: Lukas Reschke <lukas@statuscode.ch>
@LukasReschke LukasReschke requested a review from nickvergessen Sep 13, 2021
@LukasReschke LukasReschke merged commit 0dcc5c0 into master Sep 13, 2021
21 checks passed
21 checks passed
@github-actions
auto-approve-merge
Details
@github-actions
eslint eslint
Details
@github-actions
php7.3 lint
Details
@github-actions
node
Details
@github-actions
versions
Details
@github-actions
php7.4-oci
Details
@github-actions
Psalm Psalm
Details
@github-actions
Psalm Psalm
Details
@github-actions
Block fixup and squash commits
Details
@github-actions
static-code-analysis
Details
@github-actions
php7.4 lint
Details
@github-actions
php8.0 lint
Details
@github-actions
php-cs check
Details
@github-actions
test
Details
@github-actions
static-code-analysis-ocp
Details
@github-actions
jsunit
Details
@github-actions
handlebars
Details
@github-code-scanning
Psalm 27 new alerts, 28 fixes
Details
@dco
DCO DCO
Details
continuous-integration/drone/pr Build is passing
Details
@fixupbot
fixupbot No fixup commits found. The commit history is clean 👍
Details
@LukasReschke LukasReschke deleted the add-database-backend-limiter branch Sep 13, 2021
@backportbot-nextcloud
Copy link

@backportbot-nextcloud backportbot-nextcloud bot commented Sep 13, 2021

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

@backportbot-nextcloud
Copy link

@backportbot-nextcloud backportbot-nextcloud bot commented Sep 13, 2021

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

@backportbot-nextcloud
Copy link

@backportbot-nextcloud backportbot-nextcloud bot commented Sep 13, 2021

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
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants