decentraland / social-service-ea / 34162009061

91%

push

github

fix: authorize community requests and place removal before reading the target (#496)

* fix: authorize community requests and place removal before reading the target

createCommunityRequest decided whether the target was already a member, and whether it was
banned, before validatePermissionToInviteUsers ran. The outcomes are distinguishable to a
caller with no standing: 400 for a member, 401 for a non-member, and a 401 naming the ban
for a banned one. getCommunity resolves a private community regardless of privacy, so any
authenticated wallet holding a listed community id could walk a private roster one address
at a time — the roster the members endpoint refuses to non-members.

Authorize first, then read. The community is resolved without the target address now, so
the target's role comes from a getCommunityMemberRole after the permission check and the
ordering is structural rather than incidental.

removePlace has the same shape: validatePlaceExists answers 404 for a place the community
does not hold and validateOwnership 401 for one it does, both before the remove_places
check, which leaked the place list getPlaces refuses to non-members. kickMember was fixed
this way in #477.

* fix: drop the redundant owner lookup in removePlace

The role read added ahead of the authorization was a second lookup of what
validatePermissionToRemovePlacesFromCommunity already reads, kept only to let owners skip
the validator. Owners hold remove_places, so the validator passes them anyway and the
special case bought nothing but the extra query.

Call the validator directly. Authorization still precedes validatePlaceExists, so the
404/401 differential over a private community's place list stays closed.

The two unit contexts that split on the caller's role now describe the same path, since the
component no longer reads the role — collapsed into one, with the owner and moderator halves
still covered end to end by the controller tests against the real roles component.

2589 of 2987 branches covered (86.68%)

Branch coverage included in aggregate %.

11 of 11 new or added lines in 2 files covered. (100.0%)

6240 of 6692 relevant lines covered (93.25%)

93.12 hits per line