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

feat(framework) Add enable-user-auth field in TOML file and require TLS for user authentication #4850

Merged
merged 3 commits into from
Jan 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 1 addition & 1 deletion src/py/flwr/cli/config_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,7 @@ def validate_certificate_in_federation_config(
root_certificates_bytes = (app / root_certificates).read_bytes()
if insecure := bool(insecure_str):
typer.secho(
"❌ `root_certificates` were provided but the `insecure` parameter "
"❌ `root-certificates` were provided but the `insecure` parameter "
"is set to `True`.",
fg=typer.colors.RED,
bold=True,
Expand Down
2 changes: 1 addition & 1 deletion src/py/flwr/cli/log.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ def _log_with_exec_api(
run_id: int,
stream: bool,
) -> None:
auth_plugin = try_obtain_cli_auth_plugin(app, federation)
auth_plugin = try_obtain_cli_auth_plugin(app, federation, federation_config)
channel = init_channel(app, federation_config, auth_plugin)

if stream:
Expand Down
16 changes: 15 additions & 1 deletion src/py/flwr/cli/login/login.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,18 @@ def login( # pylint: disable=R0914
federation, config, federation_config_overrides
)
exit_if_no_address(federation_config, "login")

# Check if `enable-user-auth` is set to `true`
if not federation_config.get("enable-user-auth", False):
typer.secho(
f"❌ User authentication is not enabled for the federation '{federation}'. "
"To enable it, set `enable-user-auth = true` in the federation "
"configuration.",
fg=typer.colors.RED,
bold=True,
)
raise typer.Exit(code=1)

channel = init_channel(app, federation_config, None)
stub = ExecStub(channel)

Expand All @@ -73,7 +85,9 @@ def login( # pylint: disable=R0914

# Get the auth plugin
auth_type = login_response.auth_type
auth_plugin = try_obtain_cli_auth_plugin(app, federation, auth_type)
auth_plugin = try_obtain_cli_auth_plugin(
app, federation, federation_config, auth_type
)
if auth_plugin is None:
typer.secho(
f'❌ Authentication type "{auth_type}" not found',
Expand Down
2 changes: 1 addition & 1 deletion src/py/flwr/cli/ls.py
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ def ls( # pylint: disable=too-many-locals, too-many-branches, R0913, R0917
raise ValueError(
"The options '--runs' and '--run-id' are mutually exclusive."
)
auth_plugin = try_obtain_cli_auth_plugin(app, federation)
auth_plugin = try_obtain_cli_auth_plugin(app, federation, federation_config)
channel = init_channel(app, federation_config, auth_plugin)
stub = ExecStub(channel)

Expand Down
2 changes: 1 addition & 1 deletion src/py/flwr/cli/run/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ def _run_with_exec_api(
stream: bool,
output_format: str,
) -> None:
auth_plugin = try_obtain_cli_auth_plugin(app, federation)
auth_plugin = try_obtain_cli_auth_plugin(app, federation, federation_config)
channel = init_channel(app, federation_config, auth_plugin)
stub = ExecStub(channel)

Expand Down
2 changes: 1 addition & 1 deletion src/py/flwr/cli/stop.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def stop( # pylint: disable=R0914
exit_if_no_address(federation_config, "stop")

try:
auth_plugin = try_obtain_cli_auth_plugin(app, federation)
auth_plugin = try_obtain_cli_auth_plugin(app, federation, federation_config)
channel = init_channel(app, federation_config, auth_plugin)
stub = ExecStub(channel) # pylint: disable=unused-variable # noqa: F841

Expand Down
41 changes: 29 additions & 12 deletions src/py/flwr/cli/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,25 +217,42 @@ def get_user_auth_config_path(root_dir: Path, federation: str) -> Path:
def try_obtain_cli_auth_plugin(
root_dir: Path,
federation: str,
federation_config: dict[str, Any],
auth_type: Optional[str] = None,
) -> Optional[CliAuthPlugin]:
"""Load the CLI-side user auth plugin for the given auth type."""
config_path = get_user_auth_config_path(root_dir, federation)

# Load the config file if it exists
json_file: dict[str, Any] = {}
if config_path.exists():
with config_path.open("r", encoding="utf-8") as file:
json_file = json.load(file)
# This is the case when the user auth is not enabled
elif auth_type is None:
# Check if user auth is enabled
if not federation_config.get("enable-user-auth", False):
return None

# Check if TLS is enabled. If not, raise an error
if federation_config.get("root-certificates") is None:
typer.secho(
"❌ User authentication requires TLS to be enabled. "
"Please provide 'root-certificates' in the federation"
" configuration.",
fg=typer.colors.RED,
bold=True,
)
raise typer.Exit(code=1)

config_path = get_user_auth_config_path(root_dir, federation)

# Get the auth type from the config if not provided
# auth_type will be None for all CLI commands except login
if auth_type is None:
if AUTH_TYPE not in json_file:
return None
auth_type = json_file[AUTH_TYPE]
try:
with config_path.open("r", encoding="utf-8") as file:
json_file = json.load(file)
auth_type = json_file[AUTH_TYPE]
except (FileNotFoundError, KeyError):
typer.secho(
"❌ Missing or invalid credentials for user authentication. "
"Please run `flwr login` to authenticate.",
fg=typer.colors.RED,
bold=True,
)
raise typer.Exit(code=1) from None

# Retrieve auth plugin class and instantiate it
try:
Expand Down