diff --git a/backend/app/routers/oidc.py b/backend/app/routers/oidc.py index 5130b8c..35aa65a 100644 --- a/backend/app/routers/oidc.py +++ b/backend/app/routers/oidc.py @@ -178,6 +178,44 @@ def _darf_anlegen(p, anmeldung: oidc_service.Anmeldung) -> bool: return anmeldung.email.rsplit("@", 1)[-1] in p.allowed_domains +def _uebernimm_namen(db: Session, nutzer: User, anmeldung: oidc_service.Anmeldung) -> None: + """Namensaenderungen beim Anbieter nachziehen. + + Wichtig ist, WESSEN Name das jeweils ist: + + ``display_name`` ist reine Anzeige und gehoert dem Anbieter – heiratet + jemand, soll der neue Name auch hier stehen. + + ``username`` ist das, was beim Passwort-Login eingetippt wird. Bei einem + Konto MIT Passwort gehoert er dem Nutzer; ihn hinter dessen Ruecken zu + aendern hiesse, ihm seine Anmeldung wegzunehmen. Bei einem reinen + SSO-Konto gibt es nichts einzutippen – dort ist er ein Etikett, und der + Anbieter darf es bestimmen. + + Dass das ueberhaupt geht, liegt daran, dass der Name NIRGENDS ein + Schluessel ist: Buchungen, Konten und Sitzungen haengen an der Nummer des + Nutzers. Umbenennen zieht nichts mit. + + Ist der neue Name schon vergeben, bleibt der alte stehen. Eine Anmeldung + daran scheitern zu lassen waere die schlechtere Antwort. + """ + if anmeldung.name and anmeldung.name[:120] != nutzer.display_name: + nutzer.display_name = anmeldung.name[:120] + + gewuenscht = (anmeldung.preferred_username or "").strip()[:64] + if not gewuenscht or gewuenscht == nutzer.username: + return + if nutzer.password_hash is not None: + return # gehoert dem Nutzer, nicht dem Anbieter + belegt = ( + db.query(User.id) + .filter(User.username == gewuenscht, User.id != nutzer.id) + .first() + ) + if belegt is None: + nutzer.username = gewuenscht + + def _finde_oder_lege_an(db: Session, p, anmeldung: oidc_service.Anmeldung) -> User: """Zu wem gehoert dieser Anmeldende? @@ -205,6 +243,7 @@ def _finde_oder_lege_an(db: Session, p, anmeldung: oidc_service.Anmeldung) -> Us nutzer = db.get(User, vorhanden.user_id) if nutzer is None: raise oidc_service.OidcFehler("Das verknuepfte Konto gibt es nicht mehr.") + _uebernimm_namen(db, nutzer, anmeldung) return nutzer if not _darf_anlegen(p, anmeldung): diff --git a/backend/app/routers/users.py b/backend/app/routers/users.py index 8f7a6e5..04842b6 100644 --- a/backend/app/routers/users.py +++ b/backend/app/routers/users.py @@ -49,6 +49,23 @@ def update_user(user_id: int, payload: UserUpdate, db: Session = Depends(get_db) 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: + neu = payload.username.strip() + if not neu: + raise HTTPException(status.HTTP_400_BAD_REQUEST, + "Benutzername darf nicht leer sein") + # Der Name ist ein Etikett, kein Schluessel – umbenennen zieht nichts + # mit. Eindeutig bleiben muss er trotzdem: Er ist das, was beim + # Passwort-Login eingetippt wird. + belegt = ( + db.query(User.id) + .filter(User.username == neu, User.id != user.id) + .first() + ) + if belegt is not None: + raise HTTPException(status.HTTP_409_CONFLICT, + "Benutzername existiert bereits") + user.username = neu if payload.password is not None: policy_service.assert_ok(db, payload.password) user.password_hash = hash_password(payload.password) diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 39a799d..c0cd669 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -57,6 +57,11 @@ class UserCreate(BaseModel): class UserUpdate(BaseModel): + # Umbenennen ist erlaubt: Der Benutzername ist ein ETIKETT, kein Schluessel. + # Alles Fachliche haengt an der Nummer des Nutzers – aendert sich der Name + # beim Anmeldedienst, muss Certain folgen koennen, ohne dass Daten + # umziehen. + username: str | None = Field(default=None, min_length=1, max_length=64) password: str | None = Field(default=None, min_length=1, max_length=1024) role: Role | None = None display_name: str | None = Field(default=None, max_length=120) diff --git a/backend/tests/test_oidc.py b/backend/tests/test_oidc.py index eb063b0..284670e 100644 --- a/backend/tests/test_oidc.py +++ b/backend/tests/test_oidc.py @@ -483,3 +483,145 @@ def test_unerreichbarer_anbieter_zeigt_keine_json_seite(client, anbieter, monkey assert ziel.startswith("/login"), ziel assert "oidc_error" in ziel assert "Name+or+service" in ziel or "Name%20or%20service" in ziel + + +# --- Der Benutzername ist ein Etikett, kein Schluessel ------------------------ + + +def test_umbenennen_laesst_alle_daten_stehen(admin): + """Der Kern: Buchungen, Konten und Vertraege haengen an der NUMMER des + Nutzers. Aendert sich sein Name – etwa weil der Anmeldedienst einen neuen + schickt –, darf davon nichts mitwandern.""" + username, temp = new_user(admin) + # login_ready erledigt den Pflicht-Wechsel und gibt das NEUE Passwort + # zurueck – das temporaere gilt danach nicht mehr. + nutzer, passwort = login_ready(username, temp) + + konto = nutzer.post("/api/accounts", json={ + "name": "Giro " + secrets.token_hex(3), "type": "girokonto", + "currency": "EUR", "opening_balance_cents": 12345}).json() + nutzer.post("/api/transactions", json={ + "type": "expense", "date": "2026-05-05", "account_id": konto["id"], + "amount_cents": 999}) + + ich = nutzer.get("/api/auth/me").json() + neuer_name = "umbenannt" + secrets.token_hex(4) + r = admin.patch(f"/api/users/{ich['id']}", json={"username": neuer_name}) + assert r.status_code == 200, r.text + assert r.json()["username"] == neuer_name + assert r.json()["id"] == ich["id"], "Umbenennen hat einen NEUEN Nutzer erzeugt" + + # Dieselbe Sitzung, dieselben Daten – nichts ist umgezogen. + assert nutzer.get("/api/auth/me").json()["username"] == neuer_name + konten = nutzer.get("/api/accounts").json() + assert [k["id"] for k in konten] == [konto["id"]] + assert konten[0]["balance_cents"] == 12345 - 999 + + # Und der neue Name funktioniert beim Anmelden. + nutzer.post("/api/auth/logout") + r = nutzer.post("/api/auth/login", + data={"username": neuer_name, "password": passwort}) + assert r.status_code == 200, r.text + + +def test_umbenennen_auf_einen_vergebenen_namen_wird_abgewiesen(admin): + """Eindeutig muss er bleiben – er ist das, was eingetippt wird.""" + username, passwort = new_user(admin) + nutzer, _ = login_ready(username, passwort) + ich = nutzer.get("/api/auth/me").json() + + r = admin.patch(f"/api/users/{ich['id']}", json={"username": "admin"}) + assert r.status_code == 409, r.text + assert nutzer.get("/api/auth/me").json()["username"] == username + + +def test_anbieter_darf_ein_reines_sso_konto_umbenennen(client, anbieter, monkeypatch): + """Dort gibt es nichts einzutippen – der Name ist ein Etikett, und der + Anbieter bestimmt es.""" + from app import oidc_config + from app.database import SessionLocal + from app.models import User, UserIdentity + + monkeypatch.setenv("APP_OIDC_AUTHENTIK_AUTO_CREATE", "true") + oidc_config.zuruecksetzen() + sub = "rename-" + secrets.token_hex(4) + erst = "vorher" + secrets.token_hex(3) + anbieter["claims"] = {"sub": sub, "preferred_username": erst, "name": "Vorher Nachname"} + state, nonce, _ = _beginne(client) + assert "oidc_error" not in _zurueck(client, anbieter, state, nonce).headers["location"] + ich = client.get("/api/auth/me").json() + assert ich["username"] == erst + assert ich["display_name"] == "Vorher Nachname" + client.post("/api/auth/logout") + + # Beim Anbieter umbenannt – naechste Anmeldung zieht nach. + danach = "nachher" + secrets.token_hex(3) + anbieter["claims"] = {"sub": sub, "preferred_username": danach, + "name": "Nachher Nachname"} + state, nonce, _ = _beginne(client) + assert "oidc_error" not in _zurueck(client, anbieter, state, nonce).headers["location"] + jetzt = client.get("/api/auth/me").json() + assert jetzt["id"] == ich["id"], "es ist ein ZWEITES Konto entstanden" + assert jetzt["username"] == danach, "Namensaenderung nicht uebernommen" + assert jetzt["display_name"] == "Nachher Nachname" + client.post("/api/auth/logout") + + with SessionLocal() as s: + eintrag = s.query(UserIdentity).filter(UserIdentity.subject == sub).first() + s.delete(s.get(User, eintrag.user_id)) + s.commit() + + +def test_anbieter_ruehrt_den_namen_eines_passwort_kontos_nicht_an(admin, anbieter): + """Dort gehoert der Name dem Nutzer. Ihn hinter dessen Ruecken zu aendern + hiesse, ihm seine Anmeldung wegzunehmen.""" + sub = "eigen-" + secrets.token_hex(4) + anbieter["claims"] = {"sub": sub, "preferred_username": "ganzandersername", + "name": "Anzeigename vom Anbieter"} + + state, nonce, _ = _beginne(admin, "/api/auth/oidc/authentik/link") + assert "oidc_linked" in _zurueck(admin, anbieter, state, nonce).headers["location"] + admin.post("/api/auth/logout") + + state, nonce, _ = _beginne(admin) + assert "oidc_error" not in _zurueck(admin, anbieter, state, nonce).headers["location"] + ich = admin.get("/api/auth/me").json() + assert ich["username"] == "admin", "der Anbieter hat den Namen ueberschrieben" + # Der Anzeigename darf er dagegen setzen – der ist reine Anzeige. + assert ich["display_name"] == "Anzeigename vom Anbieter" + + ident = admin.get("/api/auth/oidc/identities").json()["identities"][0] + admin.delete(f"/api/auth/oidc/identities/{ident['id']}") + admin.patch(f"/api/users/{ich['id']}", json={"display_name": None}) + admin.post("/api/auth/login", + data={"username": "admin", "password": ADMIN_PASSWORD}) + + +def test_vergebener_name_vom_anbieter_laesst_den_alten_stehen(client, anbieter, + monkeypatch): + """Eine Anmeldung daran scheitern zu lassen waere die schlechtere Antwort.""" + from app import oidc_config + from app.database import SessionLocal + from app.models import User, UserIdentity + + monkeypatch.setenv("APP_OIDC_AUTHENTIK_AUTO_CREATE", "true") + oidc_config.zuruecksetzen() + sub = "kollision-" + secrets.token_hex(4) + eigen = "eigener" + secrets.token_hex(3) + anbieter["claims"] = {"sub": sub, "preferred_username": eigen} + state, nonce, _ = _beginne(client) + _zurueck(client, anbieter, state, nonce) + client.post("/api/auth/logout") + + # Der Anbieter schickt jetzt einen Namen, den es hier schon gibt. + anbieter["claims"] = {"sub": sub, "preferred_username": "admin"} + state, nonce, _ = _beginne(client) + assert "oidc_error" not in _zurueck(client, anbieter, state, nonce).headers["location"] + assert client.get("/api/auth/me").json()["username"] == eigen, \ + "fremder Name uebernommen" + client.post("/api/auth/logout") + + with SessionLocal() as s: + eintrag = s.query(UserIdentity).filter(UserIdentity.subject == sub).first() + s.delete(s.get(User, eintrag.user_id)) + s.commit()