fix(auth): invalidate scope cache on web/REST provisioning paths
The elicitation flow points users to the Astrolabe web route or the BasicAuth REST endpoint to provision their app password. Both paths stored the password without clearing the in-process scope cache, so a user who provisioned through them would keep hitting ProvisioningRequiredError for up to _SCOPE_CACHE_TTL (5 min) afterwards. Add invalidate_scope_cache(user_id) to both write-paths (matching the existing pattern in nc_auth_check_status), correct the now-misleading comment in scope_authorization.py to name all three invalidation paths, and add a one-line hint above the first elicitation patch in the test file so future authors don't "fix" the patch target to the wrong module. Addresses PR #757 round-3 review feedback. 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
7464340763
commit
f31d0544b7
@@ -21,6 +21,7 @@ from starlette.requests import Request
|
||||
from starlette.responses import JSONResponse
|
||||
|
||||
from nextcloud_mcp_server.api.management import _sanitize_error_for_client
|
||||
from nextcloud_mcp_server.auth.scope_authorization import invalidate_scope_cache
|
||||
from nextcloud_mcp_server.auth.storage import RefreshTokenStorage
|
||||
from nextcloud_mcp_server.config import get_settings
|
||||
|
||||
@@ -305,6 +306,7 @@ async def provision_app_password(request: Request) -> JSONResponse:
|
||||
await storage.store_app_password_with_scopes(
|
||||
username, app_password, scopes=scopes, username=nc_username
|
||||
)
|
||||
invalidate_scope_cache(username)
|
||||
|
||||
_record_rate_limit_attempt(path_user_id, success=True)
|
||||
logger.info(f"Provisioned app password for user: {username}")
|
||||
|
||||
@@ -25,6 +25,7 @@ from starlette.responses import HTMLResponse, JSONResponse, RedirectResponse
|
||||
|
||||
from nextcloud_mcp_server.api.management import validate_token_and_get_user
|
||||
from nextcloud_mcp_server.auth.login_flow import LoginFlowV2Client, rewrite_url_origin
|
||||
from nextcloud_mcp_server.auth.scope_authorization import invalidate_scope_cache
|
||||
from nextcloud_mcp_server.auth.storage import get_shared_storage
|
||||
from nextcloud_mcp_server.config import get_nextcloud_ssl_verify, get_settings
|
||||
|
||||
@@ -114,6 +115,7 @@ async def _poll_and_store(provision_id: str) -> None:
|
||||
scopes=None, # All scopes
|
||||
username=result.login_name,
|
||||
)
|
||||
invalidate_scope_cache(effective_user_id)
|
||||
session = _provision_sessions.get(provision_id)
|
||||
if session:
|
||||
session["status"] = "completed"
|
||||
|
||||
@@ -169,11 +169,16 @@ def require_scopes(*required_scopes: str):
|
||||
# again (which would loop).
|
||||
if elicit_result == "accepted":
|
||||
# Note: stored-scope lookups are cached for
|
||||
# _SCOPE_CACHE_TTL (5 min). Both nc_auth_provision_access
|
||||
# and the Astrolabe web route invalidate the cache when
|
||||
# they finish, but if the LFv2 poller is still in-flight
|
||||
# at acknowledge-time the next retry can still hit the
|
||||
# stale cache — hence the "wait a moment" qualifier.
|
||||
# _SCOPE_CACHE_TTL (5 min). All three provisioning
|
||||
# paths invalidate the cache on completion: the
|
||||
# in-tool poller in nc_auth_check_status
|
||||
# (auth_tools.py), the Astrolabe web route
|
||||
# (provision_routes.py), and the BasicAuth REST
|
||||
# endpoint (api/passwords.py). However, if the
|
||||
# LFv2 poller is still in-flight at acknowledge-
|
||||
# time the next retry can still hit a not-yet-
|
||||
# populated entry — hence the "wait a moment"
|
||||
# qualifier below.
|
||||
error_msg = (
|
||||
f"Access denied to {func_name}: Nextcloud "
|
||||
f"access was not provisioned at the time of "
|
||||
|
||||
@@ -141,6 +141,9 @@ async def test_decorator_elicits_and_uses_retry_message_when_user_accepts():
|
||||
"nextcloud_mcp_server.auth.token_utils.extract_user_id_from_token",
|
||||
return_value="alice",
|
||||
),
|
||||
# Patch the elicitation module (not scope_authorization) because the
|
||||
# decorator does a local import of present_provisioning_required to
|
||||
# avoid a circular import, so the name is re-fetched at call-time.
|
||||
patch(
|
||||
"nextcloud_mcp_server.auth.elicitation.present_provisioning_required",
|
||||
elicit_mock,
|
||||
|
||||
Reference in New Issue
Block a user