search

slack-ruby / slack-ruby-bot-server

A library that enables you to write a complete Slack bot service with Slack button integration, in Ruby.
MIT License
268 stars 74 forks source link

Disabling/Hiding some teams endpoints. #171

Open arturs-at-levelpath opened 1 year ago

arturs-at-levelpath commented 1 year ago

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. 🤔

dblock commented 1 year ago

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?

arturs-at-levelpath commented 1 year ago

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.

alrooney commented 1 year ago

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?

dblock commented 1 year ago

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.