Skip to content

Commit

Permalink
feat: det deploy local generates a password for you [DET-10197] (#9518)
Browse files Browse the repository at this point in the history
  • Loading branch information
jesse-amano-hpe authored Jun 20, 2024
1 parent 0cf7aba commit 1747b94
Show file tree
Hide file tree
Showing 3 changed files with 166 additions and 20 deletions.
9 changes: 9 additions & 0 deletions docs/release-notes/deploy-local-auto-password.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
:orphan:

Security Fixes

- CLI: When deploying locally using ``det deploy local`` with ``master-up`` or ``cluster-up``
commands and no user accounts have been created yet, an initial password will be automatically
generated and shown to the user (with the option to change it) if neither
``security.initial_user_password`` in ``master.yaml`` nor the ``--initial-user-password`` CLI
flag is present.
88 changes: 69 additions & 19 deletions e2e_tests/tests/deploy/test_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,14 @@ def mksess(host: str, port: int, username: str = "determined", password: str = "
return authentication.login(master_url, username=username, password=password)


def det_deploy(subcommand: List) -> None:
def det_deploy(subcommand: List, cmd_input: Optional[bytes] = None) -> subprocess.CompletedProcess:
command = [
"det",
"deploy",
"local",
] + subcommand
print(f"Running deployment: {' '.join(command)}")
subprocess.run(command, check=True)
return subprocess.run(command, check=True, stdout=subprocess.PIPE, input=cmd_input)


def resource_up(
Expand All @@ -49,7 +49,8 @@ def resource_up(
kwflags: Optional[Dict[str, str]] = None,
flags: Optional[List[str]] = None,
positional_arguments: Optional[List[str]] = None,
) -> None:
cmd_input: Optional[bytes] = None,
) -> subprocess.CompletedProcess:
"""Issue a `det deploy local` command to bring up a resource.
Ex:
Expand Down Expand Up @@ -77,12 +78,12 @@ def resource_up(
det_version = conf.DET_VERSION
if det_version is not None:
command += ["--det-version", det_version]
det_deploy(command)
return det_deploy(command, cmd_input=cmd_input)


def resource_down(resource: Resource, name: str) -> None:
def resource_down(resource: Resource, name: str) -> subprocess.CompletedProcess:
command = [f"{resource}-down", f"--{resource}-name", name]
det_deploy(command)
return det_deploy(command)


@contextlib.contextmanager
Expand All @@ -92,11 +93,14 @@ def resource_manager(
kwflags: Optional[Dict[str, str]] = None,
boolean_flags: Optional[List[str]] = None,
positional_arguments: Optional[List[str]] = None,
) -> Iterator[None]:
cmd_input: Optional[bytes] = None,
) -> Iterator[subprocess.CompletedProcess]:
"""Context manager to bring resources up and down."""
resource_up(resource, name, kwflags, boolean_flags, positional_arguments)
res = resource_up(
resource, name, kwflags, boolean_flags, positional_arguments, cmd_input=cmd_input
)
try:
yield
yield res
finally:
resource_down(resource, name)

Expand All @@ -106,7 +110,10 @@ def test_cluster_down() -> None:
name = "test_cluster_down"

with resource_manager(
Resource.CLUSTER, name, {"initial-user-password": conf.USER_PASSWORD}, ["no-gpu"]
Resource.CLUSTER,
name,
{"initial-user-password": conf.USER_PASSWORD},
["no-gpu", "delete-db"],
):
container_name = name + "_determined-master_1"
client = docker.from_env()
Expand All @@ -126,7 +133,7 @@ def test_ee_cluster_up() -> None:
Resource.CLUSTER,
name,
{"initial-user-password": conf.USER_PASSWORD},
["no-gpu", "enterprise-edition"],
["no-gpu", "enterprise-edition", "delete-db"],
):
container_name = name + "_determined-master_1"
client = docker.from_env()
Expand All @@ -148,7 +155,7 @@ def test_custom_etc() -> None:
Resource.CLUSTER,
name,
{"master-config-path": etc_path, "initial-user-password": conf.USER_PASSWORD},
["no-gpu"],
["no-gpu", "delete-db"],
):
sess = mksess("localhost", 8080)
exp.run_basic_test(
Expand All @@ -165,7 +172,10 @@ def test_agent_config_path() -> None:
cluster_name = "test_agent_config_path"
master_name = f"{cluster_name}_determined-master_1"
with resource_manager(
Resource.MASTER, master_name, {"initial-user-password": conf.USER_PASSWORD}
Resource.MASTER,
master_name,
{"initial-user-password": conf.USER_PASSWORD},
["delete-db"],
):
# Config makes it unmodified.
etc_path = str(pathlib.Path(__file__).parent.joinpath("etc/agent.yaml").resolve())
Expand Down Expand Up @@ -208,7 +218,7 @@ def test_custom_port() -> None:
Resource.CLUSTER,
name,
{"master-port": str(custom_port), "initial-user-password": conf.USER_PASSWORD},
["no-gpu"],
["no-gpu", "delete-db"],
):
sess = mksess("localhost", custom_port)
exp.run_basic_test(
Expand All @@ -227,7 +237,7 @@ def test_agents_made() -> None:
Resource.CLUSTER,
name,
{"agents": str(num_agents), "initial-user-password": conf.USER_PASSWORD},
["no-gpu"],
["no-gpu", "delete-db"],
):
container_names = [name + f"-agent-{i}" for i in range(0, num_agents)]
client = docker.from_env()
Expand All @@ -243,7 +253,10 @@ def test_master_up_down() -> None:
master_name = f"{cluster_name}_determined-master_1"

with resource_manager(
Resource.MASTER, master_name, {"initial-user-password": conf.USER_PASSWORD}
Resource.MASTER,
master_name,
{"initial-user-password": conf.USER_PASSWORD},
["delete-db"],
):
client = docker.from_env()

Expand All @@ -254,6 +267,43 @@ def test_master_up_down() -> None:
assert len(containers) == 0


@pytest.mark.parametrize(
"password_input, expect_generated",
[
("XDdOB9VUp8FLpTZ2\nXDdOB9VUp8FLpTZ2\n", False),
("\n\n", True),
("31BLj16hEQWmPNHR\nNotTheSamePassword1\n", True),
],
)
@pytest.mark.det_deploy_local
def test_master_up_interactive_password(
password_input: Optional[str], expect_generated: Optional[bool]
) -> None:
assert password_input is not None
cluster_name = "test_master_up_interactive_password"
master_name = f"{cluster_name}_determined-master_1"

with resource_manager(
Resource.MASTER,
master_name,
boolean_flags=["delete-db"],
cmd_input=bytes(password_input, "utf8"),
) as master_up_command:
client = docker.from_env()

containers = client.containers.list(filters={"name": master_name})
assert len(containers) > 0
assert (
b"Determined Master was launched without a strong initial_user_password set."
in master_up_command.stdout
)
if expect_generated:
assert b"A password has been created for you." in master_up_command.stdout

containers = client.containers.list(filters={"name": master_name})
assert len(containers) == 0


@pytest.mark.det_deploy_local
def test_ee_master_up() -> None:
cluster_name = "test_master_up"
Expand All @@ -263,7 +313,7 @@ def test_ee_master_up() -> None:
Resource.MASTER,
master_name,
{"initial-user-password": conf.USER_PASSWORD},
["enterprise-edition"],
["enterprise-edition", "delete-db"],
):
client = docker.from_env()

Expand All @@ -281,7 +331,7 @@ def test_agent_up_down() -> None:
master_name = f"{cluster_name}_determined-master_1"

with resource_manager(
Resource.MASTER, master_name, {"initial-user-password": conf.USER_PASSWORD}
Resource.MASTER, master_name, {"initial-user-password": conf.USER_PASSWORD}, ["delete-db"]
):
with resource_manager(Resource.AGENT, agent_name, {}, ["no-gpu"], [conf.MASTER_IP]):
client = docker.from_env()
Expand All @@ -299,7 +349,7 @@ def test_ee_agent_up() -> None:
master_name = f"{cluster_name}_determined-master_1"

with resource_manager(
Resource.MASTER, master_name, {"initial-user-password": conf.USER_PASSWORD}
Resource.MASTER, master_name, {"initial-user-password": conf.USER_PASSWORD}, ["delete-db"]
):
with resource_manager(
Resource.AGENT, agent_name, {}, ["no-gpu", "enterprise-edition"], [conf.MASTER_IP]
Expand Down
89 changes: 88 additions & 1 deletion harness/determined/deploy/local/cluster_utils.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,27 @@
import contextlib
import getpass
import os
import pathlib
import random
import re
import secrets
import socket
import string
import subprocess
import sys
import tempfile
import threading
import time
import warnings
from typing import Any, Callable, Dict, Generator, List, Optional, Sequence, Type

import appdirs
import docker

from determined.common import constants, util
from determined.common import api, constants, util
from determined.common.api import authentication
from determined.deploy import errors, healthcheck
from determined.experimental import client as determined

AGENT_NAME_DEFAULT = f"det-agent-{socket.gethostname()}"
MASTER_PORT_DEFAULT = 8080
Expand Down Expand Up @@ -155,6 +162,8 @@ def master_up(
) -> None:
# Some cli flags for det deploy local will cause us to write a temporary master.yaml.
make_temp_conf = False
# If we have to generate a password for the user, we should print it later.
generated_user_password = None

if master_name is None:
master_name = f"{cluster_name}_{MASTER_NAME}"
Expand All @@ -173,6 +182,24 @@ def master_up(
master_conf["security"]["initial_user_password"] = initial_user_password
make_temp_conf = True

try:
authentication.check_password_complexity(
master_conf["security"].get("initial_user_password")
)
except ValueError:
random_password_characters = string.ascii_uppercase + string.ascii_lowercase + string.digits
random_password = [
secrets.choice(string.ascii_lowercase),
secrets.choice(string.ascii_uppercase),
secrets.choice(string.digits),
]
random_password.extend([secrets.choice(random_password_characters) for _ in range(13)])
random.shuffle(random_password)
generated_user_password = "".join(random_password)

master_conf["security"]["initial_user_password"] = generated_user_password
make_temp_conf = True

if storage_host_path is not None:
master_conf["checkpoint_storage"] = {
"type": "shared_fs",
Expand Down Expand Up @@ -286,6 +313,66 @@ def defer_cleanup(fn: Callable[[], None]) -> Generator:

_wait_for_master(f"http://localhost:{port}", cluster_name)

if generated_user_password is not None:
try:
sess = authentication.login(
f"http://localhost:{port}", "admin", generated_user_password
).with_retry(util.get_max_retries_config())
sess.get("/api/v1/me")

# No exception was raised, so this generated password is the way to log in.
print(
"Determined Master was launched without a strong initial_user_password set. "
+ "Please set a strong password by following prompts, or by logging in "
+ "with generated passwords and following the password change process."
)

try:
if not sys.stdin.isatty() or sys.stdin.closed:
raise EOFError
# getpass raises this warning instead of an exception if it can't find the
# terminal, which almost always means we're not going to be able to receive
# a password interactively and securely, so should abort quickly rather
# than time out.
with warnings.catch_warnings():
warnings.filterwarnings(action="error", category=getpass.GetPassWarning)
prompt = (
"Please enter a password for the built-in "
+ "`determined` and `admin` users: "
)
new_password = getpass.getpass(prompt)
# Give one more chance if this password is too weak
try:
authentication.check_password_complexity(new_password)
except ValueError as e:
print(e)
new_password = getpass.getpass(prompt)

authentication.check_password_complexity(new_password)
new_password_check = getpass.getpass("Enter the password again: ")
if new_password != new_password_check:
raise ValueError("passwords did not match")

d = determined.Determined._from_session(sess)
user = d.get_user_by_name("determined")
user.change_password(new_password)
user = d.get_user_by_name("admin")
user.change_password(new_password)

except Exception:
# User could exit, or might be unable to pass validation,
# or this might not even be interactive; none of these
# are problems with the deployment itself, so just print
# the password so users aren't locked out
print(
"A password has been created for you. The admin and determined users "
+ f"can log in with this password:\n\t{generated_user_password}\n"
)

except api.errors.UnauthenticatedException:
# There was a non-generated password there already; carry on
pass

# Remove all cleanup methods from ExitStack.
exit_stack.pop_all()

Expand Down

0 comments on commit 1747b94

Please sign in to comment.