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

Proposal Enable basic auth for CLI #178

Closed
alexellis opened this issue Oct 18, 2017 · 11 comments
Closed

Proposal Enable basic auth for CLI #178

alexellis opened this issue Oct 18, 2017 · 11 comments

Comments

@alexellis
Copy link
Member

alexellis commented Oct 18, 2017

Expected Behaviour

Pass username/password to remote calls - deploy/list/invoke etc.

Current Behaviour

Need to use API / curl.

Possible Solution / Design

We will need to discuss and review a design here before moving forward.

Options for reading:

  • Pass a parameter password - should not follow this approach due to bash cache history
  • Read from stdin - resolves issue with parameter but is hard to script and repetitive

A combination of both approaches may involve creating a hash of the username/password and storing that in an ~/.openfaas/config.json file. For Windows/Mac the native keychain would be a better place to store sensitive data.

Other concerns:

Multiple gateways are supported so we need to find a way to accept credentials or a token per gateway.

Context

Supports blog post for WeaveWorks

@Lewiscowles1986
Copy link
Contributor

Should this wait for #165 and #166? I definitely feel like #166 (check issue for branch merged with #165 current works) provides a more centralised place to locate logic

@Lewiscowles1986
Copy link
Contributor

Btw Happy to work on this. (I take it, this would be optional and use cli arguments?)

@alexellis
Copy link
Member Author

I've updated the design brief and commented on #180. Still needs to have the design reviewed.

@Lewiscowles1986
Copy link
Contributor

Lewiscowles1986 commented Oct 19, 2017

what about using an env var instead of a file in home folder? It seems cleaner and isn't polluting the FS with secrets (which seems to be the concern over cli history). For now, it empowers users to come up with their own mechanism, and we get the functionality without painting into a corner?

@stefanprodan
Copy link
Contributor

Hi,

I would suggest adding a login command that would validate and store the authentication data on disk, and a middleware for the API client that would fetch the user/pass from the auth store based on the gateway URI.

The login command would have the following signature:

./faas-cli login --help
Log in to OpenFaaS gateway.
If no gateway is specified, the default local one will be used.

Usage:
  faas-cli login [--username USERNAME] [--password PASSWORD] [--gateway GATEWAY_URL] [flags]

Examples:
  faas-cli login -u user -p password --gateway https://localhost:8080
  cat ~/faas_pass.txt | faas-cli login -u user --password-stdin --gateway https://openfaas.com

Flags:
      --gateway string    Gateway URI (default "http://localhost:8080")
  -h, --help              help for login
  -p, --password string   Gateway password
      --password-stdin    Reads the gateway password from stdin
  -u, --username string   Gateway username

The first implementation would store a JSON array of OpenFaaS endpoints at $HOME/.openfaas/config.json on Linux/Mac and %USERPROFILE%/.openfaas/config.json on Windows. An endpoint object would contain the gateway URI, username and password in base64 format. Later on, we could extend the auth store with keychain support for Mac/Windows.

If the --password is used, we should issue a warning and advise on the alternative --password-stdin param.

We should also consider a login --remove --gateway uri.

I've started working on this, please let me know if my proposal is suitable for OpenFaaS.

@alexellis
Copy link
Member Author

alexellis commented Oct 21, 2017

Hi Stefan,

I think faas-cli login would be needed to store/collect credentials to be used with basic auth on the server. Having spoke to the Docker security team at Dockercon they suggested storing the data in the keychain for Windows/Mac and Linux. There is an implementation in the Docker CLI which is worth exploring.

In your example config over on the Weave repo we should ideally only protect the /system/ and /ui/ endpoints with basic auth and leave /function and /async-function open. This means administration and listing is protected but by default invocation is not.

At the point of storing a credential we should also validate it ahead of time. I also suspect we will want to "cowardly refuse" to send credentials over HTTP (i.e. when SSL is not being used)

The help text looks good but one thing stands out - we should probably not allow reading password value from the command-line.

-p, --password string Gateway password

Alex

@stefanprodan
Copy link
Contributor

Forcing the password to be submitted via stdin will make CI workloads harder I think. All FOSS projects I've seen using TravisCI, Drone, etc are passing the credentials to docker login via -p flag since most CI tools allows you to store the password value as a secret. The build environment gets destroyed after each run so bash history is not much of a concern.

Regarding the keychain store, OpenFaaS users will need to go get or download a keychain helper and place the binary in the PATH so faas-cli could use them, also they must initialize pass with a gpg2 key ID. Adapting the Docker keychain store to OpenFaaS is no trivial task, more details here https://github.com/docker/docker-credential-helpers

I would go for the first implementation with the JSON file and the password flag. For non SSL addresses I would add a --insecure flag if the user really wants to send credentials over HTTP.

I think enforcing a high security standard will make some users give up auth altogether. Let me know your thoughts on this.

@Lewiscowles1986
Copy link
Contributor

Lewiscowles1986 commented Oct 21, 2017

This is going to sound trivial, but would it be possible to use YAML instead of JSON? I know it might sound a bit 🙄; but lots of the tooling is already working with YAML, and by working with YAML apparently it can load JSON (I've never tried that)

If it is something you'd be interested in, then there is lots of opportunities for reuse. @johnmccabe uses this to load config from YAML in https://github.com/johnmccabe/openfaas-bitbar, for multiple servers, so I'm sure this would be equally applicable to that project. Deploy already can use, I'm borrowing Johns Code to play with an Ubuntu notification area. It means there are places to standardise and follow practices that at least one other contributor is known to be versed in.

Also as the method for adding basic auth itself already has tests, feel free to use if you want, it's 2 files

@alexellis
Copy link
Member Author

I've not seen YAML used for config much, but I had a similar thought to this earlier for consistency. We also need a config file for third party templates.

@stefanprodan
Copy link
Contributor

stefanprodan commented Oct 21, 2017

If YAML would help with consistency sure I'll use that.

Regarding openfaas-bitbar, looks like it uses ~/.openfaas/config.yaml. I could make the auth store write to ~/.openfaas/auth.yaml.

On the long run, maybe we should consider renaming the bitbar config to bitbar.yaml. Could be confusing for users that the current config.yml file is not specific to faas-cli but to some other plugin.

@alexellis
Copy link
Member Author

Makes sense. Bitbar is currently a bolt-on project by @johnmccabe - I'm sure he'd rename his file.

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