From 0df3fb4a195a9c550f0f8dc2230f555f40b7e133 Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 30 May 2026 11:50:19 +0200 Subject: [PATCH] fix(api): validate app password against Nextcloud using loginName, not UID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit provision_app_password validated the supplied app password by calling the OCS cloud/user endpoint with BasicAuth as the *path user_id* (the UID). Nextcloud keys app-password BasicAuth on the loginName, which differs from the UID for OIDC-provisioned accounts whose UID is their display name (UID "Chris Coutinho", loginName "chris@coutinho.io"). Authenticating as the UID is rejected with HTTP 401 ("App password validation failed"), so provisioning never completes. Parse the request body up front and authenticate the OCS validation as the body's `username` (the Nextcloud loginName), falling back to the path user_id for legacy callers where UID == loginName. The OCS-returned account id is still checked against the path user_id (the UID), and the password is still stored keyed by UID with the loginName alongside. Note this is not an encoding issue: BasicAuth places the user-id literally in the header (RFC 7617, no URL-encoding); %20/+/literal-space forms of the UID all fail — only the loginName authenticates. Adds a regression test asserting the OCS BasicAuth uses the loginName while storage is keyed by the UID, plus a backward-compat assertion that callers without a loginName fall back to the UID. Co-Authored-By: Claude Opus 4.8 (1M context) --- nextcloud_mcp_server/api/passwords.py | 40 ++++++++----- .../test_management_app_password_endpoints.py | 58 +++++++++++++++++++ 2 files changed, 84 insertions(+), 14 deletions(-) diff --git a/nextcloud_mcp_server/api/passwords.py b/nextcloud_mcp_server/api/passwords.py index 212355b9..19ae1bdf 100644 --- a/nextcloud_mcp_server/api/passwords.py +++ b/nextcloud_mcp_server/api/passwords.py @@ -238,6 +238,24 @@ async def provision_app_password(request: Request) -> JSONResponse: status_code=400, ) + # Parse optional scopes and the Nextcloud loginName from the request body + # up front. Nextcloud authenticates app passwords against the *loginName*, + # which can differ from the UID — e.g. OIDC-provisioned users whose UID is + # their display name (UID "Chris Coutinho", loginName "chris@coutinho.io"). + # Use the loginName for the BasicAuth validation below, falling back to the + # path user_id for legacy callers that don't send one (where UID == + # loginName). + scopes = None + nc_username = None + try: + body = await request.json() + scopes = body.get("scopes") # list[str] | None + nc_username = body.get("username") # Nextcloud loginName + except Exception: + pass # No JSON body = legacy call without scopes / loginName + + login_name = nc_username or username + # Get Nextcloud host from settings settings = get_settings() nextcloud_host = settings.nextcloud_host @@ -249,7 +267,10 @@ async def provision_app_password(request: Request) -> JSONResponse: status_code=500, ) - # Validate app password against Nextcloud + # Validate app password against Nextcloud. BasicAuth places the user-id + # literally in the header (RFC 7617 — no URL-encoding) and Nextcloud keys + # app-password auth on the loginName, so authenticate as the loginName, not + # the UID. try: async with nextcloud_httpx_client( timeout=NEXTCLOUD_VALIDATION_TIMEOUT @@ -258,7 +279,7 @@ async def provision_app_password(request: Request) -> JSONResponse: test_url = f"{nextcloud_host}/ocs/v1.php/cloud/user" response = await client.get( test_url, - auth=(username, app_password), + auth=(login_name, app_password), params={"format": "json"}, headers={"OCS-APIRequest": "true"}, ) @@ -274,10 +295,11 @@ async def provision_app_password(request: Request) -> JSONResponse: status_code=401, ) - # Verify the user ID from response matches + # Verify the authenticated account maps to the path user_id (UID): + # the loginName must resolve to the UID claimed in the URL path. data = response.json() ocs_user_id = data.get("ocs", {}).get("data", {}).get("id") - if ocs_user_id != username: + if ocs_user_id != path_user_id: logger.warning("User ID mismatch in OCS response") _record_rate_limit_attempt(path_user_id, success=False) return JSONResponse( @@ -292,16 +314,6 @@ async def provision_app_password(request: Request) -> JSONResponse: status_code=500, ) - # Parse optional scopes and username from request body - scopes = None - nc_username = None - try: - body = await request.json() - scopes = body.get("scopes") # list[str] | None - nc_username = body.get("username") # Nextcloud loginName - except Exception: - pass # No JSON body = legacy call without scopes - # Store the validated app password try: storage = await _get_app_password_storage(request) diff --git a/tests/unit/test_management_app_password_endpoints.py b/tests/unit/test_management_app_password_endpoints.py index 2f0fd0f3..0a6fcb55 100644 --- a/tests/unit/test_management_app_password_endpoints.py +++ b/tests/unit/test_management_app_password_endpoints.py @@ -248,6 +248,64 @@ async def test_provision_app_password_success(temp_storage, mocker): stored_password = await temp_storage.get_app_password("testuser") assert stored_password == "aaaaa-bbbbb-ccccc-ddddd-eeeee" + # Legacy callers send no loginName in the body → the OCS validation falls + # back to authenticating as the UID (here UID == loginName). + _, get_kwargs = mock_client.get.call_args + assert get_kwargs["auth"] == ("testuser", "aaaaa-bbbbb-ccccc-ddddd-eeeee") + + +async def test_provision_app_password_uses_loginname_not_uid(temp_storage, mocker): + """Regression: when the Nextcloud UID differs from the loginName (e.g. + OIDC-provisioned users whose UID is their display name — UID + "Chris Coutinho", loginName "chris@coutinho.io"), the OCS BasicAuth + validation must authenticate as the loginName from the request body, not + the UID. Authenticating as the UID is rejected by Nextcloud with HTTP 401. + """ + mocker.patch( + "nextcloud_mcp_server.api.passwords.get_settings", + return_value=MagicMock( + nextcloud_host="http://localhost:8080", + nextcloud_verify_ssl=True, + nextcloud_ca_bundle=None, + ), + ) + + # OCS validation succeeds and reports the UID as the account id. + mock_response = MagicMock() + mock_response.status_code = 200 + mock_response.json.return_value = {"ocs": {"data": {"id": "Chris Coutinho"}}} + + mock_client = AsyncMock() + mock_client.get = AsyncMock(return_value=mock_response) + mock_client.__aenter__ = AsyncMock(return_value=mock_client) + mock_client.__aexit__ = AsyncMock() + mocker.patch( + "nextcloud_mcp_server.api.passwords.nextcloud_httpx_client", + return_value=mock_client, + ) + + app = create_test_app(temp_storage) + client = TestClient(app) + + pw = "aaaaa-bbbbb-ccccc-ddddd-eeeee" + # A literal space in the path is encoded by the client and decoded back to + # the UID; the BasicAuth username matches that UID. + response = client.post( + "/api/v1/users/Chris Coutinho/app-password", + headers={"Authorization": create_basic_auth_header("Chris Coutinho", pw)}, + json={"username": "chris@coutinho.io"}, + ) + + assert response.status_code == 200 + assert response.json()["success"] is True + + # The OCS BasicAuth used the loginName from the body, not the UID. + _, get_kwargs = mock_client.get.call_args + assert get_kwargs["auth"] == ("chris@coutinho.io", pw) + + # Stored under the UID (the identity key). + assert await temp_storage.get_app_password("Chris Coutinho") == pw + async def test_provision_app_password_nextcloud_validation_fails(mocker): """Test that failed Nextcloud validation returns 401."""