diff --git a/pygitweb/README.md b/pygitweb/README.md index 866203d..2b93912 100644 --- a/pygitweb/README.md +++ b/pygitweb/README.md @@ -41,7 +41,7 @@ Settings load from `PYGITWEB_*` environment variables (or a `.env` file in the w - `PYGITWEB_AUTH` — `0` / `false` / `off` disables auth; `1` / `true` / `on` enables it. If **unset or empty**, auth defaults to **on**, a random password is generated at startup, and a **warning is printed to stderr** (set `PYGITWEB_AUTH` explicitly to `0` or `1` and configure credentials as below). - `PYGITWEB_ADMIN_USER` — login username when auth is on (default: `admin`) - `PYGITWEB_ADMIN_PASSWORD` — login password when auth is on (optional at first run: a random value is generated and printed if missing while auth is enabled) -- Browser login: `GET /login` (optional `?next=/path`), `POST /login` with form fields `username`, `password`, `next`; sets an HttpOnly cookie also accepted by protected routes. `GET /logout?next=/` clears it. +- Browser login: `GET /login` (optional `?next=/path`), `POST /login` with form fields `username`, `password`, `next`; sets an HttpOnly cookie with an opaque in-process session id (also accepted by protected routes via `Authorization: Bearer`). Sessions are wiped when the process exits. `GET /logout?next=/` revokes the session and clears the cookie. ## Routes diff --git a/pygitweb/auth.py b/pygitweb/auth.py index a2024c1..c4486bb 100644 --- a/pygitweb/auth.py +++ b/pygitweb/auth.py @@ -1,7 +1,8 @@ """ OAuth2 password flow (tutorial-style) with credentials from PYGITWEB_ADMIN_USER / PYGITWEB_ADMIN_PASSWORD. -Browser login: GET/POST /login sets an HttpOnly cookie; APIs still accept Authorization: Bearer. +Browser login: GET/POST /login sets an HttpOnly cookie holding an opaque session id. +APIs accept Authorization: Bearer . Sessions live in memory only (see pygitweb.sessions). """ from __future__ import annotations @@ -15,6 +16,7 @@ from fastapi.security import APIKeyCookie, OAuth2PasswordBearer, OAuth2PasswordR from pydantic import BaseModel from pygitweb.config import settings +from pygitweb.sessions import create_session, get_session, revoke_session from pygitweb.templates_env import POSTAMBLE, PREAMBLE, env ACCESS_TOKEN_COOKIE_NAME = "pygitweb_access_token" @@ -65,7 +67,10 @@ def decode_access_token(token: str) -> User | None: stored = get_auth_credentials() if stored is None: return None - if not hmac.compare_digest(stored.username, token): + rec = get_session(token) + if rec is None: + return None + if not hmac.compare_digest(stored.username, rec.username): return None return User(username=stored.username) @@ -148,10 +153,11 @@ async def login_form_submit( error="Incorrect username or password.", ) return HTMLResponse(f"{pre}{body}{POSTAMBLE}") + sid = create_session(user.username) resp = RedirectResponse(url=next_safe, status_code=303) resp.set_cookie( key=ACCESS_TOKEN_COOKIE_NAME, - value=user.username, + value=sid, httponly=True, samesite="lax", max_age=LOGIN_COOKIE_MAX_AGE, @@ -161,7 +167,11 @@ async def login_form_submit( @auth_router.get("/logout") -async def logout_page(next: Annotated[str, Query()] = "/") -> RedirectResponse: +async def logout_page( + token: Annotated[str | None, Depends(access_token_from_request)], + next: Annotated[str, Query()] = "/", +) -> RedirectResponse: + revoke_session(token) resp = RedirectResponse(url=safe_next_url(next), status_code=303) resp.delete_cookie(key=ACCESS_TOKEN_COOKIE_NAME, path="/") return resp @@ -174,7 +184,8 @@ async def login(form_data: Annotated[OAuth2PasswordRequestForm, Depends()]) -> d user = authenticate_user(form_data.username, form_data.password) if user is None: raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail="Incorrect username or password") - return {"access_token": user.username, "token_type": "bearer"} + sid = create_session(user.username) + return {"access_token": sid, "token_type": "bearer"} @auth_router.get("/auth/status") diff --git a/pygitweb/auth_test.py b/pygitweb/auth_test.py index b733b04..15c62a9 100644 --- a/pygitweb/auth_test.py +++ b/pygitweb/auth_test.py @@ -29,7 +29,11 @@ def test_token_success(client: TestClient) -> None: ): r = client.post("/token", data={"username": "admin", "password": "secret"}) assert r.status_code == 200 - assert r.json() == {"access_token": "admin", "token_type": "bearer"} + data = r.json() + assert data["token_type"] == "bearer" + assert isinstance(data["access_token"], str) + assert len(data["access_token"]) >= 32 + assert data["access_token"] != "admin" def test_token_wrong_password(client: TestClient) -> None: @@ -71,6 +75,37 @@ def test_login_form_sets_cookie_and_users_me(client: TestClient) -> None: assert r2.json()["username"] == "admin" +def test_logout_revokes_cookie_session(client: TestClient) -> None: + with ( + patch.object(settings, "AUTH", True), + patch.object(settings, "ADMIN_USER", "admin"), + patch.object(settings, "ADMIN_PASSWORD", "secret"), + ): + client.post( + "/login", + data={"username": "admin", "password": "secret", "next": "/"}, + follow_redirects=False, + ) + assert client.get("/users/me").status_code == 200 + r_out = client.get("/logout", params={"next": "/"}, follow_redirects=False) + assert r_out.status_code == 303 + assert client.get("/users/me").status_code == 401 + + +def test_logout_with_bearer_revokes_token(client: TestClient) -> None: + with ( + patch.object(settings, "AUTH", True), + patch.object(settings, "ADMIN_USER", "admin"), + patch.object(settings, "ADMIN_PASSWORD", "secret"), + ): + tok = client.post("/token", data={"username": "admin", "password": "secret"}).json()["access_token"] + headers = {"Authorization": f"Bearer {tok}"} + assert client.get("/users/me", headers=headers).status_code == 200 + r_out = client.get("/logout", params={"next": "/"}, headers=headers, follow_redirects=False) + assert r_out.status_code == 303 + assert client.get("/users/me", headers=headers).status_code == 401 + + def test_projectnamevalid_no_auth_required_when_auth_disabled(client: TestClient) -> None: with patch.object(settings, "AUTH", False), patch("pygitweb.main.project_visible_in_list", return_value=False): r = client.get("/projectnamevalid", params={"name": "newproj"}) diff --git a/pygitweb/main.py b/pygitweb/main.py index b900b38..1bb7a80 100644 --- a/pygitweb/main.py +++ b/pygitweb/main.py @@ -98,6 +98,7 @@ from pygitweb.hooks_install import ( from pygitweb.merge_requests import merge_router from pygitweb.plugin_loader import load_plugin_actions, load_subpages from pygitweb.projects import git_get_project_owner, git_get_projects_list +from pygitweb.sessions import clear_all_sessions from pygitweb.settings import router as settings_router from pygitweb.tasks import ( board_router, @@ -190,6 +191,7 @@ async def app_lifespan(app: FastAPI) -> AsyncGenerator[None, None]: app.state.shutting_down = True app.state.shutdown_event.set() clear_timeline_cache_async() + clear_all_sessions() with open("pygitweb/README.md", encoding="utf-8") as _readme_file: diff --git a/pygitweb/sessions.py b/pygitweb/sessions.py new file mode 100644 index 0000000..c06f17d --- /dev/null +++ b/pygitweb/sessions.py @@ -0,0 +1,55 @@ +""" +In-process opaque session IDs (browser cookie / Bearer token value). + +Single-process only; restarting the server clears every session (empty dict). +""" + +from __future__ import annotations + +import secrets +import threading +import time +from dataclasses import dataclass + +_lock = threading.Lock() + + +@dataclass(frozen=True) +class Session: + username: str + created_at: float + + +_sessions: dict[str, Session] = {} + + +def create_session(username: str) -> str: + sid = secrets.token_urlsafe(32) + with _lock: + _sessions[sid] = Session(username=username, created_at=time.time()) + return sid + + +def get_session(session_id: str | None) -> Session | None: + if not session_id: + return None + raw = session_id.strip() + if not raw: + return None + with _lock: + return _sessions.get(raw) + + +def revoke_session(session_id: str | None) -> None: + if not session_id: + return + raw = session_id.strip() + if not raw: + return + with _lock: + _sessions.pop(raw, None) + + +def clear_all_sessions() -> None: + with _lock: + _sessions.clear()