configuration should use environment variables rather than storing passwords in files

[davidlumley]

Thanks for creating a cool way of visualising Discord traffic!

When checking this out to use on my own server I noticed a security issue – users must store their login details within a configuration file as indicated by the readme (1), and the configuration itself (2)

(1):

Rename discord-config-example.json to discord-config.json and insert the login and server info for your Discord server(s).

(2):

"email": "test@example.com",
"password": "password",

A better way of handling this is to have the user store their secrets as environment variables, and have the JSON structure indicate the names of the environment variables to use.

This eliminates the chance of a user accidentally (or purposefully) committing sensitive configuration details to the repo.

[vegeta897]

Thanks for the advice, I'm quite new to this so I didn't really know the best practices. I will look into using environment variables.

[vegeta897]

After googling around, I'm still unsure of the best way to go about doing this. If you can point me in the right direction I'd be very grateful :)

[KyleCrowley]

I'm not an expert when it comes to node-convict, but after doing some quick research, I was able to come across this article from Mozilla that states that node-convict will use the environment variables if you give it a value in the schema as such:

password: { doc: "The password for your Discord account. Ignored if token provided.", format: String, env: "PASSWORD" },

The only thing the user would need to do is set the PASSWORD env var using PASSWORD="password", or whatever the equivalent is on their OS.

This way, users can store sensitive info in environment variables rather than in a config file.

Haven't actually tested this but it seems simple enough from the article linked above.

[vegeta897]

Thank you, I'll try to get that done soon.

[favna]

If you want to use server sided environment variables in NodeJS you should really use DotENV. Simply have a file named .env (no name, just extension, just like .gitignore and the such) in the root folder (or specify the location with the config) and any variables in there will be loaded into Node's process.env as long as you require dotenv as soon as possible in the code (i.e. before the bot is even started, line 2 in index.js would do nicely). Ignoring the file can be ensured by adding *.env to the .gitignore and it already always is for gitignore templates (i.e. those that github offers). Naturally you could also ask users to add environment variables manually into their system directly, but this has some major issues

1. It does not support pm2 (not on windows nor debian anyway) since they use their own secluded environment variables system
2. Modifying environment variables sometimes requires a reboot for Windows systems, this created a layer of extreme end-user unfriendliness.

Through using the dotenv setup you could also merge (most of) discord-config.json and (entirely) socket-config.json simply by specifying names. For example in that order

token=""
url=""
infoCommand=""
socketaddress=""
socketport=""

Now you could put the servers into dotenv as well but since it doesn't support arrays let along objects you'd have to be hacky about it and that's kinda meh

servers=1234567891234567|true|members-only;testing,9876543219876543|false|bot-spam

not really the nicest format since you'd have to split by symbols in the code then so better keep that as JSON. Same counts for misc-config.json. Instead my recommendation is that alongside using dotenv for all sensitive data we'd instead use 1 JSON config (config.json) for all free-to-publicize data:

{
  "servers": [{
    "id": "9876543219876543",
    "default": true,
    "ignoreChannels": ["members-Only", "testing"]
  }, {
    "id": "1234567891234567",
    "default": false,
    "ignoreChannels": ["bot-spam"]
  }],
  "misc": {
    "textbox": {
      "maxWidth": 96,
      "linesPerPage": 4,
      "scrollSpeeds": [
        [0, 3],
        [75, 2],
        [150, 1]
      ],
      "pageDelay": 5,
      "finalDelay": 30,
      "openTime": 10,
      "closeTime": 8,
      "bgColor": "rgba(0, 0, 0, 0.7)"
    }
  }
}

[vegeta897]

Thank you very much for your detailed input, I'll try to dive into this some time soon.

[favna]

@vegeta897 I have done some rework for my own hosting of this as described in #55 (comment). This comment also shortly goes into how I did my dotenv. For my implementation I just set the textbox stuff in the textbox.js, but for the main repo that should just be as detailed in my previous comment

[davidlumley]

Hey, thanks for the bump on this and sorry for not getting back to you with how to resolve this.

@favna's comment about using DotENV is pretty much spot on!

For production, you should always be using system environment variables via process.env for secrets (like db creds, usernames and passwords etc) and DotENV provides a great way to manage that for local use.

For security, .env should be added to the .gitignore too 👍

Happy to look into submitting a PR if this doesn't make sense.

[vegeta897]

Thanks @davidlumley; I have a pretty good grasp of environment variables and have used DotENV in another project since this issue was opened, so I think I can handle this.

[vegeta897]

I promise I will get around to this 😣

[vegeta897]

So organizing this is tricky.

The original reason for splitting the socket-config from discord-config was because, for the bundle file, browserify includes the entire json file regardless of what properties you use from it. So, the bundle needs the socket information, but should not grab anything else. Removing the token from the json is a given, but that's not the only sensitive information. Servers can have passwords, and obviously we wouldn't want those included in the bundle. But moving them to .env is awkward due to the lack of a structured format, as Favna said.

I tried googling for a way to only use only part of a JSON with browserify but came up with nothing.

So it seems the only practical thing to do is just move token to .env and leave everything else as-is.

Thoughts?

Edit: I'm going to go ahead with the above plan, since the token is really the only crucially sensitive info that should be environment-ified. But if anyone has ideas I'd like to hear them.

[Macleykun]

Shouldn't this be re-opend maybe?

[SagnikPradhan]

Seems like this can be closed.

OR. We could wait till I flesh out configuration validation.