refactor: drop OAuth-refresh background-sync path from oauth_sync.py
Follow-up to #787/#789 (ADR-022 cleanup). After
`oauth_enabled ↔ enable_login_flow` became an invariant, the
`use_basic_auth=False` branch in `vector/oauth_sync.py` — and the
parameter wiring that fed it — was no longer reachable from any
supported deployment mode. This commit removes the dead code.
- nextcloud_mcp_server/vector/oauth_sync.py:
- Deleted `get_user_client_oauth` (the OAuth-token refresh helper) and
its `VECTOR_SYNC_SCOPES` constant.
- Deleted the `get_user_client` dispatcher. Internal callers now call
`get_user_client_basic_auth` directly.
- Dropped the `use_basic_auth: bool` parameter from `user_scanner_task`,
`multi_user_processor_task`, `_run_user_scanner_with_scope`, and
`user_manager_task`.
- Dropped the `token_broker` parameter from the same four functions —
they no longer need it now that the OAuth-refresh path is gone. The
`TokenBrokerService` constructed in `app.py` is still used by the
management API revoke endpoint, just not by background sync.
- Simplified the user-list query in `user_manager_task` to always read
from the `app_passwords` table.
- Replaced all `mode_label = "BasicAuth" if use_basic_auth else "OAuth"`
with a literal `[BasicAuth]` log prefix (keeps existing log filters
working).
- Updated the module docstring to describe the post-cleanup shape.
- Dropped the now-unused `TYPE_CHECKING` import of `TokenBrokerService`.
- nextcloud_mcp_server/app.py: dropped the `use_basic_auth = True` block
and the now-stale `token_broker if not use_basic_auth else None` /
`use_basic_auth` positional args from the two `tg.start(...)` calls in
the multi-user vector-sync lifespan. Token broker construction stays —
still consumed by the management API revoke endpoint via
`app.state.oauth_context["token_broker"]`.
- tests/integration/test_app_password_provisioning.py: deleted four tests
that exercised the now-removed OAuth-refresh path
(`test_oauth_mode_uses_refresh_token_only`,
`test_oauth_mode_raises_error_without_token`,
`test_get_user_client_oauth_function`,
`test_oauth_mode_requires_token_broker`) plus the
`test_get_user_client_dispatches_to_basic_auth` test for the deleted
dispatcher. Updated the module docstring + imports accordingly. The
BasicAuth-mode tests (`test_basic_auth_mode_uses_local_storage`,
`test_multiple_users_basic_auth_mode`, etc.) all remain.
No runtime-behaviour change in any supported deployment mode — the deleted
branches were already unreachable post-PR #787. 3 files changed,
+59 / -301; 1010 unit tests pass; integration jobs for
`mcp-login-flow` and `mcp-multi-user-basic` are the critical regression
gates before merge.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chris Coutinho
co-authored by
Claude Opus 4.7
Claude Opus 4.7
parent
735a4019ed
commit
65345fd6eb
@@ -1,12 +1,12 @@
|
||||
"""Integration tests for app password provisioning via management API.
|
||||
|
||||
Tests the complete flow for multi-user BasicAuth mode:
|
||||
1. User stores app password via management API endpoint
|
||||
Tests the complete flow for multi-user BasicAuth and Login Flow v2 modes:
|
||||
1. User stores app password via management API endpoint (or Login Flow v2 browser flow)
|
||||
2. MCP server stores it locally (encrypted)
|
||||
3. Background sync uses locally stored password to access Nextcloud
|
||||
|
||||
These tests verify that BasicAuth and OAuth are completely separate concerns
|
||||
with no fallback between them.
|
||||
The earlier OAuth refresh-token background-sync path was removed in the
|
||||
ADR-022 cleanup — these tests now cover the only supported path.
|
||||
"""
|
||||
|
||||
import tempfile
|
||||
@@ -18,9 +18,7 @@ from cryptography.fernet import Fernet
|
||||
from nextcloud_mcp_server.auth.storage import RefreshTokenStorage
|
||||
from nextcloud_mcp_server.vector.oauth_sync import (
|
||||
NotProvisionedError,
|
||||
get_user_client,
|
||||
get_user_client_basic_auth,
|
||||
get_user_client_oauth,
|
||||
)
|
||||
|
||||
|
||||
@@ -85,117 +83,6 @@ async def test_basic_auth_mode_raises_error_without_app_password(temp_storage):
|
||||
assert "test_user" in str(exc_info.value)
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_get_user_client_dispatches_to_basic_auth(temp_storage, mocker):
|
||||
"""Test that get_user_client dispatches to BasicAuth mode correctly."""
|
||||
# Store an app password
|
||||
await temp_storage.store_app_password("alice", "aaaaa-bbbbb-ccccc-ddddd-eeeee")
|
||||
|
||||
# Mock RefreshTokenStorage.from_env at the source module
|
||||
mocker.patch(
|
||||
"nextcloud_mcp_server.auth.storage.RefreshTokenStorage.from_env",
|
||||
return_value=temp_storage,
|
||||
)
|
||||
# Also mock initialize since from_env returns an uninitialized instance
|
||||
mocker.patch.object(temp_storage, "initialize", return_value=None)
|
||||
|
||||
# Call get_user_client in BasicAuth mode
|
||||
client = await get_user_client(
|
||||
user_id="alice",
|
||||
token_broker=None, # No token broker needed for BasicAuth mode
|
||||
nextcloud_host="http://localhost:8080",
|
||||
use_basic_auth=True,
|
||||
)
|
||||
|
||||
# Verify client was created successfully
|
||||
assert client is not None
|
||||
assert client.username == "alice"
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_oauth_mode_uses_refresh_token_only(mocker):
|
||||
"""Test that OAuth mode uses ONLY refresh tokens, NOT app passwords.
|
||||
|
||||
In OAuth mode, app passwords are NOT used.
|
||||
This is a complete separation of concerns.
|
||||
"""
|
||||
from nextcloud_mcp_server.auth.token_broker import TokenBrokerService
|
||||
|
||||
# Mock TokenBrokerService to return an access token
|
||||
mock_token_broker = mocker.AsyncMock(spec=TokenBrokerService)
|
||||
mock_token_broker.get_background_token.return_value = "test-access-token"
|
||||
|
||||
# Call get_user_client in OAuth mode
|
||||
_client = await get_user_client(
|
||||
user_id="test_user",
|
||||
token_broker=mock_token_broker,
|
||||
nextcloud_host="http://localhost:8080",
|
||||
use_basic_auth=False, # OAuth mode
|
||||
)
|
||||
|
||||
# Verify token broker was called
|
||||
mock_token_broker.get_background_token.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_oauth_mode_raises_error_without_token(mocker):
|
||||
"""Test that OAuth mode raises NotProvisionedError if no refresh token.
|
||||
|
||||
There is NO fallback to app passwords - if no token, user must provision.
|
||||
"""
|
||||
from nextcloud_mcp_server.auth.token_broker import TokenBrokerService
|
||||
|
||||
# Mock TokenBrokerService to return None (no token)
|
||||
mock_token_broker = mocker.AsyncMock(spec=TokenBrokerService)
|
||||
mock_token_broker.get_background_token.return_value = None
|
||||
|
||||
# Call get_user_client in OAuth mode - should raise NotProvisionedError
|
||||
with pytest.raises(NotProvisionedError) as exc_info:
|
||||
await get_user_client(
|
||||
user_id="test_user",
|
||||
token_broker=mock_token_broker,
|
||||
nextcloud_host="http://localhost:8080",
|
||||
use_basic_auth=False,
|
||||
)
|
||||
|
||||
# Verify error message mentions OAuth provisioning
|
||||
assert "oauth" in str(exc_info.value).lower()
|
||||
assert "test_user" in str(exc_info.value)
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_get_user_client_oauth_function(mocker):
|
||||
"""Test the dedicated get_user_client_oauth function."""
|
||||
from nextcloud_mcp_server.auth.token_broker import TokenBrokerService
|
||||
|
||||
# Mock TokenBrokerService
|
||||
mock_token_broker = mocker.AsyncMock(spec=TokenBrokerService)
|
||||
mock_token_broker.get_background_token.return_value = "test-bearer-token"
|
||||
|
||||
# Call dedicated function
|
||||
client = await get_user_client_oauth(
|
||||
user_id="alice",
|
||||
token_broker=mock_token_broker,
|
||||
nextcloud_host="http://localhost:8080",
|
||||
)
|
||||
|
||||
assert client is not None
|
||||
assert client.username == "alice"
|
||||
mock_token_broker.get_background_token.assert_called_once()
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_oauth_mode_requires_token_broker():
|
||||
"""Test that OAuth mode requires a token broker."""
|
||||
with pytest.raises(ValueError, match="token_broker required"):
|
||||
await get_user_client(
|
||||
user_id="test_user",
|
||||
token_broker=None, # Missing token broker
|
||||
nextcloud_host="http://localhost:8080",
|
||||
use_basic_auth=False, # OAuth mode
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.integration
|
||||
async def test_multiple_users_basic_auth_mode(temp_storage, mocker):
|
||||
"""Test that multiple users can be provisioned independently."""
|
||||
|
||||
Reference in New Issue
Block a user