fix: convert astrolabe int provisioned_at to ISO before ProvisioningStatus
Round-4 review (real bug): get_background_sync_status now returns provisioned_at as Unix seconds (the wire/pact value), but ProvisioningStatus.provisioned_at is str | None (ISO). Constructing it for a provisioned user raised a Pydantic ValidationError — a path that was unreachable before the has_access fix. Convert int -> ISO at the oauth_tools boundary (mirroring the existing refresh_token branch), keeping the model schema and the int-asserting contract pact/unit tests intact. Add a regression test that drives the full _get_provisioning_status round-trip with an integer timestamp. Also surface dropped provider-state params in the verifier's _dispatch_state no-op branch (round-4 nit). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
c474f62190
commit
69c40a0479
@@ -118,7 +118,13 @@ async def _get_provisioning_status(ctx: Context, user_id: str) -> ProvisioningSt
|
||||
" get_provisioning_status: app password FOUND for user_id=%s",
|
||||
user_id,
|
||||
)
|
||||
provisioned_at_str = status.get("provisioned_at")
|
||||
# Astrolabe returns provisioned_at as Unix seconds (see the
|
||||
# contract pact); convert to the ISO string the model expects.
|
||||
provisioned_at_str = None
|
||||
provisioned_at_raw = status.get("provisioned_at")
|
||||
if provisioned_at_raw:
|
||||
dt = datetime.fromtimestamp(provisioned_at_raw, tz=timezone.utc)
|
||||
provisioned_at_str = dt.isoformat()
|
||||
return ProvisioningStatus(
|
||||
is_provisioned=True,
|
||||
provisioned_at=provisioned_at_str,
|
||||
|
||||
@@ -86,7 +86,13 @@ def _dispatch_state(state: str, **kwargs) -> None:
|
||||
"""
|
||||
handler = _PROVIDER_STATES.get(state)
|
||||
if handler is None:
|
||||
logger.warning("No provider-state handler registered for %r; no-op", state)
|
||||
# Log any params astrolabe passed so they're visible once real handlers
|
||||
# need them (e.g. given("user X exists", params={"user_id": ...})).
|
||||
logger.warning(
|
||||
"No provider-state handler registered for %r (params=%s); no-op",
|
||||
state,
|
||||
kwargs,
|
||||
)
|
||||
return
|
||||
handler()
|
||||
|
||||
|
||||
@@ -8,6 +8,7 @@ read and clear that store too, otherwise they report "not provisioned" while
|
||||
tools still work, and "nothing to revoke" while the credential persists.
|
||||
"""
|
||||
|
||||
from datetime import datetime
|
||||
from types import SimpleNamespace
|
||||
from unittest.mock import AsyncMock, MagicMock
|
||||
|
||||
@@ -58,6 +59,39 @@ async def test_status_reports_provisioned_for_app_password_store(
|
||||
storage.get_refresh_token.assert_not_awaited() # app password short-circuits
|
||||
|
||||
|
||||
async def test_status_converts_astrolabe_int_timestamp_to_iso(mocker):
|
||||
"""Astrolabe returns provisioned_at as Unix seconds (per the contract pact),
|
||||
but ProvisioningStatus.provisioned_at is an ISO string. The int must be
|
||||
converted at the boundary, else constructing the model raises ValidationError
|
||||
for every provisioned user."""
|
||||
mocker.patch.object(
|
||||
oauth_tools,
|
||||
"get_settings",
|
||||
return_value=SimpleNamespace(
|
||||
oidc_client_id="mcp",
|
||||
oidc_client_secret="secret",
|
||||
nextcloud_host="https://cloud.example.com",
|
||||
),
|
||||
)
|
||||
astrolabe = MagicMock()
|
||||
astrolabe.get_background_sync_status = AsyncMock(
|
||||
return_value={
|
||||
"has_access": True,
|
||||
"credential_type": "app_password",
|
||||
"provisioned_at": 1717000000,
|
||||
}
|
||||
)
|
||||
mocker.patch.object(oauth_tools, "AstrolabeClient", return_value=astrolabe)
|
||||
|
||||
status = await _get_provisioning_status(MagicMock(), "alice")
|
||||
|
||||
assert status.is_provisioned is True
|
||||
assert status.credential_type == "app_password"
|
||||
# Converted from Unix seconds to an ISO-8601 string that round-trips back.
|
||||
assert isinstance(status.provisioned_at, str)
|
||||
assert datetime.fromisoformat(status.provisioned_at).timestamp() == 1717000000
|
||||
|
||||
|
||||
async def test_revoke_deletes_app_password(mocker, _no_astrolabe_settings):
|
||||
"""Revoke must delete the app password from storage (not just refresh tokens)."""
|
||||
storage = MagicMock()
|
||||
|
||||
Reference in New Issue
Block a user