Page MenuHomePhabricator

[keyserver] Validate community ID corresponds to a community root in role-creator

Authored by rohan on Wed, Nov 15, 12:01 PM.



This diff adds a validation check to make sure the community ID passed in to the server corresponds to a community root (where community roles are valid)

This was some feedback from @ashoat to include this check, and I should have originally done it when writing this code

Depends on D9897

Test Plan

Made sure that it works for valid thread types, and that an error was thrown if a non-community ID was passed in

Diff Detail

rCOMM Comm
Lint Not Applicable
Tests Not Applicable

Event Timeline

rohan published this revision for review.Wed, Nov 15, 12:26 PM
92–96 ↗(On Diff #33284)

Can checkThreadPermission be rewritten to avoid fetching from the threads table twice? (We already need to fetch once in fetchThreadInfos below.)

I think I asked you to do something similar in another diff recently, but in that case we had fetchServerThreadInfos instead of fetchThreadInfos. It should be a bit easier in this case, but let me know if you need pointers!

92–110 ↗(On Diff #33284)

Can these two checks be performed simultaneously rather than sequentially?

Use threadHasPermission. Tested this by:

  • Making sure users who should be able to create roles still can
  • Allowed role creation for all users client-side, but kept this server check. When a regular member tried to create a role, hasPermission was false and an error was thrown
This revision is now accepted and ready to land.Thu, Nov 16, 10:01 AM