add user and ssh management #12
No reviewers
Labels
No Label
Bug
Contributions welcome
Did not do
Duplicate
Feature
Module
Backups
Module
GraphQL
Priority
High
Priority
Low
Priority
Medium
Refactor
Severity
High
Severity
Low
Severity
Medium
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: SelfPrivacy/selfprivacy-rest-api#12
Loading…
Reference in New Issue
There is no content yet.
Delete Branch "def_graphql"
Deleting a branch is permanent. Although the deleted branch may exist for a short time before cleaning up, in most cases it CANNOT be undone. Continue?
add user and ssh management functionsto WIP: add user and ssh management functions@ -0,0 +11,4 @@
username: str
# userHomeFolderspace: UserHomeFolderUsage
sshKeys: typing.Optional[typing.List[str]] = None
Возможно будет лучше сделать тип списком, который по умолчанию пустой.
@ -0,0 +7,4 @@
from selfprivacy_api.graphql import IsAuthenticated
from selfprivacy_api.graphql.common_types.user import User, UserMutationReturn
from selfprivacy_api.graphql.mutations.mutation_interface import (
MutationReturnInterface,
Неиспользованный импорт. Сверяйся с линтером перед коммитом.
Среда должна быть настроена на использование питона из nix-shell, чтобы линтер работал корректно.
@ -0,0 +30,4 @@
"""Mutations ssh"""
@strawberry.mutation(permission_classes=[IsAuthenticated])
def create_ssh(self, settings: SshMutationsInput) -> UserMutationReturn:
settings
→input
@ -0,0 +33,4 @@
def create_ssh(self, settings: SshMutationsInput) -> UserMutationReturn:
"""Create a new ssh"""
with ReadUserData() as data:
if settings.username == "root":
Эта проверка была из-за legacy кода со стороны клиента.
С новой API это больше не нужно, эта функция может быть использована для добавления ключей рут пользователю.
@ -0,0 +46,4 @@
success=False,
message="Error: Invalid key type. Only ssh-ed25519 and ssh-rsa are supported",
code=400,
user=User("root", data["ssh"]["rootKeys"]),
Возврат этого ненастоящего пользователя не имеет смысла. Мог быть запрошен другой пользователь.
@ -0,0 +106,4 @@
)
@strawberry.mutation(permission_classes=[IsAuthenticated])
def delete_ssh(self, settings: SshMutationsInput) -> UserMutationReturn:
settings
→input
@ -0,0 +107,4 @@
@strawberry.mutation(permission_classes=[IsAuthenticated])
def delete_ssh(self, settings: SshMutationsInput) -> UserMutationReturn:
"""Delete ssh"""
"Delete ssh key from user" было бы яснее.
@ -0,0 +126,4 @@
success=True,
message="SSH key deleted",
code=200,
user=User("root", data["ssh"]["rootKeys"]),
data["ssh"]["rootKeys"]
мог быть удалён несколькими строками ранее, в этом случае произошла бы ошибка.Лучше убрать удаление массива с 124 строки, оно не имеет практического смысла.
@ -0,0 +145,4 @@
success=True,
message="SSH key deleted",
code=200,
user=User("root", data["ssh"]["rootKeys"]),
Здесь мы работаем с основным пользователем, а не рутом. Надо вернуть его, а не рута.
@ -0,0 +26,4 @@
"""Mutations change user settings"""
@strawberry.mutation(permission_classes=[IsAuthenticated])
def create_user(self, settings: UserMutationsInput) -> UserMutationReturn:
settings
→user
@ -0,0 +28,4 @@
@strawberry.mutation(permission_classes=[IsAuthenticated])
def create_user(self, settings: UserMutationsInput) -> UserMutationReturn:
"""Create a new user"""
hashing_command = ["mkpasswd", "-m", "sha-512", settings.password]
Стоит вынести код для получения хэша в другое место, и вызывать эту функцию и отсюда, и из REST кода.
Папка utils может для этого подойти.
@ -0,0 +43,4 @@
if is_username_forbidden(settings.username):
return UserMutationReturn(
success=False,
message="Error: Username is forbidden",
Error:
излишне, мы и так знаем что случилась ошибка.@ -0,0 +127,4 @@
return MutationReturnInterface(
success=True,
message="Nice frag - user deleted",
Эти сообщения будут отображаться в приложении, оставайся нейтральным.
@ -0,0 +132,4 @@
)
@strawberry.mutation(permission_classes=[IsAuthenticated])
def update_user(self, settings: UserMutationsInput) -> UserMutationReturn:
settings
→user
@ -0,0 +134,4 @@
@strawberry.mutation(permission_classes=[IsAuthenticated])
def update_user(self, settings: UserMutationsInput) -> UserMutationReturn:
"""Update user mutation"""
hashing_command = ["mkpasswd", "-m", "sha-512", settings.password]
Определённо надо вынести в отдельную функцию, см комментарий выше.
Код вызывающий subproccess потенциально опасен, и его должно быть как можно меньше.
@ -0,0 +8,4 @@
def get_users() -> typing.List[User]:
"""Get users"""
В список не попадает основной пользователь. Но ты мне напомнил что приложению нужно знать тип пользователя.
Давай определим enum
UserType
у которого могут быть следующие значения:NORMAL
для обычных пользователейPRIMARY
для основного пользователя, который указывается при создании сервера.ROOT
для рута.Добавляем в тип
User
полеtype
с типомUserType
.Внёс эту правку также в документ в Figma.
@ -0,0 +14,4 @@
for user in data["users"]:
user_list.append(User(**user))
user_list.append(User(data["username"], data["sshKeys"]))
Может случиться ошибка если поля
data["sshKeys"]
нет. Обрати внимание на проверки, что были в REST версии.501c4ce43c/selfprivacy_api/resources/services/ssh.py (L204)
@ -0,0 +5,4 @@
)
from enum import Enum
from selfprivacy_api.utils import ReadUserData, ensure_ssh_and_users_fields_exist
Беспорядочные импорты. Надо стремиться оформлять их так, что сначала идут системные, потом библиотечные, и потом уже локальные.
@ -0,0 +17,4 @@
@strawberry.type
class User:
"""Users management"""
Вводящий в заблуждение комментарий, подразумевающий что в этом классе есть функции для управления юзером.
@ -0,0 +1,54 @@
#!/usr/bin/env python3
"""Users management module"""
# pylint: disable=too-few-public-methods
Ненужные пустые строки.
@ -0,0 +29,4 @@
@strawberry.mutation(permission_classes=[IsAuthenticated])
def create_ssh(self, ssh_input: SshMutationInput) -> UserMutationReturn:
"""Create a new ssh"""
Некорректный комментарий и название функции.
В референсе было указано
add_ssh_key
.@ -0,0 +41,4 @@
)
@strawberry.mutation(permission_classes=[IsAuthenticated])
def delete_ssh(self, ssh_input: SshMutationInput) -> UserMutationReturn:
remove_ssh_key
@ -0,0 +5,4 @@
)
def create_ssh_key(username, ssh_key):
Как насчёт указывать тип аргументов и возвращаемые значения? Относится ко всем функциям в файле.
@ -0,0 +8,4 @@
from selfprivacy_api.utils import hash_password
def create_user_util(username, password):
Суффикс
util
лишён смысла. Касается всех функций в файле.Не хватает типов аргументов и возвращаемых значений
@ -162,0 +175,4 @@
return hashed_password
def ensure_ssh_and_users_fields_exist(data):
Этой функции тут не место, мне кажется. У тебя уже есть файл user_utils
@ -0,0 +46,4 @@
API_CREATE_SSH_MUTATION = """
mutation createSsh($sshInput: SshMutationInput!) {
createSsh(sshInput: $sshInput) {
Нужно будет поправить
@ -0,0 +59,4 @@
"""
def test_graphql_add_ssh_unauthorized(client, some_users, mock_subprocess_popen):
test_graphql_add_ssh_key_unauthorized
Аналогично ко всему остальному.
@ -0,0 +178,4 @@
assert response.json["data"]["createSsh"]["success"] is False
def test_graphql_add_ssh_404user(authorized_client, some_users, mock_subprocess_popen):
Что такое
404user
?test_graphql_add_ssh_key_nonexistent_user
@ -0,0 +200,4 @@
API_DELETE_SSH_MUTATION = """
mutation deleteSsh($sshInput: SshMutationInput!) {
Удаляем ключ, а не SSH
@ -0,0 +214,4 @@
"""
def test_graphql_dell_ssh_unauthorized(client, some_users, mock_subprocess_popen):
Причём тут Dell?
@ -0,0 +291,4 @@
assert response.json["data"]["users"]["getUser"] is None
API_CHANGE_USERS_MUTATION = """
Так меняем или создаём?
@ -0,0 +550,4 @@
assert response.json["data"]["createUser"]["user"] is None
API_DELETE_USERS_MUTATION = """
Удаляем одного, а не нескольких.
@ -0,0 +646,4 @@
assert response.json["data"]["deleteUser"]["success"] is False
API_UPDATE_USERS_MUTATION = """
Меняем одного, а не нескольких
WIP: add user and ssh management functionsto add user and ssh management functionsadd user and ssh management functionsto add user and ssh management