phybros commented 4 years ago

Currently the API is completely unrestricted. Anyone who can access the ip/port where the server is running can talk to it.

There are a few different options that might be feasible:

1. JWT

This was the first idea I had, and started spiking it out in #5 - unfortunately, without a signin mechanism, this is basically as secure as just sending a shared secret. Yes, you can generate short lived tokens on the client side using the shared HMAC secret, but that might be a bit too complicated for users of this plugin? We want this thing to be easy to use for the most amount of people.

2. Usernames/Passwords

Just create a map of username: bcrypt(password) in config.yml and then authenticate based on that using HTTP BASIC auth.

chestnutcase commented 4 years ago

Considering how dangerous a malicious application can be when they have access to the API, I think security is more important than catering to users who don't understand basic networking (see: dynmap github issues)

Allow the user to generate secrets with specific scopes for each external application (e.g. this application can only view the list of online players and nothing else). This way, users are aware of what permissions they are granting to each application. A simplified version of OAuth if you will. Then pass them in the header or something (X-APPLICATION-ID and X-API-KEY), I think it's easier than HTTP basic auth.

For the record, if you do imagine the majority of client applications are going to be user-facing applications like the web console (and not headless data aggregators / automation scripts), then a full OAuth implementation might be worth looking into.

morgverd commented 3 years ago

For now, a simple and yet effective method I use is just to write a very simple API to sit in-between requests to servertap. Then I can just run the servertap API locally only and preform the authentication I need on the public facing mediation API.

A tad over engineered but it gets the job done.

phybros commented 2 years ago

Currently working on an implementation that will allow you to put something like this in your config.yml

auth:
  # a key that has ultimate power
  - name: my-key
    key: 123abcsupersecretkey
    allow:
      - "*"
  # A key that can only access worlds and players
  - name: basic-key
    key: redrumredrum4567889
    deny:
      - "*"
    allow:
      - "world"
      - "worlds"
      - "players/*"

Just need to figure out if we shuold have separate permissions for read v.s. write, or just have a flag called readOnly on each key that will determine its level of access to the specified endpoints.

I welcome thoughts, ideas, critique etc

koutsie commented 2 years ago

Currently working on an implementation that will allow you to put something like this in your config.yml
auth:
  # a key that has ultimate power
  - name: my-key
    key: 123abcsupersecretkey
    allow:
      - "*"
  # A key that can only access worlds and players
  - name: basic-key
    key: redrumredrum4567889
    deny:
      - "*"
    allow:
      - "world"
      - "worlds"
      - "players/*"
Just need to figure out if we shuold have separate permissions for read v.s. write, or just have a flag called readOnly on each key that will determine its level of access to the specified endpoints.

I welcome thoughts, ideas, critique etc

I really like the simplicity of this being an "readOnly" flag.

dominik-korsa commented 2 years ago

I think it would be beneficial if the config didn't store the token (key), but it's hash (preferably using bcrypt). This is how for example GitHub or Discord does it. The token could be either sent as simple HTTP auth or as Authorization: Bearer <token> header. The benefit of not storing the token is the same as not storing plaintext passwords in a database - if the configuration file leaks, it doesn't give attackers access to the API. And a configuration file leaking is very probable - a server admin could send someone their config file to get support for your plugin or they might share a full server backup with others.

So the user would need to generate a token and hash. This could be accomplished in multiple ways, some that I could think of are:

Spigot server command that generates a token and it's hash. It shows the token to the command sender and creates a new entry in config.yml with the hash. There should be a note that the token will be shown only once. One thing to keep in mind is that you should avoid logging the token when showing it to the sender.
A command that starts a web GUI - after all this is a web server plugin :) The link generated by the command would contain a session id, which is checked when entering the GUI This option is probably overkill, but nonetheless it's something to consider
A simple HTTP + JS generator. You already have a domain for this plugin, there could be a page which can generate both the token and hash. The user is expected to put the hash in the config themselves.

I guess the token and hash should have a specific format, like STh(...) for the hash and STt(...), so the plugin can detect if the user put the token in the config or sent a hash as the authorization header. Here's a blog post by GitHub about their token format: https://github.blog/2021-04-05-behind-githubs-new-authentication-token-formats/

phybros commented 2 years ago

@dominik-korsa I think I like your first idea. What if it worked like this?

server admin installs ServerTap
server starts, and if it doesn't find a file called .first-run (or something) it will refuse to serve any endpoints until...
server admin does /servertap enable-auth or something like that on the server console, ServerTap prints something like this

Authentication enabled. Your server token is: `st_4jlh3bqwlk5jbl588bwrklbjwe5kjhb32wkj45cca`. Note: this token will never be shown again

And then it also writes that (or the bcrypt() of that) into config.yml?

Then any request to any ServerTap REST API methods check that the user has provided: Authorization: Bearer <the token>?

dominik-korsa commented 2 years ago

The problem with step 3. Is that the token will be logged in in the console and will remain in the log files. The point of storing only the hash is that even if someone gets to see the server files, they still would not be able to access the API.

Also I don't think there is a need for the .first-run file at all. There could be any number of tokens generated with different scopes (permissions). If there is no entry in the config, every request will get code 403/401, because their token would obviously be invalid. You can show a warning in the console if there is no entry in config. The 401/403 page could describe the steps to authorize the request.

If you still want to use the command, I would rename it to /server-tap generate-token, because there could be many tokens.

Also as a last thought: I think there should be no way for the authentication to be disabled. Most of the users will choose the simplest way to do something, so why would they bother to set up a token?

servertap-io / servertap

Authentication #6

1. JWT

2. Usernames/Passwords