Added rate limit bypass for app_api requests

[nc-fkl]

• Resolves: #

Summary

When doing a request via app_api the rate limiting prevents more than 5 requests per 2 minutes, with checking if the app_api session exsists we bypass the rate limiting for app_api requests.

Checklist

• Code is properly formatted
• Sign-off message is added to all commits
• Tests (unit, integration, api and/or acceptance) are included
• Screenshots before/after for front-end changes
• Documentation (manuals or wiki) has been updated or is not required
• Backports requested where applicable (ex: critical bugfixes)

• Resolves: #

[nc-fkl]

/backport to stable28

[nickvergessen]

It's not okay to generally disable ratelimit like this.

Can you maybe point out which endpoint is a problem?

[bigcat88]

Please, make squash commit with force push with correct message for "Block unconventional commits" check.

Added logic in this PR for me looks good, requests from ExApps should always bypass any rate limits.

[nickvergessen]

requests from ExApps should always bypass any rate limits.

Why is that? We have several API endpoints which have a rate limit also for logged in users.

[nc-fkl]

requests from ExApps should always bypass any rate limits.

Why is that? We have several API endpoints which have a rate limit also for logged in users.

Hi,

we need to utilize:
/ocs/v2.php/textprocessing/schedule
multiple times in a short period of time, but every >= 6th request is failing because of
429 too many requests
which is coming from the rate-limiting.

As a example we want to utilize some AI features for summarizing a long chatlog. We need to split the chatlog into multiple chunks (lets say 10 chunks) and for each we need to do a request to the /ocs/v2.php/textprocessing/schedule endpoint.
For the first 5 its going well but then the rate limiting is blocking us to process the other requests also, therefore and also for other app_api requests in the future, we need to bypass the rate limiting for app_api requests.

[bigcat88]

Why is that? We have several API endpoints which have a rate limit also for logged in users.

We need this for the Summurize bot (which was supposed to be released a week(?) ago..) calling text-processing endpoints to schedule a summarization task.
The bot does not have a user context; it is called from the background task.
As a result, it falls into the Anonymous rate limit (5 requests).
But the user rate limit is not enough for us too(20 requests), if the number of chats is more than 20, then the user rate limit will also block..

[nickvergessen]

Then let's either bump or remove the user limit from that endpoint for now.
But removing rate limit completely is not an option!

[nickvergessen]

Or additionally we add a AppAPIRateLimit attribute additionally which is checked in case the session flag is there and we fall back to the UserRateLimit if it's not set.

[bigcat88]

Then let's either bump or remove the user limit from that endpoint for now.

value of rate limit will depend on size of the Nextcloud instance, we can not know the value.

removing rate limit form that endpoint completely will allow to easy ddos cloud without even an account, as that endpoint allows anonymous access already..

Or additionally we add a AppAPIRateLimit attribute additionally which is checked in case the session flag is there and we fall back to the UserRateLimit if it's not set.

Good proposal

[nickvergessen]

removing rate limit form that endpoint completely will allow to easy ddos cloud without even an account, as that endpoint allows anonymous access already.

Well AppAPI is not anonymous, right? So it should be using the UserRateLimit, and you could simply adjust that to 100 per second.
You will need to do something alike also with an AppAPIRateLimit anyway.

[bigcat88]

Well AppAPI is not anonymous, right

Summarise bot is anonymous, it is not triggered by user, it is triggered by background job once a day, so we can't not know what "userId" to use in OCS requests.

[bigcat88]

My proposal: we have such a concept in ExApp as a “system application” - it is indicated by a separate flag.
These applications are able to impersonate the user and they do not interact directly with users, and Summarize Chat Bot is the first system application, afaik.

We can make sure that this flag is added to the session and add a check for this flag, and remove the rate limit only for such system applications.

Will this be ok?

Ordinary ExApps really don’t need a separate RateLimit, as they always have a UserId.

This is draft for adding that session variable: https://github.com/cloud-py-api/app_api/pull/248/files

[nickvergessen]

I guess so, but we are running out of time (behind API and feature freeze)

[nc-fkl]

I guess so, but we are running out of time (behind API and feature freeze)

As @bigcat88 mentioned, we are now checking in the session for app_api_system - it shouldn't break anything. Are you fine with that @nickvergessen ?

[bigcat88]

I guess so, but we are running out of time (behind API and feature freeze)

From my humble point of view, this is not a new feature, but rather a bug fix that we missed when implementing AppAPI authentication.
Bypassing two-factor authentication and CORS was treated like a bugfix..

[nickvergessen]

I guess so, but we are running out of time (behind API and feature freeze)

From my humble point of view, this is not a new feature, but rather a bug fix that we missed when implementing AppAPI authentication.

My point was less to block merging, but more to point out that this should be speeded up, so we also run the final result of crucial points before the release

[bigcat88]

cypress errors look unrelated

[welcome]

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

[bigcat88]

/backport to stable28