From 026bbc4eff8b42ac57a08ff7f552b83280360ebe Mon Sep 17 00:00:00 2001 From: phg Date: Thu, 3 Sep 2026 18:35:20 +0200 Subject: [PATCH] Delegate privileged authentication to sudo PAM --- README.md | 5 +- docs/user-guide.md | 11 ++- src/hosts/core/manager.py | 89 +++++++++++----------- src/hosts/tui/app.py | 64 ++++++++++------ src/hosts/tui/password_modal.py | 98 ------------------------ src/hosts/tui/styles.py | 37 ---------- tests/test_app.py | 127 ++++++++++++++++++++++++++++++++ tests/test_manager.py | 71 +++++++++++------- 8 files changed, 266 insertions(+), 236 deletions(-) delete mode 100644 src/hosts/tui/password_modal.py create mode 100644 tests/test_app.py diff --git a/README.md b/README.md index 4c7fbbc..dde5bd0 100644 --- a/README.md +++ b/README.md @@ -47,8 +47,9 @@ uvx git+https://git.s1q.dev/phg/hosts.git > limitations before granting sudo access. Press `Ctrl+E` to enter Privileged Mode. The application uses an existing sudo -authorization or asks for your password, then creates a Pre-edit Backup before -enabling changes. +authorization or temporarily returns control of the terminal to `sudo` and its +configured PAM authentication method. It then creates a Pre-edit Backup before +enabling changes. `hosts` never collects or handles authentication credentials. Start with the [user guide](docs/user-guide.md) for the complete workflow, including persistence behavior and manual recovery. diff --git a/docs/user-guide.md b/docs/user-guide.md index 4452dc9..fa17ced 100644 --- a/docs/user-guide.md +++ b/docs/user-guide.md @@ -46,7 +46,8 @@ before entering Privileged Mode if you sorted only for inspection. ## Enter Privileged Mode Press `Ctrl+E`. The application first checks for existing sudo authorization. -If authorization is not already available, it asks for your password. Before +If authorization is not already available, it temporarily returns control of +the terminal to `sudo` and its configured PAM authentication method. Before enabling mutations it verifies write access and creates a Pre-edit Backup. If authorization, permission validation, or backup creation fails, the @@ -192,10 +193,12 @@ reachable through the TUI. Editing them manually is not a supported workflow. ## Troubleshooting -### Privileged Mode asks for a password +### Privileged Mode needs authentication -This is expected when no cached sudo authorization is available. Canceling the -password prompt leaves the application in Read-only Mode. +This is expected when no cached sudo authorization is available. `hosts` never +collects credentials; `sudo` and PAM present any configured authentication +method, such as Touch ID or a password prompt. Canceling authentication leaves +the application in Read-only Mode. ### Privileged Mode cannot be enabled diff --git a/src/hosts/core/manager.py b/src/hosts/core/manager.py index 4ccbdc0..db7383d 100644 --- a/src/hosts/core/manager.py +++ b/src/hosts/core/manager.py @@ -35,57 +35,47 @@ class PermissionManager: self.has_sudo = False self._sudo_validated = False - def request_sudo(self, password: str = None) -> Tuple[bool, str]: + def request_sudo(self, interactive: bool = False) -> Tuple[bool, str]: """ Request sudo permissions for hosts file editing. Args: - password: Optional password for sudo authentication + interactive: Whether to run foreground sudo for a PAM conversation Returns: Tuple of (success, message) """ try: - # Test sudo access with a simple command - result = subprocess.run( - ["sudo", "-n", "true"], capture_output=True, text=True, timeout=5 - ) - - if result.returncode == 0: - # Already have sudo access - self.has_sudo = True - self._sudo_validated = True - return True, "Sudo access already available" - - # If no password provided, indicate we need password input - if password is None: - return False, "Password required for sudo access" - - # Use password for sudo authentication - result = subprocess.run( - ["sudo", "-S", "-v"], - input=password + "\n", - capture_output=True, - text=True, - timeout=10, - ) - - if result.returncode == 0: - self.has_sudo = True - self._sudo_validated = True - return True, "Sudo access granted" + if interactive: + # The foreground process inherits the terminal so sudo and PAM own + # prompts, authentication methods, retries, and explanatory output. + result = subprocess.run(["sudo", "-v"]) else: - # Check if it's a password error - if ( - "incorrect password" in result.stderr.lower() - or "authentication failure" in result.stderr.lower() - ): - return False, "Incorrect password" - else: - return False, f"Sudo access denied: {result.stderr}" + result = subprocess.run( + ["sudo", "-n", "-v"], + capture_output=True, + text=True, + timeout=5, + ) + + if result.returncode != 0: + return False, "Interactive authorization required" + + if result.returncode == 0: + self.has_sudo = True + self._sudo_validated = True + message = ( + "Sudo access granted" + if interactive + else "Sudo access already available" + ) + return True, message + return False, "Authorization was not granted" except subprocess.TimeoutExpired: return False, "Sudo request timed out" + except FileNotFoundError: + return False, "sudo is unavailable; remaining in Read-only Mode" except Exception as e: return False, f"Error requesting sudo: {e}" @@ -137,24 +127,35 @@ class HostsManager: self._backup_path: Optional[Path] = None self.undo_redo_history = UndoRedoHistory() - def enter_edit_mode(self, password: str = None) -> Tuple[bool, str]: + def enter_edit_mode(self) -> Tuple[bool, str]: """ Enter edit mode with proper permission management. - Args: - password: Optional password for sudo authentication - Returns: Tuple of (success, message) """ if self.edit_mode: return True, "Already in edit mode" - # Request sudo permissions - success, message = self.permission_manager.request_sudo(password) + # Request cached sudo authorization. + success, message = self.permission_manager.request_sudo() if not success: return False, message + return self.finish_entering_edit_mode() + + def authorize_interactively(self) -> Tuple[bool, str]: + """Request foreground sudo authorization without entering Privileged Mode.""" + if self.edit_mode: + return True, "Already in edit mode" + + return self.permission_manager.request_sudo(interactive=True) + + def finish_entering_edit_mode(self) -> Tuple[bool, str]: + """Validate write access and create a backup after authorization.""" + if self.edit_mode: + return True, "Already in edit mode" + # Validate write permissions if not self.permission_manager.validate_permissions(str(self.parser.file_path)): return False, "Cannot write to hosts file even with sudo" diff --git a/src/hosts/tui/app.py b/src/hosts/tui/app.py index 71780a0..80e388a 100644 --- a/src/hosts/tui/app.py +++ b/src/hosts/tui/app.py @@ -5,7 +5,7 @@ This module contains the main application class that orchestrates all the handlers and provides the primary user interface. """ -from textual.app import App, ComposeResult +from textual.app import App, ComposeResult, SuspendNotSupported from textual.containers import Horizontal, Vertical from textual.widgets import ( Header, @@ -25,7 +25,6 @@ from ..core.manager import HostsManager from ..core.dns import DNSService from ..core.filters import EntryFilter, FilterOptions from .config_modal import ConfigModal -from .password_modal import PasswordModal from .add_entry_modal import AddEntryModal from .delete_confirmation_modal import DeleteConfirmationModal from .filter_modal import FilterModal @@ -490,39 +489,56 @@ class HostsManagerApp(App): else: self.update_status(f"Error exiting edit mode: {message}") else: - # Enter edit mode - first try without password + # First check whether the current sudo authorization is cached. success, message = self.manager.enter_edit_mode() if success: self.edit_mode = True self.update_status(message) - elif "Password required" in message: - # Show password modal - self._request_sudo_password() + elif message == "Interactive authorization required": + self._enter_edit_mode_interactively() else: self.update_status(f"Error entering edit mode: {message}") - def _request_sudo_password(self) -> None: - """Show password modal and attempt sudo authentication.""" + def _enter_edit_mode_interactively(self) -> None: + """Run one foreground sudo/PAM conversation outside the TUI.""" + interrupted = False + try: + with self.suspend(): + try: + success, message = self.manager.authorize_interactively() + except KeyboardInterrupt: + interrupted = True + except SuspendNotSupported: + self.update_status( + "Interactive authorization requires terminal suspension; remaining in Read-only Mode." + ) + return - def handle_password(password: str) -> None: - if password is None: - # User cancelled - self.update_status("Edit mode cancelled") - return + if interrupted: + self.update_status( + "Authorization was not granted; remaining in Read-only Mode." + ) + return - # Try to enter edit mode with password - success, message = self.manager.enter_edit_mode(password) - if success: - self.edit_mode = True - self.update_status(message) - elif "Incorrect password" in message: - # Show error and try again - self.update_status("❌ Incorrect password. Please try again.") - self.set_timer(2.0, lambda: self._request_sudo_password()) + if not success: + if message == "Authorization was not granted": + self.update_status( + "Authorization was not granted; remaining in Read-only Mode." + ) else: - self.update_status(f"❌ Error entering edit mode: {message}") + self.update_status(f"Error entering edit mode: {message}") + return - self.push_screen(PasswordModal(), handle_password) + success, message = self.manager.finish_entering_edit_mode() + if success: + self.edit_mode = True + self.update_status(message) + elif message == "Authorization was not granted": + self.update_status( + "Authorization was not granted; remaining in Read-only Mode." + ) + else: + self.update_status(f"Error entering edit mode: {message}") def action_edit_entry(self) -> None: """Enter edit mode for the selected entry.""" diff --git a/src/hosts/tui/password_modal.py b/src/hosts/tui/password_modal.py deleted file mode 100644 index d445e66..0000000 --- a/src/hosts/tui/password_modal.py +++ /dev/null @@ -1,98 +0,0 @@ -""" -Password input modal window for sudo authentication. - -This module provides a secure password input modal for sudo operations. -""" - -from textual.app import ComposeResult -from textual.containers import Vertical -from textual.widgets import Static, Button, Input -from textual.screen import ModalScreen -from textual.binding import Binding - -from .styles import PASSWORD_MODAL_CSS - - -class PasswordModal(ModalScreen): - """ - Modal screen for secure password input. - - Provides a floating window for entering sudo password with proper masking. - """ - - CSS = PASSWORD_MODAL_CSS - - BINDINGS = [ - Binding("escape", "cancel", "Cancel"), - Binding("enter", "submit", "Submit"), - ] - - def __init__(self): - super().__init__() - self.error_message = "" - - def compose(self) -> ComposeResult: - """Create the password modal layout.""" - with Vertical(classes="password-container"): - yield Static("Sudo Authentication", classes="password-title") - - with Vertical(classes="default-section") as password_input: - password_input.border_title = "Enter sudo Password" - yield Input( - placeholder="Password", - password=True, - id="password-input", - classes="default-input", - ) - - # Error message placeholder (initially empty) - yield Static("", id="error-message", classes="error-message") - - def on_mount(self) -> None: - """Focus the password input when modal opens.""" - password_input = self.query_one("#password-input", Input) - password_input.focus() - - def on_button_pressed(self, event: Button.Pressed) -> None: - """Handle button presses.""" - if event.button.id == "ok-button": - self.action_submit() - elif event.button.id == "cancel-button": - self.action_cancel() - - def on_input_submitted(self, event: Input.Submitted) -> None: - """Handle Enter key in password input field.""" - if event.input.id == "password-input": - self.action_submit() - - def action_submit(self) -> None: - """Submit the password and close modal.""" - password_input = self.query_one("#password-input", Input) - password = password_input.value - - if not password: - self.show_error("Password cannot be empty") - return - - # Clear any previous error - self.clear_error() - - # Return the password - self.dismiss(password) - - def action_cancel(self) -> None: - """Cancel password input and close modal.""" - self.dismiss(None) - - def show_error(self, message: str) -> None: - """Show an error message in the modal.""" - error_static = self.query_one("#error-message", Static) - error_static.update(message) - # Keep focus on password input - password_input = self.query_one("#password-input", Input) - password_input.focus() - - def clear_error(self) -> None: - """Clear the error message.""" - error_static = self.query_one("#error-message", Static) - error_static.update("") diff --git a/src/hosts/tui/styles.py b/src/hosts/tui/styles.py index 51723e2..60c4bea 100644 --- a/src/hosts/tui/styles.py +++ b/src/hosts/tui/styles.py @@ -280,43 +280,6 @@ DeleteConfirmationModal { """ ) -# Password Modal CSS -PASSWORD_MODAL_CSS = ( - COMMON_CSS - + """ -PasswordModal { - align: center middle; -} - -.password-container { - width: 60; - height: 11; - background: $surface; - border: thick $primary; - padding: 1; -} - -.password-title { - text-align: center; - text-style: bold; - color: $primary; - margin-bottom: 1; -} - -.password-message { - text-align: center; - color: $text; - margin-bottom: 1; -} - -.error-message { - color: $error; - text-align: center; - margin: 1 0; -} -""" -) - # Config Modal CSS CONFIG_MODAL_CSS = ( COMMON_CSS diff --git a/tests/test_app.py b/tests/test_app.py new file mode 100644 index 0000000..57710fb --- /dev/null +++ b/tests/test_app.py @@ -0,0 +1,127 @@ +"""Tests for application-level authorization flow.""" + +from contextlib import contextmanager +from unittest.mock import Mock + +from textual.app import SuspendNotSupported + +from src.hosts.tui.app import HostsManagerApp + + +@contextmanager +def suspended_tui(): + """Stand in for Textual's terminal suspension in action tests.""" + yield + + +class TestPrivilegedModeAuthorization: + """Test the user-visible privileged-mode authorization flow.""" + + def test_interactive_authorization_suspends_and_restores_the_tui(self): + """Test foreground PAM authorization runs while Textual is suspended.""" + events = [] + + @contextmanager + def suspended_tui(): + events.append("suspended") + yield + events.append("restored") + + def authorize_interactively(): + assert events == ["suspended"] + events.append("authorized") + return True, "Sudo access granted" + + def finish_entering_edit_mode(): + assert events == ["suspended", "authorized", "restored"] + events.append("edit mode enabled") + return True, "Edit mode enabled" + + app = HostsManagerApp() + app.manager = Mock() + app.manager.enter_edit_mode.return_value = ( + False, + "Interactive authorization required", + ) + app.manager.authorize_interactively.side_effect = authorize_interactively + app.manager.finish_entering_edit_mode.side_effect = finish_entering_edit_mode + app.suspend = Mock(return_value=suspended_tui()) + app.update_status = Mock() + + app.action_toggle_edit_mode() + + assert app.edit_mode + app.suspend.assert_called_once_with() + app.manager.enter_edit_mode.assert_called_once_with() + app.manager.authorize_interactively.assert_called_once_with() + app.manager.finish_entering_edit_mode.assert_called_once_with() + assert events == ["suspended", "authorized", "restored", "edit mode enabled"] + app.update_status.assert_called_once_with("Edit mode enabled") + + def test_rejected_authorization_keeps_the_app_read_only(self): + """Test a rejected PAM conversation reports the required message.""" + app = HostsManagerApp() + app.manager = Mock() + app.manager.enter_edit_mode.return_value = ( + False, + "Interactive authorization required", + ) + app.manager.authorize_interactively.return_value = ( + False, + "Authorization was not granted", + ) + app.suspend = Mock(return_value=suspended_tui()) + app.update_status = Mock() + + app.action_toggle_edit_mode() + + assert not app.edit_mode + app.update_status.assert_called_once_with( + "Authorization was not granted; remaining in Read-only Mode." + ) + + def test_interrupted_authorization_keeps_the_app_read_only(self): + """Test interruption during PAM authorization restores read-only mode.""" + events = [] + + @contextmanager + def suspended_tui(): + events.append("suspended") + yield + events.append("restored") + + app = HostsManagerApp() + app.manager = Mock() + app.manager.enter_edit_mode.return_value = ( + False, + "Interactive authorization required", + ) + app.manager.authorize_interactively.side_effect = KeyboardInterrupt() + app.suspend = Mock(return_value=suspended_tui()) + app.update_status = Mock() + + app.action_toggle_edit_mode() + + assert not app.edit_mode + assert events == ["suspended", "restored"] + app.update_status.assert_called_once_with( + "Authorization was not granted; remaining in Read-only Mode." + ) + + def test_unsupported_suspension_keeps_the_app_read_only(self): + """Test the user receives an actionable suspension failure message.""" + app = HostsManagerApp() + app.manager = Mock() + app.manager.enter_edit_mode.return_value = ( + False, + "Interactive authorization required", + ) + app.suspend = Mock(side_effect=SuspendNotSupported()) + app.update_status = Mock() + + app.action_toggle_edit_mode() + + assert not app.edit_mode + app.update_status.assert_called_once_with( + "Interactive authorization requires terminal suspension; remaining in Read-only Mode." + ) diff --git a/tests/test_manager.py b/tests/test_manager.py index ae5f6c0..5eefa79 100644 --- a/tests/test_manager.py +++ b/tests/test_manager.py @@ -24,9 +24,8 @@ class TestPermissionManager: assert not pm._sudo_validated @patch("subprocess.run") - def test_request_sudo_already_available(self, mock_run): - """Test requesting sudo when already available.""" - # Mock successful sudo -n true + def test_request_sudo_uses_cached_authorization(self, mock_run): + """Test cached sudo authorization enters without an interaction.""" mock_run.return_value = Mock(returncode=0) pm = PermissionManager() @@ -38,56 +37,62 @@ class TestPermissionManager: assert pm._sudo_validated mock_run.assert_called_once_with( - ["sudo", "-n", "true"], capture_output=True, text=True, timeout=5 + ["sudo", "-n", "-v"], capture_output=True, text=True, timeout=5 ) @patch("subprocess.run") - def test_request_sudo_prompt_success(self, mock_run): - """Test requesting sudo with password prompt success.""" - # First call (sudo -n true) fails, second call (sudo -S -v) succeeds - mock_run.side_effect = [ - Mock(returncode=1), # sudo -n true fails - Mock(returncode=0), # sudo -S -v succeeds - ] + def test_request_sudo_interactively_uses_foreground_pam(self, mock_run): + """Test an uncached authorization delegates to foreground sudo.""" + mock_run.return_value = Mock(returncode=0) pm = PermissionManager() - success, message = pm.request_sudo("testpassword") + success, message = pm.request_sudo(interactive=True) assert success assert "access granted" in message assert pm.has_sudo assert pm._sudo_validated - - assert mock_run.call_count == 2 + mock_run.assert_called_once_with(["sudo", "-v"]) @patch("subprocess.run") - def test_request_sudo_no_password(self, mock_run): - """Test requesting sudo when no password is provided.""" - # sudo -n true fails (password needed) + def test_request_sudo_requires_interaction_when_cache_unavailable(self, mock_run): + """Test an uncached authorization does not start PAM from the TUI.""" mock_run.return_value = Mock(returncode=1) pm = PermissionManager() success, message = pm.request_sudo() assert not success - assert "Password required" in message + assert message == "Interactive authorization required" + assert not pm.has_sudo + assert not pm._sudo_validated + mock_run.assert_called_once_with( + ["sudo", "-n", "-v"], capture_output=True, text=True, timeout=5 + ) + + @patch("subprocess.run") + def test_request_sudo_interactively_denied(self, mock_run): + """Test a rejected PAM conversation keeps sudo unavailable.""" + mock_run.return_value = Mock(returncode=1) + + pm = PermissionManager() + success, message = pm.request_sudo(interactive=True) + + assert not success + assert message == "Authorization was not granted" assert not pm.has_sudo assert not pm._sudo_validated @patch("subprocess.run") - def test_request_sudo_denied(self, mock_run): - """Test requesting sudo when access is denied.""" - # Both calls fail - mock_run.side_effect = [ - Mock(returncode=1), # sudo -n true fails - Mock(returncode=1, stderr="access denied"), # sudo -S -v fails - ] + def test_request_sudo_interactively_cancelled(self, mock_run): + """Test cancellation during PAM authorization keeps sudo unavailable.""" + mock_run.return_value = Mock(returncode=130) pm = PermissionManager() - success, message = pm.request_sudo("testpassword") + success, message = pm.request_sudo(interactive=True) assert not success - assert "denied" in message + assert message == "Authorization was not granted" assert not pm.has_sudo assert not pm._sudo_validated @@ -115,6 +120,18 @@ class TestPermissionManager: assert "Test error" in message assert not pm.has_sudo + @patch("subprocess.run") + def test_request_sudo_reports_when_sudo_is_unavailable(self, mock_run): + """Test a missing sudo executable produces an actionable result.""" + mock_run.side_effect = FileNotFoundError() + + pm = PermissionManager() + success, message = pm.request_sudo() + + assert not success + assert message == "sudo is unavailable; remaining in Read-only Mode" + assert not pm.has_sudo + @patch("subprocess.run") def test_validate_permissions_success(self, mock_run): """Test validating permissions successfully."""