Skip to content

fix(files_sharing): rate limit share creation 20 times per 10 minutes#50905

Merged
skjnldsv merged 4 commits intomasterfrom
fix/rate-limit-share-creation
Apr 15, 2025
Merged

fix(files_sharing): rate limit share creation 20 times per 10 minutes#50905
skjnldsv merged 4 commits intomasterfrom
fix/rate-limit-share-creation

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Feb 19, 2025

And adjust existing email sending UserRateLimit

@skjnldsv skjnldsv added the 3. to review Waiting for reviews label Feb 19, 2025
@skjnldsv skjnldsv added this to the Nextcloud 32 milestone Feb 19, 2025
@skjnldsv skjnldsv requested review from a team, nickvergessen and provokateurin February 19, 2025 15:12
@skjnldsv skjnldsv self-assigned this Feb 19, 2025
@skjnldsv skjnldsv requested review from come-nc and yemkareems and removed request for a team February 19, 2025 15:12
@skjnldsv
Copy link
Member Author

skjnldsv commented Feb 19, 2025

/backport to stable31

@skjnldsv
Copy link
Member Author

skjnldsv commented Feb 19, 2025

/backport to stable30

susnux
susnux reviewed Feb 20, 2025
View reviewed changes
@skjnldsv skjnldsv requested a review from susnux February 20, 2025 13:24
susnux
susnux approved these changes Feb 20, 2025
View reviewed changes
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about the values, but code is fine.

@susnux
Copy link
Contributor

susnux commented Feb 20, 2025

For integration tests you need to create some app config for bruteForce with whitelist_1 and value the local ip in the test setup so that the tests are not rate limited.

@skjnldsv
Copy link
Member Author

skjnldsv commented Feb 20, 2025

Not sure about the values, but code is fine.

yeah, me neither tbh 🤷
I used Joas's suggestion

@skjnldsv
Copy link
Member Author

skjnldsv commented Feb 21, 2025

Integration needs excemption from rate limiting 🙈

@AndyScherzinger AndyScherzinger force-pushed the fix/rate-limit-share-creation branch from 6575855 to 7d282f2 Compare February 25, 2025 20:29
@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Feb 26, 2025
skjnldsv and others added 2 commits April 15, 2025 08:45
@skjnldsv @nickvergessen
fix(files_sharing): rate limit share creation 10 times per 10 minutes
338ab64 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@nickvergessen
test: Disable ratelimit as tests trigger it
59d1a49 
Signed-off-by: Joas Schilling <coding@schilljs.com>
@nickvergessen nickvergessen force-pushed the fix/rate-limit-share-creation branch from 7d282f2 to 59d1a49 Compare April 15, 2025 06:46
@nickvergessen nickvergessen requested a review from a team as a code owner April 15, 2025 06:46
@nickvergessen nickvergessen requested review from artonge and removed request for a team April 15, 2025 06:46
@nickvergessen
Copy link
Member

nickvergessen commented Apr 15, 2025

Integration needs excemption from rate limiting 🙈

Added

@skjnldsv
Copy link
Member Author

skjnldsv commented Apr 15, 2025 

--- Failed scenarios:

    /home/runner/actions-runner/_work/server/server/build/integration/features/ratelimiting.feature:10
    /home/runner/actions-runner/_work/server/server/build/integration/features/ratelimiting.feature:23
    /home/runner/actions-runner/_work/server/server/build/integration/features/ratelimiting.feature:36

ahah

@skjnldsv
Copy link
Member Author

skjnldsv commented Apr 15, 2025

On it

@skjnldsv
test: enable rate limiting for ratelimiting.feature
75063cd 
Signed-off-by: skjnldsv <skjnldsv@protonmail.com>
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 2. developing Work in progress labels Apr 15, 2025
@skjnldsv
fix(files_sharing): adjust rate limit share creation to 20 over 10 mi…
50dcbde 
…nutes

Signed-off-by: John Molakvoæ <skjnldsv@users.noreply.github.com>
@skjnldsv skjnldsv changed the title fix(files_sharing): rate limit share creation 10 times per 10 minutes fix(files_sharing): rate limit share creation 20 times per 10 minutes Apr 15, 2025
@skjnldsv skjnldsv disabled auto-merge April 15, 2025 08:52
@skjnldsv skjnldsv merged commit fbb7727 into master Apr 15, 2025
186 of 191 checks passed
@skjnldsv skjnldsv deleted the fix/rate-limit-share-creation branch April 15, 2025 08:57
This was referenced Apr 15, 2025
Merged
Merged
@bigcat88 bigcat88 mentioned this pull request Apr 16, 2025
Merged
@oleksandr-nc oleksandr-nc mentioned this pull request Apr 16, 2025
Merged
@nabim777 nabim777 mentioned this pull request Apr 16, 2025
Merged
8 tasks
@nextcloud nextcloud deleted a comment from kesselb May 5, 2025
@nextcloud-bot nextcloud-bot mentioned this pull request Aug 19, 2025
Merged
@skjnldsv skjnldsv modified the milestones: Nextcloud 32, Nextcloud 33 Sep 28, 2025
@biredel biredel mentioned this pull request Jan 20, 2026
Open
8 tasks
biredel added a commit to biredel/nextcloud that referenced this pull request Jan 20, 2026
@biredel
rate limit share creation 200 times per 10 minutes
68cded4 
Was unlimited before nextcloud#50905 setting to 20/600s which proved to be too low for some legitimate human actions, e.g. chat messages with attachments in Talk. Fix that by bumping to some arbitrary higher value, until some clearer justification for having it lower or higher is determined.

Signed-off-by: biredel <67849440+biredel@users.noreply.github.com>
This was referenced Jan 20, 2026
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@susnux susnux susnux approved these changes

@come-nc come-nc come-nc approved these changes

@nickvergessen nickvergessen Awaiting requested review from nickvergessen

@provokateurin provokateurin Awaiting requested review from provokateurin

@yemkareems yemkareems Awaiting requested review from yemkareems yemkareems is a code owner automatically assigned from nextcloud/server-backend

@artonge artonge Awaiting requested review from artonge artonge is a code owner automatically assigned from nextcloud/server-backend

Assignees

@skjnldsv skjnldsv

Labels

4. to release Ready to be released and/or waiting for tests to finish feature: sharing

Projects

None yet

Milestone

Nextcloud 32

Development

Successfully merging this pull request may close these issues.

4 participants

@skjnldsv @susnux @nickvergessen @come-nc