From 72c2731e4a976b66aabc30143696655ec5ced2cb Mon Sep 17 00:00:00 2001 From: Alex Date: Sat, 14 Sep 2024 21:18:41 +0200 Subject: [PATCH 01/50] added frontend MPSK Clients added a new Subsection for MPSK Clients for the creating of them #744 --- web/app.py | 3 +- web/blueprints/user/__init__.py | 3 + web/blueprints/wlanhost/__init__.py | 91 ++++++++++++++++++++++++ web/blueprints/wlanhost/forms.py | 20 ++++++ web/blueprints/wlanhost/tables.py | 58 +++++++++++++++ web/templates/user/_user_show_wlans.html | 2 + web/templates/user/user_show.html | 6 ++ 7 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 web/blueprints/wlanhost/__init__.py create mode 100644 web/blueprints/wlanhost/forms.py create mode 100644 web/blueprints/wlanhost/tables.py create mode 100644 web/templates/user/_user_show_wlans.html diff --git a/web/app.py b/web/app.py index beb9df791..6e10f46d5 100644 --- a/web/app.py +++ b/web/app.py @@ -31,7 +31,7 @@ from web.blueprints import task from . import template_filters, template_tests from .blueprints import ( - facilities, finance, infrastructure, login, properties, user, host, health + facilities, finance, infrastructure, login, properties, user, host, health, wlanhost ) from .blueprints.login import login_manager @@ -99,6 +99,7 @@ def make_app(hades_logs: bool = True) -> PycroftFlask: app.register_blueprint(login.bp) app.register_blueprint(api.bp, url_prefix="/api/v0") app.register_blueprint(health.bp, url_prefix="/health") + app.register_blueprint(wlanhost.bp, url_prefix="/wlan-host") template_filters.register_filters(app) template_tests.register_checks(app) diff --git a/web/blueprints/user/__init__.py b/web/blueprints/user/__init__.py index e55725125..143203073 100644 --- a/web/blueprints/user/__init__.py +++ b/web/blueprints/user/__init__.py @@ -68,6 +68,7 @@ from web.blueprints.helpers.form import refill_room_data from web.blueprints.helpers.user import get_user_or_404, get_pre_member_or_404 from web.blueprints.host.tables import HostTable +from web.blueprints.wlanhost.tables import WlanHostTable from web.blueprints.navigation import BlueprintNavigation from web.blueprints.task.tables import TaskTable from web.blueprints.user.forms import ( @@ -349,6 +350,8 @@ def user_show(user_id: int) -> ResponseReturnValue: ), host_table=HostTable(data_url=url_for("host.user_hosts_json", user_id=user.id), user_id=user.id), + wlan_host_table=WlanHostTable(data_url=url_for("wlan-host.user_hosts_json", user_id=user.id), + user_id=user.id), task_table=TaskTable(data_url=url_for("task.json_tasks_for_user", user_id=user.id), hidden_columns=['user']), finance_table_regular=FinanceTable( diff --git a/web/blueprints/wlanhost/__init__.py b/web/blueprints/wlanhost/__init__.py new file mode 100644 index 000000000..6b7eede5c --- /dev/null +++ b/web/blueprints/wlanhost/__init__.py @@ -0,0 +1,91 @@ +import re + +from flask import Blueprint, flash, abort, redirect, url_for, render_template, request +from flask.typing import ResponseReturnValue +from flask_login import current_user +from flask_wtf import FlaskForm +from netaddr import IPAddress + +from pycroft.exc import PycroftException +from pycroft.helpers.net import mac_regex, get_interface_manufacturer +from pycroft.lib import host as lib_host +from pycroft.lib.net import get_subnets_for_room +from pycroft.lib.facilities import get_room +from pycroft.model import session +from pycroft.model.host import Host, Interface +from pycroft.model.user import User +from web.blueprints.access import BlueprintAccess +from web.blueprints.helpers.exception import abort_on_error +from web.blueprints.helpers.form import refill_room_data +from web.blueprints.helpers.user import get_user_or_404 +from web.blueprints.host.forms import InterfaceForm, HostForm +from web.blueprints.host.tables import InterfaceTable, HostTable, HostRow, InterfaceRow +from web.table.table import TableResponse, BtnColResponse, LinkColResponse + +bp = Blueprint('wlan-host', __name__) +access = BlueprintAccess(bp, required_properties=['user_show']) + + +def get_wlan_host_or_404(host_id: int) -> Host: + #if (host := session.session.get(Host, host_id)) is None: + flash("Host existiert nicht.", "error") + abort(404) + #return host + +@bp.route('/create', methods=['GET', 'POST']) +@access.require('hosts_change') +def host_create() -> ResponseReturnValue: + user = get_user_or_404(request.args.get("user_id", default=None, type=int)) + form = HostForm(owner_id=user.id) + + def default_response() -> ResponseReturnValue: + form_args = { + 'form': form, + 'cancel_to': url_for('user.user_show', user_id=user.id) + } + + return render_template( + 'generic_form.html', + page_title="Host erstellen", form_args=form_args, form=form + ) + if not form.is_submitted(): + refill_room_data(form, user.room) + + if not form.validate_on_submit(): + return default_response() + + # existence verified by validator + # TODO can't we provide an attribute for that on the form? + owner = session.session.get(User, form.owner_id.data) + with abort_on_error(default_response), session.session.begin_nested(): + if not ( + room := get_room( + # TODO I know this is a double query, + # but we should fix this on the `get_room` side. + building_id=form.building.data.id, + level=form.level.data, + room_number=form.room_number.data, + ) + ): + form.room_number.errors.append("room does not exist") + return default_response() + + host = lib_host.host_create(owner, room, form.name.data, processor=current_user) + session.session.commit() + + flash("Host erfolgreich erstellt.", "success") + return redirect( + url_for( + ".interface_create", user_id=host.owner_id, host_id=host.id, _anchor="hosts" + ) + ) + + +@bp.route("/") +def user_hosts_json(user_id: int) -> ResponseReturnValue: + user = get_wlan_host_or_404(user_id) + # TODO: Importend when the for the returning of actual mpsk mac addresses for MPSK devices + return "" + #return TableResponse[HostRow]( + # items=[_host_row(host, user_id) for host in user.hosts] + #).model_dump() diff --git a/web/blueprints/wlanhost/forms.py b/web/blueprints/wlanhost/forms.py new file mode 100644 index 000000000..705bf5c31 --- /dev/null +++ b/web/blueprints/wlanhost/forms.py @@ -0,0 +1,20 @@ +# Copyright (c) 2024. The Pycroft Authors. See the AUTHORS file. +# This file is part of the Pycroft project and licensed under the terms of +# the Apache License, Version 2.0. See the LICENSE file for details + +from flask_wtf import FlaskForm as Form +from wtforms.validators import Optional +from wtforms_widgets.fields.core import TextField, SelectMultipleField +from wtforms_widgets.fields.custom import MacField +from wtforms_widgets.fields.validators import MacAddress + +from web.blueprints.facilities.forms import SelectRoomForm +from web.blueprints.helpers.host import UniqueMac +from web.form.widgets import UserIDField + + + +class WLANInterfaceForm(Form): + name = TextField("Name", validators=[Optional()]) + + mac = MacField("MAC", [MacAddress(message="MAC ist ungültig!")], UniqueMac(annex_field=None)) diff --git a/web/blueprints/wlanhost/tables.py b/web/blueprints/wlanhost/tables.py new file mode 100644 index 000000000..3b0be738d --- /dev/null +++ b/web/blueprints/wlanhost/tables.py @@ -0,0 +1,58 @@ +# Copyright (c) 2024. The Pycroft Authors. See the AUTHORS file. +# This file is part of the Pycroft project and licensed under the terms of +# the Apache License, Version 2.0. See the LICENSE file for details +import typing as t + +from flask import url_for +from pydantic import BaseModel + +from web.blueprints.helpers.user import no_hosts_change +from web.table.lazy_join import HasDunderStr +from web.table.table import ( + BootstrapTable, + Column, + button_toolbar, + MultiBtnColumn, + BtnColResponse, + MultiLinkColumn, + LinkColResponse, +) + +class HostRow(BaseModel): + name: str | None = None + actions: list[BtnColResponse] + interfaces_table_link: str + interface_create_link: str + id: int + + +class WlanHostTable(BootstrapTable): + """A table for displaying hosts + """ + name = Column("Name") + interface_create_link = Column("Mac") + actions = MultiBtnColumn("Aktionen", hide_if=no_hosts_change, width=3) + #interfaces_table_link = Column("", hide_if=lambda: True) + + id = Column("", hide_if=lambda: True) + + def __init__(self, *, user_id: int | None = None, **kw: t.Any) -> None: + table_args = kw.pop('table_args', {}) + table_args.setdefault('data-load-subtables', "true") + table_args.setdefault('data-detail-view', "true") + table_args.setdefault('data-detail-formatter', "table.hostDetailFormatter") + table_args.setdefault('data-response-handler', "table.userHostResponseHandler") + kw['table_args'] = table_args + + super().__init__(**kw) + self.user_id = user_id + + @property + def toolbar(self) -> HasDunderStr | None: + if self.user_id is None: + return None + if no_hosts_change(): + return None + + href = url_for("wlan-host.host_create", user_id=self.user_id) + return button_toolbar("Host", href) diff --git a/web/templates/user/_user_show_wlans.html b/web/templates/user/_user_show_wlans.html new file mode 100644 index 000000000..22c8ee347 --- /dev/null +++ b/web/templates/user/_user_show_wlans.html @@ -0,0 +1,2 @@ + +{{ wlan_host_table.render("user_wlans") }} diff --git a/web/templates/user/user_show.html b/web/templates/user/user_show.html index dfa9a5c35..19ae23448 100644 --- a/web/templates/user/user_show.html +++ b/web/templates/user/user_show.html @@ -16,6 +16,12 @@ 'name': 'Hosts', 'badge': user.hosts | length }, + { + 'id': 'wlans', + 'icon': 'fa-clipboard-check', + 'name': 'MPSK Clients', + 'badge': user.tasks | length, + }, { 'id': 'tasks', 'icon': 'fa-clipboard-check', From 4e06e420e1dbcc94cc071ca5cde9b9323fae7f06 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 15 Sep 2024 10:15:44 +0200 Subject: [PATCH 02/50] added MSPK Client Model added the database model for storing added the needed functions as well as the relation to the user and back Signed-off-by: Alex #744 --- pycroft/lib/mpsk_client.py | 78 ++++++++++++++++++++++++++++++++++++ pycroft/model/_all.py | 1 + pycroft/model/mspk_client.py | 36 +++++++++++++++++ pycroft/model/user.py | 6 +++ 4 files changed, 121 insertions(+) create mode 100644 pycroft/lib/mpsk_client.py create mode 100644 pycroft/model/mspk_client.py diff --git a/pycroft/lib/mpsk_client.py b/pycroft/lib/mpsk_client.py new file mode 100644 index 000000000..41a2e36d7 --- /dev/null +++ b/pycroft/lib/mpsk_client.py @@ -0,0 +1,78 @@ +# Copyright (c) 2024. The Pycroft Authors. See the AUTHORS file. +# This file is part of the Pycroft project and licensed under the terms of +# the Apache License, Version 2.0. See the LICENSE file for details +from pycroft.model.mspk_client import MSPKClient +from pycroft.model.user import User +from pycroft.model.session import with_transaction, session +from pycroft.lib.logging import log_user_event +from pycroft.helpers.i18n import deferred_gettext + + +@with_transaction +def host_delete(host: MSPKClient, processor: User) -> None: + message = deferred_gettext("Deleted host '{}'.").format(host.name) + log_user_event(author=processor, user=host.owner, message=message.to_json()) + + session.delete(host) + + +@with_transaction +def change_mac(client: MSPKClient, mac: str, processor: User) -> MSPKClient: + """ + This method will change the mac address of the given interface to the new + mac address. + + :param interface: the interface which should become a new mac address. + :param mac: the new mac address. + :param processor: the user who initiated the mac address change. + :return: the changed interface with the new mac address. + """ + old_mac = client.mac + client.mac = mac + message = deferred_gettext("Changed MAC address from {} to {}.").format(old_mac, mac) + if client.host.owner: + log_user_event(message.to_json(), processor, client.host.owner) + return client + + +@with_transaction +def mpsk_client_create(owner: User, name: str, mac: str, processor: User) -> MSPKClient: + client = MSPKClient(name=name, owner_id=owner.id, mac=mac) + + session.add(client) + + message = deferred_gettext("Created MPSK Client '{name}' with MAC: {mac}.").format( + name=client.name, + mac=client.mac, + ) + + log_user_event(author=processor, user=owner, message=message.to_json()) + + return client + + +@with_transaction +def mpsk_edit(client: MSPKClient, owner: User, name: str, mac: str, processor: User) -> None: + if client.name != name: + message = deferred_gettext("Changed name of client '{}' to '{}'.").format(client.name, name) + client.name = name + + log_user_event(author=processor, user=owner, message=message.to_json()) + + if client.owner_id != owner.id: + message = deferred_gettext("Transferred Host '{}' to {}.").format(client.name, owner.id) + log_user_event(author=processor, user=client.owner, message=message.to_json()) + + message = deferred_gettext("Transferred Host '{}' from {}.").format( + client.name, client.owner.id + ) + log_user_event(author=processor, user=owner, message=message.to_json()) + + client.owner = owner + + if client.mac != mac: + message = deferred_gettext("Changed MAC address of client '{}' to '{}'.").format( + client.name, mac + ) + log_user_event(author=processor, user=owner, message=message.to_json()) + client.mac = mac diff --git a/pycroft/model/_all.py b/pycroft/model/_all.py index 481eccf57..b0c9ebb4a 100644 --- a/pycroft/model/_all.py +++ b/pycroft/model/_all.py @@ -16,6 +16,7 @@ from .facilities import * from .finance import * from .host import * +from .mspk_client import * from .logging import * from .net import * from .port import * diff --git a/pycroft/model/mspk_client.py b/pycroft/model/mspk_client.py new file mode 100644 index 000000000..fe8712aec --- /dev/null +++ b/pycroft/model/mspk_client.py @@ -0,0 +1,36 @@ +# Copyright (c) 2024. The Pycroft Authors. See the AUTHORS file. +# This file is part of the Pycroft project and licensed under the terms of +# the Apache License, Version 2.0. See the LICENSE file for details + +from __future__ import annotations + +from sqlalchemy import ForeignKey +from sqlalchemy.orm import relationship, validates, Mapped, mapped_column +from sqlalchemy.types import String + +from pycroft.helpers.net import mac_regex +from pycroft.model.base import IntegerIdModel +from pycroft.model.host import MulticastFlagException +from pycroft.model.type_aliases import mac_address +from pycroft.model.types import InvalidMACAddressException +from pycroft.model.user import User + + +class MSPKClient(IntegerIdModel): + + name: Mapped[str | None] = mapped_column(String) + + owner_id: Mapped[int | None] = mapped_column( + ForeignKey(User.id, ondelete="CASCADE"), index=True + ) + owner: Mapped[User] = relationship(User, back_populates="mspks") + mac: Mapped[mac_address] = mapped_column(unique=True) + + @validates("mac") + def validate_mac(self, _, mac_address): + match = mac_regex.match(mac_address) + if not match: + raise InvalidMACAddressException("MAC address '" + mac_address + "' is not valid") + if int(mac_address[0:2], base=16) & 1: + raise MulticastFlagException("Multicast bit set in MAC address") + return mac_address diff --git a/pycroft/model/user.py b/pycroft/model/user.py index dee3e70e9..e6daf18dd 100644 --- a/pycroft/model/user.py +++ b/pycroft/model/user.py @@ -75,6 +75,7 @@ # Backrefs from .logging import LogEntry, UserLogEntry, TaskLogEntry from .host import Host + from .mspk_client import MSPKClient from .swdd import Tenancy from .task import UserTask from .traffic import TrafficVolume @@ -252,6 +253,11 @@ class User(BaseUser, UserMixin): hosts: Mapped[list[Host]] = relationship( back_populates="owner", cascade="all, delete-orphan" ) + + mspks: Mapped[list[MSPKClient]] = relationship( + back_populates="owner", cascade="all, delete-orphan" + ) + authored_log_entries: Mapped[list[LogEntry]] = relationship( back_populates="author", viewonly=True ) From 6363c737b39a4f3d624022dc7d55f22bcb8971f2 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 15 Sep 2024 23:48:52 +0200 Subject: [PATCH 03/50] changed name to mspk client module changed the name Signed-off-by: Alex --- web/app.py | 12 +- web/blueprints/mpskclient/__init__.py | 158 ++++++++++++++++++ .../{wlanhost => mpskclient}/forms.py | 12 +- .../{wlanhost => mpskclient}/tables.py | 33 ++-- web/blueprints/user/__init__.py | 17 +- web/blueprints/wlanhost/__init__.py | 91 ---------- web/templates/user/user_show.html | 4 +- 7 files changed, 198 insertions(+), 129 deletions(-) create mode 100644 web/blueprints/mpskclient/__init__.py rename web/blueprints/{wlanhost => mpskclient}/forms.py (64%) rename web/blueprints/{wlanhost => mpskclient}/tables.py (60%) delete mode 100644 web/blueprints/wlanhost/__init__.py diff --git a/web/app.py b/web/app.py index 6e10f46d5..40c5fec98 100644 --- a/web/app.py +++ b/web/app.py @@ -31,7 +31,15 @@ from web.blueprints import task from . import template_filters, template_tests from .blueprints import ( - facilities, finance, infrastructure, login, properties, user, host, health, wlanhost + facilities, + finance, + infrastructure, + login, + properties, + user, + host, + health, + mpskclient, ) from .blueprints.login import login_manager @@ -99,7 +107,7 @@ def make_app(hades_logs: bool = True) -> PycroftFlask: app.register_blueprint(login.bp) app.register_blueprint(api.bp, url_prefix="/api/v0") app.register_blueprint(health.bp, url_prefix="/health") - app.register_blueprint(wlanhost.bp, url_prefix="/wlan-host") + app.register_blueprint(mpskclient.bp, url_prefix="/wlan-host") template_filters.register_filters(app) template_tests.register_checks(app) diff --git a/web/blueprints/mpskclient/__init__.py b/web/blueprints/mpskclient/__init__.py new file mode 100644 index 000000000..573d4a7e6 --- /dev/null +++ b/web/blueprints/mpskclient/__init__.py @@ -0,0 +1,158 @@ +import re + +from flask import Blueprint, flash, abort, redirect, url_for, render_template, request +from flask.typing import ResponseReturnValue +from flask_login import current_user +from flask_wtf import FlaskForm +from netaddr import IPAddress + +from pycroft.exc import PycroftException +from pycroft.helpers.net import mac_regex, get_interface_manufacturer +from pycroft.lib import mpsk_client as lib_mspk +from pycroft.lib.net import get_subnets_for_room +from pycroft.lib.facilities import get_room +from pycroft.model import session +from pycroft.model.host import Host, Interface +from pycroft.model.mspk_client import MSPKClient +from pycroft.model.user import User +from web.blueprints.access import BlueprintAccess +from web.blueprints.helpers.exception import abort_on_error +from web.blueprints.helpers.form import refill_room_data +from web.blueprints.helpers.user import get_user_or_404 +from web.blueprints.host.forms import InterfaceForm, HostForm +from web.blueprints.host.tables import InterfaceTable, HostTable, HostRow, InterfaceRow +from web.blueprints.mpskclient.forms import WLANInterfaceForm +from web.blueprints.mpskclient.tables import MSPKRow +from web.table.table import TableResponse, BtnColResponse, LinkColResponse + +bp = Blueprint("wlan-host", __name__) +access = BlueprintAccess(bp, required_properties=["user_show"]) + + +def get_mpsk_client_or_404(mspk_id: int) -> MSPKClient: + if (mspk := session.session.get(MSPKClient, mspk_id)) is None: + flash("Host existiert nicht.", "error") + abort(404) + return mspk + + +@bp.route("/create", methods=["GET", "POST"]) +@access.require("hosts_change") +def host_create() -> ResponseReturnValue: + user = get_user_or_404(request.args.get("user_id", default=None, type=int)) + form = WLANInterfaceForm(owner_id=user.id) + + def default_response() -> ResponseReturnValue: + form_args = {"form": form, "cancel_to": url_for("user.user_show", user_id=user.id)} + + return render_template( + "generic_form.html", page_title="MSPK Client erstellen", form_args=form_args, form=form + ) + + if not form.is_submitted(): + return default_response() + refill_room_data(form, user.room) + + if not form.validate_on_submit(): + return default_response() + + # existence verified by validator + # TODO can't we provide an attribute for that on the form? + owner = session.session.get(User, form.owner_id.data) + with abort_on_error(default_response), session.session.begin_nested(): + + host = lib_mspk.mpsk_client_create( + owner, form.name.data, form.mac.data, processor=current_user + ) + session.session.commit() + + flash("MSPK Client erfolgreich erstellt.", "success") + return redirect(url_for("user.user_show", user_id=host.owner.id, _anchor="mspks")) + + +@bp.route("//delete", methods=["GET", "POST"]) +@access.require("hosts_change") +def mpsk_delete(mpsk_id: int) -> ResponseReturnValue: + mpsk = get_mpsk_client_or_404(mpsk_id) + owner = mpsk.owner + form = FlaskForm() + + def default_response() -> ResponseReturnValue: + form_args = { + "form": form, + "cancel_to": url_for("user.user_show", user_id=owner.id), + "submit_text": "Löschen", + "actions_offset": 0, + } + + return render_template( + "generic_form.html", page_title="MPSK Client löschen", form_args=form_args, form=form + ) + + if not form.is_submitted(): + return default_response() + + with abort_on_error(default_response), session.session.begin_nested(): + lib_mspk.mpsk_delete(mpsk, current_user) + session.session.commit() + + flash("MPSK Client erfolgreich gelöscht.", "success") + return redirect(url_for("user.user_show", user_id=owner.id, _anchor="mspks")) + + +@bp.route("//edit", methods=["GET", "POST"]) +@access.require("hosts_change") +def mpsk_edit(mpsk_id: int) -> ResponseReturnValue: + mpsk = get_mpsk_client_or_404(mpsk_id) + + form = WLANInterfaceForm(obj=mpsk) + + def default_response() -> ResponseReturnValue: + form_args = {"form": form, "cancel_to": url_for("user.user_show", user_id=mpsk.owner_id)} + + return render_template( + "generic_form.html", page_title="MPSK Client editieren", form_args=form_args + ) + + if not form.is_submitted() or not form.validate(): + return default_response() + + with abort_on_error(default_response), session.session.begin_nested(): + lib_mspk.mpsk_edit(mpsk, mpsk.owner, form.name.data, form.mac.data, current_user) + session.session.commit() + flash("MPSK Client erfolgreich bearbeitet.", "success") + return redirect(url_for("user.user_show", user_id=mpsk.owner_id, _anchor="mspks")) + + +@bp.route("/") +def user_hosts_json(user_id: int) -> ResponseReturnValue: + user = get_user_or_404(user_id) + # TODO: Importend when the for the returning of actual mpsk mac addresses for MPSK devices + # return "" + return TableResponse[MSPKRow]( + items=[_mspk_row(mpsk, user_id) for mpsk in user.mspks] + ).model_dump() + + +def _mspk_row(client, user_id: int) -> MSPKRow: + # println(f"{client}") + # client = get_wlan_host_or_404(client) + return MSPKRow( + id=client.id, + name=client.name, + mac=client.mac, + actions=[ + BtnColResponse( + href=url_for(".mpsk_edit", mpsk_id=client.id), + title="Bearbeiten", + icon="fa-edit", + btn_class="btn-link", + ), + BtnColResponse( + href=url_for(".mpsk_delete", mpsk_id=client.id), + title="Löschen", + icon="fa-trash", + btn_class="btn-link", + ), + ], + ) diff --git a/web/blueprints/wlanhost/forms.py b/web/blueprints/mpskclient/forms.py similarity index 64% rename from web/blueprints/wlanhost/forms.py rename to web/blueprints/mpskclient/forms.py index 705bf5c31..5471fd660 100644 --- a/web/blueprints/wlanhost/forms.py +++ b/web/blueprints/mpskclient/forms.py @@ -3,18 +3,18 @@ # the Apache License, Version 2.0. See the LICENSE file for details from flask_wtf import FlaskForm as Form -from wtforms.validators import Optional -from wtforms_widgets.fields.core import TextField, SelectMultipleField +from wtforms.validators import DataRequired +from wtforms_widgets.fields.core import TextField from wtforms_widgets.fields.custom import MacField from wtforms_widgets.fields.validators import MacAddress -from web.blueprints.facilities.forms import SelectRoomForm from web.blueprints.helpers.host import UniqueMac from web.form.widgets import UserIDField - class WLANInterfaceForm(Form): - name = TextField("Name", validators=[Optional()]) + name = TextField("Name", validators=[DataRequired(message="Es muss ein Name gesetzt werden!")]) + owner_id = UserIDField("Besitzer-ID") - mac = MacField("MAC", [MacAddress(message="MAC ist ungültig!")], UniqueMac(annex_field=None)) + mac = MacField("MAC", [MacAddress(message="MAC ist ungültig!"), UniqueMac(annex_field=None)]) + _order = ("name", "owner_id") diff --git a/web/blueprints/wlanhost/tables.py b/web/blueprints/mpskclient/tables.py similarity index 60% rename from web/blueprints/wlanhost/tables.py rename to web/blueprints/mpskclient/tables.py index 3b0be738d..c1f63698a 100644 --- a/web/blueprints/wlanhost/tables.py +++ b/web/blueprints/mpskclient/tables.py @@ -14,36 +14,20 @@ button_toolbar, MultiBtnColumn, BtnColResponse, - MultiLinkColumn, - LinkColResponse, ) -class HostRow(BaseModel): - name: str | None = None - actions: list[BtnColResponse] - interfaces_table_link: str - interface_create_link: str - id: int +class MPSKTable(BootstrapTable): + """A table for displaying hosts""" -class WlanHostTable(BootstrapTable): - """A table for displaying hosts - """ name = Column("Name") - interface_create_link = Column("Mac") + mac = Column("Mac") actions = MultiBtnColumn("Aktionen", hide_if=no_hosts_change, width=3) - #interfaces_table_link = Column("", hide_if=lambda: True) + # interfaces_table_link = Column("", hide_if=lambda: True) id = Column("", hide_if=lambda: True) def __init__(self, *, user_id: int | None = None, **kw: t.Any) -> None: - table_args = kw.pop('table_args', {}) - table_args.setdefault('data-load-subtables', "true") - table_args.setdefault('data-detail-view', "true") - table_args.setdefault('data-detail-formatter', "table.hostDetailFormatter") - table_args.setdefault('data-response-handler', "table.userHostResponseHandler") - kw['table_args'] = table_args - super().__init__(**kw) self.user_id = user_id @@ -55,4 +39,11 @@ def toolbar(self) -> HasDunderStr | None: return None href = url_for("wlan-host.host_create", user_id=self.user_id) - return button_toolbar("Host", href) + return button_toolbar("Client", href) + + +class MSPKRow(BaseModel): + name: str | None = None + mac: str + actions: list[BtnColResponse] + id: int diff --git a/web/blueprints/user/__init__.py b/web/blueprints/user/__init__.py index 143203073..23a88bc77 100644 --- a/web/blueprints/user/__init__.py +++ b/web/blueprints/user/__init__.py @@ -68,7 +68,7 @@ from web.blueprints.helpers.form import refill_room_data from web.blueprints.helpers.user import get_user_or_404, get_pre_member_or_404 from web.blueprints.host.tables import HostTable -from web.blueprints.wlanhost.tables import WlanHostTable +from web.blueprints.mpskclient.tables import MPSKTable from web.blueprints.navigation import BlueprintNavigation from web.blueprints.task.tables import TaskTable from web.blueprints.user.forms import ( @@ -348,12 +348,15 @@ def user_show(user_id: int) -> ResponseReturnValue: user_id=user.id, data_url=_membership_endpoint(group_filter="active"), ), - host_table=HostTable(data_url=url_for("host.user_hosts_json", user_id=user.id), - user_id=user.id), - wlan_host_table=WlanHostTable(data_url=url_for("wlan-host.user_hosts_json", user_id=user.id), - user_id=user.id), - task_table=TaskTable(data_url=url_for("task.json_tasks_for_user", user_id=user.id), - hidden_columns=['user']), + host_table=HostTable( + data_url=url_for("host.user_hosts_json", user_id=user.id), user_id=user.id + ), + wlan_host_table=MPSKTable( + data_url=url_for("wlan-host.user_hosts_json", user_id=user.id), user_id=user.id + ), + task_table=TaskTable( + data_url=url_for("task.json_tasks_for_user", user_id=user.id), hidden_columns=["user"] + ), finance_table_regular=FinanceTable( data_url=tbl_data_url, user_id=user.id, diff --git a/web/blueprints/wlanhost/__init__.py b/web/blueprints/wlanhost/__init__.py deleted file mode 100644 index 6b7eede5c..000000000 --- a/web/blueprints/wlanhost/__init__.py +++ /dev/null @@ -1,91 +0,0 @@ -import re - -from flask import Blueprint, flash, abort, redirect, url_for, render_template, request -from flask.typing import ResponseReturnValue -from flask_login import current_user -from flask_wtf import FlaskForm -from netaddr import IPAddress - -from pycroft.exc import PycroftException -from pycroft.helpers.net import mac_regex, get_interface_manufacturer -from pycroft.lib import host as lib_host -from pycroft.lib.net import get_subnets_for_room -from pycroft.lib.facilities import get_room -from pycroft.model import session -from pycroft.model.host import Host, Interface -from pycroft.model.user import User -from web.blueprints.access import BlueprintAccess -from web.blueprints.helpers.exception import abort_on_error -from web.blueprints.helpers.form import refill_room_data -from web.blueprints.helpers.user import get_user_or_404 -from web.blueprints.host.forms import InterfaceForm, HostForm -from web.blueprints.host.tables import InterfaceTable, HostTable, HostRow, InterfaceRow -from web.table.table import TableResponse, BtnColResponse, LinkColResponse - -bp = Blueprint('wlan-host', __name__) -access = BlueprintAccess(bp, required_properties=['user_show']) - - -def get_wlan_host_or_404(host_id: int) -> Host: - #if (host := session.session.get(Host, host_id)) is None: - flash("Host existiert nicht.", "error") - abort(404) - #return host - -@bp.route('/create', methods=['GET', 'POST']) -@access.require('hosts_change') -def host_create() -> ResponseReturnValue: - user = get_user_or_404(request.args.get("user_id", default=None, type=int)) - form = HostForm(owner_id=user.id) - - def default_response() -> ResponseReturnValue: - form_args = { - 'form': form, - 'cancel_to': url_for('user.user_show', user_id=user.id) - } - - return render_template( - 'generic_form.html', - page_title="Host erstellen", form_args=form_args, form=form - ) - if not form.is_submitted(): - refill_room_data(form, user.room) - - if not form.validate_on_submit(): - return default_response() - - # existence verified by validator - # TODO can't we provide an attribute for that on the form? - owner = session.session.get(User, form.owner_id.data) - with abort_on_error(default_response), session.session.begin_nested(): - if not ( - room := get_room( - # TODO I know this is a double query, - # but we should fix this on the `get_room` side. - building_id=form.building.data.id, - level=form.level.data, - room_number=form.room_number.data, - ) - ): - form.room_number.errors.append("room does not exist") - return default_response() - - host = lib_host.host_create(owner, room, form.name.data, processor=current_user) - session.session.commit() - - flash("Host erfolgreich erstellt.", "success") - return redirect( - url_for( - ".interface_create", user_id=host.owner_id, host_id=host.id, _anchor="hosts" - ) - ) - - -@bp.route("/") -def user_hosts_json(user_id: int) -> ResponseReturnValue: - user = get_wlan_host_or_404(user_id) - # TODO: Importend when the for the returning of actual mpsk mac addresses for MPSK devices - return "" - #return TableResponse[HostRow]( - # items=[_host_row(host, user_id) for host in user.hosts] - #).model_dump() diff --git a/web/templates/user/user_show.html b/web/templates/user/user_show.html index 19ae23448..b248280cb 100644 --- a/web/templates/user/user_show.html +++ b/web/templates/user/user_show.html @@ -18,8 +18,8 @@ }, { 'id': 'wlans', - 'icon': 'fa-clipboard-check', - 'name': 'MPSK Clients', + 'icon': 'fa-tablet', + 'name': 'MSPK Clients', 'badge': user.tasks | length, }, { From eaf6a564552b964154f2e285cec3d9148db50145 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 15 Sep 2024 23:51:42 +0200 Subject: [PATCH 04/50] changes to model now enforces name for mpsk client refactoring changed name Signed-off-by: Alex #744 --- pycroft/lib/mpsk_client.py | 6 +++--- pycroft/model/mspk_client.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/pycroft/lib/mpsk_client.py b/pycroft/lib/mpsk_client.py index 41a2e36d7..43df4400b 100644 --- a/pycroft/lib/mpsk_client.py +++ b/pycroft/lib/mpsk_client.py @@ -9,7 +9,7 @@ @with_transaction -def host_delete(host: MSPKClient, processor: User) -> None: +def mpsk_delete(host: MSPKClient, processor: User) -> None: message = deferred_gettext("Deleted host '{}'.").format(host.name) log_user_event(author=processor, user=host.owner, message=message.to_json()) @@ -30,8 +30,8 @@ def change_mac(client: MSPKClient, mac: str, processor: User) -> MSPKClient: old_mac = client.mac client.mac = mac message = deferred_gettext("Changed MAC address from {} to {}.").format(old_mac, mac) - if client.host.owner: - log_user_event(message.to_json(), processor, client.host.owner) + if client.owner: + log_user_event(message.to_json(), processor, client.owner) return client diff --git a/pycroft/model/mspk_client.py b/pycroft/model/mspk_client.py index fe8712aec..d85bcf79e 100644 --- a/pycroft/model/mspk_client.py +++ b/pycroft/model/mspk_client.py @@ -18,7 +18,7 @@ class MSPKClient(IntegerIdModel): - name: Mapped[str | None] = mapped_column(String) + name: Mapped[str] = mapped_column(String, nullable=False) owner_id: Mapped[int | None] = mapped_column( ForeignKey(User.id, ondelete="CASCADE"), index=True From 24eedaaffebb318f26d86e1ea25d28822f396184 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 16 Sep 2024 09:27:32 +0200 Subject: [PATCH 05/50] refactoring refactoring and renaming for better usability Signed-off-by: Alex #744 --- web/app.py | 2 +- web/blueprints/mpskclient/__init__.py | 12 ++++++------ web/blueprints/mpskclient/forms.py | 2 +- web/blueprints/mpskclient/tables.py | 2 +- web/blueprints/user/__init__.py | 4 ++-- web/templates/user/_user_show_mpsk.html | 2 ++ web/templates/user/_user_show_wlans.html | 2 -- web/templates/user/user_show.html | 2 +- 8 files changed, 14 insertions(+), 14 deletions(-) create mode 100644 web/templates/user/_user_show_mpsk.html delete mode 100644 web/templates/user/_user_show_wlans.html diff --git a/web/app.py b/web/app.py index 40c5fec98..03d8b9975 100644 --- a/web/app.py +++ b/web/app.py @@ -107,7 +107,7 @@ def make_app(hades_logs: bool = True) -> PycroftFlask: app.register_blueprint(login.bp) app.register_blueprint(api.bp, url_prefix="/api/v0") app.register_blueprint(health.bp, url_prefix="/health") - app.register_blueprint(mpskclient.bp, url_prefix="/wlan-host") + app.register_blueprint(mpskclient.bp, url_prefix="/wifi-mpsk") template_filters.register_filters(app) template_tests.register_checks(app) diff --git a/web/blueprints/mpskclient/__init__.py b/web/blueprints/mpskclient/__init__.py index 573d4a7e6..b11cc8163 100644 --- a/web/blueprints/mpskclient/__init__.py +++ b/web/blueprints/mpskclient/__init__.py @@ -21,11 +21,11 @@ from web.blueprints.helpers.user import get_user_or_404 from web.blueprints.host.forms import InterfaceForm, HostForm from web.blueprints.host.tables import InterfaceTable, HostTable, HostRow, InterfaceRow -from web.blueprints.mpskclient.forms import WLANInterfaceForm +from web.blueprints.mpskclient.forms import WiFiInterfaceForm from web.blueprints.mpskclient.tables import MSPKRow from web.table.table import TableResponse, BtnColResponse, LinkColResponse -bp = Blueprint("wlan-host", __name__) +bp = Blueprint("wifi-mpsk", __name__) access = BlueprintAccess(bp, required_properties=["user_show"]) @@ -40,7 +40,7 @@ def get_mpsk_client_or_404(mspk_id: int) -> MSPKClient: @access.require("hosts_change") def host_create() -> ResponseReturnValue: user = get_user_or_404(request.args.get("user_id", default=None, type=int)) - form = WLANInterfaceForm(owner_id=user.id) + form = WiFiInterfaceForm(owner_id=user.id) def default_response() -> ResponseReturnValue: form_args = {"form": form, "cancel_to": url_for("user.user_show", user_id=user.id)} @@ -66,7 +66,7 @@ def default_response() -> ResponseReturnValue: ) session.session.commit() - flash("MSPK Client erfolgreich erstellt.", "success") + flash("MPSK Client erfolgreich erstellt.", "success") return redirect(url_for("user.user_show", user_id=host.owner.id, _anchor="mspks")) @@ -105,7 +105,7 @@ def default_response() -> ResponseReturnValue: def mpsk_edit(mpsk_id: int) -> ResponseReturnValue: mpsk = get_mpsk_client_or_404(mpsk_id) - form = WLANInterfaceForm(obj=mpsk) + form = WiFiInterfaceForm(obj=mpsk) def default_response() -> ResponseReturnValue: form_args = {"form": form, "cancel_to": url_for("user.user_show", user_id=mpsk.owner_id)} @@ -125,7 +125,7 @@ def default_response() -> ResponseReturnValue: @bp.route("/") -def user_hosts_json(user_id: int) -> ResponseReturnValue: +def user_clients_json(user_id: int) -> ResponseReturnValue: user = get_user_or_404(user_id) # TODO: Importend when the for the returning of actual mpsk mac addresses for MPSK devices # return "" diff --git a/web/blueprints/mpskclient/forms.py b/web/blueprints/mpskclient/forms.py index 5471fd660..29cf359fa 100644 --- a/web/blueprints/mpskclient/forms.py +++ b/web/blueprints/mpskclient/forms.py @@ -12,7 +12,7 @@ from web.form.widgets import UserIDField -class WLANInterfaceForm(Form): +class WiFiInterfaceForm(Form): name = TextField("Name", validators=[DataRequired(message="Es muss ein Name gesetzt werden!")]) owner_id = UserIDField("Besitzer-ID") diff --git a/web/blueprints/mpskclient/tables.py b/web/blueprints/mpskclient/tables.py index c1f63698a..92d9239a5 100644 --- a/web/blueprints/mpskclient/tables.py +++ b/web/blueprints/mpskclient/tables.py @@ -38,7 +38,7 @@ def toolbar(self) -> HasDunderStr | None: if no_hosts_change(): return None - href = url_for("wlan-host.host_create", user_id=self.user_id) + href = url_for("wifi-mpsk.host_create", user_id=self.user_id) return button_toolbar("Client", href) diff --git a/web/blueprints/user/__init__.py b/web/blueprints/user/__init__.py index 23a88bc77..6afff3dbf 100644 --- a/web/blueprints/user/__init__.py +++ b/web/blueprints/user/__init__.py @@ -351,8 +351,8 @@ def user_show(user_id: int) -> ResponseReturnValue: host_table=HostTable( data_url=url_for("host.user_hosts_json", user_id=user.id), user_id=user.id ), - wlan_host_table=MPSKTable( - data_url=url_for("wlan-host.user_hosts_json", user_id=user.id), user_id=user.id + wifi_client_table=MPSKTable( + data_url=url_for("wifi-mpsk.user_clients_json", user_id=user.id), user_id=user.id ), task_table=TaskTable( data_url=url_for("task.json_tasks_for_user", user_id=user.id), hidden_columns=["user"] diff --git a/web/templates/user/_user_show_mpsk.html b/web/templates/user/_user_show_mpsk.html new file mode 100644 index 000000000..03438116d --- /dev/null +++ b/web/templates/user/_user_show_mpsk.html @@ -0,0 +1,2 @@ + +{{ wifi_client_table.render("user_mpsks") }} diff --git a/web/templates/user/_user_show_wlans.html b/web/templates/user/_user_show_wlans.html deleted file mode 100644 index 22c8ee347..000000000 --- a/web/templates/user/_user_show_wlans.html +++ /dev/null @@ -1,2 +0,0 @@ - -{{ wlan_host_table.render("user_wlans") }} diff --git a/web/templates/user/user_show.html b/web/templates/user/user_show.html index b248280cb..3dbec4439 100644 --- a/web/templates/user/user_show.html +++ b/web/templates/user/user_show.html @@ -17,7 +17,7 @@ 'badge': user.hosts | length }, { - 'id': 'wlans', + 'id': 'mpsk', 'icon': 'fa-tablet', 'name': 'MSPK Clients', 'badge': user.tasks | length, From 3f56a87e7ad00e2885fee080bb156138f796a806 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 16 Sep 2024 10:08:21 +0200 Subject: [PATCH 06/50] added api mpsk added the api background for adding mpsk clients and editing them Signed-off-by: Alex --- web/api/v0/__init__.py | 81 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index 28f20989f..67a3370b1 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -17,6 +17,7 @@ from pycroft.helpers import utc from pycroft.helpers.i18n import Message from pycroft.lib.finance import estimate_balance, get_last_import_date +from pycroft.lib.mpsk_client import mpsk_edit, mpsk_client_create, mpsk_delete from pycroft.lib.host import change_mac, host_create, interface_create, host_edit from pycroft.lib.net import SubnetFullException from pycroft.lib.swdd import get_swdd_person_id, get_relevant_tenancies, \ @@ -57,6 +58,7 @@ from pycroft.model.task import Task from pycroft.model.types import IPAddress, InvalidMACAddressException from pycroft.model.user import User, IllegalEmailError, IllegalLoginError +from web.blueprints.mpskclient import get_mpsk_client_or_404 api = Api() @@ -343,6 +345,85 @@ def get(self, ipv4: IPv4Address | IPv6Address) -> ResponseReturnValue: api.add_resource(UserByIPResource, '/user/from-ip') +class MPSKSClientAddResource(Resource): + @use_kwargs( + { + "password": fields.Str(required=True), + "mac": fields.Str(required=True), + "name": fields.Str(required=True), + }, + location="form", + ) + def post(self, user_id: int, password: str, mac: str, name: str) -> ResponseReturnValue: + user = get_authenticated_user(user_id, password) + + try: + mpsk_client_create(user, mac, name, user) + session.session.commit() + except InvalidMACAddressException: + abort(400, message="Invalid MAC address.") + except IntegrityError: + abort(400, message="Mac address is already in use.") + return "mpsk has been added." + + +api.add_resource(MPSKSClientAddResource, "/user//add-mpsk") + + +class MPSKSClientDeleteResource(Resource): + @use_kwargs( + { + "password": fields.Str(required=True), + }, + location="form", + ) + def post(self, user_id: int, mpsk_id: int, password: str) -> ResponseReturnValue: + user = get_authenticated_user(user_id, password) + mpsk = get_mpsk_client_or_404(mpsk_id) + + if not user == mpsk.owner: + abort(401, message="You are not the owner of the mpsk.") + + mpsk_delete(mpsk) + session.session().commit() + + return "mpsk client was deleted" + + +api.add_resource(MPSKSClientDeleteResource, "/user//delete-mpsk/") + + +class MPSKSClientChangeResource(Resource): + @use_kwargs( + { + "password": fields.Str(required=True), + "mac": fields.Str(required=True), + "name": fields.Str(required=True), + }, + location="form", + ) + def post( + self, user_id: int, mpsks_id: int, password: str, mac: str, name: str + ) -> ResponseReturnValue: + user = get_authenticated_user(user_id, password) + mpsk = get_mpsk_client_or_404(mpsks_id) + + if not user == mpsk.owner: + abort(404, message=f"User {user_id} does not the mpsk client with the id {mpsks_id}") + + try: + mpsk_edit(mpsk, user, name, mac, user) + session.session.add(mpsk) + session.session.commit() + except InvalidMACAddressException: + abort(400, message="Invalid MAC address.") + except IntegrityError: + abort(400, message="Mac address is already in use.") + return "mpsk has been changed." + + +api.add_resource(MPSKSClientChangeResource, "/user//change-mpsk/") + class UserInterfaceResource(Resource): @use_kwargs( From bcf8a0d734aafde09ed238b4c10f2f18e7f2b338 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 16 Sep 2024 23:30:12 +0200 Subject: [PATCH 07/50] renaming for matching Signed-off-by: Alex --- pycroft/lib/mpsk_client.py | 12 +++---- pycroft/model/_all.py | 2 +- .../model/{mspk_client.py => mpsk_client.py} | 4 +-- pycroft/model/user.py | 4 +-- tests/factories/__init__.py | 1 + web/blueprints/mpskclient/__init__.py | 34 +++++++++---------- web/blueprints/mpskclient/tables.py | 2 +- 7 files changed, 30 insertions(+), 29 deletions(-) rename pycroft/model/{mspk_client.py => mpsk_client.py} (93%) diff --git a/pycroft/lib/mpsk_client.py b/pycroft/lib/mpsk_client.py index 43df4400b..1367f7001 100644 --- a/pycroft/lib/mpsk_client.py +++ b/pycroft/lib/mpsk_client.py @@ -1,7 +1,7 @@ # Copyright (c) 2024. The Pycroft Authors. See the AUTHORS file. # This file is part of the Pycroft project and licensed under the terms of # the Apache License, Version 2.0. See the LICENSE file for details -from pycroft.model.mspk_client import MSPKClient +from pycroft.model.mpsk_client import MPSKClient from pycroft.model.user import User from pycroft.model.session import with_transaction, session from pycroft.lib.logging import log_user_event @@ -9,7 +9,7 @@ @with_transaction -def mpsk_delete(host: MSPKClient, processor: User) -> None: +def mpsk_delete(host: MPSKClient, processor: User) -> None: message = deferred_gettext("Deleted host '{}'.").format(host.name) log_user_event(author=processor, user=host.owner, message=message.to_json()) @@ -17,7 +17,7 @@ def mpsk_delete(host: MSPKClient, processor: User) -> None: @with_transaction -def change_mac(client: MSPKClient, mac: str, processor: User) -> MSPKClient: +def change_mac(client: MPSKClient, mac: str, processor: User) -> MPSKClient: """ This method will change the mac address of the given interface to the new mac address. @@ -36,8 +36,8 @@ def change_mac(client: MSPKClient, mac: str, processor: User) -> MSPKClient: @with_transaction -def mpsk_client_create(owner: User, name: str, mac: str, processor: User) -> MSPKClient: - client = MSPKClient(name=name, owner_id=owner.id, mac=mac) +def mpsk_client_create(owner: User, name: str, mac: str, processor: User) -> MPSKClient: + client = MPSKClient(name=name, owner_id=owner.id, mac=mac) session.add(client) @@ -52,7 +52,7 @@ def mpsk_client_create(owner: User, name: str, mac: str, processor: User) -> MSP @with_transaction -def mpsk_edit(client: MSPKClient, owner: User, name: str, mac: str, processor: User) -> None: +def mpsk_edit(client: MPSKClient, owner: User, name: str, mac: str, processor: User) -> None: if client.name != name: message = deferred_gettext("Changed name of client '{}' to '{}'.").format(client.name, name) client.name = name diff --git a/pycroft/model/_all.py b/pycroft/model/_all.py index b0c9ebb4a..3286f04fd 100644 --- a/pycroft/model/_all.py +++ b/pycroft/model/_all.py @@ -16,7 +16,7 @@ from .facilities import * from .finance import * from .host import * -from .mspk_client import * +from .mpsk_client import * from .logging import * from .net import * from .port import * diff --git a/pycroft/model/mspk_client.py b/pycroft/model/mpsk_client.py similarity index 93% rename from pycroft/model/mspk_client.py rename to pycroft/model/mpsk_client.py index d85bcf79e..7066f7650 100644 --- a/pycroft/model/mspk_client.py +++ b/pycroft/model/mpsk_client.py @@ -16,14 +16,14 @@ from pycroft.model.user import User -class MSPKClient(IntegerIdModel): +class MPSKClient(IntegerIdModel): name: Mapped[str] = mapped_column(String, nullable=False) owner_id: Mapped[int | None] = mapped_column( ForeignKey(User.id, ondelete="CASCADE"), index=True ) - owner: Mapped[User] = relationship(User, back_populates="mspks") + owner: Mapped[User] = relationship(User, back_populates="mpsks") mac: Mapped[mac_address] = mapped_column(unique=True) @validates("mac") diff --git a/pycroft/model/user.py b/pycroft/model/user.py index e6daf18dd..98fb3d414 100644 --- a/pycroft/model/user.py +++ b/pycroft/model/user.py @@ -75,7 +75,7 @@ # Backrefs from .logging import LogEntry, UserLogEntry, TaskLogEntry from .host import Host - from .mspk_client import MSPKClient + from .mpsk_client import MPSKClient from .swdd import Tenancy from .task import UserTask from .traffic import TrafficVolume @@ -254,7 +254,7 @@ class User(BaseUser, UserMixin): back_populates="owner", cascade="all, delete-orphan" ) - mspks: Mapped[list[MSPKClient]] = relationship( + mpsks: Mapped[list[MPSKClient]] = relationship( back_populates="owner", cascade="all, delete-orphan" ) diff --git a/tests/factories/__init__.py b/tests/factories/__init__.py index d3fac5caf..f1053b6af 100644 --- a/tests/factories/__init__.py +++ b/tests/factories/__init__.py @@ -19,6 +19,7 @@ SwitchFactory, SwitchPortFactory, ) +from .mpsk import MPSKFactory, BareMPSKFactory from .net import SubnetFactory, VLANFactory from .property import ( PropertyGroupFactory, diff --git a/web/blueprints/mpskclient/__init__.py b/web/blueprints/mpskclient/__init__.py index b11cc8163..a72cd11a0 100644 --- a/web/blueprints/mpskclient/__init__.py +++ b/web/blueprints/mpskclient/__init__.py @@ -8,12 +8,12 @@ from pycroft.exc import PycroftException from pycroft.helpers.net import mac_regex, get_interface_manufacturer -from pycroft.lib import mpsk_client as lib_mspk +from pycroft.lib import mpsk_client as lib_mpsk from pycroft.lib.net import get_subnets_for_room from pycroft.lib.facilities import get_room from pycroft.model import session from pycroft.model.host import Host, Interface -from pycroft.model.mspk_client import MSPKClient +from pycroft.model.mpsk_client import MPSKClient from pycroft.model.user import User from web.blueprints.access import BlueprintAccess from web.blueprints.helpers.exception import abort_on_error @@ -22,18 +22,18 @@ from web.blueprints.host.forms import InterfaceForm, HostForm from web.blueprints.host.tables import InterfaceTable, HostTable, HostRow, InterfaceRow from web.blueprints.mpskclient.forms import WiFiInterfaceForm -from web.blueprints.mpskclient.tables import MSPKRow +from web.blueprints.mpskclient.tables import MPSKRow from web.table.table import TableResponse, BtnColResponse, LinkColResponse bp = Blueprint("wifi-mpsk", __name__) access = BlueprintAccess(bp, required_properties=["user_show"]) -def get_mpsk_client_or_404(mspk_id: int) -> MSPKClient: - if (mspk := session.session.get(MSPKClient, mspk_id)) is None: +def get_mpsk_client_or_404(mpsk_id: int) -> MPSKClient: + if (mpsk := session.session.get(MPSKClient, mpsk_id)) is None: flash("Host existiert nicht.", "error") abort(404) - return mspk + return mpsk @bp.route("/create", methods=["GET", "POST"]) @@ -46,7 +46,7 @@ def default_response() -> ResponseReturnValue: form_args = {"form": form, "cancel_to": url_for("user.user_show", user_id=user.id)} return render_template( - "generic_form.html", page_title="MSPK Client erstellen", form_args=form_args, form=form + "generic_form.html", page_title="MPSK Client erstellen", form_args=form_args, form=form ) if not form.is_submitted(): @@ -61,13 +61,13 @@ def default_response() -> ResponseReturnValue: owner = session.session.get(User, form.owner_id.data) with abort_on_error(default_response), session.session.begin_nested(): - host = lib_mspk.mpsk_client_create( + host = lib_mpsk.mpsk_client_create( owner, form.name.data, form.mac.data, processor=current_user ) session.session.commit() flash("MPSK Client erfolgreich erstellt.", "success") - return redirect(url_for("user.user_show", user_id=host.owner.id, _anchor="mspks")) + return redirect(url_for("user.user_show", user_id=host.owner.id, _anchor="mpsks")) @bp.route("//delete", methods=["GET", "POST"]) @@ -93,11 +93,11 @@ def default_response() -> ResponseReturnValue: return default_response() with abort_on_error(default_response), session.session.begin_nested(): - lib_mspk.mpsk_delete(mpsk, current_user) + lib_mpsk.mpsk_delete(mpsk, current_user) session.session.commit() flash("MPSK Client erfolgreich gelöscht.", "success") - return redirect(url_for("user.user_show", user_id=owner.id, _anchor="mspks")) + return redirect(url_for("user.user_show", user_id=owner.id, _anchor="mpsks")) @bp.route("//edit", methods=["GET", "POST"]) @@ -118,10 +118,10 @@ def default_response() -> ResponseReturnValue: return default_response() with abort_on_error(default_response), session.session.begin_nested(): - lib_mspk.mpsk_edit(mpsk, mpsk.owner, form.name.data, form.mac.data, current_user) + lib_mpsk.mpsk_edit(mpsk, mpsk.owner, form.name.data, form.mac.data, current_user) session.session.commit() flash("MPSK Client erfolgreich bearbeitet.", "success") - return redirect(url_for("user.user_show", user_id=mpsk.owner_id, _anchor="mspks")) + return redirect(url_for("user.user_show", user_id=mpsk.owner_id, _anchor="mpsks")) @bp.route("/") @@ -129,15 +129,15 @@ def user_clients_json(user_id: int) -> ResponseReturnValue: user = get_user_or_404(user_id) # TODO: Importend when the for the returning of actual mpsk mac addresses for MPSK devices # return "" - return TableResponse[MSPKRow]( - items=[_mspk_row(mpsk, user_id) for mpsk in user.mspks] + return TableResponse[MPSKRow]( + items=[_mpsk_row(mpsk, user_id) for mpsk in user.mpsks] ).model_dump() -def _mspk_row(client, user_id: int) -> MSPKRow: +def _mpsk_row(client, user_id: int) -> MPSKRow: # println(f"{client}") # client = get_wlan_host_or_404(client) - return MSPKRow( + return MPSKRow( id=client.id, name=client.name, mac=client.mac, diff --git a/web/blueprints/mpskclient/tables.py b/web/blueprints/mpskclient/tables.py index 92d9239a5..eabc3dad0 100644 --- a/web/blueprints/mpskclient/tables.py +++ b/web/blueprints/mpskclient/tables.py @@ -42,7 +42,7 @@ def toolbar(self) -> HasDunderStr | None: return button_toolbar("Client", href) -class MSPKRow(BaseModel): +class MPSKRow(BaseModel): name: str | None = None mac: str actions: list[BtnColResponse] From adc3ce8622cb741542f72765710e1f4a8b1d6aa2 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 17 Sep 2024 17:24:49 +0200 Subject: [PATCH 08/50] added naming constraint added the naming constraint that names can not be empty or just be constructed out of witespaces Signed-off-by: Alex #744 --- pycroft/model/mpsk_client.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/pycroft/model/mpsk_client.py b/pycroft/model/mpsk_client.py index 7066f7650..e600e66aa 100644 --- a/pycroft/model/mpsk_client.py +++ b/pycroft/model/mpsk_client.py @@ -4,6 +4,7 @@ from __future__ import annotations +from packaging.utils import InvalidName from sqlalchemy import ForeignKey from sqlalchemy.orm import relationship, validates, Mapped, mapped_column from sqlalchemy.types import String @@ -21,7 +22,7 @@ class MPSKClient(IntegerIdModel): name: Mapped[str] = mapped_column(String, nullable=False) owner_id: Mapped[int | None] = mapped_column( - ForeignKey(User.id, ondelete="CASCADE"), index=True + ForeignKey(User.id, ondelete="CASCADE"), index=True, nullable=False ) owner: Mapped[User] = relationship(User, back_populates="mpsks") mac: Mapped[mac_address] = mapped_column(unique=True) @@ -34,3 +35,10 @@ def validate_mac(self, _, mac_address): if int(mac_address[0:2], base=16) & 1: raise MulticastFlagException("Multicast bit set in MAC address") return mac_address + + @validates("name") + def validate_name(self, _, name): + if len(name.replace(" ", "")) == 0: + raise InvalidName("Name cannot be empty") + + return name From a784c5eb8c672d66a30369ebf9bc1eae20f29cb7 Mon Sep 17 00:00:00 2001 From: Alex Date: Tue, 17 Sep 2024 17:26:59 +0200 Subject: [PATCH 09/50] added model test for mpsk added tests for the model of mpsk Signed-off-by: Alex #744 --- tests/factories/mpsk.py | 26 +++++++++++++ tests/model/test_mpsk.py | 81 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+) create mode 100644 tests/factories/mpsk.py create mode 100644 tests/model/test_mpsk.py diff --git a/tests/factories/mpsk.py b/tests/factories/mpsk.py new file mode 100644 index 000000000..9c3357d1c --- /dev/null +++ b/tests/factories/mpsk.py @@ -0,0 +1,26 @@ +# Copyright (c) 2024. The Pycroft Authors. See the AUTHORS file. +# This file is part of the Pycroft project and licensed under the terms of +# the Apache License, Version 2.0. See the LICENSE file for details +import factory +from tests.factories.base import BaseFactory +from pycroft.model.mpsk_client import MPSKClient + +from tests.factories.user import UserFactory +from tests.factories.host import UnicastMacProvider + +factory.Faker.add_provider(UnicastMacProvider) + + +class BareMPSKFactory(BaseFactory): + """A host without owner or interface.""" + + class Meta: + model = MPSKClient + + mac = factory.Faker("unicast_mac_address") + name = factory.Faker("name") + + +class MPSKFactory(BareMPSKFactory): + + owner = factory.SubFactory(UserFactory) diff --git a/tests/model/test_mpsk.py b/tests/model/test_mpsk.py new file mode 100644 index 000000000..5251e5794 --- /dev/null +++ b/tests/model/test_mpsk.py @@ -0,0 +1,81 @@ +# Copyright (c) 2024. The Pycroft Authors. See the AUTHORS file. +# This file is part of the Pycroft project and licensed under the terms of +# the Apache License, Version 2.0. See the LICENSE file for details + +import pytest +from packaging.utils import InvalidName +from sqlalchemy.exc import IntegrityError +from sqlalchemy.orm.base import object_state + +from pycroft.model.mpsk_client import MPSKClient +from pycroft.model.types import InvalidMACAddressException +from .. import factories + + +class TestMPSKValidators: + + @pytest.fixture(scope="class") + def user(self, class_session): + user = factories.UserFactory.build(with_host=True) + class_session.add(user) + return user + + @pytest.fixture(scope="class") + def mpsk_client(self, user): + return factories.MPSKFactory() + + @pytest.mark.parametrize( + "mac", + [ + "ff:ff:ff:ff:ff", + "ff:ff:ff:ff:ff:ff", + "ff:asjfjsdaf:ff:ff:ff:ff", + "aj:00:ff:1f:ff:ff", + "ff:ff:ff:ff:ff:ff:ff", + ], + ) + def test_bad_macs(self, session, user, mac): + + mpsk_client = MPSKClient( + name="the needs of the many outweigh the needs of the few", owner=user + ) + assert object_state(mpsk_client).transient + with pytest.raises(InvalidMACAddressException): + mpsk_client.mac = mac + with pytest.raises(IntegrityError): + session.add(mpsk_client) + session.flush() + + def test_no_name(self, session, user): + mpsk_client = MPSKClient(mac="00:00:00:00:00:00", owner=user) + with pytest.raises(IntegrityError): + session.add(mpsk_client) + session.flush() + with pytest.raises(InvalidName): + MPSKClient(mac="00:00:00:00:00:00", name="", owner=user) + + def test_no_mac(self, session, user): + mpsk_client = MPSKClient(name="00:00:00:00:00:00", owner=user) + with pytest.raises(IntegrityError): + session.add(mpsk_client) + session.flush() + + def test_no_owner(self, session, user): + mpsk_client = MPSKClient(mac="00:00:00:00:00:00", name="as") + with pytest.raises(IntegrityError): + session.add(mpsk_client) + session.flush() + + @pytest.mark.parametrize( + "name", + [ + " a ", + " a", + "ff:asjfjsdaf:ff:ff:ff:ff", + "aj:00:ff:1f:ff:ff", + "ff:ff:ff:ff:ff:ff:ff", + ], + ) + def test_names(self, session, user, name): + mpsk_client = MPSKClient(mac="00:00:00:00:00:00", name=name, owner=user) + assert mpsk_client.name == name From cbcf2c4387c1c29bbfab34d674d6193f6146f187 Mon Sep 17 00:00:00 2001 From: Alex <61407455+agmes4@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:20:00 +0200 Subject: [PATCH 10/50] Update pycroft/model/mpsk_client.py Co-authored-by: Lukas Juhrich --- pycroft/model/mpsk_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pycroft/model/mpsk_client.py b/pycroft/model/mpsk_client.py index e600e66aa..ad028ec75 100644 --- a/pycroft/model/mpsk_client.py +++ b/pycroft/model/mpsk_client.py @@ -31,7 +31,7 @@ class MPSKClient(IntegerIdModel): def validate_mac(self, _, mac_address): match = mac_regex.match(mac_address) if not match: - raise InvalidMACAddressException("MAC address '" + mac_address + "' is not valid") + raise InvalidMACAddressException(f"MAC address {mac_address!r} is not valid") if int(mac_address[0:2], base=16) & 1: raise MulticastFlagException("Multicast bit set in MAC address") return mac_address From f7df0d364b1cdd8f6e4cc9d35d22f0a11fd76ada Mon Sep 17 00:00:00 2001 From: Alex <61407455+agmes4@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:20:44 +0200 Subject: [PATCH 11/50] Update pycroft/model/mpsk_client.py Co-authored-by: Lukas Juhrich --- pycroft/model/mpsk_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pycroft/model/mpsk_client.py b/pycroft/model/mpsk_client.py index ad028ec75..4e41ea6a7 100644 --- a/pycroft/model/mpsk_client.py +++ b/pycroft/model/mpsk_client.py @@ -38,7 +38,7 @@ def validate_mac(self, _, mac_address): @validates("name") def validate_name(self, _, name): - if len(name.replace(" ", "")) == 0: + if name.strip(): raise InvalidName("Name cannot be empty") return name From 6e9c980be47cfccb8cb04792d7745cfd27e02ee0 Mon Sep 17 00:00:00 2001 From: Alex <61407455+agmes4@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:30:20 +0200 Subject: [PATCH 12/50] Update web/api/v0/__init__.py Co-authored-by: Lukas Juhrich --- web/api/v0/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index 67a3370b1..15119645c 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -408,7 +408,7 @@ def post( user = get_authenticated_user(user_id, password) mpsk = get_mpsk_client_or_404(mpsks_id) - if not user == mpsk.owner: + if user != mpsk.owner: abort(404, message=f"User {user_id} does not the mpsk client with the id {mpsks_id}") try: From c0093d1eee8de12370b2c3a0eb9ffbe4875a82ed Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 18 Sep 2024 17:28:56 +0200 Subject: [PATCH 13/50] fixed suggestion there was a small error which just checked rather Signed-off-by: Alex #744 --- pycroft/model/mpsk_client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pycroft/model/mpsk_client.py b/pycroft/model/mpsk_client.py index 4e41ea6a7..f2ac994f2 100644 --- a/pycroft/model/mpsk_client.py +++ b/pycroft/model/mpsk_client.py @@ -38,7 +38,7 @@ def validate_mac(self, _, mac_address): @validates("name") def validate_name(self, _, name): - if name.strip(): + if name.strip() == "": raise InvalidName("Name cannot be empty") return name From 65ca87fc49abfb0c3133e50b78848775d5f63dfc Mon Sep 17 00:00:00 2001 From: Alex <61407455+agmes4@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:36:12 +0200 Subject: [PATCH 14/50] Update web/api/v0/__init__.py Co-authored-by: Lukas Juhrich --- web/api/v0/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index 15119645c..94b5b59b5 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -385,7 +385,7 @@ def post(self, user_id: int, mpsk_id: int, password: str) -> ResponseReturnValue abort(401, message="You are not the owner of the mpsk.") mpsk_delete(mpsk) - session.session().commit() + session.session.commit() return "mpsk client was deleted" From 77bdef1cbc51dc104943ccc1d2155fb4df83ee82 Mon Sep 17 00:00:00 2001 From: Alex <61407455+agmes4@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:36:21 +0200 Subject: [PATCH 15/50] Update web/blueprints/mpskclient/__init__.py Co-authored-by: Lukas Juhrich --- web/blueprints/mpskclient/__init__.py | 1 - 1 file changed, 1 deletion(-) diff --git a/web/blueprints/mpskclient/__init__.py b/web/blueprints/mpskclient/__init__.py index a72cd11a0..a27f80f10 100644 --- a/web/blueprints/mpskclient/__init__.py +++ b/web/blueprints/mpskclient/__init__.py @@ -60,7 +60,6 @@ def default_response() -> ResponseReturnValue: # TODO can't we provide an attribute for that on the form? owner = session.session.get(User, form.owner_id.data) with abort_on_error(default_response), session.session.begin_nested(): - host = lib_mpsk.mpsk_client_create( owner, form.name.data, form.mac.data, processor=current_user ) From a7c17e34dba8b177f7e2809f2cab670de5d823ed Mon Sep 17 00:00:00 2001 From: Alex <61407455+agmes4@users.noreply.github.com> Date: Wed, 18 Sep 2024 17:36:41 +0200 Subject: [PATCH 16/50] Update tests/model/test_mpsk.py Co-authored-by: Lukas Juhrich --- tests/model/test_mpsk.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/model/test_mpsk.py b/tests/model/test_mpsk.py index 5251e5794..85f7a46cc 100644 --- a/tests/model/test_mpsk.py +++ b/tests/model/test_mpsk.py @@ -35,7 +35,6 @@ def mpsk_client(self, user): ], ) def test_bad_macs(self, session, user, mac): - mpsk_client = MPSKClient( name="the needs of the many outweigh the needs of the few", owner=user ) From 9eefa0ae23140fd6c88bd8a1b6b41c830f8629c6 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 18 Sep 2024 17:52:27 +0200 Subject: [PATCH 17/50] removed decorator removed the old decorator Signed-off-by: Alex #744 --- pycroft/lib/mpsk_client.py | 6 +----- web/blueprints/mpskclient/__init__.py | 5 ++--- 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/pycroft/lib/mpsk_client.py b/pycroft/lib/mpsk_client.py index 1367f7001..ed359d5a6 100644 --- a/pycroft/lib/mpsk_client.py +++ b/pycroft/lib/mpsk_client.py @@ -3,12 +3,11 @@ # the Apache License, Version 2.0. See the LICENSE file for details from pycroft.model.mpsk_client import MPSKClient from pycroft.model.user import User -from pycroft.model.session import with_transaction, session +from pycroft.model.session import session from pycroft.lib.logging import log_user_event from pycroft.helpers.i18n import deferred_gettext -@with_transaction def mpsk_delete(host: MPSKClient, processor: User) -> None: message = deferred_gettext("Deleted host '{}'.").format(host.name) log_user_event(author=processor, user=host.owner, message=message.to_json()) @@ -16,7 +15,6 @@ def mpsk_delete(host: MPSKClient, processor: User) -> None: session.delete(host) -@with_transaction def change_mac(client: MPSKClient, mac: str, processor: User) -> MPSKClient: """ This method will change the mac address of the given interface to the new @@ -35,7 +33,6 @@ def change_mac(client: MPSKClient, mac: str, processor: User) -> MPSKClient: return client -@with_transaction def mpsk_client_create(owner: User, name: str, mac: str, processor: User) -> MPSKClient: client = MPSKClient(name=name, owner_id=owner.id, mac=mac) @@ -51,7 +48,6 @@ def mpsk_client_create(owner: User, name: str, mac: str, processor: User) -> MPS return client -@with_transaction def mpsk_edit(client: MPSKClient, owner: User, name: str, mac: str, processor: User) -> None: if client.name != name: message = deferred_gettext("Changed name of client '{}' to '{}'.").format(client.name, name) diff --git a/web/blueprints/mpskclient/__init__.py b/web/blueprints/mpskclient/__init__.py index a27f80f10..996a118d3 100644 --- a/web/blueprints/mpskclient/__init__.py +++ b/web/blueprints/mpskclient/__init__.py @@ -118,6 +118,7 @@ def default_response() -> ResponseReturnValue: with abort_on_error(default_response), session.session.begin_nested(): lib_mpsk.mpsk_edit(mpsk, mpsk.owner, form.name.data, form.mac.data, current_user) + session.session.add(mpsk) session.session.commit() flash("MPSK Client erfolgreich bearbeitet.", "success") return redirect(url_for("user.user_show", user_id=mpsk.owner_id, _anchor="mpsks")) @@ -133,9 +134,7 @@ def user_clients_json(user_id: int) -> ResponseReturnValue: ).model_dump() -def _mpsk_row(client, user_id: int) -> MPSKRow: - # println(f"{client}") - # client = get_wlan_host_or_404(client) +def _mpsk_row(client: MPSKClient, user_id: int) -> MPSKRow: return MPSKRow( id=client.id, name=client.name, From d1689b282f68f96ad164a7e5bb41c3edcacf48dd Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 18 Sep 2024 20:59:27 +0200 Subject: [PATCH 18/50] refactoring test Signed-off-by: Alex #744 --- tests/model/test_mpsk.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/tests/model/test_mpsk.py b/tests/model/test_mpsk.py index 85f7a46cc..c26057dbd 100644 --- a/tests/model/test_mpsk.py +++ b/tests/model/test_mpsk.py @@ -14,16 +14,6 @@ class TestMPSKValidators: - @pytest.fixture(scope="class") - def user(self, class_session): - user = factories.UserFactory.build(with_host=True) - class_session.add(user) - return user - - @pytest.fixture(scope="class") - def mpsk_client(self, user): - return factories.MPSKFactory() - @pytest.mark.parametrize( "mac", [ @@ -78,3 +68,13 @@ def test_no_owner(self, session, user): def test_names(self, session, user, name): mpsk_client = MPSKClient(mac="00:00:00:00:00:00", name=name, owner=user) assert mpsk_client.name == name + + @pytest.fixture(scope="class") + def user(self, class_session): + user = factories.UserFactory.build(with_host=True) + class_session.add(user) + return user + + @pytest.fixture(scope="class") + def mpsk_client(self): + return factories.MPSKFactory() From dc42b96ffae37668b4b4d0d337de65294c41682d Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 18 Sep 2024 21:08:58 +0200 Subject: [PATCH 19/50] fixed API mpsk delete added the user who deleted the mpsk client as processor Signed-off-by: Alex #744 --- web/api/v0/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index 94b5b59b5..a2031ecbe 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -384,7 +384,7 @@ def post(self, user_id: int, mpsk_id: int, password: str) -> ResponseReturnValue if not user == mpsk.owner: abort(401, message="You are not the owner of the mpsk.") - mpsk_delete(mpsk) + mpsk_delete(mpsk, user) session.session.commit() return "mpsk client was deleted" From ec21f158bc900b610412a8cf752e7b9fcf644935 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 19 Sep 2024 10:26:46 +0200 Subject: [PATCH 20/50] added error when exceeding max amount MPSKS added an error indecating the maximum amount of clients was exceeded added a check for the client to exceed the maximum number of mpsks devices Signed-off-by: Alex #744 --- pycroft/lib/mpsk_client.py | 7 +++++++ pycroft/model/types.py | 14 ++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/pycroft/lib/mpsk_client.py b/pycroft/lib/mpsk_client.py index ed359d5a6..b223625ec 100644 --- a/pycroft/lib/mpsk_client.py +++ b/pycroft/lib/mpsk_client.py @@ -2,6 +2,7 @@ # This file is part of the Pycroft project and licensed under the terms of # the Apache License, Version 2.0. See the LICENSE file for details from pycroft.model.mpsk_client import MPSKClient +from pycroft.model.types import AmountExceededError from pycroft.model.user import User from pycroft.model.session import session from pycroft.lib.logging import log_user_event @@ -34,6 +35,12 @@ def change_mac(client: MPSKClient, mac: str, processor: User) -> MPSKClient: def mpsk_client_create(owner: User, name: str, mac: str, processor: User) -> MPSKClient: + + if len(owner.mpsks) >= 10: + raise AmountExceededError( + "the limit of added mpsks clients is exceeded", limit=10, actual=len(owner.mpsks) + ) + client = MPSKClient(name=name, owner_id=owner.id, mac=mac) session.add(client) diff --git a/pycroft/model/types.py b/pycroft/model/types.py index 88cbcd6ad..0e5398e9e 100644 --- a/pycroft/model/types.py +++ b/pycroft/model/types.py @@ -216,6 +216,20 @@ class InvalidMACAddressException(PycroftModelException, ValueError): pass +class AmountExceededError(PycroftModelException): + + def __init__(self, message="Amount exceeded the allowed limit", limit=None, actual=None): + self.message = message + self.limit = limit + self.actual = actual + super().__init__(self.message) + + def __str__(self): + if self.limit and self.actual: + return f"{self.message}. Limit: {self.limit}, Actual: {self.actual}" + return self.message + + class DateTimeTz(DateTime): """A sqlalchemy type decorator corresponding to `datetime` types with time zone. From f30891dcd8bbba9716b3fd9ddd435453f0166430 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 19 Sep 2024 10:27:49 +0200 Subject: [PATCH 21/50] checking for naming error and amount exceeded checking for amount exceeded error as well as naming error in MPSKS clients Signed-off-by: Alex #744 --- web/api/v0/__init__.py | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index a2031ecbe..d795231b6 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -7,6 +7,7 @@ from flask import jsonify, current_app, Response from flask.typing import ResponseReturnValue from flask_restful import Api, Resource as FlaskRestfulResource, abort +from packaging.utils import InvalidName from sqlalchemy.exc import IntegrityError from sqlalchemy import select from sqlalchemy.orm import joinedload, selectinload, undefer, with_polymorphic @@ -56,7 +57,7 @@ from pycroft.model.host import IP, Interface, Host from pycroft.model.session import current_timestamp from pycroft.model.task import Task -from pycroft.model.types import IPAddress, InvalidMACAddressException +from pycroft.model.types import IPAddress, InvalidMACAddressException, AmountExceededError from pycroft.model.user import User, IllegalEmailError, IllegalLoginError from web.blueprints.mpskclient import get_mpsk_client_or_404 @@ -364,6 +365,10 @@ def post(self, user_id: int, password: str, mac: str, name: str) -> ResponseRetu abort(400, message="Invalid MAC address.") except IntegrityError: abort(400, message="Mac address is already in use.") + except InvalidName: + abort(400, message="No proper name was provided.") + except AmountExceededError: + abort(400, message="User has the maximum count of mpsk clients.") return "mpsk has been added." @@ -419,6 +424,8 @@ def post( abort(400, message="Invalid MAC address.") except IntegrityError: abort(400, message="Mac address is already in use.") + except InvalidName: + abort(400, message="No proper name was provided.") return "mpsk has been changed." From b60a881ca51ad246164104ee428bc3814cb251df Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 19 Sep 2024 10:28:51 +0200 Subject: [PATCH 22/50] added test for exceeded clients added a test for too much mpsks clients added Signed-off-by: Alex #744 --- tests/model/test_mpsk.py | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tests/model/test_mpsk.py b/tests/model/test_mpsk.py index c26057dbd..6a0a3371f 100644 --- a/tests/model/test_mpsk.py +++ b/tests/model/test_mpsk.py @@ -7,8 +7,9 @@ from sqlalchemy.exc import IntegrityError from sqlalchemy.orm.base import object_state +from pycroft.lib.mpsk_client import mpsk_client_create from pycroft.model.mpsk_client import MPSKClient -from pycroft.model.types import InvalidMACAddressException +from pycroft.model.types import InvalidMACAddressException, AmountExceededError from .. import factories @@ -69,6 +70,20 @@ def test_names(self, session, user, name): mpsk_client = MPSKClient(mac="00:00:00:00:00:00", name=name, owner=user) assert mpsk_client.name == name + def test_exceeds_max(self, session, user): + mac = "00:00:00:00:00:0" + for i in range(10): + mac_client = mac + hex(i)[2:] + c = mpsk_client_create(user, "Hallo", mac_client, user) + user.mpsks.append(c) + session.flush() + assert len(user.mpsks) == i + 1 + + for i in range(10, 15): + mac_client = mac + hex(i)[2:] + with pytest.raises(AmountExceededError): + c = mpsk_client_create(user, "Hallo", mac_client, user) + @pytest.fixture(scope="class") def user(self, class_session): user = factories.UserFactory.build(with_host=True) From 9475c515e7cce334108b839c4d791b97d4a7fd73 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 19 Sep 2024 11:23:48 +0200 Subject: [PATCH 23/50] added key arg creat mpsks client added key arg which is for determining rather the amount of mpsks clients should be validated for a amount Signed-off-by: Alex #744 --- pycroft/lib/mpsk_client.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pycroft/lib/mpsk_client.py b/pycroft/lib/mpsk_client.py index b223625ec..ae64e2b42 100644 --- a/pycroft/lib/mpsk_client.py +++ b/pycroft/lib/mpsk_client.py @@ -18,10 +18,10 @@ def mpsk_delete(host: MPSKClient, processor: User) -> None: def change_mac(client: MPSKClient, mac: str, processor: User) -> MPSKClient: """ - This method will change the mac address of the given interface to the new + This method will change the mac address of the given mpsks client to the new mac address. - :param interface: the interface which should become a new mac address. + :param client: the mpsks which should become a new mac address. :param mac: the new mac address. :param processor: the user who initiated the mac address change. :return: the changed interface with the new mac address. @@ -34,9 +34,17 @@ def change_mac(client: MPSKClient, mac: str, processor: User) -> MPSKClient: return client -def mpsk_client_create(owner: User, name: str, mac: str, processor: User) -> MPSKClient: +def mpsk_client_create(owner: User, name: str, mac: str, processor: User, api=False) -> MPSKClient: + """ + creates a mpsks client for a given user with a mac address. - if len(owner.mpsks) >= 10: + :param owner: the user who initiated the mac address change. + :param name: the name of the mpsks client. + :param mac: the new mac address. + :param processor: the user who initiated the mac address change. + :param api: whether to create an api client or not. If set Ture checks rather a user exceeds the maximum of clients (set to 10). + """ + if len(owner.mpsks) >= 10 and api: raise AmountExceededError( "the limit of added mpsks clients is exceeded", limit=10, actual=len(owner.mpsks) ) From 6cff7d1ecdf0df4174d250e17a40c4d0ee0f88c3 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 19 Sep 2024 11:24:51 +0200 Subject: [PATCH 24/50] Test MPSK: added test exceed admin added test for clients exceeding admin fixed test for api call Signed-off-by: Alex #744 --- tests/model/test_mpsk.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/model/test_mpsk.py b/tests/model/test_mpsk.py index 6a0a3371f..c336b02f7 100644 --- a/tests/model/test_mpsk.py +++ b/tests/model/test_mpsk.py @@ -70,11 +70,11 @@ def test_names(self, session, user, name): mpsk_client = MPSKClient(mac="00:00:00:00:00:00", name=name, owner=user) assert mpsk_client.name == name - def test_exceeds_max(self, session, user): + def test_exceeds_max_api(self, session, user): mac = "00:00:00:00:00:0" for i in range(10): mac_client = mac + hex(i)[2:] - c = mpsk_client_create(user, "Hallo", mac_client, user) + c = mpsk_client_create(user, "Hallo", mac_client, user, api=True) user.mpsks.append(c) session.flush() assert len(user.mpsks) == i + 1 @@ -82,7 +82,16 @@ def test_exceeds_max(self, session, user): for i in range(10, 15): mac_client = mac + hex(i)[2:] with pytest.raises(AmountExceededError): - c = mpsk_client_create(user, "Hallo", mac_client, user) + c = mpsk_client_create(user, "Hallo", mac_client, user, api=True) + + def test_admin_exceeds(self, session, user): + mac = "00:00:00:00:00:0" + for i in range(15): + mac_client = mac + hex(i)[2:] + c = mpsk_client_create(user, "Hallo", mac_client, user, api=False) + user.mpsks.append(c) + session.flush() + assert len(user.mpsks) == i + 1 @pytest.fixture(scope="class") def user(self, class_session): From ab23c4489616cd551cb2a1be7d84c52c3cce3b6e Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 19 Sep 2024 11:25:56 +0200 Subject: [PATCH 25/50] changed Create MPSK changed the creat mpsks so that it now validates for the length of added clients Signed-off-by: Alex #744 --- web/api/v0/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index d795231b6..42a590156 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -359,7 +359,7 @@ def post(self, user_id: int, password: str, mac: str, name: str) -> ResponseRetu user = get_authenticated_user(user_id, password) try: - mpsk_client_create(user, mac, name, user) + mpsk_client_create(user, mac, name, user, api=True) session.session.commit() except InvalidMACAddressException: abort(400, message="Invalid MAC address.") From 3cafd5cb4f1374a007c1d8a660e8a4f2c4453d95 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 19 Sep 2024 14:08:48 +0200 Subject: [PATCH 26/50] api mpsk api more clients the api is now able to add more clients Signed-off-by: Alex #744 --- pycroft/lib/mpsk_client.py | 4 ++-- tests/model/test_mpsk.py | 34 +++++++++++++++++----------------- 2 files changed, 19 insertions(+), 19 deletions(-) diff --git a/pycroft/lib/mpsk_client.py b/pycroft/lib/mpsk_client.py index ae64e2b42..ca29f365c 100644 --- a/pycroft/lib/mpsk_client.py +++ b/pycroft/lib/mpsk_client.py @@ -44,9 +44,9 @@ def mpsk_client_create(owner: User, name: str, mac: str, processor: User, api=Fa :param processor: the user who initiated the mac address change. :param api: whether to create an api client or not. If set Ture checks rather a user exceeds the maximum of clients (set to 10). """ - if len(owner.mpsks) >= 10 and api: + if len(owner.mpsks) >= 30 and api: raise AmountExceededError( - "the limit of added mpsks clients is exceeded", limit=10, actual=len(owner.mpsks) + "the limit of added mpsks clients is exceeded", limit=30, actual=len(owner.mpsks) ) client = MPSKClient(name=name, owner_id=owner.id, mac=mac) diff --git a/tests/model/test_mpsk.py b/tests/model/test_mpsk.py index c336b02f7..3d361439d 100644 --- a/tests/model/test_mpsk.py +++ b/tests/model/test_mpsk.py @@ -71,27 +71,27 @@ def test_names(self, session, user, name): assert mpsk_client.name == name def test_exceeds_max_api(self, session, user): - mac = "00:00:00:00:00:0" - for i in range(10): - mac_client = mac + hex(i)[2:] - c = mpsk_client_create(user, "Hallo", mac_client, user, api=True) - user.mpsks.append(c) - session.flush() - assert len(user.mpsks) == i + 1 + mac = "00:00:00:00:00:" + for j in range(1, 4): + for i in range(10): + mac_client = mac + hex(j)[2:] + hex(i)[2:] + c = mpsk_client_create(user, "Hallo", mac_client, user, api=True) + user.mpsks.append(c) + session.flush() - for i in range(10, 15): - mac_client = mac + hex(i)[2:] + for i in range(15): + mac_client = mac + "0" + hex(i)[2:] with pytest.raises(AmountExceededError): - c = mpsk_client_create(user, "Hallo", mac_client, user, api=True) + mpsk_client_create(user, "Hallo", mac_client, user, api=True) def test_admin_exceeds(self, session, user): - mac = "00:00:00:00:00:0" - for i in range(15): - mac_client = mac + hex(i)[2:] - c = mpsk_client_create(user, "Hallo", mac_client, user, api=False) - user.mpsks.append(c) - session.flush() - assert len(user.mpsks) == i + 1 + mac = "00:00:00:00:00:" + for j in range(0, 4): + for i in range(15): + mac_client = mac + hex(j)[2:] + hex(i)[2:] + c = mpsk_client_create(user, "Hallo", mac_client, user) + user.mpsks.append(c) + session.flush() @pytest.fixture(scope="class") def user(self, class_session): From cda3369ab5d36d62acbc0be545b1827db30a0354 Mon Sep 17 00:00:00 2001 From: Alex <61407455+agmes4@users.noreply.github.com> Date: Thu, 19 Sep 2024 14:16:11 +0200 Subject: [PATCH 27/50] Update web/api/v0/__init__.py Co-authored-by: Lukas Juhrich --- web/api/v0/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index 42a590156..edfcc6605 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -414,7 +414,7 @@ def post( mpsk = get_mpsk_client_or_404(mpsks_id) if user != mpsk.owner: - abort(404, message=f"User {user_id} does not the mpsk client with the id {mpsks_id}") + abort(404, message=f"User {user_id} does not own the mpsk client with the id {mpsks_id}") try: mpsk_edit(mpsk, user, name, mac, user) From 22dc893e8c8cea9d6d1d7bb9d7c500ae0f6980a1 Mon Sep 17 00:00:00 2001 From: Alex Date: Thu, 19 Sep 2024 14:42:36 +0200 Subject: [PATCH 28/50] fixed typo Signed-off-by: Alex #744 --- web/templates/user/user_show.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/templates/user/user_show.html b/web/templates/user/user_show.html index 3dbec4439..a299e1905 100644 --- a/web/templates/user/user_show.html +++ b/web/templates/user/user_show.html @@ -19,7 +19,7 @@ { 'id': 'mpsk', 'icon': 'fa-tablet', - 'name': 'MSPK Clients', + 'name': 'MPSK Clients', 'badge': user.tasks | length, }, { From 99c47afbd3cbd58ac87973197e2d71431d22f83a Mon Sep 17 00:00:00 2001 From: Alex Date: Sat, 21 Sep 2024 16:43:11 +0200 Subject: [PATCH 29/50] lib mpsks added session as parameter added the db session now as parameter removed kargs for determining api calls moved check for number of mpsks clients to api Signed-off-by: Alex #744 --- pycroft/lib/mpsk_client.py | 27 ++++++++++++--------------- tests/model/test_mpsk.py | 18 ++---------------- web/api/v0/__init__.py | 18 +++++++++++++----- web/blueprints/mpskclient/__init__.py | 8 +++++--- 4 files changed, 32 insertions(+), 39 deletions(-) diff --git a/pycroft/lib/mpsk_client.py b/pycroft/lib/mpsk_client.py index ca29f365c..09f99cc46 100644 --- a/pycroft/lib/mpsk_client.py +++ b/pycroft/lib/mpsk_client.py @@ -2,21 +2,19 @@ # This file is part of the Pycroft project and licensed under the terms of # the Apache License, Version 2.0. See the LICENSE file for details from pycroft.model.mpsk_client import MPSKClient -from pycroft.model.types import AmountExceededError from pycroft.model.user import User -from pycroft.model.session import session from pycroft.lib.logging import log_user_event from pycroft.helpers.i18n import deferred_gettext -def mpsk_delete(host: MPSKClient, processor: User) -> None: - message = deferred_gettext("Deleted host '{}'.").format(host.name) - log_user_event(author=processor, user=host.owner, message=message.to_json()) +def mpsk_delete(session, mpsk_client: MPSKClient, processor: User) -> None: + message = deferred_gettext("Deleted mpsk client '{}'.").format(mpsk_client.name) + log_user_event(author=processor, user=mpsk_client.owner, message=message.to_json()) - session.delete(host) + session.delete(mpsk_client) -def change_mac(client: MPSKClient, mac: str, processor: User) -> MPSKClient: +def change_mac(session, client: MPSKClient, mac: str, processor: User) -> MPSKClient: """ This method will change the mac address of the given mpsks client to the new mac address. @@ -31,24 +29,20 @@ def change_mac(client: MPSKClient, mac: str, processor: User) -> MPSKClient: message = deferred_gettext("Changed MAC address from {} to {}.").format(old_mac, mac) if client.owner: log_user_event(message.to_json(), processor, client.owner) + session.add(client) return client -def mpsk_client_create(owner: User, name: str, mac: str, processor: User, api=False) -> MPSKClient: +def mpsk_client_create(session, owner: User, name: str, mac: str, processor: User) -> MPSKClient: """ creates a mpsks client for a given user with a mac address. + :param session: session to use with the database. :param owner: the user who initiated the mac address change. :param name: the name of the mpsks client. :param mac: the new mac address. :param processor: the user who initiated the mac address change. - :param api: whether to create an api client or not. If set Ture checks rather a user exceeds the maximum of clients (set to 10). """ - if len(owner.mpsks) >= 30 and api: - raise AmountExceededError( - "the limit of added mpsks clients is exceeded", limit=30, actual=len(owner.mpsks) - ) - client = MPSKClient(name=name, owner_id=owner.id, mac=mac) session.add(client) @@ -63,7 +57,9 @@ def mpsk_client_create(owner: User, name: str, mac: str, processor: User, api=Fa return client -def mpsk_edit(client: MPSKClient, owner: User, name: str, mac: str, processor: User) -> None: +def mpsk_edit( + session, client: MPSKClient, owner: User, name: str, mac: str, processor: User +) -> None: if client.name != name: message = deferred_gettext("Changed name of client '{}' to '{}'.").format(client.name, name) client.name = name @@ -87,3 +83,4 @@ def mpsk_edit(client: MPSKClient, owner: User, name: str, mac: str, processor: U ) log_user_event(author=processor, user=owner, message=message.to_json()) client.mac = mac + session.add(client) diff --git a/tests/model/test_mpsk.py b/tests/model/test_mpsk.py index 3d361439d..b28111984 100644 --- a/tests/model/test_mpsk.py +++ b/tests/model/test_mpsk.py @@ -9,7 +9,7 @@ from pycroft.lib.mpsk_client import mpsk_client_create from pycroft.model.mpsk_client import MPSKClient -from pycroft.model.types import InvalidMACAddressException, AmountExceededError +from pycroft.model.types import InvalidMACAddressException from .. import factories @@ -70,26 +70,12 @@ def test_names(self, session, user, name): mpsk_client = MPSKClient(mac="00:00:00:00:00:00", name=name, owner=user) assert mpsk_client.name == name - def test_exceeds_max_api(self, session, user): - mac = "00:00:00:00:00:" - for j in range(1, 4): - for i in range(10): - mac_client = mac + hex(j)[2:] + hex(i)[2:] - c = mpsk_client_create(user, "Hallo", mac_client, user, api=True) - user.mpsks.append(c) - session.flush() - - for i in range(15): - mac_client = mac + "0" + hex(i)[2:] - with pytest.raises(AmountExceededError): - mpsk_client_create(user, "Hallo", mac_client, user, api=True) - def test_admin_exceeds(self, session, user): mac = "00:00:00:00:00:" for j in range(0, 4): for i in range(15): mac_client = mac + hex(j)[2:] + hex(i)[2:] - c = mpsk_client_create(user, "Hallo", mac_client, user) + c = mpsk_client_create(session, user, "Hallo", mac_client, user) user.mpsks.append(c) session.flush() diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index edfcc6605..607ac8f65 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -359,7 +359,14 @@ def post(self, user_id: int, password: str, mac: str, name: str) -> ResponseRetu user = get_authenticated_user(user_id, password) try: - mpsk_client_create(user, mac, name, user, api=True) + # checks rather the user has all settable mpsks clients created + if len(user.mpsks) > current_app.config.get("MAX_MPSKS", 30): + abort(400, message="User has the maximum count of mpsk clients.") + + if not user.wifi_password: + abort(400, message="Legacy wifi password change of password is required.") + + mpsk_client_create(session, user, mac, name, user) session.session.commit() except InvalidMACAddressException: abort(400, message="Invalid MAC address.") @@ -389,7 +396,7 @@ def post(self, user_id: int, mpsk_id: int, password: str) -> ResponseReturnValue if not user == mpsk.owner: abort(401, message="You are not the owner of the mpsk.") - mpsk_delete(mpsk, user) + mpsk_delete(session, mpsk, user) session.session.commit() return "mpsk client was deleted" @@ -414,11 +421,12 @@ def post( mpsk = get_mpsk_client_or_404(mpsks_id) if user != mpsk.owner: - abort(404, message=f"User {user_id} does not own the mpsk client with the id {mpsks_id}") + abort( + 404, message=f"User {user_id} does not own the mpsk client with the id {mpsks_id}" + ) try: - mpsk_edit(mpsk, user, name, mac, user) - session.session.add(mpsk) + mpsk_edit(session, mpsk, user, name, mac, user) session.session.commit() except InvalidMACAddressException: abort(400, message="Invalid MAC address.") diff --git a/web/blueprints/mpskclient/__init__.py b/web/blueprints/mpskclient/__init__.py index 996a118d3..70f6a075c 100644 --- a/web/blueprints/mpskclient/__init__.py +++ b/web/blueprints/mpskclient/__init__.py @@ -61,7 +61,7 @@ def default_response() -> ResponseReturnValue: owner = session.session.get(User, form.owner_id.data) with abort_on_error(default_response), session.session.begin_nested(): host = lib_mpsk.mpsk_client_create( - owner, form.name.data, form.mac.data, processor=current_user + session.session, owner, form.name.data, form.mac.data, processor=current_user ) session.session.commit() @@ -92,7 +92,7 @@ def default_response() -> ResponseReturnValue: return default_response() with abort_on_error(default_response), session.session.begin_nested(): - lib_mpsk.mpsk_delete(mpsk, current_user) + lib_mpsk.mpsk_delete(session.session, mpsk, current_user) session.session.commit() flash("MPSK Client erfolgreich gelöscht.", "success") @@ -117,7 +117,9 @@ def default_response() -> ResponseReturnValue: return default_response() with abort_on_error(default_response), session.session.begin_nested(): - lib_mpsk.mpsk_edit(mpsk, mpsk.owner, form.name.data, form.mac.data, current_user) + lib_mpsk.mpsk_edit( + session.session, mpsk, mpsk.owner, form.name.data, form.mac.data, current_user + ) session.session.add(mpsk) session.session.commit() flash("MPSK Client erfolgreich bearbeitet.", "success") From 26ed4131040b0c73693166a1db0bae29301f7cec Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 25 Sep 2024 19:35:07 +0200 Subject: [PATCH 30/50] MPSKS Lib: added type hint session added the type hint for Session --- pycroft/lib/mpsk_client.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/pycroft/lib/mpsk_client.py b/pycroft/lib/mpsk_client.py index 09f99cc46..cb8ff5ca8 100644 --- a/pycroft/lib/mpsk_client.py +++ b/pycroft/lib/mpsk_client.py @@ -1,24 +1,27 @@ # Copyright (c) 2024. The Pycroft Authors. See the AUTHORS file. # This file is part of the Pycroft project and licensed under the terms of # the Apache License, Version 2.0. See the LICENSE file for details +from sqlalchemy.orm import Session + from pycroft.model.mpsk_client import MPSKClient from pycroft.model.user import User from pycroft.lib.logging import log_user_event from pycroft.helpers.i18n import deferred_gettext -def mpsk_delete(session, mpsk_client: MPSKClient, processor: User) -> None: +def mpsk_delete(session: Session, mpsk_client: MPSKClient, processor: User) -> None: message = deferred_gettext("Deleted mpsk client '{}'.").format(mpsk_client.name) log_user_event(author=processor, user=mpsk_client.owner, message=message.to_json()) session.delete(mpsk_client) -def change_mac(session, client: MPSKClient, mac: str, processor: User) -> MPSKClient: +def change_mac(session: Session, client: MPSKClient, mac: str, processor: User) -> MPSKClient: """ This method will change the mac address of the given mpsks client to the new mac address. + :param session: session to use with the database. :param client: the mpsks which should become a new mac address. :param mac: the new mac address. :param processor: the user who initiated the mac address change. @@ -33,7 +36,9 @@ def change_mac(session, client: MPSKClient, mac: str, processor: User) -> MPSKCl return client -def mpsk_client_create(session, owner: User, name: str, mac: str, processor: User) -> MPSKClient: +def mpsk_client_create( + session: Session, owner: User, name: str, mac: str, processor: User +) -> MPSKClient: """ creates a mpsks client for a given user with a mac address. @@ -58,7 +63,7 @@ def mpsk_client_create(session, owner: User, name: str, mac: str, processor: Use def mpsk_edit( - session, client: MPSKClient, owner: User, name: str, mac: str, processor: User + session: Session, client: MPSKClient, owner: User, name: str, mac: str, processor: User ) -> None: if client.name != name: message = deferred_gettext("Changed name of client '{}' to '{}'.").format(client.name, name) From 8f7f359da0e212f12007cf66322de7bf025cf0e1 Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 25 Sep 2024 19:39:01 +0200 Subject: [PATCH 31/50] fixed wrong type in passings fixed api wrong type --- web/api/v0/__init__.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index 607ac8f65..8c3732655 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -366,7 +366,7 @@ def post(self, user_id: int, password: str, mac: str, name: str) -> ResponseRetu if not user.wifi_password: abort(400, message="Legacy wifi password change of password is required.") - mpsk_client_create(session, user, mac, name, user) + mpsk_client_create(session.session, user, mac, name, user) session.session.commit() except InvalidMACAddressException: abort(400, message="Invalid MAC address.") @@ -396,7 +396,7 @@ def post(self, user_id: int, mpsk_id: int, password: str) -> ResponseReturnValue if not user == mpsk.owner: abort(401, message="You are not the owner of the mpsk.") - mpsk_delete(session, mpsk, user) + mpsk_delete(session.session, mpsk, user) session.session.commit() return "mpsk client was deleted" @@ -426,7 +426,7 @@ def post( ) try: - mpsk_edit(session, mpsk, user, name, mac, user) + mpsk_edit(session.session, mpsk, user, name, mac, user) session.session.commit() except InvalidMACAddressException: abort(400, message="Invalid MAC address.") From 85febde483be37943a1afc85f8a3a0a2ccbf3663 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 28 Sep 2024 12:08:28 +0200 Subject: [PATCH 32/50] schema: add `mpsk_client` --- .../versions/dda39ad43536_add_mpskclient.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 pycroft/model/alembic/versions/dda39ad43536_add_mpskclient.py diff --git a/pycroft/model/alembic/versions/dda39ad43536_add_mpskclient.py b/pycroft/model/alembic/versions/dda39ad43536_add_mpskclient.py new file mode 100644 index 000000000..e5ed10466 --- /dev/null +++ b/pycroft/model/alembic/versions/dda39ad43536_add_mpskclient.py @@ -0,0 +1,36 @@ +"""Add MPSKClient + +Revision ID: dda39ad43536 +Revises: 5234d7ac2b4a +Create Date: 2024-09-28 10:01:39.952235 + +""" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# revision identifiers, used by Alembic. +revision = "dda39ad43536" +down_revision = "5234d7ac2b4a" +branch_labels = None +depends_on = None + + +def upgrade(): + op.create_table( + "mpsk_client", + sa.Column("name", sa.String(), nullable=False), + sa.Column("owner_id", sa.Integer(), nullable=False), + sa.Column("mac", postgresql.types.MACADDR, nullable=False), + sa.Column("id", sa.Integer(), nullable=False), + sa.ForeignKeyConstraint(["owner_id"], ["user.id"], ondelete="CASCADE"), + sa.PrimaryKeyConstraint("id"), + sa.UniqueConstraint("mac"), + ) + op.create_index(op.f("ix_mpsk_client_owner_id"), "mpsk_client", ["owner_id"], unique=False) + + +def downgrade(): + op.drop_index(op.f("ix_mpsk_client_owner_id"), table_name="mpsk_client") + op.drop_table("mpsk_client") From 10002d00430ba616ced85a089f35f3f192ea3f07 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 28 Sep 2024 12:23:39 +0200 Subject: [PATCH 33/50] Remove now unused AmountExceededError Since the amount check moved from lib to API, this is not raised anymore. --- pycroft/model/types.py | 14 -------------- web/api/v0/__init__.py | 4 +--- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/pycroft/model/types.py b/pycroft/model/types.py index 0e5398e9e..88cbcd6ad 100644 --- a/pycroft/model/types.py +++ b/pycroft/model/types.py @@ -216,20 +216,6 @@ class InvalidMACAddressException(PycroftModelException, ValueError): pass -class AmountExceededError(PycroftModelException): - - def __init__(self, message="Amount exceeded the allowed limit", limit=None, actual=None): - self.message = message - self.limit = limit - self.actual = actual - super().__init__(self.message) - - def __str__(self): - if self.limit and self.actual: - return f"{self.message}. Limit: {self.limit}, Actual: {self.actual}" - return self.message - - class DateTimeTz(DateTime): """A sqlalchemy type decorator corresponding to `datetime` types with time zone. diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index 8c3732655..03feef6ed 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -57,7 +57,7 @@ from pycroft.model.host import IP, Interface, Host from pycroft.model.session import current_timestamp from pycroft.model.task import Task -from pycroft.model.types import IPAddress, InvalidMACAddressException, AmountExceededError +from pycroft.model.types import IPAddress, InvalidMACAddressException from pycroft.model.user import User, IllegalEmailError, IllegalLoginError from web.blueprints.mpskclient import get_mpsk_client_or_404 @@ -374,8 +374,6 @@ def post(self, user_id: int, password: str, mac: str, name: str) -> ResponseRetu abort(400, message="Mac address is already in use.") except InvalidName: abort(400, message="No proper name was provided.") - except AmountExceededError: - abort(400, message="User has the maximum count of mpsk clients.") return "mpsk has been added." From 62902d22363ef6b8d4ffaaa61bb38f2070d6c0cf Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 28 Sep 2024 14:55:10 +0200 Subject: [PATCH 34/50] Fix error reporting for frontend tests This uses the data attached by the webargs handler, which is responsible for throwing 422 unprocessable entity. See https://webargs.readthedocs.io/en/latest/framework_support.html#error-handling Also, note that https://github.com/pallets/flask/issues/593 is now closed, making the workaround superfluous. --- tests/frontend/assertions.py | 9 +++++++-- web/api/__init__.py | 24 +++++++++++++++++++----- web/app.py | 6 ------ 3 files changed, 26 insertions(+), 13 deletions(-) diff --git a/tests/frontend/assertions.py b/tests/frontend/assertions.py index 32abb3982..a93b298e4 100644 --- a/tests/frontend/assertions.py +++ b/tests/frontend/assertions.py @@ -41,8 +41,13 @@ def assert_url_response_code( """ __tracebackhide__ = True resp = self.open(url, method=method, **kw) - assert resp.status_code == code, \ - f"Expected url {url} to return {code}, got {resp.status_code}" + assert resp.status_code == code, "\n".join( + ( + f"Expected url {url} to return {code}, got {resp.status}.", + "Additional data:", + resp.text, + ) + ) if autoclose: resp.close() return resp diff --git a/web/api/__init__.py b/web/api/__init__.py index 7e22787b7..8a5297e06 100644 --- a/web/api/__init__.py +++ b/web/api/__init__.py @@ -1,14 +1,28 @@ from flask import Blueprint, Flask from flask.typing import ResponseReturnValue +from werkzeug.exceptions import HTTPException from . import v0 -bp = Blueprint('api', __name__) +bp = Blueprint("api", __name__) v0.api.init_app(bp) -app_for_sphinx = Flask(__name__) -app_for_sphinx.register_blueprint(bp, url_prefix="/api/v0") +@bp.errorhandler(422) +@bp.errorhandler(400) +def handle_error(err: Exception) -> ResponseReturnValue: + """Use `err.data` values from marshmallow for the response.""" + if not hasattr(err, "data") or not isinstance(err, HTTPException): + return v0.api.handle_error(err) + + assert hasattr(err, "data") + headers = err.data.get("headers", None) + messages = err.data.get("messages", ["Invalid request."]) + + if headers: + return {"errors": messages}, err.code, headers + return {"errors": messages}, err.code -def errorpage(e: Exception) -> ResponseReturnValue: - return v0.api.handle_error(e) + +app_for_sphinx = Flask(__name__) +app_for_sphinx.register_blueprint(bp, url_prefix="/api/v0") diff --git a/web/app.py b/web/app.py index 03d8b9975..e9401e0ff 100644 --- a/web/app.py +++ b/web/app.py @@ -141,12 +141,6 @@ def errorpage(e: Exception) -> ResponseReturnValue: :param e: The error from the errorhandler """ - # We need this path hard-coding because the global app errorhandlers have higher - # precedence than anything registered to a blueprint. - # A clean solution would be flask supporting nested blueprints (see flask #539) - if request.path.startswith('/api/'): - return api.errorpage(e) - code = getattr(e, "code", 500) if code == 500: From c9148d21326e1ac569bc91ee8775bc12d5a3bd09 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 28 Sep 2024 15:15:53 +0200 Subject: [PATCH 35/50] api: Make certain error messages more specific --- web/api/v0/__init__.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index 03feef6ed..d2d034722 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -115,7 +115,7 @@ def get_user_or_404(user_id: int, options: t.Sequence[ORMOption] | None = None) def get_authenticated_user(user_id: int, password: str) -> User: user = get_user_or_404(user_id) if user is None or not user.check_password(password): - abort(401, message="Authentication failed") + abort(401, message=f"Authentication of user {user_id} failed") return user @@ -368,10 +368,10 @@ def post(self, user_id: int, password: str, mac: str, name: str) -> ResponseRetu mpsk_client_create(session.session, user, mac, name, user) session.session.commit() - except InvalidMACAddressException: - abort(400, message="Invalid MAC address.") - except IntegrityError: - abort(400, message="Mac address is already in use.") + except InvalidMACAddressException as e: + abort(400, message=f"Invalid MAC address: {e}") + except IntegrityError as e: + abort(400, message=f"Mac address is already in use: {e}") except InvalidName: abort(400, message="No proper name was provided.") return "mpsk has been added." From a52ec266d968c4ca5c6b32f2da210b08dc897aeb Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 28 Sep 2024 15:37:24 +0200 Subject: [PATCH 36/50] tests: POC API test for `add-mpsk` --- tests/factories/user.py | 1 + tests/frontend/test_api.py | 65 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 tests/frontend/test_api.py diff --git a/tests/factories/user.py b/tests/factories/user.py index f81f7784a..3b9d129a3 100644 --- a/tests/factories/user.py +++ b/tests/factories/user.py @@ -28,6 +28,7 @@ class Meta: registered_at = Faker('date_time') password = None passwd_hash = PASSWORD + wifi_passwd_hash = "{clear}password" email = Faker('email') account = factory.SubFactory(AccountFactory, type="USER_ASSET") room = factory.SubFactory(RoomFactory) diff --git a/tests/frontend/test_api.py b/tests/frontend/test_api.py new file mode 100644 index 000000000..6564d4c26 --- /dev/null +++ b/tests/frontend/test_api.py @@ -0,0 +1,65 @@ +import pytest +from pycroft.model.user import User + +from tests import factories as f +from tests.frontend.assertions import TestClient + + +def test_api_get(client, auth_header, session, user): + resp = client.assert_url_ok( + f"api/v0/user/{user.id}/add-mpsk", + headers=auth_header, + method="POST", + data={"password": "password", "mac": "00:de:ad:be:ef:00", "name": "Fancy TV"}, + ) + # TODO negative test cases, check for adequate response (id) + session.refresh(user) + assert len(mpsk_clients := user.mpsk_clients) == 1 + + assert (j := resp.json) + assert j.get("id") == mpsk_clients[0].id + + +def test_add_mpsk_needs_wifi_hash(client, auth_header, user_without_wifi_pw): + # TODO what kind of response do we expect? + _ = client.assert_url_response_code( + f"api/v0/user/{user_without_wifi_pw.id}/add-mpsk", + 400, + headers=auth_header, + method="POST", + data={"password": "password", "mac": "00:de:ad:be:ef:00", "name": "Fancy TV"}, + ) + + +@pytest.fixture(scope="module") +def user(module_session) -> User: + return f.UserFactory() + + +@pytest.fixture +def user_without_wifi_pw(module_session) -> User: + return f.UserFactory(wifi_passwd_hash=None) + + +@pytest.fixture +def user_with_encrypted_wifi(module_session) -> User: + return f.UserFactory(wifi_passwd_hash="{somecryptprefix}garbledpasswordhash") + + +@pytest.fixture(scope="module", autouse=True) +def client(module_test_client: TestClient) -> TestClient: + return module_test_client + + +@pytest.fixture(scope="module", autouse=True) +def api_key(app) -> str: + api_key = "secrettestapikey" + app.config["PYCROFT_API_KEY"] = api_key + return api_key + + +# TODO put this into the client +@pytest.fixture(scope="module") +def auth_header(api_key) -> dict[str, str]: + # see `api.v0.parse_authorization_header` + return {"AUTHORIZATION": f"apikey {api_key}"} From 97373c67333b6a79b638b1f08711be721cbc709f Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 28 Sep 2024 15:54:43 +0200 Subject: [PATCH 37/50] lib.mpsk_client: Enforce keyword-only arguments This fixes a subtle parameter swap in the API call. --- pycroft/lib/mpsk_client.py | 8 ++++---- web/api/v0/__init__.py | 6 +++--- web/blueprints/mpskclient/__init__.py | 15 ++++++++++++--- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/pycroft/lib/mpsk_client.py b/pycroft/lib/mpsk_client.py index cb8ff5ca8..5992b324f 100644 --- a/pycroft/lib/mpsk_client.py +++ b/pycroft/lib/mpsk_client.py @@ -9,14 +9,14 @@ from pycroft.helpers.i18n import deferred_gettext -def mpsk_delete(session: Session, mpsk_client: MPSKClient, processor: User) -> None: +def mpsk_delete(session: Session, *, mpsk_client: MPSKClient, processor: User) -> None: message = deferred_gettext("Deleted mpsk client '{}'.").format(mpsk_client.name) log_user_event(author=processor, user=mpsk_client.owner, message=message.to_json()) session.delete(mpsk_client) -def change_mac(session: Session, client: MPSKClient, mac: str, processor: User) -> MPSKClient: +def change_mac(session: Session, *, client: MPSKClient, mac: str, processor: User) -> MPSKClient: """ This method will change the mac address of the given mpsks client to the new mac address. @@ -37,7 +37,7 @@ def change_mac(session: Session, client: MPSKClient, mac: str, processor: User) def mpsk_client_create( - session: Session, owner: User, name: str, mac: str, processor: User + session: Session, *, owner: User, name: str, mac: str, processor: User ) -> MPSKClient: """ creates a mpsks client for a given user with a mac address. @@ -63,7 +63,7 @@ def mpsk_client_create( def mpsk_edit( - session: Session, client: MPSKClient, owner: User, name: str, mac: str, processor: User + session: Session, *, client: MPSKClient, owner: User, name: str, mac: str, processor: User ) -> None: if client.name != name: message = deferred_gettext("Changed name of client '{}' to '{}'.").format(client.name, name) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index d2d034722..e32946ffb 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -366,7 +366,7 @@ def post(self, user_id: int, password: str, mac: str, name: str) -> ResponseRetu if not user.wifi_password: abort(400, message="Legacy wifi password change of password is required.") - mpsk_client_create(session.session, user, mac, name, user) + mpsk_client_create(session.session, owner=user, mac=mac, name=name, processor=user) session.session.commit() except InvalidMACAddressException as e: abort(400, message=f"Invalid MAC address: {e}") @@ -394,7 +394,7 @@ def post(self, user_id: int, mpsk_id: int, password: str) -> ResponseReturnValue if not user == mpsk.owner: abort(401, message="You are not the owner of the mpsk.") - mpsk_delete(session.session, mpsk, user) + mpsk_delete(session.session, mpsk_client=mpsk, processor=user) session.session.commit() return "mpsk client was deleted" @@ -424,7 +424,7 @@ def post( ) try: - mpsk_edit(session.session, mpsk, user, name, mac, user) + mpsk_edit(session.session, client=mpsk, owner=user, name=name, mac=mac, processor=user) session.session.commit() except InvalidMACAddressException: abort(400, message="Invalid MAC address.") diff --git a/web/blueprints/mpskclient/__init__.py b/web/blueprints/mpskclient/__init__.py index 70f6a075c..380bb05b1 100644 --- a/web/blueprints/mpskclient/__init__.py +++ b/web/blueprints/mpskclient/__init__.py @@ -61,7 +61,11 @@ def default_response() -> ResponseReturnValue: owner = session.session.get(User, form.owner_id.data) with abort_on_error(default_response), session.session.begin_nested(): host = lib_mpsk.mpsk_client_create( - session.session, owner, form.name.data, form.mac.data, processor=current_user + session.session, + owner=owner, + name=form.name.data, + mac=form.mac.data, + processor=current_user, ) session.session.commit() @@ -92,7 +96,7 @@ def default_response() -> ResponseReturnValue: return default_response() with abort_on_error(default_response), session.session.begin_nested(): - lib_mpsk.mpsk_delete(session.session, mpsk, current_user) + lib_mpsk.mpsk_delete(session.session, mpsk_client=mpsk, processor=current_user) session.session.commit() flash("MPSK Client erfolgreich gelöscht.", "success") @@ -118,7 +122,12 @@ def default_response() -> ResponseReturnValue: with abort_on_error(default_response), session.session.begin_nested(): lib_mpsk.mpsk_edit( - session.session, mpsk, mpsk.owner, form.name.data, form.mac.data, current_user + session.session, + client=mpsk, + owner=mpsk.owner, + name=form.name.data, + mac=form.mac.data, + processor=current_user, ) session.session.add(mpsk) session.session.commit() From 4fc6e29dc0c58f6d2f1642dcb94797d0d77f072d Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 28 Sep 2024 16:36:36 +0200 Subject: [PATCH 38/50] model: Allow accessing `user.wifi_password` even if stored in crypt In that case, it returns `None` just as if the password weren't set --- pycroft/model/user.py | 8 ++++---- web/api/v0/__init__.py | 7 ++----- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/pycroft/model/user.py b/pycroft/model/user.py index 98fb3d414..83fe2d73c 100644 --- a/pycroft/model/user.py +++ b/pycroft/model/user.py @@ -374,15 +374,15 @@ def latest_log_entry(self) -> UserLogEntry | None: return max(le, key=operator.attrgetter("created_at")) @property - def wifi_password(self): - """Store a hash of a given plaintext passwd for the user. + def wifi_password(self) -> str | None: + """return the cleartext wifi password (without crypt prefix) if available. + :returns: `None` if the `wifi_passwd_hash` is not set or is not cleartext. """ - if self.wifi_passwd_hash is not None and self.wifi_passwd_hash.startswith(clear_password_prefix): return self.wifi_passwd_hash.replace(clear_password_prefix, '', 1) - raise ValueError("Cleartext password not available.") + return None @wifi_password.setter def wifi_password(self, value): diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index e32946ffb..1a24850fc 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -158,10 +158,7 @@ class _Entry(t.TypedDict): last_import_ts = get_last_import_date(session.session) last_finance_update = last_import_ts and last_import_ts.date() or None - try: - wifi_password = user.wifi_password - except ValueError: - wifi_password = None + wifi_password = user.wifi_password med = scheduled_membership_end(user) mbd = scheduled_membership_start(user) @@ -364,7 +361,7 @@ def post(self, user_id: int, password: str, mac: str, name: str) -> ResponseRetu abort(400, message="User has the maximum count of mpsk clients.") if not user.wifi_password: - abort(400, message="Legacy wifi password change of password is required.") + abort(400, message="Please generate a wifi password first") mpsk_client_create(session.session, owner=user, mac=mac, name=name, processor=user) session.session.commit() From 3b0b18c2c5f1f5030a46785d4231483dda047634 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 28 Sep 2024 16:59:32 +0200 Subject: [PATCH 39/50] =?UTF-8?q?model:=20rename=20backref=20`User.mpsks`?= =?UTF-8?q?=20=E2=86=92=20`User.mpsk=5Fclients`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pycroft/model/mpsk_client.py | 2 +- pycroft/model/user.py | 2 +- tests/model/test_mpsk.py | 2 +- web/api/v0/__init__.py | 2 +- web/blueprints/mpskclient/__init__.py | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pycroft/model/mpsk_client.py b/pycroft/model/mpsk_client.py index f2ac994f2..f774e4f74 100644 --- a/pycroft/model/mpsk_client.py +++ b/pycroft/model/mpsk_client.py @@ -24,7 +24,7 @@ class MPSKClient(IntegerIdModel): owner_id: Mapped[int | None] = mapped_column( ForeignKey(User.id, ondelete="CASCADE"), index=True, nullable=False ) - owner: Mapped[User] = relationship(User, back_populates="mpsks") + owner: Mapped[User] = relationship(User, back_populates="mpsk_clients") mac: Mapped[mac_address] = mapped_column(unique=True) @validates("mac") diff --git a/pycroft/model/user.py b/pycroft/model/user.py index 83fe2d73c..b45daf827 100644 --- a/pycroft/model/user.py +++ b/pycroft/model/user.py @@ -254,7 +254,7 @@ class User(BaseUser, UserMixin): back_populates="owner", cascade="all, delete-orphan" ) - mpsks: Mapped[list[MPSKClient]] = relationship( + mpsk_clients: Mapped[list[MPSKClient]] = relationship( back_populates="owner", cascade="all, delete-orphan" ) diff --git a/tests/model/test_mpsk.py b/tests/model/test_mpsk.py index b28111984..39fa7735d 100644 --- a/tests/model/test_mpsk.py +++ b/tests/model/test_mpsk.py @@ -76,7 +76,7 @@ def test_admin_exceeds(self, session, user): for i in range(15): mac_client = mac + hex(j)[2:] + hex(i)[2:] c = mpsk_client_create(session, user, "Hallo", mac_client, user) - user.mpsks.append(c) + user.mpsk_clients.append(c) session.flush() @pytest.fixture(scope="class") diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index 1a24850fc..c9ce98b69 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -357,7 +357,7 @@ def post(self, user_id: int, password: str, mac: str, name: str) -> ResponseRetu try: # checks rather the user has all settable mpsks clients created - if len(user.mpsks) > current_app.config.get("MAX_MPSKS", 30): + if len(user.mpsk_clients) > current_app.config.get("MAX_MPSKS", 30): abort(400, message="User has the maximum count of mpsk clients.") if not user.wifi_password: diff --git a/web/blueprints/mpskclient/__init__.py b/web/blueprints/mpskclient/__init__.py index 380bb05b1..206739879 100644 --- a/web/blueprints/mpskclient/__init__.py +++ b/web/blueprints/mpskclient/__init__.py @@ -141,7 +141,7 @@ def user_clients_json(user_id: int) -> ResponseReturnValue: # TODO: Importend when the for the returning of actual mpsk mac addresses for MPSK devices # return "" return TableResponse[MPSKRow]( - items=[_mpsk_row(mpsk, user_id) for mpsk in user.mpsks] + items=[_mpsk_row(mpsk, user_id) for mpsk in user.mpsk_clients] ).model_dump() From f5dc3ff0ac45d70cadd214aaec7b354f665b27ac Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Sat, 28 Sep 2024 17:23:49 +0200 Subject: [PATCH 40/50] tests: move `test_api` to `api.test_mpsk` and introduce class --- tests/frontend/api/conftest.py | 22 +++++++++++ tests/frontend/api/test_mpsk.py | 63 ++++++++++++++++++++++++++++++++ tests/frontend/test_api.py | 65 --------------------------------- 3 files changed, 85 insertions(+), 65 deletions(-) create mode 100644 tests/frontend/api/conftest.py create mode 100644 tests/frontend/api/test_mpsk.py delete mode 100644 tests/frontend/test_api.py diff --git a/tests/frontend/api/conftest.py b/tests/frontend/api/conftest.py new file mode 100644 index 000000000..45eedb2c7 --- /dev/null +++ b/tests/frontend/api/conftest.py @@ -0,0 +1,22 @@ +import pytest + +from tests.frontend.assertions import TestClient + + +@pytest.fixture(scope="module", autouse=True) +def client(module_test_client: TestClient) -> TestClient: + return module_test_client + + +@pytest.fixture(scope="module", autouse=True) +def api_key(app) -> str: + api_key = "secrettestapikey" + app.config["PYCROFT_API_KEY"] = api_key + return api_key + + +# TODO put this into the client +@pytest.fixture(scope="module") +def auth_header(api_key) -> dict[str, str]: + # see `api.v0.parse_authorization_header` + return {"AUTHORIZATION": f"apikey {api_key}"} diff --git a/tests/frontend/api/test_mpsk.py b/tests/frontend/api/test_mpsk.py new file mode 100644 index 000000000..b7de6a759 --- /dev/null +++ b/tests/frontend/api/test_mpsk.py @@ -0,0 +1,63 @@ +import pytest +from pycroft.model.user import User + +from tests import factories as f + + +class TestAddMpsk: + VALID_DATA = {"password": "password", "mac": "00:de:ad:be:ef:00", "name": "Fancy TV"} + + def test_valid_mpsk_device(self, client, auth_header, url, session, user): + resp = client.assert_url_ok(url, headers=auth_header, method="POST", data=self.VALID_DATA) + session.refresh(user) + assert len(mpsk_clients := user.mpsk_clients) == 1 + + assert isinstance(j := resp.json, dict) + assert j.get("id") == mpsk_clients[0].id + + # TODO negative test cases + + @pytest.mark.parametrize( + "data", + ( + {}, + {"password": "password"}, + {"mac": "00:de:ad:be:ef:00"}, + {"name": "Fancy TV"}, + VALID_DATA | {"mac": "badmac"}, + ), + ) + def test_bad_data(self, client, auth_header, url, data): + client.assert_url_response_code( + url, code=422, headers=auth_header, method="POST", data=data + ) + + @pytest.fixture(scope="module") + def url(self, user) -> str: + return f"api/v0/user/{user.id}/add-mpsk" + + def test_add_mpsk_needs_wifi_hash(self, client, auth_header, user_without_wifi_pw): + client.assert_url_response_code( + f"api/v0/user/{user_without_wifi_pw.id}/add-mpsk", + # 412 precondition failed + # The request itself is valid, so 422 (unprocessable entity) is slightly inaccurate + code=412, + headers=auth_header, + method="POST", + data=self.VALID_DATA, + ) + + +@pytest.fixture(scope="module") +def user(module_session) -> User: + return f.UserFactory() + + +@pytest.fixture +def user_without_wifi_pw(module_session) -> User: + return f.UserFactory(wifi_passwd_hash=None) + + +@pytest.fixture +def user_with_encrypted_wifi(module_session) -> User: + return f.UserFactory(wifi_passwd_hash="{somecryptprefix}garbledpasswordhash") diff --git a/tests/frontend/test_api.py b/tests/frontend/test_api.py deleted file mode 100644 index 6564d4c26..000000000 --- a/tests/frontend/test_api.py +++ /dev/null @@ -1,65 +0,0 @@ -import pytest -from pycroft.model.user import User - -from tests import factories as f -from tests.frontend.assertions import TestClient - - -def test_api_get(client, auth_header, session, user): - resp = client.assert_url_ok( - f"api/v0/user/{user.id}/add-mpsk", - headers=auth_header, - method="POST", - data={"password": "password", "mac": "00:de:ad:be:ef:00", "name": "Fancy TV"}, - ) - # TODO negative test cases, check for adequate response (id) - session.refresh(user) - assert len(mpsk_clients := user.mpsk_clients) == 1 - - assert (j := resp.json) - assert j.get("id") == mpsk_clients[0].id - - -def test_add_mpsk_needs_wifi_hash(client, auth_header, user_without_wifi_pw): - # TODO what kind of response do we expect? - _ = client.assert_url_response_code( - f"api/v0/user/{user_without_wifi_pw.id}/add-mpsk", - 400, - headers=auth_header, - method="POST", - data={"password": "password", "mac": "00:de:ad:be:ef:00", "name": "Fancy TV"}, - ) - - -@pytest.fixture(scope="module") -def user(module_session) -> User: - return f.UserFactory() - - -@pytest.fixture -def user_without_wifi_pw(module_session) -> User: - return f.UserFactory(wifi_passwd_hash=None) - - -@pytest.fixture -def user_with_encrypted_wifi(module_session) -> User: - return f.UserFactory(wifi_passwd_hash="{somecryptprefix}garbledpasswordhash") - - -@pytest.fixture(scope="module", autouse=True) -def client(module_test_client: TestClient) -> TestClient: - return module_test_client - - -@pytest.fixture(scope="module", autouse=True) -def api_key(app) -> str: - api_key = "secrettestapikey" - app.config["PYCROFT_API_KEY"] = api_key - return api_key - - -# TODO put this into the client -@pytest.fixture(scope="module") -def auth_header(api_key) -> dict[str, str]: - # see `api.v0.parse_authorization_header` - return {"AUTHORIZATION": f"apikey {api_key}"} From 9c9ad267ba2a1be95dfab61777f4594ac39b27e4 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 29 Sep 2024 15:30:46 +0200 Subject: [PATCH 41/50] added api tests added same mac test, invalid password, exceeds the max clients added delete mpsk; test not found, unauthed, successful delete Signed-off-by: Alex #744 --- tests/frontend/api/conftest.py | 7 +++ tests/frontend/api/test_mpsk.py | 90 +++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/tests/frontend/api/conftest.py b/tests/frontend/api/conftest.py index 45eedb2c7..7b5515e1c 100644 --- a/tests/frontend/api/conftest.py +++ b/tests/frontend/api/conftest.py @@ -15,6 +15,13 @@ def api_key(app) -> str: return api_key +@pytest.fixture(scope="module", autouse=True) +def max_clients(app) -> int: + max_clients = 5 + app.config["MAX_MPSKS"] = max_clients + return max_clients + + # TODO put this into the client @pytest.fixture(scope="module") def auth_header(api_key) -> dict[str, str]: diff --git a/tests/frontend/api/test_mpsk.py b/tests/frontend/api/test_mpsk.py index b7de6a759..b12766c0e 100644 --- a/tests/frontend/api/test_mpsk.py +++ b/tests/frontend/api/test_mpsk.py @@ -1,4 +1,6 @@ import pytest + +from pycroft.lib.mpsk_client import mpsk_client_create from pycroft.model.user import User from tests import factories as f @@ -6,6 +8,8 @@ class TestAddMpsk: VALID_DATA = {"password": "password", "mac": "00:de:ad:be:ef:00", "name": "Fancy TV"} + INVALID_PASSWORD = {"password": "p", "mac": "00:de:ad:be:ef:00", "name": "Fancy TV"} + INVALID_name = {"password": "password", "mac": "00:de:ad:be:ef:00"} def test_valid_mpsk_device(self, client, auth_header, url, session, user): resp = client.assert_url_ok(url, headers=auth_header, method="POST", data=self.VALID_DATA) @@ -15,7 +19,29 @@ def test_valid_mpsk_device(self, client, auth_header, url, session, user): assert isinstance(j := resp.json, dict) assert j.get("id") == mpsk_clients[0].id + def test_mpsk_device_same_mac(self, client, auth_header, url, session, user): + client.assert_url_ok(url, headers=auth_header, method="POST", data=self.VALID_DATA) + session.refresh(user) + assert len(user.mpsk_clients) == 1 + + client.assert_url_response_code( + url, code=409, headers=auth_header, method="POST", data=self.VALID_DATA + ) + # TODO negative test cases + def test_invalid_password(self, client, auth_header, url, session, user): + client.assert_url_response_code( + url, code=401, headers=auth_header, method="POST", data=self.INVALID_PASSWORD + ) + session.refresh(user) + assert len(user.mpsk_clients) == 0 + + @pytest.mark.parametrize("name", ("", " ", " ", " ", " ")) + def test_invalid_name(self, client, auth_header, url, session, user, name): + self.INVALID_name["name"] = name + client.assert_url_response_code( + url, code=400, headers=auth_header, method="POST", data=self.INVALID_name + ) @pytest.mark.parametrize( "data", @@ -47,6 +73,70 @@ def test_add_mpsk_needs_wifi_hash(self, client, auth_header, user_without_wifi_p data=self.VALID_DATA, ) + @pytest.mark.parametrize( + "data", + ( + { + "password": "password", + "mac": "00:de:ad:be:ef:0", + "name": "Fancy TV", + }, + ), + ) + def test_max_mpsk_clients(self, client, auth_header, user, session, max_clients, url, data): + + for i in range(max_clients): + default = data["mac"] + data["mac"] = f"{data['mac']}{i}" + client.assert_url_ok(url, headers=auth_header, method="POST", data=data) + data["mac"] = default + session.refresh(user) + assert len(user.mpsk_clients) == i + 1 + + data["mac"] = f"{data['mac']}{max_clients}" + client.assert_url_response_code( + url, code=400, headers=auth_header, method="POST", data=data + ) + + +class TestDeleteMpsk: + VALID_PASSWORD = {"password": "password"} + INVALID_PASSWORD = {"password": "p"} + + def test_delete_no_mpsk(self, client, auth_header, session, user): + client.assert_url_response_code( + f"api/v0/user/{user.id}/delete-mpsk/0", + code=404, + headers=auth_header, + method="POST", + data=self.VALID_PASSWORD, + ) + + def test_delete_mpsk_unauthed(self, client, auth_header, url, session, user): + mpsk_client = mpsk_client_create( + session, owner=user, mac="00:de:ad:be:ef:00", name="Fancy TV", processor=user + ) + session.flush() + client.assert_url_response_code( + url + str(mpsk_client.id), + code=401, + headers=auth_header, + method="POST", + data=self.INVALID_PASSWORD, + ) + + def test_delete_mpsk(self, client, auth_header, url, session, user): + mpsk_client = mpsk_client_create( + session, owner=user, mac="00:de:ad:be:ef:00", name="Fancy TV", processor=user + ) + session.flush() + client.assert_url_ok( + url + str(mpsk_client.id), headers=auth_header, method="POST", data=self.VALID_PASSWORD + ) + + @pytest.fixture(scope="module") + def url(self, user) -> str: + return f"api/v0/user/{user.id}/delete-mpsk/" @pytest.fixture(scope="module") def user(module_session) -> User: From a26a4964fff71db03351ed464bfa502ba04cc2c0 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 29 Sep 2024 15:31:53 +0200 Subject: [PATCH 42/50] refactoring and renaming of error codes refactored code renamed some error codes for better readability Signed-off-by: Alex #744 --- web/api/v0/__init__.py | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index c9ce98b69..0093564d2 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -354,24 +354,31 @@ class MPSKSClientAddResource(Resource): ) def post(self, user_id: int, password: str, mac: str, name: str) -> ResponseReturnValue: user = get_authenticated_user(user_id, password) + # checks rather the user has all settable mpsks clients created + if len(user.mpsk_clients) >= current_app.config.get("MAX_MPSKS", 30): + abort(400, message="User has the maximum count of mpsk clients.") - try: - # checks rather the user has all settable mpsks clients created - if len(user.mpsk_clients) > current_app.config.get("MAX_MPSKS", 30): - abort(400, message="User has the maximum count of mpsk clients.") - - if not user.wifi_password: - abort(400, message="Please generate a wifi password first") + if not user.wifi_password: + abort(412, message="Please generate a wifi password first") - mpsk_client_create(session.session, owner=user, mac=mac, name=name, processor=user) + try: + mpsk_client = mpsk_client_create( + session.session, owner=user, mac=mac, name=name, processor=user + ) session.session.commit() except InvalidMACAddressException as e: - abort(400, message=f"Invalid MAC address: {e}") + abort(422, message=f"Invalid MAC address: {e}") except IntegrityError as e: - abort(400, message=f"Mac address is already in use: {e}") + abort(409, message=f"Mac address is already in use: {e}") except InvalidName: abort(400, message="No proper name was provided.") - return "mpsk has been added." + return jsonify( + { + "name": mpsk_client.name, + "id": mpsk_client.id, + "mac": mpsk_client.mac, + } + ) api.add_resource(MPSKSClientAddResource, "/user//add-mpsk") From 0f8547601d828dd1b49d611066d700066653bafc Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 29 Sep 2024 15:45:30 +0200 Subject: [PATCH 43/50] added test for API added get mpsks clients tests Signed-off-by: Alex #744 --- tests/frontend/api/test_mpsk.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/tests/frontend/api/test_mpsk.py b/tests/frontend/api/test_mpsk.py index b12766c0e..e5761479e 100644 --- a/tests/frontend/api/test_mpsk.py +++ b/tests/frontend/api/test_mpsk.py @@ -138,6 +138,17 @@ def test_delete_mpsk(self, client, auth_header, url, session, user): def url(self, user) -> str: return f"api/v0/user/{user.id}/delete-mpsk/" + +class TestGetMPSKS: + @pytest.fixture(scope="module") + def url(self, user) -> str: + return f"api/v0/user/{user.id}/get-mpsks" + + def test_get(self, client, auth_header, user, url, session): + resp = client.assert_url_ok(url, headers=auth_header, method="GET") + assert isinstance(j := resp.json, list) + assert len(j) == len(user.mpsk_clients) + @pytest.fixture(scope="module") def user(module_session) -> User: return f.UserFactory() From 886e4b3cf8d0000dec4a44b6b0f55c408c08bb43 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 29 Sep 2024 15:45:40 +0200 Subject: [PATCH 44/50] API: added get mpsks clients added api route for getting all the mpsks clients of a user Signed-off-by: Alex #744 --- web/api/v0/__init__.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index 0093564d2..0209b54be 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -343,6 +343,24 @@ def get(self, ipv4: IPv4Address | IPv6Address) -> ResponseReturnValue: api.add_resource(UserByIPResource, '/user/from-ip') +class MPSKSClientsResource(Resource): + def get(self, user_id: int) -> ResponseReturnValue: + user = get_user_or_404(user_id) + + return jsonify( + [ + { + "name": mpsk_client.name, + "id": mpsk_client.id, + "mac": mpsk_client.mac, + } + for mpsk_client in user.mpsk_clients + ] + ) + + +api.add_resource(MPSKSClientsResource, "/user//get-mpsks") + class MPSKSClientAddResource(Resource): @use_kwargs( { From e4dec3aa69f1c9f1e3fb8932fe028bdef19f8146 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 29 Sep 2024 16:54:14 +0200 Subject: [PATCH 45/50] added mpsks change tests added test for changing the mpsk clients Signed-off-by: Alex #744 --- tests/frontend/api/test_mpsk.py | 92 +++++++++++++++++++++++++++++++++ 1 file changed, 92 insertions(+) diff --git a/tests/frontend/api/test_mpsk.py b/tests/frontend/api/test_mpsk.py index e5761479e..725f6d15a 100644 --- a/tests/frontend/api/test_mpsk.py +++ b/tests/frontend/api/test_mpsk.py @@ -1,6 +1,7 @@ import pytest from pycroft.lib.mpsk_client import mpsk_client_create +from pycroft.model.mpsk_client import MPSKClient from pycroft.model.user import User from tests import factories as f @@ -149,6 +150,97 @@ def test_get(self, client, auth_header, user, url, session): assert isinstance(j := resp.json, list) assert len(j) == len(user.mpsk_clients) + def test_none_list(self, client, auth_header, user, url, session): + user.mpsk_clients.clear() + session.add(user) + session.flush() + + resp = client.assert_url_ok(url, headers=auth_header, method="GET") + assert isinstance(j := resp.json, list) + assert len(j) == 0 + + +class TestEditMPSK: + VALID_DATA = {"password": "password", "mac": "00:de:ad:be:ef:00", "name": "Fancy TV"} + INVALID_PASSWORD = {"password": "p", "mac": "00:de:ad:be:ef:00", "name": "Fancy TV"} + + @pytest.fixture(scope="module") + def url(self, user) -> str: + return f"api/v0/user/{user.id}/change-mpsk/" + + @pytest.mark.parametrize( + "data", + ( + {}, + {"password": "password"}, + {"mac": "00:de:ad:be:ef:00"}, + {"name": "Fancy TV"}, + VALID_DATA | {"mac": "badmac"}, + ), + ) + def test_unfound_change(self, client, auth_header, url, data, session): + client.assert_url_response_code( + url + "0", code=404, headers=auth_header, method="POST", data=self.VALID_DATA + ) + + def get_mpsk(self, user, session, mac="00:de:ad:be:ef:00") -> MPSKClient: + client = mpsk_client_create(session, owner=user, mac=mac, name="Fancy TV", processor=user) + session.flush() + return client + + def test_unautherized_change(self, client, auth_header, url, session, user): + mpsk = self.get_mpsk(user, session) + client.assert_url_response_code( + url + str(mpsk.id), + code=401, + headers=auth_header, + method="POST", + data=self.INVALID_PASSWORD, + ) + + def test_no_change(self, client, auth_header, url, session, user): + mpsk = self.get_mpsk(user, session) + client.assert_url_ok( + url + str(mpsk.id), headers=auth_header, method="POST", data=self.VALID_DATA + ) + + def test_duplicate_mac(self, client, auth_header, url, session, user): + mpsk = self.get_mpsk(user, session) + mpsk1 = self.get_mpsk(user, session, mac="00:de:ad:be:ef:01") + + data = self.VALID_DATA | {"mac": mpsk1.mac} + + client.assert_url_response_code( + url + str(mpsk.id), code=409, headers=auth_header, data=data, method="POST" + ) + + @pytest.mark.parametrize( + "data", + ( + VALID_DATA | {"name": ""}, + VALID_DATA | {"name": " "}, + ), + ) + def test_invalid_name(self, client, auth_header, url, session, user, data): + mpsk = self.get_mpsk(user, session) + client.assert_url_response_code( + url + str(mpsk.id), code=400, headers=auth_header, data=data, method="POST" + ) + + @pytest.mark.parametrize( + "data", + ( + VALID_DATA | {"mac": ""}, + VALID_DATA | {"mac": "ff:ff:ff:ff:ff"}, + ), + ) + def test_invalid_mac(self, client, auth_header, url, session, user, data): + mpsk = self.get_mpsk(user, session) + client.assert_url_response_code( + url + str(mpsk.id), code=422, headers=auth_header, data=data, method="POST" + ) + + @pytest.fixture(scope="module") def user(module_session) -> User: return f.UserFactory() From da89cae1f358d33f9a88583ffc60ce38d206bdd1 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 29 Sep 2024 16:55:28 +0200 Subject: [PATCH 46/50] API: renaming of error codes MPSK changed the error codes for changing mpsk client so they match the ones in adding Signed-off-by: Alex #744 --- web/api/v0/__init__.py | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index 0209b54be..85bfde4b6 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -435,23 +435,21 @@ class MPSKSClientChangeResource(Resource): location="form", ) def post( - self, user_id: int, mpsks_id: int, password: str, mac: str, name: str + self, user_id: int, mpsk_id: int, password: str, mac: str, name: str ) -> ResponseReturnValue: user = get_authenticated_user(user_id, password) - mpsk = get_mpsk_client_or_404(mpsks_id) + mpsk = get_mpsk_client_or_404(mpsk_id) if user != mpsk.owner: - abort( - 404, message=f"User {user_id} does not own the mpsk client with the id {mpsks_id}" - ) + abort(404, message=f"User {user_id} does not own the mpsk client with the id {mpsk_id}") try: mpsk_edit(session.session, client=mpsk, owner=user, name=name, mac=mac, processor=user) session.session.commit() except InvalidMACAddressException: - abort(400, message="Invalid MAC address.") + abort(422, message="Invalid MAC address.") except IntegrityError: - abort(400, message="Mac address is already in use.") + abort(409, message="Mac address is already in use.") except InvalidName: abort(400, message="No proper name was provided.") return "mpsk has been changed." From 139778a682522983553daae2dca9b5689919dea0 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 29 Sep 2024 16:57:58 +0200 Subject: [PATCH 47/50] API: refactoring Signed-off-by: Alex #744 --- tests/frontend/api/test_mpsk.py | 4 +++- web/api/v0/__init__.py | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/frontend/api/test_mpsk.py b/tests/frontend/api/test_mpsk.py index 725f6d15a..be7004d65 100644 --- a/tests/frontend/api/test_mpsk.py +++ b/tests/frontend/api/test_mpsk.py @@ -85,7 +85,9 @@ def test_add_mpsk_needs_wifi_hash(self, client, auth_header, user_without_wifi_p ), ) def test_max_mpsk_clients(self, client, auth_header, user, session, max_clients, url, data): - + user.mpsk_clients.clear() + session.add(user) + session.flush() for i in range(max_clients): default = data["mac"] data["mac"] = f"{data['mac']}{i}" diff --git a/web/api/v0/__init__.py b/web/api/v0/__init__.py index 85bfde4b6..c3d598334 100644 --- a/web/api/v0/__init__.py +++ b/web/api/v0/__init__.py @@ -343,6 +343,7 @@ def get(self, ipv4: IPv4Address | IPv6Address) -> ResponseReturnValue: api.add_resource(UserByIPResource, '/user/from-ip') + class MPSKSClientsResource(Resource): def get(self, user_id: int) -> ResponseReturnValue: user = get_user_or_404(user_id) @@ -361,6 +362,7 @@ def get(self, user_id: int) -> ResponseReturnValue: api.add_resource(MPSKSClientsResource, "/user//get-mpsks") + class MPSKSClientAddResource(Resource): @use_kwargs( { From 489c55ffcac37d2863a562feb5e368a91c0b19a8 Mon Sep 17 00:00:00 2001 From: Alex Date: Sun, 29 Sep 2024 19:13:23 +0200 Subject: [PATCH 48/50] fixed added admin mpsk test added karg parameter Signed-off-by: Alex #744 --- tests/frontend/api/test_mpsk.py | 4 +--- tests/model/test_mpsk.py | 4 +++- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/frontend/api/test_mpsk.py b/tests/frontend/api/test_mpsk.py index be7004d65..725f6d15a 100644 --- a/tests/frontend/api/test_mpsk.py +++ b/tests/frontend/api/test_mpsk.py @@ -85,9 +85,7 @@ def test_add_mpsk_needs_wifi_hash(self, client, auth_header, user_without_wifi_p ), ) def test_max_mpsk_clients(self, client, auth_header, user, session, max_clients, url, data): - user.mpsk_clients.clear() - session.add(user) - session.flush() + for i in range(max_clients): default = data["mac"] data["mac"] = f"{data['mac']}{i}" diff --git a/tests/model/test_mpsk.py b/tests/model/test_mpsk.py index 39fa7735d..9ced12a9e 100644 --- a/tests/model/test_mpsk.py +++ b/tests/model/test_mpsk.py @@ -75,7 +75,9 @@ def test_admin_exceeds(self, session, user): for j in range(0, 4): for i in range(15): mac_client = mac + hex(j)[2:] + hex(i)[2:] - c = mpsk_client_create(session, user, "Hallo", mac_client, user) + c = mpsk_client_create( + session, owner=user, name="Hallo", mac=mac_client, processor=user + ) user.mpsk_clients.append(c) session.flush() From 7534a7d43d56277e88655b19e56224fb793b7987 Mon Sep 17 00:00:00 2001 From: Alex Date: Mon, 30 Sep 2024 23:18:36 +0200 Subject: [PATCH 49/50] added lib.mpsk_clients pyproject.toml imported the pycroft.lib.mpsk_client to strict typechecking --- pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/pyproject.toml b/pyproject.toml index 7cf82ec0e..d5515172d 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -176,6 +176,7 @@ module = [ "pycroft.lib.user.*", "web.blueprints.finance", "web.blueprints.finance.*", + "pycroft.lib.mpsk_client", ] strict_optional = true From c350c1bab17d444592a67b9fea95bbea5250f825 Mon Sep 17 00:00:00 2001 From: Lukas Juhrich Date: Thu, 3 Oct 2024 16:41:58 +0200 Subject: [PATCH 50/50] Change mpsk icon to `wifi` The tablet icon was hard to idenfity. --- web/templates/user/user_show.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/web/templates/user/user_show.html b/web/templates/user/user_show.html index a299e1905..5d1d56354 100644 --- a/web/templates/user/user_show.html +++ b/web/templates/user/user_show.html @@ -18,7 +18,7 @@ }, { 'id': 'mpsk', - 'icon': 'fa-tablet', + 'icon': 'fa-wifi', 'name': 'MPSK Clients', 'badge': user.tasks | length, },