refactor: remove RFC 8693 token exchange and Keycloak OAuth implementation
Nextcloud doesn't support OAuth bearer tokens without upstream patches, making the RFC 8693 token exchange path untestable and dead code. Removed: - nextcloud_mcp_server/auth/token_exchange.py (597 lines) - nextcloud_mcp_server/auth/keycloak_oauth.py (586 lines) - OAUTH_TOKEN_EXCHANGE deployment mode from AuthMode enum - get_session_client_from_context() from context_helper.py - get_session_token() from token_broker.py - enable_token_exchange / token_exchange_cache_ttl config fields - oauth_token_exchange_total Prometheus metric - Keycloak fixture block from tests/conftest.py (~408 lines) - Token exchange unit tests from test_config_validators.py, test_unified_verifier.py, test_management_status_endpoint.py Preserved: - Multi-audience OAuth mode (OAUTH_SINGLE_AUDIENCE) - Login Flow v2 provisioning with elicitation support - Token broker background token management - All existing test coverage for non-exchange paths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chris Coutinho
co-authored by
Claude Opus 4.6
Claude Opus 4.6
parent
c6316dbb91
commit
5730313574
@@ -22,16 +22,6 @@ from nextcloud_mcp_server.config_validators import (
|
||||
class TestModeDetection:
|
||||
"""Test auth mode detection from configuration."""
|
||||
|
||||
def test_token_exchange_mode_detection(self):
|
||||
"""Test token exchange mode is detected."""
|
||||
settings = Settings(
|
||||
nextcloud_host="http://localhost",
|
||||
enable_token_exchange=True,
|
||||
)
|
||||
|
||||
mode = detect_auth_mode(settings)
|
||||
assert mode == AuthMode.OAUTH_TOKEN_EXCHANGE
|
||||
|
||||
def test_multi_user_basic_mode_detection(self):
|
||||
"""Test multi-user BasicAuth mode is detected."""
|
||||
settings = Settings(
|
||||
@@ -62,18 +52,6 @@ class TestModeDetection:
|
||||
mode = detect_auth_mode(settings)
|
||||
assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE
|
||||
|
||||
def test_mode_priority_token_exchange_over_basic(self):
|
||||
"""Test token exchange has priority over BasicAuth."""
|
||||
settings = Settings(
|
||||
nextcloud_host="http://localhost",
|
||||
nextcloud_username="admin",
|
||||
nextcloud_password="password",
|
||||
enable_token_exchange=True,
|
||||
)
|
||||
|
||||
mode = detect_auth_mode(settings)
|
||||
assert mode == AuthMode.OAUTH_TOKEN_EXCHANGE
|
||||
|
||||
|
||||
class TestSingleUserBasicValidation:
|
||||
"""Test validation for single-user BasicAuth mode."""
|
||||
@@ -165,21 +143,6 @@ class TestSingleUserBasicValidation:
|
||||
# It will fail multi-user validation because username/password are forbidden
|
||||
assert len(errors) > 0
|
||||
|
||||
def test_forbidden_token_exchange(self):
|
||||
"""Test error when ENABLE_TOKEN_EXCHANGE is set."""
|
||||
settings = Settings(
|
||||
nextcloud_host="http://localhost",
|
||||
nextcloud_username="admin",
|
||||
nextcloud_password="password",
|
||||
enable_token_exchange=True,
|
||||
)
|
||||
|
||||
# Note: This will detect as OAUTH_TOKEN_EXCHANGE due to priority
|
||||
mode, errors = validate_configuration(settings)
|
||||
|
||||
assert mode == AuthMode.OAUTH_TOKEN_EXCHANGE
|
||||
# It will fail OAuth validation
|
||||
|
||||
def test_vector_sync_without_embedding_provider_uses_fallback(self):
|
||||
"""Test that vector sync works with Simple provider fallback (no config needed)."""
|
||||
settings = Settings(
|
||||
@@ -419,51 +382,6 @@ class TestOAuthSingleAudienceValidation:
|
||||
assert settings.enable_offline_access is True
|
||||
|
||||
|
||||
class TestOAuthTokenExchangeValidation:
|
||||
"""Test validation for OAuth token exchange mode."""
|
||||
|
||||
def test_valid_minimal_config(self):
|
||||
"""Test valid minimal OAuth token exchange config."""
|
||||
settings = Settings(
|
||||
nextcloud_host="http://localhost",
|
||||
enable_token_exchange=True,
|
||||
)
|
||||
|
||||
mode, errors = validate_configuration(settings)
|
||||
|
||||
assert mode == AuthMode.OAUTH_TOKEN_EXCHANGE
|
||||
assert len(errors) == 0
|
||||
|
||||
def test_valid_with_credentials(self):
|
||||
"""Test valid config with OAuth credentials."""
|
||||
settings = Settings(
|
||||
nextcloud_host="http://localhost",
|
||||
enable_token_exchange=True,
|
||||
oidc_client_id="test-client",
|
||||
oidc_client_secret="test-secret",
|
||||
)
|
||||
|
||||
mode, errors = validate_configuration(settings)
|
||||
|
||||
assert mode == AuthMode.OAUTH_TOKEN_EXCHANGE
|
||||
assert len(errors) == 0
|
||||
|
||||
def test_forbidden_username_password(self):
|
||||
"""Test error when username/password are set."""
|
||||
settings = Settings(
|
||||
nextcloud_host="http://localhost",
|
||||
enable_token_exchange=True,
|
||||
nextcloud_username="admin",
|
||||
nextcloud_password="password",
|
||||
)
|
||||
|
||||
mode, errors = validate_configuration(settings)
|
||||
|
||||
assert mode == AuthMode.OAUTH_TOKEN_EXCHANGE
|
||||
assert any("nextcloud_username" in err.lower() for err in errors)
|
||||
assert any("nextcloud_password" in err.lower() for err in errors)
|
||||
|
||||
|
||||
class TestModeSummary:
|
||||
"""Test mode summary generation."""
|
||||
|
||||
@@ -477,14 +395,6 @@ class TestModeSummary:
|
||||
assert "NEXTCLOUD_PASSWORD" in summary
|
||||
assert "VECTOR_SYNC_ENABLED" in summary
|
||||
|
||||
def test_oauth_token_exchange_summary(self):
|
||||
"""Test summary for OAuth token exchange mode."""
|
||||
summary = get_mode_summary(AuthMode.OAUTH_TOKEN_EXCHANGE)
|
||||
|
||||
assert "oauth_exchange" in summary
|
||||
assert "ENABLE_TOKEN_EXCHANGE" in summary
|
||||
assert "RFC 8693" in summary
|
||||
|
||||
|
||||
class TestEdgeCases:
|
||||
"""Test edge cases and boundary conditions."""
|
||||
@@ -800,23 +710,6 @@ class TestExplicitModeSelection:
|
||||
|
||||
assert mode == AuthMode.OAUTH_SINGLE_AUDIENCE
|
||||
|
||||
def test_explicit_oauth_token_exchange_mode(self):
|
||||
"""Test explicit oauth_token_exchange mode selection."""
|
||||
with patch.dict(
|
||||
os.environ,
|
||||
{
|
||||
"NEXTCLOUD_HOST": "http://localhost:8080",
|
||||
"MCP_DEPLOYMENT_MODE": "oauth_token_exchange",
|
||||
},
|
||||
clear=True,
|
||||
):
|
||||
from nextcloud_mcp_server.config import get_settings
|
||||
|
||||
settings = get_settings()
|
||||
mode = detect_auth_mode(settings)
|
||||
|
||||
assert mode == AuthMode.OAUTH_TOKEN_EXCHANGE
|
||||
|
||||
def test_invalid_deployment_mode_raises_error(self):
|
||||
"""Test invalid MCP_DEPLOYMENT_MODE raises ValueError."""
|
||||
with patch.dict(
|
||||
|
||||
@@ -37,7 +37,6 @@ def create_mock_settings(
|
||||
oidc_issuer: str | None = None,
|
||||
vector_sync_enabled: bool = False,
|
||||
nextcloud_url: str = "http://localhost",
|
||||
enable_token_exchange: bool = False,
|
||||
mcp_client_id: str | None = None,
|
||||
mcp_client_secret: str | None = None,
|
||||
):
|
||||
@@ -49,7 +48,6 @@ def create_mock_settings(
|
||||
settings.oidc_issuer = oidc_issuer
|
||||
settings.vector_sync_enabled = vector_sync_enabled
|
||||
settings.nextcloud_url = nextcloud_url
|
||||
settings.enable_token_exchange = enable_token_exchange
|
||||
settings.mcp_client_id = mcp_client_id
|
||||
settings.mcp_client_secret = mcp_client_secret
|
||||
return settings
|
||||
|
||||
@@ -29,18 +29,9 @@ def base_settings():
|
||||
nextcloud_resource_uri="http://localhost:8080",
|
||||
jwks_uri="https://idp.example.com/jwks",
|
||||
introspection_uri="https://idp.example.com/introspect",
|
||||
enable_token_exchange=False, # Multi-audience mode
|
||||
token_exchange_cache_ttl=300,
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def exchange_settings(base_settings):
|
||||
"""Create settings for token exchange mode."""
|
||||
base_settings.enable_token_exchange = True
|
||||
return base_settings
|
||||
|
||||
|
||||
class TestUnifiedTokenVerifierInit:
|
||||
"""Test UnifiedTokenVerifier initialization."""
|
||||
|
||||
@@ -50,11 +41,11 @@ class TestUnifiedTokenVerifierInit:
|
||||
assert verifier.mode == "multi-audience"
|
||||
assert verifier.settings == base_settings
|
||||
|
||||
def test_init_exchange_mode(self, exchange_settings):
|
||||
"""Test verifier initialization in token exchange mode."""
|
||||
verifier = UnifiedTokenVerifier(exchange_settings)
|
||||
assert verifier.mode == "exchange"
|
||||
assert verifier.settings == exchange_settings
|
||||
def test_init_always_multi_audience(self, base_settings):
|
||||
"""Test verifier always initializes in multi-audience mode."""
|
||||
verifier = UnifiedTokenVerifier(base_settings)
|
||||
assert verifier.mode == "multi-audience"
|
||||
assert verifier.settings == base_settings
|
||||
|
||||
|
||||
class TestAudienceValidation:
|
||||
@@ -117,9 +108,9 @@ class TestAudienceValidation:
|
||||
# Should pass - we only validate MCP audience per RFC 7519
|
||||
assert verifier._has_mcp_audience(payload) is True
|
||||
|
||||
def test_has_mcp_audience_with_client_id(self, exchange_settings):
|
||||
def test_has_mcp_audience_with_client_id(self, base_settings):
|
||||
"""Test MCP audience validation with client ID."""
|
||||
verifier = UnifiedTokenVerifier(exchange_settings)
|
||||
verifier = UnifiedTokenVerifier(base_settings)
|
||||
payload = {
|
||||
"aud": ["test-client-id"],
|
||||
"sub": "testuser",
|
||||
@@ -128,9 +119,9 @@ class TestAudienceValidation:
|
||||
|
||||
assert verifier._has_mcp_audience(payload) is True
|
||||
|
||||
def test_has_mcp_audience_with_server_url(self, exchange_settings):
|
||||
def test_has_mcp_audience_with_server_url(self, base_settings):
|
||||
"""Test MCP audience validation with server URL."""
|
||||
verifier = UnifiedTokenVerifier(exchange_settings)
|
||||
verifier = UnifiedTokenVerifier(base_settings)
|
||||
payload = {
|
||||
"aud": ["http://localhost:8000"],
|
||||
"sub": "testuser",
|
||||
@@ -139,9 +130,9 @@ class TestAudienceValidation:
|
||||
|
||||
assert verifier._has_mcp_audience(payload) is True
|
||||
|
||||
def test_has_mcp_audience_missing(self, exchange_settings):
|
||||
def test_has_mcp_audience_missing(self, base_settings):
|
||||
"""Test MCP audience validation fails without MCP audience."""
|
||||
verifier = UnifiedTokenVerifier(exchange_settings)
|
||||
verifier = UnifiedTokenVerifier(base_settings)
|
||||
payload = {
|
||||
"aud": ["http://localhost:8080"], # Wrong audience
|
||||
"sub": "testuser",
|
||||
@@ -292,12 +283,12 @@ class TestMultiAudienceVerification:
|
||||
assert result.resource == "testuser"
|
||||
|
||||
|
||||
class TestExchangeModeVerification:
|
||||
"""Test token exchange mode verification."""
|
||||
class TestMcpAudienceVerification:
|
||||
"""Test MCP audience verification."""
|
||||
|
||||
async def test_verify_mcp_audience_only_success(self, exchange_settings):
|
||||
async def test_verify_mcp_audience_only_success(self, base_settings):
|
||||
"""Test MCP-only audience verification succeeds with MCP audience."""
|
||||
verifier = UnifiedTokenVerifier(exchange_settings)
|
||||
verifier = UnifiedTokenVerifier(base_settings)
|
||||
|
||||
# Mock introspection response with MCP audience only
|
||||
introspection_response = {
|
||||
@@ -318,9 +309,9 @@ class TestExchangeModeVerification:
|
||||
assert result is not None
|
||||
assert result.resource == "testuser"
|
||||
|
||||
async def test_verify_mcp_audience_only_fails_without_mcp(self, exchange_settings):
|
||||
async def test_verify_mcp_audience_only_fails_without_mcp(self, base_settings):
|
||||
"""Test MCP audience verification fails without MCP audience."""
|
||||
verifier = UnifiedTokenVerifier(exchange_settings)
|
||||
verifier = UnifiedTokenVerifier(base_settings)
|
||||
|
||||
# Mock introspection response without MCP audience
|
||||
introspection_response = {
|
||||
@@ -503,9 +494,9 @@ class TestVerifyTokenFlow:
|
||||
assert result is not None
|
||||
assert result.resource == "testuser"
|
||||
|
||||
async def test_verify_token_exchange_mode(self, exchange_settings):
|
||||
"""Test verify_token in exchange mode."""
|
||||
verifier = UnifiedTokenVerifier(exchange_settings)
|
||||
async def test_verify_token_mcp_audience_only(self, base_settings):
|
||||
"""Test verify_token with MCP audience only."""
|
||||
verifier = UnifiedTokenVerifier(base_settings)
|
||||
|
||||
introspection_response = {
|
||||
"active": True,
|
||||
|
||||
Reference in New Issue
Block a user