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

Enable basic-auth by default #743

Merged
merged 4 commits into from
Jul 16, 2018
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 46 additions & 2 deletions deploy_stack.sh
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,50 @@ if ! [ -x "$(command -v docker)" ]; then
exit 1
fi

echo "Deploying stack"
docker stack deploy func --compose-file docker-compose.yml
export BASIC_AUTH="true"
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rgee0 this sets the value to true


sha_cmd="shasum -a 256"
if ! command -v shasum >/dev/null; then
sha_cmd="sha256sum"
fi

while [ ! $# -eq 0 ]
do
case "$1" in
--no-auth | -n)
export BASIC_AUTH="false"
;;
--help | -h)
echo "Usage: \n [default]\tdeploy the OpenFaaS core services\n --no-auth [-n]\tdisable basic authentication.\n --help\tdisplays this screen"
exit
;;
esac
shift
done

# Secrets should be created even if basic-auth is disabled.
echo "Attempting to create credentials for gateway.."
echo "admin" | docker secret create basic-auth-user -
secret=$(head -c 16 /dev/urandom| $sha_cmd | cut -d " " -f 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be good if there is an easy way for the user to create own credentials or edit the password without the need to set-up secrets manually.

What about passing and argument to deploy_stack.sh?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly in a follow-up PR. I think it's out of scope for this change. The user really must run the faas-cli login command though.

echo "$secret" | docker secret create basic-auth-password -
if [ $? = 0 ];
then
echo "[Credentials]\n username: admin \n password: $secret\n echo -n "$secret" | faas-cli login --username=admin --password-stdin"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The echo -n... needs some context rather than just printing along with the username and password. Ie blank line spacer and some text telling the user to use this to log into the gateway with the client.

Or even better, offer to do so on behalf of the user (if faas-cli is found on the path).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also make very clear that the user should record/store the generated user credentials as they'll not be made available again.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What context should we give about -n?

else
echo "[Credentials]\n already exist, not creating"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be inclined to give the user the option to replace these secrets. Could be in a situation where they've been setup in the past but the user is no longer sure what they had been set to (cough me).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about docker secret create?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about something like this in the troubleshooting guide?

docker service create --image alpine:3.7 --secret secret-name cat /run/secrets/secret-name 

fi

if [ $BASIC_AUTH = "true" ];
then
echo ""
echo "Enabling basic authentication for gateway.."
echo ""
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you need to include the sed command here as well.

Say for example you deploy a stack with --no-auth this will alter the docker-compose.yml disabling auth, but if you then tear down the stack and run again without disabling auth, then you'd get the same altered compose file so auth would remain disabled.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using envvars in the compose file instead (https://docs.docker.com/compose/environment-variables/) set enabled=true/false in these conditionals then run the final deploy with:

OPENFAAS_AUTH=$enabled docker stack deploy func --compose-file docker-compose.yml

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer the env-var approach with a default set.

else
echo ""
echo "Disabling basic authentication for gateway.."
echo ""
fi

echo "Deploying OpenFaaS core services"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency you'd want the ellipsis at the end of this output.


docker stack deploy func --compose-file docker-compose.yml