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."""