mirror of
https://github.com/freedombox/FreedomBox.git
synced 2026-01-21 07:55:00 +00:00
users: Delete or move home folder when user is deleted or renamed
On user deletion, user's home folder is also deleted. Admins have an
option to avoid deleting user's home by inactivating the user instead.
This commit also removes user deletion buttons from the user's list
page and adds this option to the user edit page. The user's edit form
asks for a confirmation if the user deletion is requested. This change
also means that the confirmation password is now required to delete a user.
Also:
- Add a simple username validation to the privileged actions.
- Functional tests: Create a fixture to login as an admin before every test.
- Functional tests: Add a test to check that SSH passwordless login works
after user is renamed to validate correct SSH related path permissions.
- Privileged tests: Add `test_` prefix to the generated random string which
makes easier to check and cleanup created home folders.
- Minor quote fixes.
Tests performed in stable and testing containers:
- Run all the users module tests twice, no failures in tests.
- When user is the last admin, both "Active" and "Delete user"
checkboxes are disabled.
Closes #2451.
[sunil]
- Refactor the JS code:
- Ensure that DOM elements are lookup after DOM content is loaded.
- Styling changes. Reduce the number of globals, name the global names
somewhat more unique.
- Click the button instead of submitting the form to disable the button.
- Template changes:
- Add a body for the confirmation dialog to talk about disabling the user and
deleting the home directory.
- Change the label of the confirm button to make it more
explicit (recommendation from many UX guides).
- Styling.
- Functional tests:
- Fix visibility checking of an element to use the correct splinter API.
- Simplify clicking the edit user link.
- Minor update to form checkbox help text.
Signed-off-by: Veiko Aasa <veiko17@disroot.org>
Signed-off-by: Sunil Mohan Adapa <sunil@medhas.org>
Reviewed-by: Sunil Mohan Adapa <sunil@medhas.org>
This commit is contained in:
Veiko Aasa
Sunil Mohan Adapa
parent
0eac9c3260
commit
f12e634bc9
@ -19,6 +19,7 @@ import plinth.forms
|
||||
import plinth.modules.ssh.privileged as ssh_privileged
|
||||
from plinth.modules import first_boot
|
||||
from plinth.utils import is_user_admin
|
||||
from plinth.views import messages_error
|
||||
|
||||
from . import get_last_admin_user, privileged
|
||||
from .components import UsersAndGroups
|
||||
@ -247,11 +248,18 @@ class UserUpdateForm(ValidNewUsernameCheckMixin, PasswordConfirmForm,
|
||||
|
||||
language = plinth.forms.LanguageSelectionFormMixin.language
|
||||
|
||||
delete = forms.BooleanField(
|
||||
label=gettext_lazy('Delete user'), required=False,
|
||||
help_text=gettext_lazy(
|
||||
'Deleting the user account will also remove all the files '
|
||||
'related to the user. Deleting files can be avoided by '
|
||||
'setting the user account as inactive.'))
|
||||
|
||||
class Meta:
|
||||
"""Metadata to control automatic form building."""
|
||||
|
||||
fields = ('username', 'email', 'groups', 'ssh_keys', 'language',
|
||||
'is_active', 'confirm_password')
|
||||
'is_active', 'delete', 'confirm_password')
|
||||
model = User
|
||||
widgets = {
|
||||
'groups': plinth.forms.CheckboxSelectMultipleWithReadOnly(),
|
||||
@ -274,6 +282,7 @@ class UserUpdateForm(ValidNewUsernameCheckMixin, PasswordConfirmForm,
|
||||
|
||||
if self.is_last_admin_user:
|
||||
self.fields['is_active'].disabled = True
|
||||
self.fields['delete'].disabled = True
|
||||
|
||||
def save(self, commit=True):
|
||||
"""Update LDAP user name and groups after saving user model."""
|
||||
@ -287,6 +296,19 @@ class UserUpdateForm(ValidNewUsernameCheckMixin, PasswordConfirmForm,
|
||||
user.save()
|
||||
self.save_m2m()
|
||||
|
||||
if self.cleaned_data.get('delete'):
|
||||
try:
|
||||
# Remove system user
|
||||
privileged.remove_user(user.get_username(), auth_username,
|
||||
confirm_password)
|
||||
except Exception as error:
|
||||
messages_error(self.request, _('Failed to delete user.'),
|
||||
error)
|
||||
else:
|
||||
# Remove Django user
|
||||
user.delete()
|
||||
return user
|
||||
|
||||
old_groups = privileged.get_user_groups(self.username)
|
||||
old_groups = [group for group in old_groups if group]
|
||||
|
||||
|
||||
@ -3,6 +3,7 @@
|
||||
|
||||
import logging
|
||||
import os
|
||||
import pathlib
|
||||
import re
|
||||
import shutil
|
||||
import subprocess
|
||||
@ -45,6 +46,12 @@ def _validate_password(username, password):
|
||||
raise PermissionError('Invalid credentials')
|
||||
|
||||
|
||||
def _validate_username(username):
|
||||
"""Validate username."""
|
||||
if pathlib.Path(username).parts[-1] != username:
|
||||
raise ValueError('Invalid username')
|
||||
|
||||
|
||||
@privileged
|
||||
def first_setup():
|
||||
"""Perform initial setup of LDAP."""
|
||||
@ -277,8 +284,7 @@ def _configure_ldapscripts():
|
||||
|
||||
def _lock_ldap_user(username: str):
|
||||
"""Lock user."""
|
||||
if not _get_user_ids(username):
|
||||
# User not found
|
||||
if not _user_exists(username):
|
||||
return None
|
||||
|
||||
# Replace command adds the attribute if it doesn't exist.
|
||||
@ -286,13 +292,12 @@ def _lock_ldap_user(username: str):
|
||||
replace: pwdAccountLockedTime
|
||||
pwdAccountLockedTime: 000001010000Z
|
||||
'''
|
||||
_run(["ldapmodifyuser", username], input=input.encode())
|
||||
_run(['ldapmodifyuser', username], input=input.encode())
|
||||
|
||||
|
||||
def _unlock_ldap_user(username: str):
|
||||
"""Unlock user."""
|
||||
if not _get_user_ids(username):
|
||||
# User not found
|
||||
if not _user_exists(username):
|
||||
return None
|
||||
|
||||
# Replace command without providing a value will remove the attribute
|
||||
@ -344,41 +349,83 @@ def _disconnect_samba_user(username):
|
||||
raise
|
||||
|
||||
|
||||
def _get_user_home(username):
|
||||
"""Return the user home directory."""
|
||||
output = subprocess.check_output(['getent', 'passwd', username], text=True)
|
||||
return pathlib.Path(output.split(':')[5])
|
||||
|
||||
|
||||
@privileged
|
||||
def create_user(username: str, password: secret_str,
|
||||
auth_user: str | None = None,
|
||||
auth_password: secret_str | None = None):
|
||||
"""Create an LDAP user, set password and flush cache."""
|
||||
_validate_username(username)
|
||||
_validate_user(auth_user, auth_password)
|
||||
|
||||
_run(['ldapadduser', username, 'users'])
|
||||
|
||||
_set_user_password(username, password)
|
||||
_flush_cache()
|
||||
_set_samba_user(username, password)
|
||||
|
||||
|
||||
@privileged
|
||||
def remove_user(username: str, password: secret_str | None = None):
|
||||
def remove_user(username: str, auth_user: str, auth_password: secret_str):
|
||||
"""Remove an LDAP user."""
|
||||
_validate_username(username)
|
||||
_validate_user(auth_user, auth_password)
|
||||
groups = _get_user_groups(username)
|
||||
|
||||
# require authentication if the user is last admin user
|
||||
if _get_group_users('admin') == [username]:
|
||||
_validate_password(username, password)
|
||||
|
||||
_delete_samba_user(username)
|
||||
|
||||
for group in groups:
|
||||
_remove_user_from_group(username, group)
|
||||
|
||||
_run(['ldapdeleteuser', username])
|
||||
if _user_exists(username):
|
||||
# remove the home folder if it's owned by the user
|
||||
home_folder = _get_user_home(username)
|
||||
if home_folder.is_dir():
|
||||
try:
|
||||
owner = home_folder.owner()
|
||||
except KeyError: # owner not found
|
||||
pass
|
||||
else:
|
||||
if owner == username:
|
||||
shutil.rmtree(home_folder, ignore_errors=True)
|
||||
|
||||
_run(['ldapdeleteuser', username])
|
||||
|
||||
_flush_cache()
|
||||
|
||||
|
||||
def _rename_ldap_user(old_username: str, new_username: str,
|
||||
new_home: pathlib.Path | None):
|
||||
"""Rename LDAP user and user parameters."""
|
||||
_run(['ldaprenameuser', old_username, new_username])
|
||||
|
||||
input = f'''changetype: modify
|
||||
replace: cn
|
||||
cn: {new_username}
|
||||
-
|
||||
replace: gecos
|
||||
gecos: {new_username}
|
||||
'''
|
||||
|
||||
if new_home:
|
||||
input += f'''-
|
||||
replace: homeDirectory
|
||||
homeDirectory: {str(new_home)}
|
||||
'''
|
||||
|
||||
_run(['ldapmodifyuser', new_username], input=input.encode())
|
||||
|
||||
|
||||
@privileged
|
||||
def rename_user(old_username: str, new_username: str):
|
||||
"""Rename an LDAP user."""
|
||||
_validate_username(old_username)
|
||||
_validate_username(new_username)
|
||||
groups = _get_user_groups(old_username)
|
||||
|
||||
_delete_samba_user(old_username)
|
||||
@ -386,7 +433,16 @@ def rename_user(old_username: str, new_username: str):
|
||||
for group in groups:
|
||||
_remove_user_from_group(old_username, group)
|
||||
|
||||
_run(['ldaprenameuser', old_username, new_username])
|
||||
old_home = _get_user_home(old_username)
|
||||
new_home = old_home.with_name(new_username)
|
||||
|
||||
if new_home.exists():
|
||||
new_home = None # Do not rename home
|
||||
else:
|
||||
if old_home.is_dir():
|
||||
old_home.rename(new_home)
|
||||
|
||||
_rename_ldap_user(old_username, new_username, new_home)
|
||||
|
||||
for group in groups:
|
||||
_add_user_to_group(new_username, group)
|
||||
@ -465,6 +521,11 @@ def _get_user_ids(username: str) -> str | None:
|
||||
return process.stdout.decode().strip()
|
||||
|
||||
|
||||
def _user_exists(username):
|
||||
"""Return whether the user exists."""
|
||||
return _get_user_ids(username) is not None
|
||||
|
||||
|
||||
def _get_group_users(groupname):
|
||||
"""Return list of members in the group."""
|
||||
try:
|
||||
|
||||
60
plinth/modules/users/static/users.js
Normal file
60
plinth/modules/users/static/users.js
Normal file
@ -0,0 +1,60 @@
|
||||
// SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
/**
|
||||
* @licstart The following is the entire license notice for the JavaScript
|
||||
* code in this page.
|
||||
*
|
||||
* This file is part of FreedomBox.
|
||||
*
|
||||
* This program is free software: you can redistribute it and/or modify
|
||||
* it under the terms of the GNU Affero General Public License as
|
||||
* published by the Free Software Foundation, either version 3 of the
|
||||
* License, or (at your option) any later version.
|
||||
*
|
||||
* This program is distributed in the hope that it will be useful,
|
||||
* but WITHOUT ANY WARRANTY; without even the implied warranty of
|
||||
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
|
||||
* GNU Affero General Public License for more details.
|
||||
*
|
||||
* You should have received a copy of the GNU Affero General Public License
|
||||
* along with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
*
|
||||
* @licend The above is the entire license notice for the JavaScript code
|
||||
* in this page.
|
||||
*/
|
||||
|
||||
var deleteConfirmed = false;
|
||||
|
||||
document.addEventListener('DOMContentLoaded', (event) => {
|
||||
const form = document.querySelector('form.form-update');
|
||||
form.addEventListener('submit', onUserUpdateSubmit);
|
||||
|
||||
const confirmDeleteButton = document.querySelector(
|
||||
'#user-delete-confirm-dialog button.confirm');
|
||||
confirmDeleteButton.addEventListener('click', () => {
|
||||
onUserDeleteConfirmed(form);
|
||||
});
|
||||
});
|
||||
|
||||
// Show the confirmation dialog if the delete checkbox is selected
|
||||
function onUserUpdateSubmit(event) {
|
||||
const deleteUserCheckbox = document.getElementById('id_delete');
|
||||
if (!deleteUserCheckbox.checked) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (deleteConfirmed) { // Deletion is already confirmed
|
||||
deleteConfirmed = false;
|
||||
return;
|
||||
}
|
||||
|
||||
event.preventDefault();
|
||||
$("#user-delete-confirm-dialog").modal('show');
|
||||
};
|
||||
|
||||
// Submit the user edit form
|
||||
function onUserDeleteConfirmed(form) {
|
||||
deleteConfirmed = true;
|
||||
$('#user-delete-confirm-dialog').modal('hide');
|
||||
// Click instead of submit to disable the submission button
|
||||
form.querySelector('input[type=submit]').click();
|
||||
};
|
||||
@ -1,26 +0,0 @@
|
||||
{% extends "base.html" %}
|
||||
{% comment %}
|
||||
# SPDX-License-Identifier: AGPL-3.0-or-later
|
||||
{% endcomment %}
|
||||
|
||||
{% load bootstrap %}
|
||||
{% load i18n %}
|
||||
|
||||
{% block content %}
|
||||
|
||||
<h3>{% trans "Delete User" %}</h3>
|
||||
|
||||
<p>
|
||||
{% blocktrans trimmed with username=object.username %}
|
||||
Delete user <strong>{{ username }}</strong> permanently?
|
||||
{% endblocktrans %}
|
||||
</p>
|
||||
|
||||
<form class="form form-delete" method="post">
|
||||
{% csrf_token %}
|
||||
|
||||
<input type="submit" class="btn btn-md btn-danger"
|
||||
value="{% blocktrans with username=object.username %}Delete {{ username }}{% endblocktrans %}"/>
|
||||
</form>
|
||||
|
||||
{% endblock %}
|
||||
@ -19,8 +19,8 @@
|
||||
</div>
|
||||
|
||||
<div class="row">
|
||||
<div class="col-md-6">
|
||||
<div class="list-group list-group-two-column">
|
||||
<div class="col-md-4">
|
||||
<div class="list-group">
|
||||
{% for user in object_list %}
|
||||
<div class="list-group-item">
|
||||
<a class='user-edit-label primary'
|
||||
@ -34,15 +34,6 @@
|
||||
aria-hidden="true"></span>
|
||||
{% endif %}
|
||||
|
||||
{% if user.username != last_admin_user %}
|
||||
<a href="{% url 'users:delete' user.username %}"
|
||||
class="btn btn-default btn-sm secondary"
|
||||
role="button"
|
||||
title="{% blocktrans with username=user.username %}Delete user {{ username }}{% endblocktrans %}">
|
||||
<span class="fa fa-trash-o"
|
||||
aria-hidden="true"></span>
|
||||
</a>
|
||||
{% endif %}
|
||||
</div>
|
||||
{% endfor %}
|
||||
</div>
|
||||
|
||||
@ -5,6 +5,7 @@
|
||||
|
||||
{% load bootstrap %}
|
||||
{% load i18n %}
|
||||
{% load static %}
|
||||
|
||||
{% block content %}
|
||||
<h3>
|
||||
@ -30,5 +31,42 @@
|
||||
<input type="submit" class="btn btn-primary"
|
||||
value="{% trans "Save Changes" %}"/>
|
||||
</form>
|
||||
|
||||
<div id="user-delete-confirm-dialog" class="modal" tabindex="-1"
|
||||
role="dialog">
|
||||
<div class="modal-dialog modal-dialog-centered" role="document">
|
||||
<div class="modal-content">
|
||||
<div class="modal-header">
|
||||
<h5 class="modal-title">
|
||||
{% blocktrans trimmed with username=object.username %}
|
||||
Delete user <em>{{ username }}</em> and all the user's files?
|
||||
{% endblocktrans %}
|
||||
</h5>
|
||||
<button type="button" class="close" data-dismiss="modal"
|
||||
aria-label="{% trans 'Close' %}">
|
||||
<span aria-hidden="true">×</span>
|
||||
</button>
|
||||
</div>
|
||||
<div class="modal-body">
|
||||
{% blocktrans trimmed %}
|
||||
Deleting a user account also removes all the files user's home
|
||||
directory. If you wish to keep these files, disable the user account
|
||||
instead.
|
||||
{% endblocktrans %}
|
||||
</div>
|
||||
<div class="modal-footer">
|
||||
<button type="button" class="btn btn-danger confirm">
|
||||
{% trans "Delete user and files" %}
|
||||
</button>
|
||||
<button type="button" class="btn btn-secondary" data-dismiss="modal">
|
||||
{% trans "Cancel" %}
|
||||
</button>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
</div>
|
||||
{% endblock %}
|
||||
|
||||
{% block page_js %}
|
||||
<script type="text/javascript" src="{% static 'users/users.js' %}"></script>
|
||||
{% endblock %}
|
||||
|
||||
@ -54,16 +54,21 @@ _config_page_title_language_map = {
|
||||
|
||||
@pytest.fixture(scope='module', autouse=True)
|
||||
def fixture_background(session_browser):
|
||||
"""Unset language."""
|
||||
yield
|
||||
functional.login(session_browser)
|
||||
functional.user_set_language(session_browser, _language_codes['None'])
|
||||
|
||||
|
||||
@pytest.fixture(scope='function', autouse=True)
|
||||
def fixture_login(session_browser):
|
||||
"""Login."""
|
||||
functional.login(session_browser)
|
||||
yield
|
||||
functional.user_set_language(session_browser, _language_codes['None'])
|
||||
|
||||
|
||||
def test_create_user(session_browser):
|
||||
"""Test creating a user."""
|
||||
if functional.user_exists(session_browser, 'alice'):
|
||||
functional.delete_user(session_browser, 'alice')
|
||||
_delete_user(session_browser, 'alice')
|
||||
|
||||
functional.create_user(session_browser, 'alice', email='alice@example.com')
|
||||
assert functional.user_exists(session_browser, 'alice')
|
||||
@ -73,8 +78,7 @@ def test_create_user(session_browser):
|
||||
def test_rename_user(session_browser):
|
||||
"""Test renaming a user."""
|
||||
_non_admin_user_exists(session_browser, 'alice')
|
||||
if functional.user_exists(session_browser, 'bob'):
|
||||
functional.delete_user(session_browser, 'bob')
|
||||
_delete_user(session_browser, 'bob')
|
||||
|
||||
_rename_user(session_browser, 'alice', 'bob')
|
||||
assert not functional.user_exists(session_browser, 'alice')
|
||||
@ -94,7 +98,6 @@ def test_non_admin_users_can_change_own_ssh_keys(session_browser):
|
||||
'alice')
|
||||
_set_ssh_keys(session_browser, 'somekey456')
|
||||
assert _get_ssh_keys(session_browser) == 'somekey456'
|
||||
functional.login(session_browser)
|
||||
|
||||
|
||||
def test_admin_users_can_change_other_users_ssh_keys(session_browser):
|
||||
@ -121,6 +124,26 @@ def test_users_can_connect_passwordless_over_ssh(session_browser,
|
||||
_should_connect_passwordless_over_ssh(session_browser, tmp_path_factory)
|
||||
|
||||
|
||||
def test_ssh_passwordless_after_user_rename(session_browser, tmp_path_factory):
|
||||
"""Test that users can connect passwordless after user is renamed."""
|
||||
username_old = 'bob'
|
||||
username_new = 'bob2'
|
||||
functional.app_enable(session_browser, 'ssh')
|
||||
_non_admin_user_exists(session_browser, username_old,
|
||||
groups=['freedombox-ssh'])
|
||||
_delete_user(session_browser, username_new)
|
||||
_configure_ssh_keys(session_browser, tmp_path_factory,
|
||||
username=username_old)
|
||||
_should_connect_passwordless_over_ssh(session_browser, tmp_path_factory,
|
||||
username=username_old)
|
||||
|
||||
_rename_user(session_browser, username_old, username_new)
|
||||
|
||||
assert functional.user_exists(session_browser, username_new)
|
||||
_should_connect_passwordless_over_ssh(session_browser, tmp_path_factory,
|
||||
username=username_new)
|
||||
|
||||
|
||||
def test_users_cannot_connect_passwordless_over_ssh(session_browser,
|
||||
tmp_path_factory):
|
||||
"""Test that users cannot connect passwordless over ssh if the keys aren't
|
||||
@ -173,8 +196,6 @@ def test_user_states(session_browser, tmp_path_factory):
|
||||
_should_connect_passwordless_over_ssh(session_browser, tmp_path_factory,
|
||||
username=username)
|
||||
|
||||
functional.login(session_browser)
|
||||
|
||||
|
||||
def test_admin_users_can_change_own_password(session_browser):
|
||||
"""Test that admin users can change their own password."""
|
||||
@ -183,7 +204,6 @@ def test_admin_users_can_change_own_password(session_browser):
|
||||
'testadmin')
|
||||
_change_password(session_browser, 'newpassword456')
|
||||
_can_log_in_with_password(session_browser, 'testadmin', 'newpassword456')
|
||||
functional.login(session_browser)
|
||||
|
||||
|
||||
def test_admin_users_can_change_others_password(session_browser):
|
||||
@ -191,7 +211,6 @@ def test_admin_users_can_change_others_password(session_browser):
|
||||
_non_admin_user_exists(session_browser, 'alice')
|
||||
_change_password(session_browser, 'secretsecret567', username='alice')
|
||||
_can_log_in_with_password(session_browser, 'alice', 'secretsecret567')
|
||||
functional.login(session_browser)
|
||||
|
||||
|
||||
def test_non_admin_users_can_change_own_password(session_browser):
|
||||
@ -201,7 +220,6 @@ def test_non_admin_users_can_change_own_password(session_browser):
|
||||
'alice')
|
||||
_change_password(session_browser, 'newpassword123')
|
||||
_can_log_in_with_password(session_browser, 'alice', 'newpassword123')
|
||||
functional.login(session_browser)
|
||||
|
||||
|
||||
def test_delete_user(session_browser):
|
||||
@ -211,15 +229,19 @@ def test_delete_user(session_browser):
|
||||
assert not functional.user_exists(session_browser, 'alice')
|
||||
|
||||
|
||||
def _admin_user_exists(session_browser, name):
|
||||
def _delete_user(session_browser, name):
|
||||
"""Delete a user."""
|
||||
if functional.user_exists(session_browser, name):
|
||||
functional.delete_user(session_browser, name)
|
||||
|
||||
|
||||
def _admin_user_exists(session_browser, name):
|
||||
_delete_user(session_browser, name)
|
||||
functional.create_user(session_browser, name, groups=['admin'])
|
||||
|
||||
|
||||
def _non_admin_user_exists(session_browser, name, groups=[]):
|
||||
if functional.user_exists(session_browser, name):
|
||||
functional.delete_user(session_browser, name)
|
||||
_delete_user(session_browser, name)
|
||||
functional.create_user(session_browser, name, groups=groups)
|
||||
|
||||
|
||||
|
||||
@ -36,17 +36,17 @@ def _is_ldap_set_up():
|
||||
|
||||
pytestmark: list[pytest.MarkDecorator] = [
|
||||
pytest.mark.usefixtures('needs_root', 'load_cfg', 'mock_privileged'),
|
||||
pytest.mark.skipif(not _is_ldap_set_up(), reason="LDAP is not configured")
|
||||
pytest.mark.skipif(not _is_ldap_set_up(), reason='LDAP is not configured')
|
||||
]
|
||||
privileged_modules_to_mock = [
|
||||
'plinth.modules.users.privileged', 'plinth.modules.security.privileged'
|
||||
]
|
||||
|
||||
|
||||
def _random_string(length=8):
|
||||
def _random_string():
|
||||
"""Return a random string created from lower case ascii."""
|
||||
return ''.join(
|
||||
[random.choice(string.ascii_lowercase) for _ in range(length)])
|
||||
random_chars = [random.choice(string.ascii_lowercase) for _ in range(8)]
|
||||
return 'test_' + ''.join(random_chars)
|
||||
|
||||
|
||||
def _get_password_hash(username):
|
||||
@ -135,11 +135,8 @@ def _create_user(username=None, groups=None):
|
||||
|
||||
def _delete_user(username):
|
||||
"""Utility to delete an LDAP and Samba user"""
|
||||
admin_password = None
|
||||
if privileged.get_group_users('admin') == [username]:
|
||||
_, admin_password = _get_admin_user_password()
|
||||
|
||||
privileged.remove_user(username, admin_password)
|
||||
admin_user, admin_password = _get_admin_user_password()
|
||||
privileged.remove_user(username, admin_user, admin_password)
|
||||
|
||||
|
||||
def _create_admin_if_does_not_exist():
|
||||
@ -195,6 +192,13 @@ def test_create_user():
|
||||
_create_user(username)
|
||||
|
||||
|
||||
def test_create_invalid_user():
|
||||
"""Test invalid username validation."""
|
||||
username = 'invalid/user'
|
||||
with pytest.raises(ValueError):
|
||||
_create_user(username)
|
||||
|
||||
|
||||
def test_change_user_password():
|
||||
"""Test changing user password."""
|
||||
_create_admin_if_does_not_exist()
|
||||
@ -293,6 +297,18 @@ def test_rename_user():
|
||||
_rename_user(existing_user, new_username=new_username)
|
||||
|
||||
|
||||
def test_rename_invalid_user():
|
||||
"""Test rename invalid username"""
|
||||
invalid_username = 'invalid/user'
|
||||
valid_username = _random_string()
|
||||
|
||||
with pytest.raises(ValueError):
|
||||
_rename_user(invalid_username, new_username=valid_username)
|
||||
|
||||
with pytest.raises(ValueError):
|
||||
_rename_user(valid_username, new_username=invalid_username)
|
||||
|
||||
|
||||
def test_delete_user():
|
||||
"""Test to check whether LDAP users can be deleted"""
|
||||
_create_admin_if_does_not_exist()
|
||||
@ -313,10 +329,17 @@ def test_delete_user():
|
||||
|
||||
|
||||
def test_delete_non_existent_user():
|
||||
"""Deleting a non-existent user should fail."""
|
||||
"""Deleting a non-existent user doesn't fail."""
|
||||
non_existent_user = _random_string()
|
||||
with pytest.raises(subprocess.CalledProcessError):
|
||||
privileged.remove_user(non_existent_user)
|
||||
_delete_user(non_existent_user)
|
||||
|
||||
|
||||
def test_delete_invalid_user():
|
||||
"""Deleting invalid username should fail."""
|
||||
invalid_username = 'invalid/user'
|
||||
|
||||
with pytest.raises(ValueError):
|
||||
_delete_user(invalid_username)
|
||||
|
||||
|
||||
def test_groups():
|
||||
|
||||
@ -84,15 +84,14 @@ def make_request(request, view, as_admin=True, **kwargs):
|
||||
|
||||
|
||||
def test_users_list_view(rf):
|
||||
"""Test that users list view has correct view data."""
|
||||
"""Test users list view has correct view data."""
|
||||
with (patch('plinth.views.AppView.get_context_data',
|
||||
return_value={'is_enabled': True}),
|
||||
patch('plinth.views.AppView.app', return_value=None)):
|
||||
view = views.UserList.as_view()
|
||||
response, messages = make_request(rf.get('/'), view)
|
||||
|
||||
assert response.context_data['last_admin_user'] == 'admin'
|
||||
assert response.status_code == 200
|
||||
assert response.status_code == 200
|
||||
|
||||
|
||||
@pytest.mark.parametrize('username', ['test-new', 'test-neW@2_'])
|
||||
@ -268,10 +267,15 @@ def test_update_user_without_permissions_view(rf):
|
||||
def test_delete_user_view(rf):
|
||||
"""Test that user deletion succeeds."""
|
||||
user = 'tester'
|
||||
form_data = {
|
||||
'username': user,
|
||||
'delete': True,
|
||||
'confirm_password': 'adminpassword',
|
||||
}
|
||||
|
||||
url = urls.reverse('users:delete', kwargs={'slug': user})
|
||||
request = rf.post(url)
|
||||
view = views.UserDelete.as_view()
|
||||
url = urls.reverse('users:edit', kwargs={'slug': user})
|
||||
request = rf.post(url, data=form_data)
|
||||
view = views.UserUpdate.as_view()
|
||||
response, messages = make_request(request, view, as_admin=True, slug=user)
|
||||
|
||||
assert list(messages)[0].message == 'User {} deleted.'.format(user)
|
||||
|
||||
@ -16,8 +16,6 @@ urlpatterns = [
|
||||
re_path(r'^sys/users/create/$', views.UserCreate.as_view(), name='create'),
|
||||
re_path(r'^sys/users/(?P<slug>[\w.@+-]+)/edit/$',
|
||||
non_admin_view(views.UserUpdate.as_view()), name='edit'),
|
||||
re_path(r'^sys/users/(?P<slug>[\w.@+-]+)/delete/$',
|
||||
views.UserDelete.as_view(), name='delete'),
|
||||
re_path(r'^sys/users/(?P<slug>[\w.@+-]+)/change_password/$',
|
||||
non_admin_view(views.UserChangePassword.as_view()),
|
||||
name='change_password'),
|
||||
|
||||
@ -2,17 +2,14 @@
|
||||
"""Django views for user app."""
|
||||
|
||||
import django.views.generic
|
||||
from django.contrib import messages
|
||||
from django.contrib.auth import update_session_auth_hash
|
||||
from django.contrib.auth.models import User
|
||||
from django.contrib.messages.views import SuccessMessageMixin
|
||||
from django.core.exceptions import PermissionDenied
|
||||
from django.http import HttpResponseRedirect
|
||||
from django.urls import reverse, reverse_lazy
|
||||
from django.utils.translation import gettext as _
|
||||
from django.utils.translation import gettext_lazy
|
||||
from django.views.generic.edit import (CreateView, DeleteView, FormView,
|
||||
UpdateView)
|
||||
from django.views.generic.edit import (CreateView, FormView, UpdateView)
|
||||
|
||||
import plinth.modules.ssh.privileged as ssh_privileged
|
||||
from plinth import translation
|
||||
@ -20,7 +17,7 @@ from plinth.modules import first_boot
|
||||
from plinth.utils import is_user_admin
|
||||
from plinth.views import AppView
|
||||
|
||||
from . import get_last_admin_user, privileged
|
||||
from . import privileged
|
||||
from .forms import (CreateUserForm, FirstBootForm, UserChangePasswordForm,
|
||||
UserUpdateForm)
|
||||
|
||||
@ -64,11 +61,6 @@ class UserList(AppView, ContextMixin, django.views.generic.ListView):
|
||||
title = gettext_lazy('Users')
|
||||
app_id = 'users'
|
||||
|
||||
def get_context_data(self, *args, **kwargs):
|
||||
context = super().get_context_data(*args, **kwargs)
|
||||
context['last_admin_user'] = get_last_admin_user()
|
||||
return context
|
||||
|
||||
|
||||
class UserUpdate(ContextMixin, SuccessMessageMixin, UpdateView):
|
||||
"""View to update a user's details."""
|
||||
@ -113,7 +105,13 @@ class UserUpdate(ContextMixin, SuccessMessageMixin, UpdateView):
|
||||
|
||||
def form_valid(self, form):
|
||||
"""Set the user language if necessary."""
|
||||
|
||||
is_user_deleted = form.cleaned_data.get('delete')
|
||||
if is_user_deleted:
|
||||
self.success_message = gettext_lazy('User %(username)s deleted.')
|
||||
response = super().form_valid(form)
|
||||
if is_user_deleted:
|
||||
return HttpResponseRedirect(reverse_lazy('users:index'))
|
||||
|
||||
# If user is updating their own profile then set the language for
|
||||
# current session too.
|
||||
@ -124,59 +122,6 @@ class UserUpdate(ContextMixin, SuccessMessageMixin, UpdateView):
|
||||
return response
|
||||
|
||||
|
||||
class UserDelete(ContextMixin, DeleteView):
|
||||
"""Handle deleting users, showing a confirmation dialog first.
|
||||
|
||||
On GET, display a confirmation page.
|
||||
On POST, delete the user.
|
||||
"""
|
||||
|
||||
template_name = 'users_delete.html'
|
||||
model = User
|
||||
slug_field = 'username'
|
||||
success_url = reverse_lazy('users:index')
|
||||
title = gettext_lazy('Delete User')
|
||||
|
||||
def __init__(self, *args, **kwargs):
|
||||
"""Initialize view, override delete method."""
|
||||
super().__init__(*args, **kwargs)
|
||||
|
||||
# Avoid a warning with Django 4.0 that delete member should not be
|
||||
# overridden. Remove this line and _delete() after Django 4.0 reaches
|
||||
# Debian Stable.
|
||||
self.delete = self._delete
|
||||
|
||||
def _delete_from_ldap(self):
|
||||
"""Remove user from LDAP and show a success/error message."""
|
||||
message = _('User {user} deleted.').format(user=self.kwargs['slug'])
|
||||
messages.success(self.request, message)
|
||||
|
||||
try:
|
||||
privileged.remove_user(self.kwargs['slug'])
|
||||
except Exception:
|
||||
messages.error(self.request, _('Deleting LDAP user failed.'))
|
||||
|
||||
def _delete(self, *args, **kwargs):
|
||||
"""Set the success message of deleting the user.
|
||||
|
||||
The SuccessMessageMixin doesn't work with the DeleteView on Django1.7,
|
||||
so set the success message manually here.
|
||||
"""
|
||||
output = super().delete(*args, **kwargs)
|
||||
self._delete_from_ldap()
|
||||
return output
|
||||
|
||||
def form_valid(self, form):
|
||||
"""Perform additional operations after delete.
|
||||
|
||||
Since Django 4.0, DeleteView inherits form_view and a call to delete()
|
||||
is not made.
|
||||
"""
|
||||
response = super().form_valid(form) # NOQA, pylint: disable=no-member
|
||||
self._delete_from_ldap()
|
||||
return response
|
||||
|
||||
|
||||
class UserChangePassword(ContextMixin, SuccessMessageMixin, FormView):
|
||||
"""View to change user password."""
|
||||
|
||||
|
||||
@ -664,12 +664,20 @@ def create_user(browser, name, password=None, groups=[], email=None):
|
||||
def delete_user(browser, name):
|
||||
"""Delete a user."""
|
||||
nav_to_module(browser, 'users')
|
||||
delete_link = browser.links.find_by_href(
|
||||
f'/plinth/sys/users/{name}/delete/')
|
||||
browser.links.find_by_href(f'/plinth/sys/users/{name}/edit/').first.click()
|
||||
|
||||
with wait_for_page_update(browser):
|
||||
delete_link.first.click()
|
||||
submit(browser, form_class='form-delete')
|
||||
browser.find_by_id('id_delete').check()
|
||||
browser.find_by_id('id_confirm_password').fill(
|
||||
config['DEFAULT']['password'])
|
||||
|
||||
browser.find_by_css('.form-update input[type=submit]').first.click()
|
||||
|
||||
confirm_button = browser.find_by_css(
|
||||
'#user-delete-confirm-dialog button.confirm').first
|
||||
eventually(lambda: confirm_button.visible)
|
||||
assert confirm_button.visible
|
||||
with wait_for_page_update(browser, expected_url='/plinth/sys/users/'):
|
||||
confirm_button.click()
|
||||
|
||||
|
||||
def user_exists(browser, name):
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user