Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disabling/Hiding some teams endpoints. #171

Open
arturs-at-levelpath opened this issue Jul 6, 2023 · 4 comments
Open

Disabling/Hiding some teams endpoints. #171

arturs-at-levelpath opened this issue Jul 6, 2023 · 4 comments

Comments

@arturs-at-levelpath
Copy link

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
Copy link
Collaborator

dblock commented Jul 6, 2023

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
Copy link
Author

arturs-at-levelpath commented Jul 13, 2023

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
Copy link

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
Copy link
Collaborator

dblock commented Aug 31, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants