From b566055d0af42fb3e89d58ae98fbdf5209fbccc8 Mon Sep 17 00:00:00 2001 From: stephan Date: Sun, 28 Dec 2025 16:47:43 +0100 Subject: [PATCH] auth: improve pam flow and user status --- CHANGES.md | 32 ++++++++++++++++++++++++++++++++ backend/actions.py | 11 +++++++++-- backend/app.py | 16 +++++++++++++++- backend/auth.py | 15 +++++++++++++++ backend/models.py | 1 + backend/templates/index.html | 4 +++- 6 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 CHANGES.md diff --git a/CHANGES.md b/CHANGES.md new file mode 100644 index 0000000..49d9ed2 --- /dev/null +++ b/CHANGES.md @@ -0,0 +1,32 @@ +# Changes + +## Kontext +- Problem: PAM-Login schlug fehl und spaeter wurden keine Benutzer im UI angezeigt. +- Ursache: Der Dienst lief als User `skd`. PAM kann nur den aktuellen Nutzer pruefen; deshalb schlug die Authentifizierung fuer andere Nutzer fehl. +- UI-Eindruck: Benutzerliste erscheint erst nach "Status laden" (kein Auto-Refresh nach erfolgreichem Login, nur Token setzen). + +## Service/Deploy-Anpassungen +- `skd.service` wird als `root` gestartet, damit PAM andere Nutzer authentifizieren kann. (Systemd Unit unter `/etc/systemd/system/skd.service`) +- `SKD_ALLOWED_USERS` in `/etc/skd/env` auf leer gesetzt, damit alle "echten" Nutzer (uid >= 1000, mit Shell) gelistet werden. + +## Code-Aenderungen +- `backend/actions.py` + - `pkill`-Exit-Code 1 (keine Prozesse) wird jetzt als normaler Zustand behandelt, kein 500-Fehler mehr. +- `backend/auth.py` + - `is_account_locked()` hinzugefuegt, liest `/etc/shadow` und erkennt gesperrte Accounts. +- `backend/models.py` + - `UserStatus` um `account_locked: bool` erweitert. +- `backend/app.py` + - `/users` liefert jetzt `account_locked`. + - Schutz: der aktuell angemeldete User kann sich nicht selbst deaktivieren. +- `backend/templates/index.html` + - Statusanzeige zeigt jetzt "aktiv/deaktiviert" pro Benutzer. + +## Client-Seite / Verhalten +- Benutzerliste erscheint erst nach "Status laden". Das UI setzt nach dem Login zwar den Token, laedt aber nicht automatisch die Benutzerliste, es sei denn die Funktion wird explizit aufgerufen. +- Workaround: nach Login einmal "Status laden" klicken. + +## Relevante Logs/Beobachtungen +- Vor Umstellung auf root: `unix_chkpwd: check pass; user unknown` und 401 bei PAM-Login. +- Nach Umstellung: Login erfolgreich, `/users` liefert 200. + diff --git a/backend/actions.py b/backend/actions.py index 04152e4..70e921e 100644 --- a/backend/actions.py +++ b/backend/actions.py @@ -133,8 +133,15 @@ def disable_user( if play_sound: _play_sound_if_available() - _run(["sudo", "pkill", "-KILL", "-u", user]) - steps.append("sessions terminated") + try: + _run(["sudo", "pkill", "-KILL", "-u", user]) + steps.append("sessions terminated") + except subprocess.CalledProcessError as exc: + if exc.returncode == 1: + # pkill returns 1 when no matching processes exist; not an error here. + steps.append("no sessions to terminate") + else: + raise if logged_in: _run(["sudo", "shutdown", "now"]) diff --git a/backend/app.py b/backend/app.py index a46f2ce..4a01eea 100644 --- a/backend/app.py +++ b/backend/app.py @@ -11,6 +11,7 @@ from backend.auth import ( authenticate_admin_user, get_current_admin, is_authorized_admin, + is_account_locked, issue_token, list_manageable_users, ) @@ -142,7 +143,14 @@ def oidc_status(settings: Settings = Depends(get_settings)) -> dict: def users(settings: Settings = Depends(get_settings)) -> List[UserStatus]: logged_in = set(actions.list_logged_in_users()) targets = list_manageable_users(settings) - return [UserStatus(user=user, logged_in=user in logged_in) for user in targets] + return [ + UserStatus( + user=user, + logged_in=user in logged_in, + account_locked=is_account_locked(user), + ) + for user in targets + ] @app.post( @@ -151,10 +159,16 @@ def users(settings: Settings = Depends(get_settings)) -> List[UserStatus]: dependencies=[Depends(get_current_admin)], ) def disable_user( + current_user: str = Depends(get_current_admin), username: str = Depends(validate_user), payload: ActionRequest | None = Body(default=None), settings: Settings = Depends(get_settings), ) -> ActionResponse: + if username == current_user: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail="Refusing to disable the currently authenticated user", + ) try: steps = actions.disable_user( username, diff --git a/backend/auth.py b/backend/auth.py index b012197..17e58c9 100644 --- a/backend/auth.py +++ b/backend/auth.py @@ -1,6 +1,7 @@ import datetime as dt import grp import pwd +import spwd from typing import List, Set, Optional import jwt @@ -119,3 +120,17 @@ def list_manageable_users(settings: Settings) -> List[str]: candidates.append(entry.pw_name) candidates.sort() return candidates + + +def is_account_locked(username: str) -> bool: + """Return True when the account is locked in /etc/shadow.""" + try: + shadow_entry = spwd.getspnam(username) + except KeyError: + return False + except PermissionError: + return False + shadow_password = shadow_entry.sp_pwdp or "" + if shadow_password in ("*", "!", "!!"): + return True + return shadow_password.startswith("!") diff --git a/backend/models.py b/backend/models.py index 4cbea83..b290194 100644 --- a/backend/models.py +++ b/backend/models.py @@ -23,6 +23,7 @@ class ActionResponse(BaseModel): class UserStatus(BaseModel): user: str logged_in: bool + account_locked: bool class LoginRequest(BaseModel): diff --git a/backend/templates/index.html b/backend/templates/index.html index 257941c..3e6e3f9 100644 --- a/backend/templates/index.html +++ b/backend/templates/index.html @@ -154,7 +154,9 @@ statusDiv.textContent = 'Lade...'; try { const data = await api('/users'); - statusDiv.textContent = data.map(u => `${u.user}: ${u.logged_in ? 'eingeloggt' : 'aus'}`).join('\n') || 'Keine Daten'; + statusDiv.textContent = data + .map(u => `${u.user}: ${u.logged_in ? 'eingeloggt' : 'aus'} | ${u.account_locked ? 'deaktiviert' : 'aktiv'}`) + .join('\n') || 'Keine Daten'; const select = document.getElementById('username'); select.innerHTML = ''; data.forEach(u => {