Skip to content

fix(dav): Rate limit calendar/subscription creation - #43732

Merged
ChristophWurst merged 1 commit into
masterfrom
fix/dav/rate-limit-create-calendar
Feb 23, 2024
Merged

fix(dav): Rate limit calendar/subscription creation#43732
ChristophWurst merged 1 commit into
masterfrom
fix/dav/rate-limit-create-calendar

Conversation

@ChristophWurst

@ChristophWurstChristophWurst commented Feb 21, 2024•
edited
Loading

Copy link
Copy Markdown
Member

Summary

Adds rate limiting for new calendars and subscriptions.

Checklist

@ChristophWurstChristophWurst added bug 3. to review Waiting for reviews feature: dav feature: caldav Related to CalDAV internals labels Feb 21, 2024
@ChristophWurstChristophWurst added this to the Nextcloud 29 milestone Feb 21, 2024
@ChristophWurstChristophWurst self-assigned this Feb 21, 2024
ChristophWurst
ChristophWurst commented Feb 21, 2024
View reviewed changes
Comment threadapps/dav/lib/CalDAV/Security/RateLimitingPlugin.php Outdated
throw new TooManyRequests('Too many calendars created', 0, $e);
}

$calendarLimit = $this->config->getValueInt('dav', 'maximum_calendars', 30);

@ChristophWurstChristophWurstFeb 21, 2024

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I'm open for input on a reasonable limit for 99% of users. someone will exceed it and they need the admin to increase the limit. this is acceptable.

github-advanced-security[bot]
github-advanced-securityAI found potential problems Feb 21, 2024
View reviewed changes
Comment threadapps/dav/lib/CalDAV/Security/RateLimitingPlugin.php Fixed
Comment threadapps/dav/lib/CalDAV/Security/RateLimitingPlugin.php Fixed
Comment threadapps/dav/lib/CalDAV/Security/RateLimitingPlugin.php Fixed
Comment threadapps/dav/lib/CalDAV/Security/RateLimitingPlugin.php Fixed
miaulalala
miaulalala approved these changes Feb 21, 2024
View reviewed changes

@miaulalalamiaulalala left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

joshtrichards
joshtrichards reviewed Feb 21, 2024
View reviewed changes
Comment thread.htaccess Outdated
tcitworld
tcitworld reviewed Feb 21, 2024
View reviewed changes

@tcitworldtcitworld left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you test how the Calendar app currently handles the TooManyRequests exception? When creating calendars/subscriptions/holiday calendars/imports

Mind creating an issue to make sure this is tracked?

Comment threadapps/dav/lib/Server.php
Comment threadapps/dav/lib/CalDAV/Security/RateLimitingPlugin.php Outdated
Comment threadapps/dav/lib/CalDAV/Security/RateLimitingPlugin.php Outdated
@ChristophWurstChristophWurst mentioned this pull request Feb 22, 2024
Open
@ChristophWurst

ChristophWurst commented Feb 22, 2024

Copy link
Copy Markdown
MemberAuthor

Did you test how the Calendar app currently handles the TooManyRequests exception? When creating calendars/subscriptions/holiday calendars/imports

Mind creating an issue to make sure this is tracked?

Sure! nextcloud/calendar#5792

@ChristophWurst
ChristophWurstforce-pushed the fix/dav/rate-limit-create-calendar branch from 7f07be8 to 9bfa1a3CompareFebruary 22, 2024 08:20
@ChristophWurstChristophWurst mentioned this pull request Feb 22, 2024
Merged
@ChristophWurstChristophWurst added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Feb 22, 2024
This was referenced Feb 22, 2024
Merged
Merged
Merged
@ChristophWurst

ChristophWurst commented Feb 22, 2024

Copy link
Copy Markdown
MemberAuthor

/backport! to stable28

@ChristophWurst

ChristophWurst commented Feb 22, 2024

Copy link
Copy Markdown
MemberAuthor

/backport! to stable27

@ChristophWurst

ChristophWurst commented Feb 22, 2024

Copy link
Copy Markdown
MemberAuthor

/backport! to stable26

@backportbotbackportbotBot mentioned this pull request Feb 22, 2024
Merged
2 tasks
This was referenced Feb 22, 2024
Merged
Merged
@ChristophWurst
fix(dav): Rate limit calendar/subscription creation
dc7f2ba
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
@ChristophWurst
ChristophWurstforce-pushed the fix/dav/rate-limit-create-calendar branch from 9bfa1a3 to dc7f2baCompareFebruary 23, 2024 07:53
@ChristophWurst
ChristophWurst merged commit f1c79cd into masterFeb 23, 2024
@ChristophWurst
ChristophWurst deleted the fix/dav/rate-limit-create-calendar branch February 23, 2024 12:54
@blizzzblizzz mentioned this pull request Mar 5, 2024
Merged
@ChristophWurstChristophWurst mentioned this pull request Mar 11, 2024
Merged
4 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tcitworldtcitworldtcitworld approved these changes
@miaulalalamiaulalalamiaulalala approved these changes
@nickvergessennickvergessenAwaiting requested review from nickvergessen
@joshtrichardsjoshtrichardsAwaiting requested review from joshtrichards

Assignees

@ChristophWurstChristophWurst

Labels

4. to releaseReady to be released and/or waiting for tests to finishbugfeature: caldavRelated to CalDAV internalsfeature: dav

Projects

💌 📅 👥 Groupware team
Archived in project

Milestone

Nextcloud 29

Development

Successfully merging this pull request may close these issues.

5 participants

@ChristophWurst@joshtrichards@tcitworld@miaulalala@github-advanced-security