security: require fresh authorization for high-impact actions (#341)
This commit is contained in:
1 parent
5d326c30a1
commit
3726360baa
8 files changed
+597
-9
No files matched your search
@@ -18,6 +18,7 @@ from src.session_store import SessionStore, SessionStoreError
|
||||
SESSION_COOKIE = "stackchain_session"
|
||||
CSRF_COOKIE = "stackchain_csrf"
|
||||
DEFAULT_TTL_SECONDS = 8 * 60 * 60
|
||||
STEP_UP_TTL_SECONDS = 90
|
||||
MIN_SECRET_LENGTH = 24
|
||||
OPERATOR_MODE = "operator"
|
||||
INSECURE_LOCAL_MODE = "insecure-local"
|
||||
@@ -170,6 +171,28 @@ async def revoke_managed_session(management_id: str) -> bool:
|
||||
return await asyncio.to_thread(_session_store().revoke_managed, management_id)
|
||||
|
||||
|
||||
async def issue_step_up(session: Session, *, action: str, target: str) -> str:
|
||||
return await asyncio.to_thread(
|
||||
_session_store().mint_step_up,
|
||||
session.session_id,
|
||||
action=action,
|
||||
target=target,
|
||||
ttl_seconds=STEP_UP_TTL_SECONDS,
|
||||
)
|
||||
|
||||
|
||||
async def consume_step_up(
|
||||
grant: str, session: Session, *, action: str, target: str
|
||||
) -> bool:
|
||||
return await asyncio.to_thread(
|
||||
_session_store().consume_step_up,
|
||||
grant,
|
||||
session.session_id,
|
||||
action=action,
|
||||
target=target,
|
||||
)
|
||||
|
||||
|
||||
async def request_session(request: Request) -> Session | None:
|
||||
return await asyncio.to_thread(verify_session, request.cookies.get(SESSION_COOKIE))
|
||||
|
||||
|
||||
+144
-2
@@ -146,6 +146,14 @@ class DashboardSignIn(BaseModel):
|
||||
return normalized
|
||||
|
||||
|
||||
class FreshAuthorization(BaseModel):
|
||||
access_token: str = Field(min_length=1, max_length=1_024)
|
||||
action: Literal[
|
||||
"merge_pull", "close_issue", "revoke_device", "revoke_all_sessions"
|
||||
]
|
||||
target: str = Field(min_length=1, max_length=255)
|
||||
|
||||
|
||||
def _login_attempt_store() -> LoginAttemptStore:
|
||||
state_dir = os.getenv("STACKCHAIN_STATE_DIR", ".stackchain-state")
|
||||
return LoginAttemptStore(
|
||||
@@ -160,6 +168,33 @@ def _login_attempt_store() -> LoginAttemptStore:
|
||||
)
|
||||
|
||||
|
||||
async def _require_step_up(
|
||||
request: Request,
|
||||
grant: str | None,
|
||||
*,
|
||||
action: str,
|
||||
target: str,
|
||||
) -> None:
|
||||
if dashboard_auth.mode() != dashboard_auth.OPERATOR_MODE:
|
||||
return
|
||||
try:
|
||||
valid = bool(grant) and await dashboard_auth.consume_step_up(
|
||||
grant, request.state.dashboard_session, action=action, target=target
|
||||
)
|
||||
except dashboard_auth.SessionStoreError:
|
||||
raise HTTPException(
|
||||
status_code=503, detail="Session registry is temporarily unavailable"
|
||||
)
|
||||
if not valid:
|
||||
return_payload = {
|
||||
"detail": "Fresh authorization required",
|
||||
"code": "step_up_required",
|
||||
"action": action,
|
||||
"target": target,
|
||||
}
|
||||
raise HTTPException(status_code=428, detail=return_payload)
|
||||
|
||||
|
||||
class NotificationReadBatch(BaseModel):
|
||||
ids: list[PositiveInt] = Field(min_length=1, max_length=50)
|
||||
|
||||
@@ -732,6 +767,68 @@ async def sign_in(payload: DashboardSignIn, request: Request, response: Response
|
||||
return {"authenticated": True}
|
||||
|
||||
|
||||
@app.post("/api/v1/fresh-authorization", status_code=201)
|
||||
async def fresh_authorization(payload: FreshAuthorization, request: Request):
|
||||
peer_host = request.client.host if request.client is not None else "unknown"
|
||||
source = client_source(
|
||||
peer_host,
|
||||
request.headers.get("x-forwarded-for", ""),
|
||||
os.getenv("STACKCHAIN_TRUSTED_PROXY_CIDRS", ""),
|
||||
)
|
||||
attempts = _login_attempt_store()
|
||||
try:
|
||||
retry_after = await asyncio.to_thread(attempts.retry_after, source)
|
||||
except LoginAttemptStoreError:
|
||||
return JSONResponse(
|
||||
{"detail": "Sign-in throttling is temporarily unavailable"},
|
||||
status_code=503,
|
||||
headers={"Cache-Control": "no-store"},
|
||||
)
|
||||
if retry_after:
|
||||
return JSONResponse(
|
||||
{"detail": "Too many sign-in attempts"},
|
||||
status_code=429,
|
||||
headers={"Cache-Control": "no-store", "Retry-After": str(retry_after)},
|
||||
)
|
||||
configured_token = dashboard_auth.access_token()
|
||||
if not configured_token or not hmac.compare_digest(
|
||||
payload.access_token, configured_token
|
||||
):
|
||||
try:
|
||||
await asyncio.to_thread(attempts.record_failure, source)
|
||||
except LoginAttemptStoreError:
|
||||
return JSONResponse(
|
||||
{"detail": "Sign-in throttling is temporarily unavailable"},
|
||||
status_code=503,
|
||||
headers={"Cache-Control": "no-store"},
|
||||
)
|
||||
raise HTTPException(status_code=401, detail="Invalid access token")
|
||||
try:
|
||||
await asyncio.to_thread(attempts.clear, source)
|
||||
grant = await dashboard_auth.issue_step_up(
|
||||
request.state.dashboard_session,
|
||||
action=payload.action,
|
||||
target=payload.target,
|
||||
)
|
||||
except LoginAttemptStoreError:
|
||||
return JSONResponse(
|
||||
{"detail": "Sign-in throttling is temporarily unavailable"},
|
||||
status_code=503,
|
||||
headers={"Cache-Control": "no-store"},
|
||||
)
|
||||
except dashboard_auth.SessionStoreError:
|
||||
return JSONResponse(
|
||||
{"detail": "Session registry is temporarily unavailable"},
|
||||
status_code=503,
|
||||
headers={"Cache-Control": "no-store"},
|
||||
)
|
||||
return JSONResponse(
|
||||
{"grant": grant, "expires_in": dashboard_auth.STEP_UP_TTL_SECONDS},
|
||||
status_code=201,
|
||||
headers={"Cache-Control": "no-store"},
|
||||
)
|
||||
|
||||
|
||||
@app.get("/api/v1/session")
|
||||
async def session_status(request: Request):
|
||||
session = request.state.dashboard_session
|
||||
@@ -802,7 +899,16 @@ async def revoke_active_device(
|
||||
management_id: str = PathParam(
|
||||
min_length=16, max_length=64, pattern=r"^[A-Za-z0-9_-]+$"
|
||||
),
|
||||
step_up_grant: str | None = Header(
|
||||
default=None, alias="X-Step-Up-Grant", max_length=128
|
||||
),
|
||||
):
|
||||
await _require_step_up(
|
||||
request,
|
||||
step_up_grant,
|
||||
action="revoke_device",
|
||||
target=management_id,
|
||||
)
|
||||
try:
|
||||
devices = await dashboard_auth.active_devices(request.state.dashboard_session)
|
||||
target = next(
|
||||
@@ -821,7 +927,19 @@ async def revoke_active_device(
|
||||
|
||||
|
||||
@app.delete("/api/v1/sessions")
|
||||
async def sign_out_all_devices(request: Request, response: Response):
|
||||
async def sign_out_all_devices(
|
||||
request: Request,
|
||||
response: Response,
|
||||
step_up_grant: str | None = Header(
|
||||
default=None, alias="X-Step-Up-Grant", max_length=128
|
||||
),
|
||||
):
|
||||
await _require_step_up(
|
||||
request,
|
||||
step_up_grant,
|
||||
action="revoke_all_sessions",
|
||||
target="all",
|
||||
)
|
||||
try:
|
||||
await dashboard_auth.revoke_all_sessions()
|
||||
except dashboard_auth.SessionStoreError:
|
||||
@@ -2102,8 +2220,22 @@ async def comment_on_assigned_issue(
|
||||
|
||||
|
||||
@app.patch("/api/v1/repos/{owner}/{repo}/issues/{number}/close")
|
||||
async def close_assigned_issue(owner: str, repo: str, number: int = PathParam(gt=0)):
|
||||
async def close_assigned_issue(
|
||||
request: Request,
|
||||
owner: str,
|
||||
repo: str,
|
||||
number: int = PathParam(gt=0),
|
||||
step_up_grant: str | None = Header(
|
||||
default=None, alias="X-Step-Up-Grant", max_length=128
|
||||
),
|
||||
):
|
||||
repository = f"{owner}/{repo}"
|
||||
await _require_step_up(
|
||||
request,
|
||||
step_up_grant,
|
||||
action="close_issue",
|
||||
target=f"{repository}#{number}",
|
||||
)
|
||||
|
||||
async def close_issue():
|
||||
if not await gitea_proxy.is_assigned_issue(repository, number):
|
||||
@@ -2336,11 +2468,21 @@ async def comment_on_assigned_pull(
|
||||
@app.post("/api/v1/repos/{owner}/{repo}/pulls/{number}/merge")
|
||||
async def merge_assigned_pull(
|
||||
submission: PullMergeSubmission,
|
||||
request: Request,
|
||||
owner: str,
|
||||
repo: str,
|
||||
number: int = PathParam(gt=0),
|
||||
step_up_grant: str | None = Header(
|
||||
default=None, alias="X-Step-Up-Grant", max_length=128
|
||||
),
|
||||
):
|
||||
repository = f"{owner}/{repo}"
|
||||
await _require_step_up(
|
||||
request,
|
||||
step_up_grant,
|
||||
action="merge_pull",
|
||||
target=f"{repository}#{number}",
|
||||
)
|
||||
|
||||
async def merge_pull():
|
||||
if not await gitea_proxy.is_assigned_pull(repository, number):
|
||||
|
||||
+106
-2
@@ -62,6 +62,21 @@ class SessionStore:
|
||||
)
|
||||
"""
|
||||
)
|
||||
connection.execute(
|
||||
"""
|
||||
CREATE TABLE IF NOT EXISTS step_up_grants (
|
||||
grant_hash TEXT PRIMARY KEY,
|
||||
session_hash TEXT NOT NULL,
|
||||
action TEXT NOT NULL,
|
||||
target TEXT NOT NULL,
|
||||
expires_at INTEGER NOT NULL
|
||||
)
|
||||
"""
|
||||
)
|
||||
connection.execute(
|
||||
"CREATE INDEX IF NOT EXISTS step_up_grants_session_hash "
|
||||
"ON step_up_grants(session_hash)"
|
||||
)
|
||||
columns = {
|
||||
row[1] for row in connection.execute("PRAGMA table_info(active_sessions)")
|
||||
}
|
||||
@@ -134,7 +149,11 @@ class SessionStore:
|
||||
|
||||
def revoke(self, session_id: str) -> None:
|
||||
try:
|
||||
with self._connect() as connection:
|
||||
with self._connect(initialize=True) as connection:
|
||||
connection.execute(
|
||||
"DELETE FROM step_up_grants WHERE session_hash = ?",
|
||||
(self._digest(session_id),),
|
||||
)
|
||||
connection.execute(
|
||||
"DELETE FROM active_sessions WHERE session_hash = ?",
|
||||
(self._digest(session_id),),
|
||||
@@ -169,6 +188,15 @@ class SessionStore:
|
||||
def revoke_managed(self, management_id: str) -> bool:
|
||||
try:
|
||||
with self._connect() as connection:
|
||||
row = connection.execute(
|
||||
"SELECT session_hash FROM active_sessions WHERE management_id = ?",
|
||||
(management_id,),
|
||||
).fetchone()
|
||||
if row is None:
|
||||
return False
|
||||
connection.execute(
|
||||
"DELETE FROM step_up_grants WHERE session_hash = ?", (row[0],)
|
||||
)
|
||||
cursor = connection.execute(
|
||||
"DELETE FROM active_sessions WHERE management_id = ?", (management_id,)
|
||||
)
|
||||
@@ -178,7 +206,83 @@ class SessionStore:
|
||||
|
||||
def revoke_all(self) -> None:
|
||||
try:
|
||||
with self._connect() as connection:
|
||||
with self._connect(initialize=True) as connection:
|
||||
connection.execute("DELETE FROM step_up_grants")
|
||||
connection.execute("DELETE FROM active_sessions")
|
||||
except (OSError, sqlite3.Error) as exc:
|
||||
raise SessionStoreError("Session registry is temporarily unavailable") from exc
|
||||
|
||||
def mint_step_up(
|
||||
self,
|
||||
session_id: str,
|
||||
*,
|
||||
action: str,
|
||||
target: str,
|
||||
ttl_seconds: int,
|
||||
) -> str:
|
||||
now = int(self.clock())
|
||||
grant = secrets.token_urlsafe(32)
|
||||
session_hash = self._digest(session_id)
|
||||
try:
|
||||
with self._connect(initialize=True) as connection:
|
||||
active = connection.execute(
|
||||
"SELECT 1 FROM active_sessions "
|
||||
"WHERE session_hash = ? AND expires_at > ?",
|
||||
(session_hash, now),
|
||||
).fetchone()
|
||||
if active is None:
|
||||
raise SessionStoreError("Session is no longer active")
|
||||
connection.execute(
|
||||
"DELETE FROM step_up_grants WHERE expires_at <= ?", (now,)
|
||||
)
|
||||
connection.execute(
|
||||
"INSERT INTO step_up_grants("
|
||||
"grant_hash, session_hash, action, target, expires_at"
|
||||
") VALUES (?, ?, ?, ?, ?)",
|
||||
(
|
||||
self._digest(grant),
|
||||
session_hash,
|
||||
action,
|
||||
target,
|
||||
now + max(1, ttl_seconds),
|
||||
),
|
||||
)
|
||||
except SessionStoreError:
|
||||
raise
|
||||
except (OSError, sqlite3.Error) as exc:
|
||||
raise SessionStoreError("Session registry is temporarily unavailable") from exc
|
||||
return grant
|
||||
|
||||
def consume_step_up(
|
||||
self,
|
||||
grant: str,
|
||||
session_id: str,
|
||||
*,
|
||||
action: str,
|
||||
target: str,
|
||||
) -> bool:
|
||||
now = int(self.clock())
|
||||
try:
|
||||
with self._connect() as connection:
|
||||
connection.execute(
|
||||
"DELETE FROM step_up_grants WHERE expires_at <= ?", (now,)
|
||||
)
|
||||
cursor = connection.execute(
|
||||
"DELETE FROM step_up_grants WHERE grant_hash = ? "
|
||||
"AND session_hash = ? AND action = ? AND target = ? "
|
||||
"AND expires_at > ? AND EXISTS ("
|
||||
"SELECT 1 FROM active_sessions "
|
||||
"WHERE active_sessions.session_hash = step_up_grants.session_hash "
|
||||
"AND active_sessions.expires_at > ?)",
|
||||
(
|
||||
self._digest(grant),
|
||||
self._digest(session_id),
|
||||
action,
|
||||
target,
|
||||
now,
|
||||
now,
|
||||
),
|
||||
)
|
||||
return cursor.rowcount == 1
|
||||
except (OSError, sqlite3.Error) as exc:
|
||||
raise SessionStoreError("Session registry is temporarily unavailable") from exc
|
||||
Reference in new issue
Block a user