Spaces:
Sleeping
Sleeping
github-actions[bot] commited on
Commit ·
68a9a37
1
Parent(s): dde0c6d
Sync from GitHub 37ea6f08e07613f633eb974817509cc30f24dbf9
Browse files- app.py +13 -2
- auth/oauth.py +26 -1
- tests/test_hf_oauth_state.py +102 -0
app.py
CHANGED
|
@@ -16,7 +16,13 @@ from pydantic import BaseModel, Field
|
|
| 16 |
from sqlalchemy import text
|
| 17 |
from sqlalchemy.orm import Session
|
| 18 |
|
| 19 |
-
from auth.oauth import
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 20 |
from auth.session import (
|
| 21 |
AUTH_MODE_DEV,
|
| 22 |
AUTH_MODE_HF,
|
|
@@ -454,7 +460,12 @@ async def auth_callback(request: Request, db: Session = Depends(get_db)) -> Redi
|
|
| 454 |
expected_state = request.session.get("oauth_state")
|
| 455 |
state = request.query_params.get("state")
|
| 456 |
code = request.query_params.get("code")
|
| 457 |
-
if not
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 458 |
raise HTTPException(status_code=400, detail="Invalid OAuth state.")
|
| 459 |
if not code:
|
| 460 |
raise HTTPException(status_code=400, detail="Missing OAuth code.")
|
|
|
|
| 16 |
from sqlalchemy import text
|
| 17 |
from sqlalchemy.orm import Session
|
| 18 |
|
| 19 |
+
from auth.oauth import (
|
| 20 |
+
HFOAuthError,
|
| 21 |
+
build_hf_authorize_url,
|
| 22 |
+
exchange_code_for_hf_user,
|
| 23 |
+
generate_oauth_state,
|
| 24 |
+
is_valid_oauth_state,
|
| 25 |
+
)
|
| 26 |
from auth.session import (
|
| 27 |
AUTH_MODE_DEV,
|
| 28 |
AUTH_MODE_HF,
|
|
|
|
| 460 |
expected_state = request.session.get("oauth_state")
|
| 461 |
state = request.query_params.get("state")
|
| 462 |
code = request.query_params.get("code")
|
| 463 |
+
if not state:
|
| 464 |
+
raise HTTPException(status_code=400, detail="Invalid OAuth state.")
|
| 465 |
+
if expected_state:
|
| 466 |
+
if state != expected_state:
|
| 467 |
+
raise HTTPException(status_code=400, detail="Invalid OAuth state.")
|
| 468 |
+
elif not is_valid_oauth_state(state):
|
| 469 |
raise HTTPException(status_code=400, detail="Invalid OAuth state.")
|
| 470 |
if not code:
|
| 471 |
raise HTTPException(status_code=400, detail="Missing OAuth code.")
|
auth/oauth.py
CHANGED
|
@@ -7,6 +7,7 @@ from typing import Any
|
|
| 7 |
from urllib.parse import urlencode
|
| 8 |
|
| 9 |
import httpx
|
|
|
|
| 10 |
|
| 11 |
|
| 12 |
class HFOAuthError(RuntimeError):
|
|
@@ -23,6 +24,17 @@ class HFOAuthSettings:
|
|
| 23 |
scope: str
|
| 24 |
|
| 25 |
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 26 |
def get_hf_oauth_settings() -> HFOAuthSettings:
|
| 27 |
client_id = os.getenv("HF_OAUTH_CLIENT_ID", "").strip()
|
| 28 |
client_secret = os.getenv("HF_OAUTH_CLIENT_SECRET", "").strip()
|
|
@@ -40,7 +52,20 @@ def get_hf_oauth_settings() -> HFOAuthSettings:
|
|
| 40 |
|
| 41 |
|
| 42 |
def generate_oauth_state() -> str:
|
| 43 |
-
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 44 |
|
| 45 |
|
| 46 |
def build_hf_authorize_url(redirect_uri: str, state: str) -> str:
|
|
|
|
| 7 |
from urllib.parse import urlencode
|
| 8 |
|
| 9 |
import httpx
|
| 10 |
+
from itsdangerous import BadSignature, SignatureExpired, URLSafeTimedSerializer
|
| 11 |
|
| 12 |
|
| 13 |
class HFOAuthError(RuntimeError):
|
|
|
|
| 24 |
scope: str
|
| 25 |
|
| 26 |
|
| 27 |
+
OAUTH_STATE_SALT = "hf-oauth-state"
|
| 28 |
+
DEFAULT_OAUTH_STATE_SECRET = "dev-only-session-secret-change-me"
|
| 29 |
+
|
| 30 |
+
|
| 31 |
+
def _oauth_state_serializer() -> URLSafeTimedSerializer:
|
| 32 |
+
secret = os.getenv("APP_SESSION_SECRET", DEFAULT_OAUTH_STATE_SECRET).strip()
|
| 33 |
+
if not secret:
|
| 34 |
+
secret = DEFAULT_OAUTH_STATE_SECRET
|
| 35 |
+
return URLSafeTimedSerializer(secret_key=secret, salt=OAUTH_STATE_SALT)
|
| 36 |
+
|
| 37 |
+
|
| 38 |
def get_hf_oauth_settings() -> HFOAuthSettings:
|
| 39 |
client_id = os.getenv("HF_OAUTH_CLIENT_ID", "").strip()
|
| 40 |
client_secret = os.getenv("HF_OAUTH_CLIENT_SECRET", "").strip()
|
|
|
|
| 52 |
|
| 53 |
|
| 54 |
def generate_oauth_state() -> str:
|
| 55 |
+
payload = {"nonce": secrets.token_urlsafe(32)}
|
| 56 |
+
return _oauth_state_serializer().dumps(payload)
|
| 57 |
+
|
| 58 |
+
|
| 59 |
+
def is_valid_oauth_state(state: str) -> bool:
|
| 60 |
+
ttl_seconds = int(os.getenv("AUTH_OAUTH_STATE_TTL_SECONDS", "600"))
|
| 61 |
+
try:
|
| 62 |
+
payload = _oauth_state_serializer().loads(state, max_age=ttl_seconds)
|
| 63 |
+
except (BadSignature, SignatureExpired):
|
| 64 |
+
return False
|
| 65 |
+
if not isinstance(payload, dict):
|
| 66 |
+
return False
|
| 67 |
+
nonce = payload.get("nonce")
|
| 68 |
+
return isinstance(nonce, str) and bool(nonce.strip())
|
| 69 |
|
| 70 |
|
| 71 |
def build_hf_authorize_url(redirect_uri: str, state: str) -> str:
|
tests/test_hf_oauth_state.py
ADDED
|
@@ -0,0 +1,102 @@
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
|
| 1 |
+
from __future__ import annotations
|
| 2 |
+
|
| 3 |
+
import pathlib
|
| 4 |
+
import sys
|
| 5 |
+
|
| 6 |
+
import pytest
|
| 7 |
+
from fastapi.testclient import TestClient
|
| 8 |
+
from sqlalchemy import create_engine
|
| 9 |
+
from sqlalchemy.orm import sessionmaker
|
| 10 |
+
|
| 11 |
+
ROOT = pathlib.Path(__file__).resolve().parents[1]
|
| 12 |
+
sys.path.insert(0, str(ROOT))
|
| 13 |
+
|
| 14 |
+
import app as app_module
|
| 15 |
+
from app import app
|
| 16 |
+
from auth.oauth import generate_oauth_state, is_valid_oauth_state
|
| 17 |
+
from data.db import Base, get_db
|
| 18 |
+
|
| 19 |
+
|
| 20 |
+
@pytest.fixture()
|
| 21 |
+
def db_engine(tmp_path):
|
| 22 |
+
db_file = tmp_path / "test_hf_oauth_state.db"
|
| 23 |
+
engine = create_engine(
|
| 24 |
+
f"sqlite:///{db_file}",
|
| 25 |
+
connect_args={"check_same_thread": False},
|
| 26 |
+
)
|
| 27 |
+
import data.models # noqa: F401
|
| 28 |
+
|
| 29 |
+
Base.metadata.create_all(bind=engine)
|
| 30 |
+
yield engine
|
| 31 |
+
Base.metadata.drop_all(bind=engine)
|
| 32 |
+
engine.dispose()
|
| 33 |
+
|
| 34 |
+
|
| 35 |
+
@pytest.fixture()
|
| 36 |
+
def db_session(db_engine):
|
| 37 |
+
Session = sessionmaker(autocommit=False, autoflush=False, bind=db_engine)
|
| 38 |
+
session = Session()
|
| 39 |
+
yield session
|
| 40 |
+
session.close()
|
| 41 |
+
|
| 42 |
+
|
| 43 |
+
@pytest.fixture()
|
| 44 |
+
def client(db_session):
|
| 45 |
+
def _override_get_db():
|
| 46 |
+
yield db_session
|
| 47 |
+
|
| 48 |
+
app.dependency_overrides[get_db] = _override_get_db
|
| 49 |
+
with TestClient(app, raise_server_exceptions=True) as c:
|
| 50 |
+
yield c
|
| 51 |
+
app.dependency_overrides.clear()
|
| 52 |
+
|
| 53 |
+
|
| 54 |
+
def test_oauth_state_is_signed_and_valid(monkeypatch):
|
| 55 |
+
monkeypatch.setenv("APP_SESSION_SECRET", "oauth-state-test-secret")
|
| 56 |
+
state = generate_oauth_state()
|
| 57 |
+
assert state
|
| 58 |
+
assert is_valid_oauth_state(state) is True
|
| 59 |
+
assert is_valid_oauth_state("plain-random-state") is False
|
| 60 |
+
|
| 61 |
+
|
| 62 |
+
def test_callback_accepts_signed_state_without_session(client, monkeypatch):
|
| 63 |
+
monkeypatch.setenv("AUTH_MODE", "hf_oauth")
|
| 64 |
+
monkeypatch.setenv("APP_SESSION_SECRET", "oauth-state-test-secret")
|
| 65 |
+
monkeypatch.setenv("HF_OAUTH_CLIENT_ID", "test-client-id")
|
| 66 |
+
monkeypatch.setenv("HF_OAUTH_CLIENT_SECRET", "test-client-secret")
|
| 67 |
+
monkeypatch.setenv("HF_OAUTH_REDIRECT_URI", "http://testserver/auth/callback")
|
| 68 |
+
monkeypatch.setenv("AUTH_SUCCESS_REDIRECT_URL", "http://testserver/")
|
| 69 |
+
|
| 70 |
+
async def _fake_exchange(*, code: str, redirect_uri: str):
|
| 71 |
+
return {
|
| 72 |
+
"email": "oauth-user@example.com",
|
| 73 |
+
"display_name": "OAuth User",
|
| 74 |
+
"avatar_url": None,
|
| 75 |
+
"provider_sub": "sub-123",
|
| 76 |
+
}
|
| 77 |
+
|
| 78 |
+
monkeypatch.setattr(app_module, "exchange_code_for_hf_user", _fake_exchange)
|
| 79 |
+
|
| 80 |
+
response = client.get(
|
| 81 |
+
"/auth/callback",
|
| 82 |
+
params={"state": generate_oauth_state(), "code": "test-code"},
|
| 83 |
+
follow_redirects=False,
|
| 84 |
+
)
|
| 85 |
+
assert response.status_code == 302
|
| 86 |
+
location = response.headers.get("location", "")
|
| 87 |
+
assert "auth_bridge=" in location
|
| 88 |
+
|
| 89 |
+
|
| 90 |
+
def test_callback_rejects_invalid_state_without_session(client, monkeypatch):
|
| 91 |
+
monkeypatch.setenv("AUTH_MODE", "hf_oauth")
|
| 92 |
+
monkeypatch.setenv("APP_SESSION_SECRET", "oauth-state-test-secret")
|
| 93 |
+
monkeypatch.setenv("HF_OAUTH_CLIENT_ID", "test-client-id")
|
| 94 |
+
monkeypatch.setenv("HF_OAUTH_CLIENT_SECRET", "test-client-secret")
|
| 95 |
+
|
| 96 |
+
response = client.get(
|
| 97 |
+
"/auth/callback",
|
| 98 |
+
params={"state": "invalid-state", "code": "test-code"},
|
| 99 |
+
follow_redirects=False,
|
| 100 |
+
)
|
| 101 |
+
assert response.status_code == 400
|
| 102 |
+
assert response.json()["detail"] == "Invalid OAuth state."
|