feat: make consequential reviews passkey-native (Closes #509)
This commit is contained in:
parent
28e70f6415
commit
877dc475dd
|
|
@ -149,6 +149,8 @@
|
|||
all_sessions_revoked: 'All device access revoked',
|
||||
issue_closed: 'Issue closed',
|
||||
pull_merged: 'Pull request merged',
|
||||
pull_review_approved: 'Pull request approved',
|
||||
pull_review_changes_requested: 'Changes requested',
|
||||
};
|
||||
page.events.forEach(event => {
|
||||
const row = root.document.createElement('article');
|
||||
|
|
|
|||
75
src/main.py
75
src/main.py
|
|
@ -200,17 +200,20 @@ class DashboardSignIn(BaseModel):
|
|||
return normalized
|
||||
|
||||
|
||||
StepUpAction = Literal[
|
||||
"merge_pull",
|
||||
"submit_pull_review",
|
||||
"close_issue",
|
||||
"revoke_device",
|
||||
"revoke_all_sessions",
|
||||
"enroll_passkey",
|
||||
"revoke_passkey",
|
||||
]
|
||||
|
||||
|
||||
class FreshAuthorization(BaseModel):
|
||||
access_token: str = Field(min_length=1, max_length=1_024)
|
||||
action: Literal[
|
||||
"merge_pull",
|
||||
"submit_pull_review",
|
||||
"close_issue",
|
||||
"revoke_device",
|
||||
"revoke_all_sessions",
|
||||
"enroll_passkey",
|
||||
"revoke_passkey",
|
||||
]
|
||||
action: StepUpAction
|
||||
target: str = Field(min_length=1, max_length=255)
|
||||
|
||||
|
||||
|
|
@ -228,9 +231,7 @@ class PasskeyAuthentication(PasskeyCeremony):
|
|||
|
||||
|
||||
class PasskeyAuthorizationTarget(BaseModel):
|
||||
action: Literal[
|
||||
"merge_pull", "close_issue", "revoke_device", "revoke_all_sessions", "revoke_passkey"
|
||||
]
|
||||
action: StepUpAction
|
||||
target: str = Field(min_length=1, max_length=255)
|
||||
|
||||
|
||||
|
|
@ -3825,12 +3826,50 @@ async def submit_review(
|
|||
submission.decision,
|
||||
submission.body,
|
||||
)
|
||||
if submission.comments:
|
||||
return await gitea_proxy.submit_pull_review(
|
||||
*args,
|
||||
[comment.model_dump() for comment in submission.comments],
|
||||
)
|
||||
return await gitea_proxy.submit_pull_review(*args)
|
||||
journal = None
|
||||
operation_id = None
|
||||
if submission.decision != "comment":
|
||||
journal = _security_event_store()
|
||||
try:
|
||||
operation_id = await asyncio.to_thread(
|
||||
journal.reserve,
|
||||
(
|
||||
"pull_review_approved"
|
||||
if submission.decision == "approve"
|
||||
else "pull_review_changes_requested"
|
||||
),
|
||||
target=f"{repository}#{number}",
|
||||
)
|
||||
except SecurityEventStoreError:
|
||||
raise HTTPException(
|
||||
status_code=503,
|
||||
detail="Security activity is temporarily unavailable",
|
||||
)
|
||||
try:
|
||||
if submission.comments:
|
||||
result = await gitea_proxy.submit_pull_review(
|
||||
*args,
|
||||
[comment.model_dump() for comment in submission.comments],
|
||||
)
|
||||
else:
|
||||
result = await gitea_proxy.submit_pull_review(*args)
|
||||
except (
|
||||
HTTPException,
|
||||
gitea_proxy.StaleReviewError,
|
||||
gitea_proxy.InvalidReviewCommentError,
|
||||
):
|
||||
if journal is not None and operation_id is not None:
|
||||
try:
|
||||
await asyncio.to_thread(journal.discard, operation_id)
|
||||
except SecurityEventStoreError:
|
||||
pass
|
||||
raise
|
||||
if journal is not None and operation_id is not None:
|
||||
try:
|
||||
await asyncio.to_thread(journal.finalize, operation_id)
|
||||
except SecurityEventStoreError:
|
||||
pass
|
||||
return result
|
||||
|
||||
try:
|
||||
comment_fingerprint = tuple(
|
||||
|
|
|
|||
|
|
@ -201,6 +201,14 @@ async def test_passkey_fresh_authorization_is_exact_target_bound_and_single_use(
|
|||
json={"action": "close_issue", "target": "stackchain/api#7"},
|
||||
headers=headers,
|
||||
)
|
||||
review_options = await client.post(
|
||||
"/api/v1/passkeys/authorization/options",
|
||||
json={
|
||||
"action": "submit_pull_review",
|
||||
"target": "stackchain/api#7@abc123:approve",
|
||||
},
|
||||
headers=headers,
|
||||
)
|
||||
ceremony = {
|
||||
"challenge": options.json()["challenge"],
|
||||
"credential": {"id": "cGhvbmUtY3JlZGVudGlhbA"},
|
||||
|
|
@ -220,6 +228,8 @@ async def test_passkey_fresh_authorization_is_exact_target_bound_and_single_use(
|
|||
)
|
||||
|
||||
assert options.status_code == 200
|
||||
assert review_options.status_code == 200
|
||||
assert review_options.json()["challenge"]
|
||||
assert authorized.status_code == 201
|
||||
assert authorized.json()["grant"]
|
||||
assert authorized.json()["expires_in"] == 90
|
||||
|
|
|
|||
|
|
@ -655,6 +655,13 @@ def test_security_activity_explains_pending_outcome_confirmation():
|
|||
assert "event.status === 'pending'" in source
|
||||
|
||||
|
||||
def test_security_activity_labels_consequential_pull_review_decisions():
|
||||
source = SESSION_JS.read_text()
|
||||
|
||||
assert "pull_review_approved: 'Pull request approved'" in source
|
||||
assert "pull_review_changes_requested: 'Changes requested'" in source
|
||||
|
||||
|
||||
def test_authenticated_dashboard_load_requests_queued_delivery_resume():
|
||||
result = run_session_scenario(
|
||||
"""
|
||||
|
|
|
|||
|
|
@ -234,6 +234,117 @@ async def test_successful_protected_actions_record_only_bounded_targets(
|
|||
assert b"abc123" not in persisted
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_approved_pull_review_records_a_privacy_safe_completed_event(
|
||||
security_access, monkeypatch
|
||||
):
|
||||
async def requested(*_args):
|
||||
return True
|
||||
|
||||
async def submit(*_args):
|
||||
return {"id": 91, "state": "APPROVED"}
|
||||
|
||||
monkeypatch.setattr(main, "is_requested_review", requested)
|
||||
monkeypatch.setattr(main.gitea_proxy, "submit_pull_review", submit)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="https://test") as client:
|
||||
await client.post(
|
||||
"/api/v1/session",
|
||||
json={"access_token": "correct horse battery staple", "device_label": "Phone"},
|
||||
)
|
||||
headers = {
|
||||
"Origin": "https://test",
|
||||
"X-CSRF-Token": client.cookies["stackchain_csrf"],
|
||||
}
|
||||
grant = await client.post(
|
||||
"/api/v1/fresh-authorization",
|
||||
json={
|
||||
"access_token": "correct horse battery staple",
|
||||
"action": "submit_pull_review",
|
||||
"target": "stackchain/api#7@abc123:approve",
|
||||
},
|
||||
headers=headers,
|
||||
)
|
||||
response = await client.post(
|
||||
"/api/v1/repos/stackchain/api/pulls/7/review",
|
||||
json={
|
||||
"expected_head_sha": "abc123",
|
||||
"decision": "approve",
|
||||
"body": "Ready to ship with private review notes.",
|
||||
},
|
||||
headers={**headers, "X-Step-Up-Grant": grant.json()["grant"]},
|
||||
)
|
||||
events = (await client.get("/api/v1/security-events")).json()["events"]
|
||||
|
||||
assert response.status_code == 201
|
||||
assert events[0] == {
|
||||
"id": events[0]["id"],
|
||||
"kind": "pull_review_approved",
|
||||
"method": None,
|
||||
"device_label": None,
|
||||
"target": "stackchain/api#7",
|
||||
"created_at": events[0]["created_at"],
|
||||
"status": "completed",
|
||||
}
|
||||
persisted = (security_access / "security.sqlite3").read_bytes()
|
||||
assert b"abc123" not in persisted
|
||||
assert b"private review notes" not in persisted
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_review_reservation_failure_prevents_the_gitea_mutation(
|
||||
security_access, monkeypatch
|
||||
):
|
||||
submit_calls = []
|
||||
|
||||
async def requested(*_args):
|
||||
return True
|
||||
|
||||
async def submit(*args):
|
||||
submit_calls.append(args)
|
||||
return {"id": 91, "state": "APPROVED"}
|
||||
|
||||
class UnavailableJournal:
|
||||
def reserve(self, *_args, **_kwargs):
|
||||
raise SecurityEventStoreError("unavailable")
|
||||
|
||||
monkeypatch.setattr(main, "is_requested_review", requested)
|
||||
monkeypatch.setattr(main.gitea_proxy, "submit_pull_review", submit)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="https://test") as client:
|
||||
await client.post(
|
||||
"/api/v1/session",
|
||||
json={"access_token": "correct horse battery staple"},
|
||||
)
|
||||
headers = {
|
||||
"Origin": "https://test",
|
||||
"X-CSRF-Token": client.cookies["stackchain_csrf"],
|
||||
}
|
||||
grant = await client.post(
|
||||
"/api/v1/fresh-authorization",
|
||||
json={
|
||||
"access_token": "correct horse battery staple",
|
||||
"action": "submit_pull_review",
|
||||
"target": "stackchain/api#7@abc123:approve",
|
||||
},
|
||||
headers=headers,
|
||||
)
|
||||
monkeypatch.setattr(main, "_security_event_store", lambda: UnavailableJournal())
|
||||
response = await client.post(
|
||||
"/api/v1/repos/stackchain/api/pulls/7/review",
|
||||
json={
|
||||
"expected_head_sha": "abc123",
|
||||
"decision": "approve",
|
||||
"body": "Ready.",
|
||||
},
|
||||
headers={**headers, "X-Step-Up-Grant": grant.json()["grant"]},
|
||||
)
|
||||
|
||||
assert response.status_code == 503
|
||||
assert response.json()["detail"] == "Security activity is temporarily unavailable"
|
||||
assert submit_calls == []
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_issue_close_reservation_failure_prevents_the_gitea_mutation(
|
||||
security_access, monkeypatch
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user