Skip to content

[NEW] Limit all DDP/Websocket requests (configurable via admin panel)#13311

Merged
sampaiodiego merged 5 commits intodevelopfrom
ddp-rate-limit
Feb 1, 2019
Merged

[NEW] Limit all DDP/Websocket requests (configurable via admin panel)#13311
sampaiodiego merged 5 commits intodevelopfrom
ddp-rate-limit

Conversation

@rodrigok
Copy link
Member

@rodrigok rodrigok commented Feb 1, 2019

No description provided.

@rodrigok rodrigok added this to the 0.74.1 milestone Feb 1, 2019
@rodrigok rodrigok requested a review from sampaiodiego February 1, 2019 00:37
@engelgabriel engelgabriel temporarily deployed to rocket-chat-pr-13311 February 1, 2019 00:38 Inactive
@rodrigok rodrigok temporarily deployed to rocket-chat-pr-13311 February 1, 2019 12:28 Inactive

const callback = (message) => (reply, input) => {
if (reply.allowed === false) {
console.warn('DDP RATE LIMIT:', message);
Copy link
Member

Choose a reason for hiding this comment

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

should we send this to Logger?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't have a level for warnings in Logger, do you think this message should be an error?

Copy link
Member

Choose a reason for hiding this comment

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

it might be an error, since the client's request was denied.

I was initially thinking if this warns could flood the server's log, with no way to hide them.

Copy link
Member Author

Choose a reason for hiding this comment

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

@sampaiodiego but there is no way to hide errors as well 😄

Copy link
Member

Choose a reason for hiding this comment

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

true 😛

};

const matchedRules = self._findAllMatchingRules(input);
_.each(matchedRules, function(rule) {
Copy link
Member

Choose a reason for hiding this comment

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

underscore 🤢

Copy link
Member Author

Choose a reason for hiding this comment

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

I was keeping the meteor's code as much as possible

@rodrigok rodrigok temporarily deployed to rocket-chat-pr-13311 February 1, 2019 13:25 Inactive
@rodrigok rodrigok temporarily deployed to rocket-chat-pr-13311 February 1, 2019 13:30 Inactive
sampaiodiego
sampaiodiego previously approved these changes Feb 1, 2019
@sampaiodiego sampaiodiego merged commit 9d7d270 into develop Feb 1, 2019
@sampaiodiego sampaiodiego deleted the ddp-rate-limit branch February 1, 2019 19:32
@sampaiodiego sampaiodiego mentioned this pull request Feb 1, 2019
engelgabriel added a commit that referenced this pull request Feb 3, 2019
* develop:
  [NEW] Limit all DDP/Websocket requests (configurable via admin panel) (#13311)
  [FIX] Mobile view and re-enable E2E tests (#13322)
  [NEW] REST endpoint to forward livechat rooms (#13308)
  [FIX] Hipchat Enterprise Importer not generating subscriptions (#13293)
  [FIX] Message updating by Apps (#13294)
  [FIX]  REST endpoint for creating custom emojis (#13306)
  [FIX] Preview of image uploads were not working when apps framework is enable (#13303)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments