Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for MPSK #744

Open
mabezi opened this issue Aug 10, 2024 · 6 comments · May be fixed by #749
Open

Add support for MPSK #744

mabezi opened this issue Aug 10, 2024 · 6 comments · May be fixed by #749
Labels
feature 🔨 api 🔨 db-schema Things that touch the DB schema and probably require a migration. 🔨 web/ui Things relating to Flask routes and Jinja templates

Comments

@mabezi
Copy link
Contributor

mabezi commented Aug 10, 2024

In order for MPSK to be used by our members, an appropriate frontend, including an API endpoint, must be implemented.

The following SQL schema is currently planned:

CREATE TABLE wifi_mpsk_clients(
    owner_id INT NOT NULL,
    name character varying,
    mac macaddr NOT NULL,
    UNIQUE(mac),
    CONSTRAINT fk_owner_id FOREIGN KEY (owner_id) REFERENCES pycroft."user"(id) ON DELETE CASCADE
);

A MAC address must be uniquely assignable to a user. However, a user can create as many MAC addresses as they wish.

To better distinguish between devices, it is planned to allow users to assign names to their devices.

The frontend can thus be implemented similarly to the existing interface list. This can then be placed in a separate tab, "WiFi" or similar, on the user page.

To simplify troubleshooting in case of issues, a note should be added in the frontend if the user has a legacy WiFi password that is not stored in plain text. (Reminder: WiFi password ≠ login password)

Feel free to ask questions if necessary.

@mabezi mabezi added feature 🔨 web/ui Things relating to Flask routes and Jinja templates 🔨 db-schema Things that touch the DB schema and probably require a migration. 🔨 api labels Aug 10, 2024
@lukasjuhrich
Copy link
Collaborator

The frontend can thus be implemented similarly to the existing interface list.

To give a few pointers to anyone willing to work on this:

  1. These are the interface views

@bp.route('/interface/<int:interface_id>/delete', methods=['GET', 'POST'])
@access.require('hosts_change')
def interface_delete(interface_id: int) -> ResponseReturnValue:
interface = get_interface_or_404(interface_id)
form = FlaskForm()
def default_response() -> ResponseReturnValue:
form_args = {
'form': form,
'cancel_to': url_for('user.user_show', user_id=interface.host.owner_id),
'submit_text': 'Löschen',
'actions_offset': 0
}
return render_template('generic_form.html',
page_title="Interface 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_host.interface_delete(interface, current_user)
session.session.commit()
flash("Interface erfolgreich gelöscht.", 'success')
return redirect(url_for(
'user.user_show',
user_id=interface.host.owner_id, _anchor='hosts'
))
@bp.route('/interface/<int:interface_id>/edit', methods=['GET', 'POST'])
@access.require('hosts_change')
def interface_edit(interface_id: int) -> ResponseReturnValue:
interface = get_interface_or_404(interface_id)
subnets = get_subnets_for_room(interface.host.room)
current_ips = [ip.address for ip in interface.ips]
unused_ips = [ip for subnet in subnets for ip in subnet.unused_ips_iter()]
form = InterfaceForm(obj=interface)
form.meta.current_mac = interface.mac
form.ips.choices = [(str(ip), str(ip)) for ip in current_ips + unused_ips]
def default_response() -> ResponseReturnValue:
form_args = {
'form': form,
'cancel_to': url_for('user.user_show', user_id=interface.host.owner_id)
}
return render_template('generic_form.html',
page_title="Interface editieren",
form_args=form_args)
if not form.is_submitted():
form.ips.process_data(ip for ip in current_ips)
return default_response()
if not form.validate():
return default_response()
ips = {IPAddress(ip) for ip in form.ips.data}
with abort_on_error(default_response), session.session.begin_nested():
lib_host.interface_edit(
interface, form.name.data, form.mac.data, ips, processor=current_user
)
session.session.commit()
flash("Interface erfolgreich bearbeitet.", 'success')
return redirect(url_for('user.user_show', user_id=interface.host.owner_id, _anchor='hosts'))
@bp.route("/<int:host_id>/interface/create", methods=["GET", "POST"])
@access.require("hosts_change")
def interface_create(host_id: int) -> ResponseReturnValue:
host = get_host_or_404(host_id)
subnets = get_subnets_for_room(host.room)
form = InterfaceForm()
unused_ips = [ip for subnet in subnets for ip in subnet.unused_ips_iter()]
form.ips.choices = [(str(ip), str(ip)) for ip in unused_ips]
def default_response() -> ResponseReturnValue:
form_args = {
'form': form,
'cancel_to': url_for('user.user_show', user_id=host.owner.id)
}
return render_template('generic_form.html',
page_title="Interface erstellen",
form_args=form_args)
if not form.is_submitted():
form.ips.process_data([next(iter(unused_ips), None)])
return default_response()
if not form.validate():
return default_response()
ips = {IPAddress(ip) for ip in form.ips.data}
try:
lib_host.interface_create(
host,
form.name.data,
form.mac.data,
ips,
current_user
)
session.session.commit()
except PycroftException: # pragma: no cover
return default_response()
flash("Interface erfolgreich erstellt.", 'success')
return redirect(url_for(
'user.user_show',
user_id=host.owner.id, _anchor='hosts'
))

  1. these are the underlying lib functions containing the core CRUD (Create, Read, Update, Delete) logic.

pycroft/pycroft/lib/host.py

Lines 110 to 221 in 865ebcf

def interface_create(
host: Host,
name: str,
mac: str,
ips: t.Iterable[netaddr.IPAddress] | None,
processor: User,
) -> Interface:
interface = Interface(host=host, mac=mac, name=name)
session.add(interface)
subnets = get_subnets_for_room(interface.host.room)
if ips is None:
# this happens in only one call
ip, _ = get_free_ip(subnets)
ips = {ip}
# IP added
for ip in ips:
subnet = next(
iter([subnet for subnet in subnets if (ip in subnet.address)]),
None)
if subnet is not None:
session.add(IP(interface=interface, address=ip,
subnet=subnet))
message = deferred_gettext(
"Created interface ({}, {}) with name '{}' for host '{}'."
).format(
interface.mac,
", ".join(str(ip_.address) for ip_ in interface.ips),
interface.name,
interface.host.name,
)
log_user_event(author=processor, user=host.owner, message=message.to_json())
return interface
@with_transaction
def interface_edit(
interface: Interface,
name: str,
mac: str,
ips: t.Iterable[netaddr.IPAddress],
processor: User,
) -> None:
message = "Edited interface ({}, {}) of host '{}'.".format(
interface.mac,
", ".join(str(ip.address) for ip in interface.ips),
interface.host.name,
)
if interface.name != name:
interface.name = name
message += f" New name: '{interface.name}'."
if interface.mac != mac:
interface.mac = mac
message += f" New MAC: {interface.mac}."
ips_changed = False
current_ips = list(ip.address for ip in interface.ips)
subnets = get_subnets_for_room(interface.host.room)
new_ips = set(current_ips)
# IP removed
for ip in current_ips:
if ip not in ips:
delete_ip(session, ip)
ips_changed = True
new_ips.remove(ip)
# IP added
for ip in ips:
if ip not in current_ips:
subnet = next(
iter([subnet for subnet in subnets if (ip in subnet.address)]),
None)
if subnet is not None:
session.add(IP(interface=interface, address=ip,
subnet=subnet))
ips_changed = True
new_ips.add(netaddr.IPAddress(ip))
if ips_changed:
message += " New IPs: {}.".format(', '.join(str(ip) for ip in
new_ips))
log_user_event(author=processor,
user=interface.host.owner,
message=deferred_gettext(message).to_json())
@with_transaction
def interface_delete(interface: Interface, processor: User) -> None:
message = deferred_gettext("Deleted interface {} of host {}.").format(
interface.mac, interface.host.name
)
log_user_event(
author=processor, user=interface.host.owner, message=message.to_json()
)
session.delete(interface)
TPort = t.TypeVar("TPort", bound=SwitchPort | PatchPort)

@lukasjuhrich
Copy link
Collaborator

A straightforward way to implement this feature in the UI would be to place a second table, named "mpsk clients", below the hosts table:
image

agmes4 added a commit that referenced this issue Sep 14, 2024
added a new Subsection for MPSK Clients for the creating of them

#744
agmes4 added a commit that referenced this issue Sep 15, 2024
added the database model for storing
added the needed functions
as well as the relation to the user and back

Signed-off-by: Alex <[email protected]>
#744
@agmes4
Copy link
Contributor

agmes4 commented Sep 15, 2024

I would add a not null for the name so its better identifiable

I currently also added another tab instate of putting it below the old one, first it looks way cleaner at the fronted side and in code as well as it is better structured

agmes4 added a commit that referenced this issue Sep 15, 2024
changed the name

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 15, 2024
now enforces name for mpsk client
refactoring changed name

Signed-off-by: Alex <[email protected]>
#744
@mabezi
Copy link
Contributor Author

mabezi commented Sep 15, 2024

First of all thank you @agmes4 :)

I see that you use „WLAN“ in your implementation at the moment.
In order to maintain uniformity, it may be better to use „WiFi“. (Except in the German translation, since „WLAN“ is more common in Germany.)

@agmes4
Copy link
Contributor

agmes4 commented Sep 16, 2024

Your totally right will change this thanks :)

agmes4 added a commit that referenced this issue Sep 16, 2024
refactoring and renaming for better usability

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 16, 2024
added the api background for adding mpsk clients and editing them

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 16, 2024
added delete api endpoint for mpsks clients so they can be removed

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 16, 2024
@lukasjuhrich
Copy link
Collaborator

@agmes4 feel free to create a draft PR to allow for reviews :)

agmes4 added a commit that referenced this issue Sep 17, 2024
agmes4 added a commit that referenced this issue Sep 17, 2024
added the naming constraint that names can not be empty or just be constructed out of witespaces

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 17, 2024
added tests for the model of mpsk

Signed-off-by: Alex <[email protected]>
#744
@agmes4 agmes4 linked a pull request Sep 17, 2024 that will close this issue
@agmes4 agmes4 linked a pull request Sep 17, 2024 that will close this issue
agmes4 added a commit that referenced this issue Sep 17, 2024
added a new Subsection for MPSK Clients for the creating of them

#744
agmes4 added a commit that referenced this issue Sep 17, 2024
added the database model for storing
added the needed functions
as well as the relation to the user and back

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 17, 2024
now enforces name for mpsk client
refactoring changed name

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 17, 2024
refactoring and renaming for better usability

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 17, 2024
added the naming constraint that names can not be empty or just be constructed out of witespaces

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 17, 2024
added tests for the model of mpsk

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 18, 2024
there was a small error which just checked rather

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 18, 2024
removed the old decorator

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 18, 2024
agmes4 added a commit that referenced this issue Sep 18, 2024
added the user who deleted the mpsk client as processor

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 19, 2024
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 <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 19, 2024
checking for amount exceeded error as well as naming error in MPSKS clients

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 19, 2024
added a test for too much mpsks clients added

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 19, 2024
added key arg which is for determining rather the amount of mpsks clients should be validated for a amount

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 19, 2024
added test for clients exceeding admin
fixed test for api call

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 19, 2024
changed the creat mpsks so that it now validates for the length of added clients

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 19, 2024
the api is now able to add more clients

Signed-off-by: Alex <[email protected]>
#744
agmes4 added a commit that referenced this issue Sep 19, 2024
Signed-off-by: Alex <[email protected]>

#744
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature 🔨 api 🔨 db-schema Things that touch the DB schema and probably require a migration. 🔨 web/ui Things relating to Flask routes and Jinja templates
Projects
Status: To do
Development

Successfully merging a pull request may close this issue.

3 participants