Skip to content
This repository has been archived by the owner on May 13, 2022. It is now read-only.

password protect the port #20

Closed
wants to merge 2 commits into from
Closed

Conversation

syst3mw0rm
Copy link

Implemented basic HTTP basic auth functionality (issue #11 ) to password protect the port. Here's the sample conf file used for authentication,

basic_auth_enabled: true
username: admin
password: secret

@Jaspur
Copy link

Jaspur commented Mar 12, 2014

👍

@andre
Copy link
Contributor

andre commented Mar 12, 2014

Thanks @syst3mw0rm. I'll review this later this week or early next. The main thing I'm going to be looking at: if we're introducing a configuration file, does it make sense to make it a general-purpose configuration file, instead of being specific to basic auth?

Also, just to clarify your functionality: with this merged, basic auth is always enabled, correct?

@syst3mw0rm
Copy link
Author

There are just two files in ~/.scout namely pid_file and log_file which can't be merged with this conf file and we don't have any general-purpose conf params at the moment. Essentially this is a general-purpose conf file already with a slightly non-general-purpose name. So, we can just rename proposed scout_basic_auth.conf to general purpose scout_realtime.conf.

No, it has a parameter, basic_auth_enabled which will be false by default and only when you change it to true, basic auth will be enabled.

@itsderek23
Copy link
Contributor

It does seem confusing to only have basic auth configured via a config file.

Wondering if the following makes sense:

  • each option can be specified in the config file by its full name
  • if an option is specified on both the command line and in the config file, the command line overrides.

Also, does it make sense to always try and load the config file from the default location? It would reduce the arguments needed when starting scout_realtime.

@syst3mw0rm syst3mw0rm closed this Apr 29, 2014
@syst3mw0rm syst3mw0rm reopened this Apr 29, 2014
@syst3mw0rm
Copy link
Author

It already tries to load the config file from default location (~/.scout/scout_basic_auth.conf). I understand that it doesn't make much sense to provide user the ability to change this file path. So, I can go ahead and remove it.

What do you mean by, each option can be specified in the config file by its full name? Is it not the case already?

Yes, we can certainly have command line arguments as well to enable http basic auth but I thought it would not be a good practice to write username and password in cleartext when running the process. But, providing enable_basic_auth should be helpful.

@itsderek23
Copy link
Contributor

What do you mean by, each option can be specified in the config file by its full name? Is it not the case already?

For a new user, having a set of options configured via a config file and another set via the command line seems confusing.

Yes, we can certainly have command line arguments as well to enable http basic auth but I thought it would not be a good practice to write username and password in cleartext when running the process.

I agree with the security issues of making these command-line arguments.

Nutshell: it seems like we need to make every argument configurable from a config file (ex: port, log-path, pid-path, etc) as it's confusing to only have some options configurable in one area (command line arguments) and another set in a config file.

@Manochehrghamary
Copy link

Please support

@syst3mw0rm syst3mw0rm closed this Aug 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants