servertap-io / servertap

ServerTap is a REST API for Bukkit/Spigot/Paper Minecraft servers
https://servertap.io
MIT License
225 stars 57 forks source link

Added Config Options to disable Endpoint Categories, Swagger and specific Paths #214

Closed Velyn-N closed 1 year ago

Velyn-N commented 1 year ago

As ServerTap provides Paths to critical Server Data and Functionality (e.g. /v1/server/ops) I wanted the Option to disable some of them.

I added three things:

  1. Config Options to disable Categories of Paths. You could for example disable all Paths for Worlds.
  2. A Config Option to disable Swagger and Swagger-Docs (could be useful in Production Environments)
  3. A Config Option to disable specific Paths. If I would for example like to disable /v1/server/whitelist but keep the other Server Endpoints I could add only this one to the Blocked-Paths List. Requests will get a 403 - Forbidden returned if the Paths are blocked.

The default Config is designed so everything stays enabled by default. You have to opt-out of everything.

phybros commented 1 year ago

Thank you for the PR! I like this change a lot. The only question I have really is what about paths with dynamic components such as /v1/players/{uuid}? These would seem to be unblockable with this change (except for just blocking the whole "category"). Do you have any ideas about that?

I don't think that scenario is a blocker (pun intended) for merging this change as-is, I just wanted to make sure you had thought about it.

Velyn-N commented 1 year ago

Actually I did not think about that! I will look into that and update this PR when I have a fix!

Velyn-N commented 1 year ago

I switched the whole URL Blocking to Regular Expressions. Now you can block all sorts of URLs and use Wildcards in the config. I commented the code where I thought necessary. Since it was now obsolete I removed the Category Disabling Part. You can just Block the category using a Wildcard URL Blocker.

Due to API Design there could be an issue with /v1/players/all when you block /v1/players/{uuid} since I did not specifically replace {uuid} with a UUID-Regex-Pattern. Of course I can still add that if wanted, but that would allow lookups for incorrectly formed UUIDs so I left that out for now. Can add that tho if wanted. The "correct" solution would be to change the API to use /v1/players/all and /v1/player/{uuid} which would prevent the conflicts in the first place. Maybe a thing for an future API v2 ;)

phybros commented 1 year ago

Nice, thanks - I'll look at this at some point this weekend or Monday. The code looks good to me, just want to test it out!

Velyn-N commented 1 year ago

Hey, are there any updates on the state of this PR or #217?

phybros commented 1 year ago

Sorry, haven't had a chance yet! But soon hopefully