diff --git a/plinth/modules/ssh/__init__.py b/plinth/modules/ssh/__init__.py index 13e358aa6..e5e40a6dc 100644 --- a/plinth/modules/ssh/__init__.py +++ b/plinth/modules/ssh/__init__.py @@ -1,7 +1,5 @@ # SPDX-License-Identifier: AGPL-3.0-or-later -""" -FreedomBox app for OpenSSH server. -""" +"""FreedomBox app for OpenSSH server.""" import pathlib import re @@ -9,7 +7,6 @@ import subprocess from django.utils.translation import gettext_lazy as _ -from plinth import actions from plinth import app as app_module from plinth import menu from plinth.daemon import Daemon @@ -17,7 +14,7 @@ from plinth.modules.backups.components import BackupRestore from plinth.modules.firewall.components import Firewall from plinth.package import Packages -from . import manifest +from . import manifest, privileged _description = [ _('A Secure Shell server uses the secure shell protocol to accept ' @@ -65,7 +62,7 @@ class SSHApp(app_module.App): def setup(self, old_version): """Install and configure the app.""" super().setup(old_version) - actions.superuser_run('ssh', ['setup']) + privileged.setup() self.enable() @@ -87,9 +84,3 @@ def get_host_keys(): host_keys.append(match.groupdict()) return host_keys - - -def is_password_authentication_disabled(): - """Return if ssh password authentication is enabled.""" - return actions.superuser_run('ssh', - ['get-password-config']).strip() == 'no' diff --git a/actions/ssh b/plinth/modules/ssh/privileged.py old mode 100755 new mode 100644 similarity index 52% rename from actions/ssh rename to plinth/modules/ssh/privileged.py index acf0d5fea..ee95983b0 --- a/actions/ssh +++ b/plinth/modules/ssh/privileged.py @@ -1,52 +1,20 @@ -#!/usr/bin/python3 # SPDX-License-Identifier: AGPL-3.0-or-later -""" -Configuration helper for SSH server. -""" +"""Configuration helper for SSH server.""" -import argparse import grp import os import pwd import shutil import stat import subprocess -import sys import augeas from plinth import action_utils, utils +from plinth.actions import privileged -def parse_arguments(): - """Return parsed command line arguments as dictionary.""" - parser = argparse.ArgumentParser() - subparsers = parser.add_subparsers(dest='subcommand', help='Sub command') - - subparsers.add_parser('setup', help='Setup SSH server') - - get_keys = subparsers.add_parser('get-keys', - help='Get SSH authorized keys') - get_keys.add_argument('--username', required=True, type=_managed_user) - - set_keys = subparsers.add_parser('set-keys', - help='Set SSH authorized keys') - set_keys.add_argument('--username', required=True, type=_managed_user) - set_keys.add_argument('--keys', required=True) - set_keys.add_argument('--auth-user', required=True) - - subparsers.add_parser('get-password-config', - help='Get SSH password auth configuration') - - set_password_config = subparsers.add_parser( - 'set-password-config', help='Set SSH password auth configuration') - set_password_config.add_argument('--value') - - subparsers.required = True - return parser.parse_args() - - -def _validate_user(username, must_be_admin=True): +def _validate_user(username, password, must_be_admin=True): """Validate a user.""" if must_be_admin: try: @@ -55,29 +23,23 @@ def _validate_user(username, must_be_admin=True): admins = [] if username not in admins: - msg = '"{}" is not authorized to perform this action'.format( - username) - raise argparse.ArgumentTypeError(msg) + msg = f'"{username}" is not authorized to perform this action' + raise PermissionError(msg) - password = _read_password() if not utils.is_authenticated_user(username, password): - raise argparse.ArgumentTypeError("Invalid credentials") + raise PermissionError('Invalid credentials') def _managed_user(username): """Raise an error if the user is root.""" if pwd.getpwnam(username).pw_gid == 0: - msg = 'User {} is not managed by FreedomBox'.format(username) - raise argparse.ArgumentTypeError(msg) + raise ValueError(f'User {username} is not managed by FreedomBox') + return username -def _read_password(): - """Read the password from stdin.""" - return ''.join(sys.stdin) - - -def subcommand_setup(arguments): +@privileged +def setup(): """Setup Open SSH server. Regenerates deleted SSH keys. This is necessary when FreedomBox image is @@ -87,7 +49,6 @@ def subcommand_setup(arguments): If the keys already exist, do nothing. This is necessary when a user installs FreedomBox using an apt package. SSH keys exist and running reconfigure on the openssh-server package does not regenerate them. - """ action_utils.dpkg_reconfigure('openssh-server', {}) @@ -97,29 +58,25 @@ def get_user_homedir(username): try: return pwd.getpwnam(username).pw_dir except KeyError: - print('Username not found') - sys.exit(1) + raise ValueError('Username not found') -def subcommand_get_keys(arguments): +@privileged +def get_keys(user: str) -> str: """Get SSH authorized keys.""" - user = arguments.username - path = os.path.join(get_user_homedir(user), '.ssh', 'authorized_keys') try: with open(path, 'r', encoding='utf-8') as file_handle: - print(file_handle.read()) + return file_handle.read() except FileNotFoundError: - pass + return '' -def subcommand_set_keys(arguments): +@privileged +def set_keys(user: str, keys: str, auth_user: str, auth_password: str): """Set SSH authorized keys.""" - user = arguments.username - auth_user = arguments.auth_user - must_be_admin = user != auth_user - _validate_user(auth_user, must_be_admin=must_be_admin) + _validate_user(auth_user, auth_password, must_be_admin=must_be_admin) ssh_folder = os.path.join(get_user_homedir(user), '.ssh') key_file_path = os.path.join(ssh_folder, 'authorized_keys') @@ -131,7 +88,7 @@ def subcommand_set_keys(arguments): shutil.chown(ssh_folder, user, 'users') with open(key_file_path, 'w', encoding='utf-8') as file_handle: - file_handle.write(arguments.keys) + file_handle.write(keys) shutil.chown(key_file_path, user, 'users') os.chmod(key_file_path, stat.S_IRUSR | stat.S_IWUSR) @@ -148,30 +105,19 @@ def _load_augeas(): return aug -def subcommand_get_password_config(_): +@privileged +def is_password_authentication_enabled() -> bool: """Retrieve value of password authentication from sshd configuration.""" aug = _load_augeas() field_path = '/files/etc/ssh/sshd_config/PasswordAuthentication' get_value = aug.get(field_path) - print(get_value or 'yes') + return (get_value or 'yes') == 'yes' -def subcommand_set_password_config(arguments): +@privileged +def set_password_authentication(enable: bool): """Set value of password authentication in sshd configuration.""" + value = 'yes' if enable else 'no' aug = _load_augeas() - aug.set('/files/etc/ssh/sshd_config/PasswordAuthentication', - arguments.value) + aug.set('/files/etc/ssh/sshd_config/PasswordAuthentication', value) aug.save() - - -def main(): - """Parse arguments and perform all duties.""" - arguments = parse_arguments() - - subcommand = arguments.subcommand.replace('-', '_') - subcommand_method = globals()['subcommand_' + subcommand] - subcommand_method(arguments) - - -if __name__ == '__main__': - main() diff --git a/plinth/modules/ssh/views.py b/plinth/modules/ssh/views.py index ebddcf3d3..d35c0b0ec 100644 --- a/plinth/modules/ssh/views.py +++ b/plinth/modules/ssh/views.py @@ -1,7 +1,6 @@ # SPDX-License-Identifier: AGPL-3.0-or-later -""" -Views for the SSH module -""" +"""Views for the SSH app.""" + from django.contrib import messages from django.utils.translation import gettext_lazy as _ @@ -9,32 +8,36 @@ from plinth import actions from plinth.modules import ssh from plinth.views import AppView -from . import is_password_authentication_disabled +from . import privileged from .forms import SSHServerForm class SshAppView(AppView): + """Show ssh app main page.""" + app_id = 'ssh' template_name = 'ssh.html' form_class = SSHServerForm def get_context_data(self, *args, **kwargs): + """Return additional context for rendering the template.""" context = super().get_context_data(**kwargs) context['host_keys'] = ssh.get_host_keys() return context def get_initial(self): - """Initial form value""" + """Return initial values of the form.""" initial = super().get_initial() initial.update({ - 'password_auth_disabled': is_password_authentication_disabled(), + 'password_auth_disabled': + not privileged.is_password_authentication_enabled(), }) return initial def form_valid(self, form): - """Apply changes from the form""" + """Apply changes from the form.""" old_config = self.get_initial() new_config = form.cleaned_data @@ -43,16 +46,9 @@ class SshAppView(AppView): passwd_auth_changed = is_field_changed('password_auth_disabled') if passwd_auth_changed: - if new_config['password_auth_disabled']: - passwd_auth = 'no' - message = _('SSH authentication with password disabled.') - else: - passwd_auth = 'yes' - message = _('SSH authentication with password enabled.') - - actions.superuser_run( - 'ssh', ['set-password-config', '--value', passwd_auth]) + privileged.set_password_authentication( + not new_config['password_auth_disabled']) actions.superuser_run('service', ['reload', 'ssh']) - messages.success(self.request, message) + messages.success(self.request, _('Configuration updated')) return super().form_valid(form) diff --git a/plinth/modules/users/forms.py b/plinth/modules/users/forms.py index 5dd6b7a74..76747a9c2 100644 --- a/plinth/modules/users/forms.py +++ b/plinth/modules/users/forms.py @@ -15,6 +15,7 @@ from django.utils.translation import gettext as _ from django.utils.translation import gettext_lazy import plinth.forms +import plinth.modules.ssh.privileged as ssh_privileged from plinth import actions from plinth.errors import ActionError from plinth.modules import first_boot @@ -294,16 +295,10 @@ class UserUpdateForm(ValidNewUsernameCheckMixin, PasswordConfirmForm, _('Failed to add user to group.')) try: - actions.superuser_run('ssh', [ - 'set-keys', - '--username', - user.get_username(), - '--keys', - self.cleaned_data['ssh_keys'].strip(), - '--auth-user', - auth_username, - ], input=confirm_password.encode()) - except ActionError: + ssh_privileged.set_keys(user.get_username(), + self.cleaned_data['ssh_keys'].strip(), + auth_username, confirm_password) + except Exception: messages.error(self.request, _('Unable to set SSH keys.')) is_active = self.cleaned_data['is_active'] diff --git a/plinth/modules/users/views.py b/plinth/modules/users/views.py index 7d25fe518..005701299 100644 --- a/plinth/modules/users/views.py +++ b/plinth/modules/users/views.py @@ -1,4 +1,5 @@ # SPDX-License-Identifier: AGPL-3.0-or-later +"""Django views for user app.""" import django.views.generic from django.contrib import messages @@ -13,6 +14,7 @@ from django.utils.translation import gettext_lazy from django.views.generic.edit import (CreateView, DeleteView, FormView, UpdateView) +import plinth.modules.ssh.privileged as ssh_privileged from plinth import actions, translation from plinth.errors import ActionError from plinth.modules import first_boot @@ -36,6 +38,7 @@ class ContextMixin: class UserCreate(ContextMixin, SuccessMessageMixin, CreateView): """View to create a new user.""" + form_class = CreateUserForm template_name = 'users_create.html' model = User @@ -95,8 +98,7 @@ class UserUpdate(ContextMixin, SuccessMessageMixin, UpdateView): """Return the data for initial form load.""" initial = super().get_initial() try: - ssh_keys = actions.superuser_run( - 'ssh', ['get-keys', '--username', self.object.username]) + ssh_keys = ssh_privileged.get_keys(self.object.username) initial['ssh_keys'] = ssh_keys.strip() initial['language'] = self.object.userprofile.language except ActionError: