diff --git a/README.md b/README.md index 97d5cfc..a2fbfa9 100644 --- a/README.md +++ b/README.md @@ -183,6 +183,10 @@ und einen zweiten Block mit dessen Präfix anlegen (`OIDC__…`) – am und alle Endpunkte bleiben unverändert. - **Rollen kommen nie aus dem Anbieter.** Neu angelegte SSO-Konten sind immer *Benutzer*; Admin vergibt ein Admin von Hand. +- Der **Benutzername** wird einmal beim Anlegen übernommen und gehört danach + Vorrania – ein späterer Namenswechsel beim Anmeldedienst schlägt nicht durch. + Ändern lässt er sich jederzeit unter *Benutzer*: Sitzungen, Verlauf und + API-Tokens hängen an der Benutzer-ID, nicht am Namen. - **Automatisch** verknüpft wird nur bei bestätigter E-Mail-Adresse. Sonst könnte ein Konto im Anmeldedienst mit frei gewählter Adresse ein fremdes Vorrania-Konto übernehmen. Beim Verknüpfen über *Mein Konto* entfällt das, diff --git a/backend/app/deps.py b/backend/app/deps.py index 83bb121..318af19 100644 --- a/backend/app/deps.py +++ b/backend/app/deps.py @@ -17,16 +17,34 @@ _credentials_exc = HTTPException( ) +def _nutzer_aus_payload(db: Session, payload: dict) -> User | None: + """Loest die Sitzung auf – bevorzugt ueber die Benutzer-ID. + + Der Benutzername ist Anzeige und darf sich aendern; die ID nicht. Deshalb + benennt ein neues Token den Benutzer ueber ``uid``. + """ + uid = payload.get("uid") + if uid is not None: + try: + return db.get(User, int(uid)) + except (TypeError, ValueError): + return None + + # Token aus der Zeit vor der Umstellung (``sub`` = Benutzername). Laeuft + # spaetestens nach jwt_expire_minutes aus; dieser Zweig darf danach weg. + username = payload.get("sub") + if not username: + return None + return db.query(User).filter(User.username == username).first() + + def get_current_user( token: str = Depends(oauth2_scheme), db: Session = Depends(get_db) ) -> User: """Akzeptiert ein JWT aus dem Login oder ein langlebiges API-Token.""" payload = decode_access_token(token) if payload is not None: - username = payload.get("sub") - if not username: - raise _credentials_exc - user = db.query(User).filter(User.username == username).first() + user = _nutzer_aus_payload(db, payload) if user is None: raise _credentials_exc return user diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 5b27e4a..ff59cf3 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -21,7 +21,7 @@ def login( status_code=status.HTTP_401_UNAUTHORIZED, detail="Benutzername oder Passwort falsch", ) - token = create_access_token(subject=user.username, role=user.role.value) + token = create_access_token(user_id=user.id, role=user.role.value) return Token(access_token=token, role=user.role, username=user.username) diff --git a/backend/app/routers/oidc.py b/backend/app/routers/oidc.py index debc38c..20e2cda 100644 --- a/backend/app/routers/oidc.py +++ b/backend/app/routers/oidc.py @@ -64,7 +64,7 @@ def _provider_oder_404(slug: str) -> OidcProvider: def _token_fuer(user: User) -> Token: - token = create_access_token(subject=user.username, role=user.role.value) + token = create_access_token(user_id=user.id, role=user.role.value) return Token(access_token=token, role=user.role, username=user.username) diff --git a/backend/app/routers/users.py b/backend/app/routers/users.py index ddc6fc4..854590e 100644 --- a/backend/app/routers/users.py +++ b/backend/app/routers/users.py @@ -65,6 +65,14 @@ def update_user( user = db.get(User, user_id) if user is None: raise HTTPException(status.HTTP_404_NOT_FOUND, "Benutzer nicht gefunden") + if payload.username is not None: + neuer_name = payload.username.strip() + if not neuer_name: + raise HTTPException(status.HTTP_400_BAD_REQUEST, "Benutzername darf nicht leer sein") + if neuer_name != user.username: + if db.query(User).filter(User.username == neuer_name).first(): + raise HTTPException(status.HTTP_409_CONFLICT, "Benutzername existiert bereits") + user.username = neuer_name if payload.password is not None: user.password_hash = hash_password(payload.password) if payload.email is not None: diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 4ee838d..e8c6de5 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -106,6 +106,9 @@ class UserCreate(BaseModel): class UserUpdate(BaseModel): + # Umbenennen ist gefahrlos: Sitzungen, Bewegungen, Dashboards und API-Tokens + # haengen an der Benutzer-ID, nicht am Namen. + username: str | None = Field(default=None, min_length=1, max_length=64) password: str | None = Field(default=None, min_length=4, max_length=255) # Leerer String heisst „Adresse entfernen" – deshalb hier nicht min_length. email: str | None = Field(default=None, max_length=255) diff --git a/backend/app/security.py b/backend/app/security.py index a8048f4..76f67a9 100644 --- a/backend/app/security.py +++ b/backend/app/security.py @@ -31,9 +31,19 @@ def verify_password(password: str, password_hash: str | None) -> bool: return False -def create_access_token(subject: str, role: str) -> str: +def create_access_token(user_id: int, role: str) -> str: + """Stellt ein Sitzungs-Token aus, das den Benutzer ueber seine ID benennt. + + Frueher stand hier der Benutzername. Damit war der Name die Kennung der + Sitzung – und ein Umbenennen haette jede laufende Anmeldung dieses Benutzers + ungueltig gemacht. Ueber die ID ist der Name reine Anzeige und frei aenderbar. + + ``uid`` steht zusaetzlich zu ``sub`` drin, damit sich alte Token (``sub`` = + Name) eindeutig von neuen unterscheiden lassen – ein Benutzer koennte sonst + schlicht "42" heissen. + """ expire = datetime.now(timezone.utc) + timedelta(minutes=settings.jwt_expire_minutes) - payload = {"sub": subject, "role": role, "exp": expire} + payload = {"sub": str(user_id), "uid": user_id, "role": role, "exp": expire} return jwt.encode(payload, settings.jwt_secret, algorithm=settings.jwt_algorithm) diff --git a/backend/tests/test_sitzung.py b/backend/tests/test_sitzung.py new file mode 100644 index 0000000..5f59920 --- /dev/null +++ b/backend/tests/test_sitzung.py @@ -0,0 +1,118 @@ +"""Sitzungs-Token: der Benutzer haengt an seiner ID, nicht an seinem Namen. + +Frueher stand der Benutzername im Token. Damit war der Name die Kennung der +Sitzung – ein Umbenennen haette jede laufende Anmeldung ungueltig gemacht und +war deshalb gar nicht erst vorgesehen. +""" + +from datetime import datetime, timedelta, timezone + +import pytest +from fastapi import HTTPException +from jose import jwt + +from app.config import get_settings +from app.deps import get_current_user +from app.models import Role, User +from app.routers.users import update_user +from app.schemas import UserUpdate +from app.security import create_access_token, hash_password + + +@pytest.fixture +def eva(db) -> User: + nutzer = User(username="eva", password_hash=hash_password("geheim123"), role=Role.admin) + db.add(nutzer) + db.commit() + db.refresh(nutzer) + return nutzer + + +def test_token_benennt_die_id_nicht_den_namen(eva): + einstellungen = get_settings() + inhalt = jwt.decode( + create_access_token(user_id=eva.id, role=eva.role.value), + einstellungen.jwt_secret, + algorithms=[einstellungen.jwt_algorithm], + ) + assert inhalt["uid"] == eva.id + assert inhalt["sub"] == str(eva.id) + assert "eva" not in inhalt.values() + + +def test_umbenennen_beendet_die_sitzung_nicht(db, eva): + """Der eigentliche Gewinn: das Token gilt nach dem Umbenennen weiter.""" + token = create_access_token(user_id=eva.id, role=eva.role.value) + + update_user(user_id=eva.id, payload=UserUpdate(username="eva.neu"), db=db) + + weiterhin = get_current_user(token=token, db=db) + assert weiterhin.id == eva.id + assert weiterhin.username == "eva.neu" + + +def test_umbenennen_auf_vergebenen_namen_wird_abgelehnt(db, eva): + db.add(User(username="otto", password_hash=hash_password("geheim123"), role=Role.user)) + db.commit() + + with pytest.raises(HTTPException) as fehler: + update_user(user_id=eva.id, payload=UserUpdate(username="otto"), db=db) + + assert fehler.value.status_code == 409 + db.refresh(eva) + assert eva.username == "eva" + + +def test_gleicher_name_ist_kein_konflikt(db, eva): + """Ein Speichern ohne Namensaenderung darf nicht an sich selbst scheitern.""" + update_user(user_id=eva.id, payload=UserUpdate(username="eva"), db=db) + db.refresh(eva) + assert eva.username == "eva" + + +def test_geloeschter_benutzer_hat_keine_gueltige_sitzung(db, eva): + token = create_access_token(user_id=eva.id, role=eva.role.value) + db.delete(eva) + db.commit() + + with pytest.raises(HTTPException) as fehler: + get_current_user(token=token, db=db) + assert fehler.value.status_code == 401 + + +def test_altes_token_mit_namen_gilt_weiter(db, eva): + """Beim Update darf niemand aus der laufenden Sitzung fliegen. + + Token aus der Zeit vor der Umstellung tragen den Namen in ``sub`` und kein + ``uid``; sie muessen bis zu ihrem Ablauf weiter funktionieren. + """ + einstellungen = get_settings() + alt = jwt.encode( + { + "sub": "eva", + "role": "admin", + "exp": datetime.now(timezone.utc) + timedelta(hours=1), + }, + einstellungen.jwt_secret, + algorithm=einstellungen.jwt_algorithm, + ) + + assert get_current_user(token=alt, db=db).id == eva.id + + +def test_benutzer_namens_42_verwirrt_die_aufloesung_nicht(db): + """Ein rein numerischer Name darf nicht als ID missdeutet werden.""" + einstellungen = get_settings() + zahlname = User(username="42", password_hash=hash_password("geheim123"), role=Role.user) + anderer = User(username="otto", password_hash=hash_password("geheim123"), role=Role.user) + db.add_all([zahlname, anderer]) + db.commit() + + # Altes Token fuer den Benutzer "42" – ohne uid, also ueber den Namen. + alt = jwt.encode( + {"sub": "42", "role": "user", "exp": datetime.now(timezone.utc) + timedelta(hours=1)}, + einstellungen.jwt_secret, + algorithm=einstellungen.jwt_algorithm, + ) + + assert get_current_user(token=alt, db=db).id == zahlname.id diff --git a/web/src/pages/Users.jsx b/web/src/pages/Users.jsx index 3e5fdc6..dc92545 100644 --- a/web/src/pages/Users.jsx +++ b/web/src/pages/Users.jsx @@ -7,7 +7,7 @@ import { useToast } from "../toast"; export default function Users() { const confirm = useConfirm(); - const { user: me } = useAuth(); + const { user: me, erneutVersuchen } = useAuth(); const toast = useToast(); const [users, setUsers] = useState([]); const [form, setForm] = useState({ username: "", password: "", email: "", role: "user" }); @@ -36,6 +36,24 @@ export default function Users() { } } + // Umbenennen ist gefahrlos: Sitzungen, Bewegungen und API-Tokens haengen an + // der Benutzer-ID, nicht am Namen. Wer sich selbst umbenennt, bleibt angemeldet. + async function changeUsername(u, username) { + const neu = username.trim(); + if (!neu || neu === u.username) { load(); return; } + setError(null); + try { + await api.updateUser(u.id, { username: neu }); + toast("Benutzername geändert."); + // Der eigene Name steht auch in der Seitenleiste – dort nachziehen. + if (u.id === me.id) await erneutVersuchen(); + load(); + } catch (err) { + setError(err.message); + load(); + } + } + // Die Adresse ist der Schluessel fuer die erste SSO-Anmeldung: nur ueber sie // findet ein Anmeldedienst das passende Bestandskonto. async function changeEmail(u, email) { @@ -94,8 +112,12 @@ export default function Users() { {users.map((u) => ( - - {u.username}{u.id === me.id && (du)} + + changeUsername(u, e.target.value)} + /> + {u.id === me.id && (du)}