Skip to content

The password provided to configure shouldn't be echo'd. #244

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.

Sign up for GitHub

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

Closed
wants to merge 2 commits into from
Closed

The password provided to configure shouldn't be echo'd. #244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e3a15f7
The password provided to configure shouldn't be echo'd.
areese Jul 8, 2019
44d3ba4
Fix test failures caused by missing field
areese Jul 8, 2019
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
18 changes: 15 additions & 3 deletions databricks_cli/configure/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,18 +32,27 @@

PROMPT_HOST = 'Databricks Host (should begin with https://)'
PROMPT_USERNAME = 'Username'
PROMPT_PASSWORD = 'Password' # NOQA
PROMPT_PASSWORD = 'Password [enter stdin to read from stdin every time]' # NOQA
PROMPT_TOKEN = 'Token' # NOQA


def _configure_cli_token(profile, insecure):
config = ProfileConfigProvider(profile).get_config() or DatabricksConfig.empty()
host = click.prompt(PROMPT_HOST, default=config.host, type=_DbfsHost())
token = click.prompt(PROMPT_TOKEN, default=config.token)
token = click.prompt(PROMPT_TOKEN, default=config.token, hide_input=True)
new_config = DatabricksConfig.from_token(host, token, insecure)
update_and_persist_config(profile, new_config)


def _configure_cli_stdin(profile, insecure):
config = ProfileConfigProvider(profile).get_config() or DatabricksConfig.empty()
host = click.prompt(PROMPT_HOST, default=config.host, type=_DbfsHost())
username = click.prompt(PROMPT_USERNAME, default=config.username)
password = 'stdin'
Copy link
Contributor

@andrewmchen andrewmchen Jul 10, 2019

Choose a reason for hiding this comment

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

What if my password is stdin? I know this password is probably not allowed in most sign up workflows in Databricks but I'd rather not special case this password..

Can we just add a flag called read_password_from_stdin or something?

Copy link
Contributor Author

@areese areese Jul 11, 2019

Choose a reason for hiding this comment

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

excellent point, let me rework this that way.
How about empty password? Would that be an ok choice or would you rather use a flag?

Copy link
Contributor

@andrewmchen andrewmchen Jul 15, 2019

Choose a reason for hiding this comment

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

I think a flag is probably just the cleanest way.

new_config = DatabricksConfig.from_password(host, username, password, insecure)
update_and_persist_config(profile, new_config)


def _configure_cli_password(profile, insecure):
config = ProfileConfigProvider(profile).get_config() or DatabricksConfig.empty()
if config.password:
Expand All @@ -64,16 +73,19 @@ def _configure_cli_password(profile, insecure):
short_help='Configures host and authentication info for the CLI.')
@click.option('--token', show_default=True, is_flag=True, default=False)
@click.option('--insecure', show_default=True, is_flag=True, default=None)
@click.option('--stdin', show_default=True, is_flag=True, default=False)
@debug_option
@profile_option
def configure_cli(token, insecure):
def configure_cli(token, insecure, stdin):
"""
Configures host and authentication info for the CLI.
"""
profile = get_profile_from_context()
insecure_str = str(insecure) if insecure is not None else None
if token:
_configure_cli_token(profile, insecure_str)
elif stdin:
_configure_cli_stdin(profile, insecure_str)
else:
_configure_cli_password(profile, insecure_str)

Expand Down
15 changes: 11 additions & 4 deletions databricks_cli/configure/provider.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import getpass
from abc import abstractmethod, ABCMeta
from configparser import ConfigParser
import os
Expand All @@ -33,7 +33,7 @@
CONFIG_FILE_ENV_VAR = "DATABRICKS_CONFIG_FILE"
HOST = 'host'
USERNAME = 'username'
PASSWORD = 'password' # NOQA
PASSWORD = 'password' # NOQA
TOKEN = 'token'
INSECURE = 'insecure'
DEFAULT_SECTION = 'DEFAULT'
Expand Down Expand Up @@ -263,10 +263,17 @@ def get_config(self):


class DatabricksConfig(object):
def __init__(self, host, username, password, token, insecure): # noqa
def __init__(self, host, username, password, token, insecure): # noqa
self.host = host
self.username = username
self.password = password

self.password = None
if self.password == 'stdin':
message = 'Password to connect to databricks at [{}]: '.format(self.host)
self.password = getpass.getpass(message)
else:
self.password = password

self.token = token
self.insecure = insecure

Expand Down