fix(webhooks): use HTTP 428 instead of 412 for unprovisioned users
428 (Precondition Required, RFC 6585) is the correct semantic — the request requires the client to complete a prerequisite step (Login Flow v2 provisioning) before retrying. 412 (Precondition Failed) is for header-based preconditions like ETags / If-Match. No behavior change beyond the status code; same JSON payload. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
a0e484d95b
commit
b3a7587f1a
@@ -91,7 +91,7 @@ async def get_installed_apps(request: Request) -> JSONResponse:
|
||||
logger.info("Provisioning required for user %s: %s", user_id, e)
|
||||
return JSONResponse(
|
||||
{"error": "Provisioning required", "message": str(e)},
|
||||
status_code=412,
|
||||
status_code=428,
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error("Error getting installed apps for user %s: %s", user_id, e)
|
||||
@@ -145,7 +145,7 @@ async def list_webhooks(request: Request) -> JSONResponse:
|
||||
logger.info("Provisioning required for user %s: %s", user_id, e)
|
||||
return JSONResponse(
|
||||
{"error": "Provisioning required", "message": str(e)},
|
||||
status_code=412,
|
||||
status_code=428,
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error("Error listing webhooks for user %s: %s", user_id, e)
|
||||
@@ -232,7 +232,7 @@ async def create_webhook(request: Request) -> JSONResponse:
|
||||
logger.info("Provisioning required for user %s: %s", user_id, e)
|
||||
return JSONResponse(
|
||||
{"error": "Provisioning required", "message": str(e)},
|
||||
status_code=412,
|
||||
status_code=428,
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error("Error creating webhook for user %s: %s", user_id, e)
|
||||
@@ -302,7 +302,7 @@ async def delete_webhook(request: Request) -> JSONResponse:
|
||||
logger.info("Provisioning required for user %s: %s", user_id, e)
|
||||
return JSONResponse(
|
||||
{"error": "Provisioning required", "message": str(e)},
|
||||
status_code=412,
|
||||
status_code=428,
|
||||
)
|
||||
except Exception as e:
|
||||
logger.error("Error deleting webhook for user %s: %s", user_id, e)
|
||||
|
||||
@@ -166,9 +166,11 @@ async def test_missing_nextcloud_host_returns_500(mocker):
|
||||
assert http_response.status_code == 500
|
||||
|
||||
|
||||
async def test_unprovisioned_user_returns_412(mocker):
|
||||
"""Users without a stored app password get HTTP 412 so the client can
|
||||
surface a 'complete Login Flow v2' UX rather than a generic 500."""
|
||||
async def test_unprovisioned_user_returns_428(mocker):
|
||||
"""Users without a stored app password get HTTP 428 (Precondition Required,
|
||||
RFC 6585) so the client can surface a 'complete Login Flow v2' UX rather
|
||||
than a generic 500. 428 is the right semantic — the request requires a
|
||||
prerequisite step (provisioning) before it can succeed."""
|
||||
_patch_token_validation(mocker)
|
||||
mocker.patch(
|
||||
"nextcloud_mcp_server.api.webhooks.get_basic_auth_for_user",
|
||||
@@ -181,5 +183,5 @@ async def test_unprovisioned_user_returns_412(mocker):
|
||||
"/api/v1/apps", headers={"Authorization": "Bearer test-token"}
|
||||
)
|
||||
|
||||
assert http_response.status_code == 412
|
||||
assert http_response.status_code == 428
|
||||
assert http_response.json()["error"] == "Provisioning required"
|
||||
|
||||
@@ -116,7 +116,7 @@ async def test_list_webhooks_uses_basic_auth(mocker):
|
||||
_assert_basic_auth_not_bearer(factory)
|
||||
|
||||
|
||||
async def test_list_webhooks_returns_412_when_unprovisioned(mocker):
|
||||
async def test_list_webhooks_returns_428_when_unprovisioned(mocker):
|
||||
_patch_token_validation(mocker)
|
||||
mocker.patch(
|
||||
"nextcloud_mcp_server.api.webhooks.get_basic_auth_for_user",
|
||||
@@ -126,7 +126,7 @@ async def test_list_webhooks_returns_412_when_unprovisioned(mocker):
|
||||
client = TestClient(_build_test_app())
|
||||
resp = client.get("/api/v1/webhooks", headers={"Authorization": "Bearer mcp-token"})
|
||||
|
||||
assert resp.status_code == 412
|
||||
assert resp.status_code == 428
|
||||
assert resp.json()["error"] == "Provisioning required"
|
||||
|
||||
|
||||
@@ -177,7 +177,7 @@ async def test_create_webhook_validates_required_fields(mocker):
|
||||
assert resp.status_code == 400
|
||||
|
||||
|
||||
async def test_create_webhook_returns_412_when_unprovisioned(mocker):
|
||||
async def test_create_webhook_returns_428_when_unprovisioned(mocker):
|
||||
_patch_token_validation(mocker)
|
||||
mocker.patch(
|
||||
"nextcloud_mcp_server.api.webhooks.get_basic_auth_for_user",
|
||||
@@ -191,7 +191,7 @@ async def test_create_webhook_returns_412_when_unprovisioned(mocker):
|
||||
json={"event": "X", "uri": "http://x"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 412
|
||||
assert resp.status_code == 428
|
||||
assert resp.json()["error"] == "Provisioning required"
|
||||
|
||||
|
||||
@@ -229,7 +229,7 @@ async def test_delete_webhook_rejects_non_integer_id(mocker):
|
||||
assert resp.status_code == 400
|
||||
|
||||
|
||||
async def test_delete_webhook_returns_412_when_unprovisioned(mocker):
|
||||
async def test_delete_webhook_returns_428_when_unprovisioned(mocker):
|
||||
_patch_token_validation(mocker)
|
||||
mocker.patch(
|
||||
"nextcloud_mcp_server.api.webhooks.get_basic_auth_for_user",
|
||||
@@ -242,7 +242,7 @@ async def test_delete_webhook_returns_412_when_unprovisioned(mocker):
|
||||
headers={"Authorization": "Bearer mcp-token"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 412
|
||||
assert resp.status_code == 428
|
||||
assert resp.json()["error"] == "Provisioning required"
|
||||
|
||||
|
||||
|
||||
Reference in New Issue
Block a user