diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index f5d6178dc..59ad13cd6 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -67,6 +67,7 @@ jobs: - "multi-user-basic" - "login-flow" - "keycloak" + - "ldap" include: # Version-specific image pins — Renovate updates these via customManagers in renovate.json # Each entry is pinned to its major version (e.g., NC 32 only gets 32.x updates) @@ -81,7 +82,7 @@ jobs: # Mode-specific properties - mode: single-user profile: single-user - markers: "(smoke and not keycloak and not login_flow and not multi_user_basic) or (integration and not keycloak and not login_flow and not multi_user_basic)" + markers: "(smoke and not keycloak and not login_flow and not multi_user_basic and not ldap) or (integration and not keycloak and not login_flow and not multi_user_basic and not ldap)" wait-port: 8000 mcp-internal-url: "http://mcp:8000" needs-playwright: false @@ -116,6 +117,19 @@ jobs: needs-playwright: true extra-args: "" + # LDAP-backend lane (GH #980 reproduction). Reuses the multi-user + # BasicAuth MCP service (port 8003) and adds the `ldap` profile + # (OpenLDAP). user_ldap is configured automatically by the app-hook + # post-installation/15-setup-ldap-backend.sh. No browser needed. + - mode: ldap + profile: multi-user-basic + extra-compose-profiles: "--profile ldap" + markers: "ldap" + wait-port: 8003 + mcp-internal-url: "http://mcp-multi-user-basic:8000" + needs-playwright: false + extra-args: "" + name: integration (${{ matrix.mode }} / nc${{ matrix.nextcloud_version }}) steps: @@ -229,6 +243,21 @@ jobs: done echo "MCP service is ready on port ${{ matrix.wait-port }}." + # user_ldap is configured automatically by the app-hook + # app-hooks/post-installation/15-setup-ldap-backend.sh (gated on the + # openldap service). This step just surfaces the resulting config for + # debugging, mirroring the keycloak verify step below. + - name: Verify LDAP backend + if: matrix.mode == 'ldap' + run: | + echo "=== user_ldap enabled? ===" + docker compose exec -T app php occ app:list 2>/dev/null | grep -i ldap || echo "user_ldap NOT enabled" + echo "=== user_ldap connection ===" + # Re-derive the config id the same way the app-hook does, rather than + # assuming s01, so the diagnostic stays accurate if discovery differs. + config_id=$(docker compose exec -T app php occ ldap:show-config 2>/dev/null | grep -E '^\| Configuration' | grep -oE 's[0-9]+' | head -n1) + docker compose exec -T app php occ ldap:test-config "${config_id:-s01}" 2>&1 || echo "ldap:test-config failed" + - name: Wait for Keycloak if: matrix.mode == 'keycloak' run: | @@ -279,7 +308,7 @@ jobs: - name: Collect service logs on failure if: failure() run: | - docker compose --profile ${{ matrix.profile }} logs --tail=500 > /tmp/docker-compose-logs.txt 2>&1 + docker compose --profile ${{ matrix.profile }} ${{ matrix.extra-compose-profiles }} logs --tail=500 > /tmp/docker-compose-logs.txt 2>&1 docker compose exec -T app cat /var/www/html/data/nextcloud.log 2>/dev/null | tail -100 > /tmp/nextcloud-app.log 2>&1 || true - name: Upload debug artifacts diff --git a/CLAUDE.md b/CLAUDE.md index 0d72db738..51c22f34e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -549,6 +549,41 @@ docker compose exec app php occ user_oidc:provider keycloak - `docs/ADR-002-vector-sync-authentication.md` - Offline access architecture - `docs/keycloak-multi-client-validation.md` - Realm-level validation +### LDAP Testing (GH #980 reproduction) + +The `ldap` lane reproduces GH #980 (DAV paths built from the loginName instead +of the canonical Nextcloud UID). It runs an OpenLDAP server (`vegardit/openldap`) +whose user `alice` logs in as `alice` but is mapped by `user_ldap` to a UID +derived from the LDAP `entryUUID` — so `loginName != UID` **and** +`/remote.php/dav/files/alice/` does not resolve to her real home. This is the +only backend that reproduces #980 live: login-by-email resolves to the real home +(email is a path alias) and `user_oidc` hardcodes `loginName == UID`. + +The reproduction test drives the **multi-user BasicAuth** MCP service (port 8003) +as `alice`, so no browser is needed. + +**Setup**: +```bash +# openldap (ldap profile) + the multi-user-basic MCP service (port 8003). +# user_ldap is auto-configured by app-hooks/post-installation/15-setup-ldap-backend.sh +# (gated on the openldap service; a no-op for every other profile). +docker compose --profile ldap --profile multi-user-basic up --build -d +uv run pytest -m ldap -v # xfails until #980's client fix lands +``` + +> The post-installation hook only runs on a **fresh** Nextcloud install. If you +> add the `ldap` profile to an already-installed dev stack, run the hook by hand: +> `docker compose exec app bash /docker-entrypoint-hooks.d/post-installation/15-setup-ldap-backend.sh` + +**Credentials**: LDAP admin `uid=admin,dc=example,dc=org` / `ldap_admin_pw`; +user `alice` / `AlicePass123!` (see `ldap/bootstrap.ldif`). + +**xfail note**: `tests/server/ldap/test_ldap_dav_principal.py` is +`xfail(strict=True)` — it fails (RED) on `master` because the bug is present, and +xpasses (GREEN) once #980's `BaseNextcloudClient._ensure_principal_id` discovery +lands. `strict=True` turns the unexpected pass into a failure, signalling that +the marker should be dropped when #980 merges. + ## Integration Testing with Docker **Nextcloud**: `docker compose exec app php occ ...` for occ commands diff --git a/app-hooks/post-installation/15-setup-ldap-backend.sh b/app-hooks/post-installation/15-setup-ldap-backend.sh new file mode 100755 index 000000000..1d36323b8 --- /dev/null +++ b/app-hooks/post-installation/15-setup-ldap-backend.sh @@ -0,0 +1,81 @@ +#!/bin/bash +# +# Configure Nextcloud's user_ldap backend against the `openldap` service. +# +# Runs only when the `ldap` docker-compose profile is active (the `openldap` +# hostname resolves). Sets up a single LDAP config so the seeded user `alice` +# logs in as `alice` but is mapped to a canonical UID derived from her LDAP +# entryUUID — the divergent loginName/UID condition that reproduces GH #980. +# +# Idempotent: reuses an existing config id if present, else creates one. +# + +set -e + +echo "====================================================================" +echo "Configuring user_ldap backend for OpenLDAP..." +echo "====================================================================" + +# Quick check: is the openldap service in the Docker network? +# When the `ldap` profile is not active, this hostname won't resolve, so the +# hook is a no-op for every other lane. +if ! getent hosts openldap >/dev/null 2>&1; then + echo " OpenLDAP service not detected in Docker network (ldap profile not active)" + echo " Skipping user_ldap configuration" + exit 0 +fi + +php /var/www/html/occ app:enable user_ldap + +# Reuse an existing config id (idempotent re-run), else create a fresh one. +# The trailing `|| true` guards the whole command substitution under `set -e` +# (grep exits non-zero when no config exists yet on a fresh install); keep it on +# this pipeline if refactoring. +CONFIG_ID=$(php /var/www/html/occ ldap:show-config 2>/dev/null \ + | grep -E '^\| Configuration' | grep -oE 's[0-9]+' | head -n1 || true) +if [ -z "$CONFIG_ID" ]; then + CONFIG_ID=$(php /var/www/html/occ ldap:create-empty-config | grep -oE 's[0-9]+' | head -n1) +fi +echo "Using user_ldap config: $CONFIG_ID" + +set_cfg() { php /var/www/html/occ ldap:set-config "$CONFIG_ID" "$1" "$2" >/dev/null; } + +# Connection (openldap = internal docker hostname; admin bind = uid=admin). +set_cfg ldapHost openldap +set_cfg ldapPort 389 +set_cfg ldapAgentName "uid=admin,dc=example,dc=org" +set_cfg ldapAgentPassword "ldap_admin_pw" +set_cfg ldapBase "dc=example,dc=org" +set_cfg ldapBaseUsers "ou=people,dc=example,dc=org" +# Filters + attribute mapping. Leaving the internal-username attribute at its +# default makes Nextcloud derive the UID from entryUUID (below), so loginName +# (uid=alice) differs from the canonical UID — the point of this lane. +set_cfg ldapUserFilter "(objectClass=inetOrgPerson)" +set_cfg ldapLoginFilter "(&(objectClass=inetOrgPerson)(uid=%uid))" +set_cfg ldapUserDisplayName "cn" +set_cfg ldapEmailAttribute "mail" +set_cfg ldapExpertUUIDUserAttr "entryUUID" +set_cfg ldapTLS 0 +set_cfg turnOffCertCheck 1 +set_cfg ldapConfigurationActive 1 + +# Wait for OpenLDAP to answer by retrying the connection test itself — no extra +# client tooling needed in the app image. This doubles as validation. +echo "Verifying LDAP connection..." +MAX_RETRIES=30 +RETRY_COUNT=0 +while [ $RETRY_COUNT -lt $MAX_RETRIES ]; do + if php /var/www/html/occ ldap:test-config "$CONFIG_ID" 2>/dev/null | grep -q "valid"; then + echo "====================================================================" + echo "✓ user_ldap backend configured ($CONFIG_ID)" + echo "====================================================================" + exit 0 + fi + echo " Waiting for OpenLDAP... (attempt $((RETRY_COUNT + 1))/$MAX_RETRIES)" + sleep 5 + RETRY_COUNT=$((RETRY_COUNT + 1)) +done + +echo "⚠ Warning: LDAP connection could not be verified after $MAX_RETRIES attempts" +php /var/www/html/occ ldap:test-config "$CONFIG_ID" || true +exit 0 diff --git a/docker-compose.yml b/docker-compose.yml index d4e2f6d62..ef85f195f 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -445,6 +445,50 @@ services: profiles: - mail + # OpenLDAP backend for the `ldap` lane — reproduces GH #980 (DAV paths built + # from loginName instead of the canonical Nextcloud UID). Nextcloud's + # user_ldap backend derives an internal UID from the LDAP UUID that differs + # from the login (`alice`), which single-user / user_oidc / login-by-email + # backends can't produce. Bootstrapped fresh on every `up` (no data volume), + # so the custom LDIF is always applied. + openldap: + image: vegardit/openldap:2.6.10@sha256:f722617af8118ea9a740ebc138bdc188fd8d5e963c88e3eacf6e07e3456c88f4 + environment: + - LDAP_INIT_ORG_DN=dc=example,dc=org + - LDAP_INIT_ORG_NAME=Example Org + - LDAP_INIT_ROOT_USER_PW=ldap_admin_pw + # Plain LDAP on 389 is enough for the internal test network; skip the + # self-signed LDAPS setup so user_ldap can connect without TLS fuss. + - LDAP_LDAPS_ENABLED=false + volumes: + # Seed ou=people + the divergent `alice` user (evaluated on first launch; + # no data volume, so it re-seeds fresh on every `up`). + - ./ldap/bootstrap.ldif:/opt/ldifs/init_org_entries.ldif:ro + ports: + - 127.0.0.1:1389:389 + # Searching for the seeded `alice` entry (not just a port probe) means + # "healthy" == "bootstrap LDIF applied" — the app-hook can rely on that. + healthcheck: + test: + [ + "CMD", + "ldapsearch", + "-x", + "-H", + "ldap://localhost", + "-b", + "uid=alice,ou=people,dc=example,dc=org", + "-D", + "uid=admin,dc=example,dc=org", + "-w", + "ldap_admin_pw", + ] + interval: 10s + timeout: 5s + retries: 30 + profiles: + - ldap + volumes: nextcloud: db: diff --git a/ldap/bootstrap.ldif b/ldap/bootstrap.ldif new file mode 100644 index 000000000..c674e770b --- /dev/null +++ b/ldap/bootstrap.ldif @@ -0,0 +1,27 @@ +# Custom bootstrap entries for the OpenLDAP `ldap` compose profile. +# Loaded by osixia/openldap into the seeded dc=example,dc=org tree. +# +# The single user `alice` is the divergent-principal fixture for reproducing +# GH #980: her LDAP login is `alice`, but Nextcloud's user_ldap backend derives +# a DIFFERENT canonical internal UID (from the UUID attribute), so DAV paths +# built from the loginName (`/remote.php/dav/files/alice/`) miss her real home +# at `/remote.php/dav/files//`. +# +# userPassword is a deterministic SSHA hash of "AlicePass123!" (salt "ncmcp980") +# so this file stays reproducible across rebuilds. + +dn: ou=people,dc=example,dc=org +objectClass: organizationalUnit +ou: people + +dn: uid=alice,ou=people,dc=example,dc=org +objectClass: inetOrgPerson +objectClass: posixAccount +cn: Alice Example +sn: Example +uid: alice +uidNumber: 10001 +gidNumber: 10001 +homeDirectory: /home/alice +mail: alice@ldap.example.org +userPassword: {SSHA}PD7foplU4ZMYi7ENTdbi3i+sG79uY21jcDk4MA== diff --git a/pyproject.toml b/pyproject.toml index 3a11b4f20..947eed6d7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -80,6 +80,7 @@ markers = [ "keycloak: OAuth tests that utilize keycloak external identity provider", "login_flow: Login Flow v2 integration tests (ADR-022)", "multi_user_basic: Multi-user BasicAuth pass-through tests (ADR-020)", + "ldap: LDAP-backend tests reproducing divergent loginName/UID DAV paths (GH #980)", "postgres: Tests requiring the docker-compose postgres-test service (ADR-026)", "contract: Pact consumer/provider contract tests (ADR-029)", ] diff --git a/tests/server/ldap/__init__.py b/tests/server/ldap/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/tests/server/ldap/conftest.py b/tests/server/ldap/conftest.py new file mode 100644 index 000000000..ccccb990f --- /dev/null +++ b/tests/server/ldap/conftest.py @@ -0,0 +1,53 @@ +"""Fixtures for the LDAP lane — reproduces GH #980 (divergent loginName/UID). + +The `ldap` docker-compose profile runs an OpenLDAP server whose user `alice` +logs in as `alice` but is mapped by Nextcloud's `user_ldap` backend to a +canonical internal UID derived from the LDAP UUID (e.g. +`c2c0a34c-09dd-1041-...`). That divergence (`loginName != UID`) is the exact +shape of GH #980 that single-user, `user_oidc`, and login-by-email backends +cannot produce — and, unlike login-by-email, the LDAP login is NOT a valid +files-path alias, so `/remote.php/dav/files/alice/` genuinely misses the real +home at `/remote.php/dav/files//`. + +These tests drive the **multi-user BasicAuth** MCP service (port 8003): the +client sends `alice`'s LDAP credentials in the Authorization header, so the +server builds DAV paths from the loginName `alice`. Without the #980 client fix +(`BaseNextcloudClient._ensure_principal_id`) every DAV op targets the wrong, +non-existent home and fails; with it, current-user-principal discovery resolves +the real UID. +""" + +import base64 +from typing import Any, AsyncGenerator + +import pytest +from mcp import ClientSession + +from tests.conftest import create_mcp_client_session + +# The multi-user BasicAuth MCP service (ADR-020) — builds the Nextcloud client +# per request from the BasicAuth username, i.e. the loginName `alice`. +MULTI_USER_BASIC_MCP_URL = "http://localhost:8003/mcp" + +# Divergent LDAP user seeded by ldap/bootstrap.ldif. +LDAP_USERNAME = "alice" +LDAP_PASSWORD = "AlicePass123!" # NOSONAR(S2068) - dev-only LDAP fixture credential + + +@pytest.fixture +async def nc_mcp_ldap_alice_client( + anyio_backend, +) -> AsyncGenerator[ClientSession, Any]: + """MCP session authenticated as the divergent LDAP user `alice` (port 8003). + + Connects to the multi-user BasicAuth service with `alice`'s LDAP credentials + so the server constructs DAV paths from her loginName (`alice`), not her + canonical UID — the condition GH #980 fixes. + """ + credentials = base64.b64encode(f"{LDAP_USERNAME}:{LDAP_PASSWORD}".encode()).decode() + async with create_mcp_client_session( + url=MULTI_USER_BASIC_MCP_URL, + headers={"Authorization": f"Basic {credentials}"}, + client_name="LDAP alice (multi-user-basic)", + ) as session: + yield session diff --git a/tests/server/ldap/test_ldap_dav_principal.py b/tests/server/ldap/test_ldap_dav_principal.py new file mode 100644 index 000000000..6a86cb1f9 --- /dev/null +++ b/tests/server/ldap/test_ldap_dav_principal.py @@ -0,0 +1,90 @@ +"""LDAP-backend reproduction of GH #980 (divergent loginName/UID DAV paths). + +The LDAP user `alice` logs in as `alice` but Nextcloud's `user_ldap` backend +maps her to a canonical internal UID (the LDAP UUID). The multi-user BasicAuth +MCP server builds DAV paths from the loginName, so every WebDAV operation +targets ``/remote.php/dav/files/alice/`` — which does NOT resolve to her real +home at ``/remote.php/dav/files//`` (unlike login-by-email, an LDAP login +is not a files-path alias, so it 404s rather than silently resolving). + +* **Without** the #980 client fix → the round-trip targets the non-existent + ``/files/alice/`` home and fails → the test below **fails** (RED). It is + therefore marked ``xfail(strict=True)``: on ``master`` (no fix) it xfails, so + CI stays green while still asserting the bug is present. +* **With** #980's ``BaseNextcloudClient._ensure_principal_id`` (a + ``PROPFIND /remote.php/dav/`` for ``current-user-principal``) the real UID is + discovered and the paths are rewritten → the round-trip succeeds → the test + **xpasses**. ``strict=True`` then turns the unexpected pass into a CI failure, + which is the signal to drop the ``xfail`` marker once #980 lands. + +This is the live RED→GREEN reproduction that the Keycloak lane (PR #993) could +not provide, because email/`user_oidc` logins don't produce a non-resolvable +divergent path on the CI Nextcloud versions. +""" + +import json + +import pytest +from mcp import ClientSession + +pytestmark = [pytest.mark.integration, pytest.mark.ldap] + + +@pytest.mark.xfail( + strict=True, + reason=( + "Reproduces GH #980: DAV paths built from the LDAP loginName miss the " + "canonical-UID home. Fixed by BaseNextcloudClient._ensure_principal_id " + "(PR #980) — drop this marker once that lands." + ), +) +async def test_webdav_round_trip_resolves_ldap_principal( + nc_mcp_ldap_alice_client: ClientSession, +): + """A full WebDAV cycle as the divergent LDAP user must hit her real home. + + create → write → read → list → delete, all as `alice`. Without #980 the + paths are built from the loginName `alice` and the very first operation + fails against the non-existent ``/files/alice/`` home. + """ + dir_path = "/LdapPrincipalTest" + file_path = f"{dir_path}/ldap_principal.txt" + content = "webdav round-trip via the divergent LDAP principal" + + mkdir_result = await nc_mcp_ldap_alice_client.call_tool( + "nc_webdav_create_directory", {"path": dir_path} + ) + assert mkdir_result.isError is False, ( + "create_directory failed — DAV path built from the LDAP loginName " + "'alice' instead of the discovered canonical UID (GH #980 not fixed?): " + f"{mkdir_result.content}" + ) + + try: + write_result = await nc_mcp_ldap_alice_client.call_tool( + "nc_webdav_write_file", + {"path": file_path, "content": content}, + ) + assert write_result.isError is False + + read_result = await nc_mcp_ldap_alice_client.call_tool( + "nc_webdav_read_file", {"path": file_path} + ) + assert read_result.isError is False + read_data = json.loads(read_result.content[0].text) + assert content in read_data.get("content", "") + + list_result = await nc_mcp_ldap_alice_client.call_tool( + "nc_webdav_list_directory", {"path": dir_path} + ) + assert list_result.isError is False + list_data = json.loads(list_result.content[0].text) + names = [f.get("name", "") for f in list_data.get("files", [])] + assert "ldap_principal.txt" in names + finally: + await nc_mcp_ldap_alice_client.call_tool( + "nc_webdav_delete_resource", {"path": file_path} + ) + await nc_mcp_ldap_alice_client.call_tool( + "nc_webdav_delete_resource", {"path": dir_path} + )