slack-ruby-bot-server icon indicating copy to clipboard operation
slack-ruby-bot-server copied to clipboard

Disabling/Hiding some teams endpoints.

Open arturs-at-levelpath opened this issue 2 years ago • 4 comments

Hi, based on current implementation of https://github.com/slack-ruby/slack-ruby-bot-server/blob/master/lib/slack-ruby-bot-server/api/endpoints/teams_endpoint.rb all those endpoints by default are public. Which means anyone could in theory list all slack integrations.

I was thinking, are there any clean ways to disable get endpoints or put them behind the auth layer and leave the post public one?

Only way how currently I can do it is through monkey patching. 🤔

arturs-at-levelpath avatar Jul 06 '23 09:07 arturs-at-levelpath

In my bots I don't mount default endpoints and rolled out my own for teams, e.g. in strava bot I only return teams that have public API enabled. You'd also want to prevent individual teams from being returned by id.

I think we have a few options:

  1. Do nothing, document how to override.
  2. Add a way to disable this and other APIs.
  3. Introduce a public_api or something similar and default it to false, ensure all existing endpoints abide by it.
  4. Add auth as you suggest.

While I like my (2) option, it will create conflicts for those like me who already rolled out their own. I think we should do 0) and 1). One possibly nicer way that could fix monkey-patching is to split the TeamsEndpoint into TeamsEndpoint::Get and TeamsEndpoint::Create, and document how to mount it.

WDYT? Want to give it a try?

dblock avatar Jul 06 '23 12:07 dblock

I like your way of thinking on option 1. Because then when patching Root endpoints, you would just add those mounts like TeamsEndpoint::Store without gets. This way its still kind of patch but with reasonable clear way how to do it.

arturs-at-levelpath avatar Jul 13 '23 14:07 arturs-at-levelpath

Also interested in this. As someone who isn't super familiar with grape, it would be great to have a recommendation/documentation for best way to approach this i.e. option 0. Also, it would be good to make folks aware that the default is open in case they miss that. I do like the suggestion of more granular mounting. Also looks like mount supports passing in options that can then get handled in an override of mount_instance. Would it be possible to add some optional options around authorization for the mount command?

alrooney avatar Aug 31 '23 13:08 alrooney

I have a ton of bots that rewrite the root API and mount it, latest in https://github.com/dblock/discord-strava. That's what I would document to begin with.

dblock avatar Aug 31 '23 16:08 dblock