Skip to content

Commit e4cd494

Browse files
authored
Merge pull request #997 from cbcoutinho/feat/openldap-980-reproduction
test(ldap): reproduce GH #980 divergent-principal DAV bug via OpenLDAP
2 parents f9ae1d7 + 2d6279e commit e4cd494

9 files changed

Lines changed: 362 additions & 2 deletions

File tree

.github/workflows/test.yml

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ jobs:
6767
- "multi-user-basic"
6868
- "login-flow"
6969
- "keycloak"
70+
- "ldap"
7071
include:
7172
# Version-specific image pins — Renovate updates these via customManagers in renovate.json
7273
# Each entry is pinned to its major version (e.g., NC 32 only gets 32.x updates)
@@ -81,7 +82,7 @@ jobs:
8182
# Mode-specific properties
8283
- mode: single-user
8384
profile: single-user
84-
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)"
85+
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)"
8586
wait-port: 8000
8687
mcp-internal-url: "http://mcp:8000"
8788
needs-playwright: false
@@ -116,6 +117,19 @@ jobs:
116117
needs-playwright: true
117118
extra-args: ""
118119

120+
# LDAP-backend lane (GH #980 reproduction). Reuses the multi-user
121+
# BasicAuth MCP service (port 8003) and adds the `ldap` profile
122+
# (OpenLDAP). user_ldap is configured automatically by the app-hook
123+
# post-installation/15-setup-ldap-backend.sh. No browser needed.
124+
- mode: ldap
125+
profile: multi-user-basic
126+
extra-compose-profiles: "--profile ldap"
127+
markers: "ldap"
128+
wait-port: 8003
129+
mcp-internal-url: "http://mcp-multi-user-basic:8000"
130+
needs-playwright: false
131+
extra-args: ""
132+
119133
name: integration (${{ matrix.mode }} / nc${{ matrix.nextcloud_version }})
120134

121135
steps:
@@ -229,6 +243,21 @@ jobs:
229243
done
230244
echo "MCP service is ready on port ${{ matrix.wait-port }}."
231245
246+
# user_ldap is configured automatically by the app-hook
247+
# app-hooks/post-installation/15-setup-ldap-backend.sh (gated on the
248+
# openldap service). This step just surfaces the resulting config for
249+
# debugging, mirroring the keycloak verify step below.
250+
- name: Verify LDAP backend
251+
if: matrix.mode == 'ldap'
252+
run: |
253+
echo "=== user_ldap enabled? ==="
254+
docker compose exec -T app php occ app:list 2>/dev/null | grep -i ldap || echo "user_ldap NOT enabled"
255+
echo "=== user_ldap connection ==="
256+
# Re-derive the config id the same way the app-hook does, rather than
257+
# assuming s01, so the diagnostic stays accurate if discovery differs.
258+
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)
259+
docker compose exec -T app php occ ldap:test-config "${config_id:-s01}" 2>&1 || echo "ldap:test-config failed"
260+
232261
- name: Wait for Keycloak
233262
if: matrix.mode == 'keycloak'
234263
run: |
@@ -279,7 +308,7 @@ jobs:
279308
- name: Collect service logs on failure
280309
if: failure()
281310
run: |
282-
docker compose --profile ${{ matrix.profile }} logs --tail=500 > /tmp/docker-compose-logs.txt 2>&1
311+
docker compose --profile ${{ matrix.profile }} ${{ matrix.extra-compose-profiles }} logs --tail=500 > /tmp/docker-compose-logs.txt 2>&1
283312
docker compose exec -T app cat /var/www/html/data/nextcloud.log 2>/dev/null | tail -100 > /tmp/nextcloud-app.log 2>&1 || true
284313
285314
- name: Upload debug artifacts

CLAUDE.md

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,6 +549,41 @@ docker compose exec app php occ user_oidc:provider keycloak
549549
- `docs/ADR-002-vector-sync-authentication.md` - Offline access architecture
550550
- `docs/keycloak-multi-client-validation.md` - Realm-level validation
551551

552+
### LDAP Testing (GH #980 reproduction)
553+
554+
The `ldap` lane reproduces GH #980 (DAV paths built from the loginName instead
555+
of the canonical Nextcloud UID). It runs an OpenLDAP server (`vegardit/openldap`)
556+
whose user `alice` logs in as `alice` but is mapped by `user_ldap` to a UID
557+
derived from the LDAP `entryUUID` — so `loginName != UID` **and**
558+
`/remote.php/dav/files/alice/` does not resolve to her real home. This is the
559+
only backend that reproduces #980 live: login-by-email resolves to the real home
560+
(email is a path alias) and `user_oidc` hardcodes `loginName == UID`.
561+
562+
The reproduction test drives the **multi-user BasicAuth** MCP service (port 8003)
563+
as `alice`, so no browser is needed.
564+
565+
**Setup**:
566+
```bash
567+
# openldap (ldap profile) + the multi-user-basic MCP service (port 8003).
568+
# user_ldap is auto-configured by app-hooks/post-installation/15-setup-ldap-backend.sh
569+
# (gated on the openldap service; a no-op for every other profile).
570+
docker compose --profile ldap --profile multi-user-basic up --build -d
571+
uv run pytest -m ldap -v # xfails until #980's client fix lands
572+
```
573+
574+
> The post-installation hook only runs on a **fresh** Nextcloud install. If you
575+
> add the `ldap` profile to an already-installed dev stack, run the hook by hand:
576+
> `docker compose exec app bash /docker-entrypoint-hooks.d/post-installation/15-setup-ldap-backend.sh`
577+
578+
**Credentials**: LDAP admin `uid=admin,dc=example,dc=org` / `ldap_admin_pw`;
579+
user `alice` / `AlicePass123!` (see `ldap/bootstrap.ldif`).
580+
581+
**xfail note**: `tests/server/ldap/test_ldap_dav_principal.py` is
582+
`xfail(strict=True)` — it fails (RED) on `master` because the bug is present, and
583+
xpasses (GREEN) once #980's `BaseNextcloudClient._ensure_principal_id` discovery
584+
lands. `strict=True` turns the unexpected pass into a failure, signalling that
585+
the marker should be dropped when #980 merges.
586+
552587
## Integration Testing with Docker
553588

554589
**Nextcloud**: `docker compose exec app php occ ...` for occ commands
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
#!/bin/bash
2+
#
3+
# Configure Nextcloud's user_ldap backend against the `openldap` service.
4+
#
5+
# Runs only when the `ldap` docker-compose profile is active (the `openldap`
6+
# hostname resolves). Sets up a single LDAP config so the seeded user `alice`
7+
# logs in as `alice` but is mapped to a canonical UID derived from her LDAP
8+
# entryUUID — the divergent loginName/UID condition that reproduces GH #980.
9+
#
10+
# Idempotent: reuses an existing config id if present, else creates one.
11+
#
12+
13+
set -e
14+
15+
echo "===================================================================="
16+
echo "Configuring user_ldap backend for OpenLDAP..."
17+
echo "===================================================================="
18+
19+
# Quick check: is the openldap service in the Docker network?
20+
# When the `ldap` profile is not active, this hostname won't resolve, so the
21+
# hook is a no-op for every other lane.
22+
if ! getent hosts openldap >/dev/null 2>&1; then
23+
echo " OpenLDAP service not detected in Docker network (ldap profile not active)"
24+
echo " Skipping user_ldap configuration"
25+
exit 0
26+
fi
27+
28+
php /var/www/html/occ app:enable user_ldap
29+
30+
# Reuse an existing config id (idempotent re-run), else create a fresh one.
31+
# The trailing `|| true` guards the whole command substitution under `set -e`
32+
# (grep exits non-zero when no config exists yet on a fresh install); keep it on
33+
# this pipeline if refactoring.
34+
CONFIG_ID=$(php /var/www/html/occ ldap:show-config 2>/dev/null \
35+
| grep -E '^\| Configuration' | grep -oE 's[0-9]+' | head -n1 || true)
36+
if [ -z "$CONFIG_ID" ]; then
37+
CONFIG_ID=$(php /var/www/html/occ ldap:create-empty-config | grep -oE 's[0-9]+' | head -n1)
38+
fi
39+
echo "Using user_ldap config: $CONFIG_ID"
40+
41+
set_cfg() { php /var/www/html/occ ldap:set-config "$CONFIG_ID" "$1" "$2" >/dev/null; }
42+
43+
# Connection (openldap = internal docker hostname; admin bind = uid=admin).
44+
set_cfg ldapHost openldap
45+
set_cfg ldapPort 389
46+
set_cfg ldapAgentName "uid=admin,dc=example,dc=org"
47+
set_cfg ldapAgentPassword "ldap_admin_pw"
48+
set_cfg ldapBase "dc=example,dc=org"
49+
set_cfg ldapBaseUsers "ou=people,dc=example,dc=org"
50+
# Filters + attribute mapping. Leaving the internal-username attribute at its
51+
# default makes Nextcloud derive the UID from entryUUID (below), so loginName
52+
# (uid=alice) differs from the canonical UID — the point of this lane.
53+
set_cfg ldapUserFilter "(objectClass=inetOrgPerson)"
54+
set_cfg ldapLoginFilter "(&(objectClass=inetOrgPerson)(uid=%uid))"
55+
set_cfg ldapUserDisplayName "cn"
56+
set_cfg ldapEmailAttribute "mail"
57+
set_cfg ldapExpertUUIDUserAttr "entryUUID"
58+
set_cfg ldapTLS 0
59+
set_cfg turnOffCertCheck 1
60+
set_cfg ldapConfigurationActive 1
61+
62+
# Wait for OpenLDAP to answer by retrying the connection test itself — no extra
63+
# client tooling needed in the app image. This doubles as validation.
64+
echo "Verifying LDAP connection..."
65+
MAX_RETRIES=30
66+
RETRY_COUNT=0
67+
while [ $RETRY_COUNT -lt $MAX_RETRIES ]; do
68+
if php /var/www/html/occ ldap:test-config "$CONFIG_ID" 2>/dev/null | grep -q "valid"; then
69+
echo "===================================================================="
70+
echo "✓ user_ldap backend configured ($CONFIG_ID)"
71+
echo "===================================================================="
72+
exit 0
73+
fi
74+
echo " Waiting for OpenLDAP... (attempt $((RETRY_COUNT + 1))/$MAX_RETRIES)"
75+
sleep 5
76+
RETRY_COUNT=$((RETRY_COUNT + 1))
77+
done
78+
79+
echo "⚠ Warning: LDAP connection could not be verified after $MAX_RETRIES attempts"
80+
php /var/www/html/occ ldap:test-config "$CONFIG_ID" || true
81+
exit 0

docker-compose.yml

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,50 @@ services:
445445
profiles:
446446
- mail
447447

448+
# OpenLDAP backend for the `ldap` lane — reproduces GH #980 (DAV paths built
449+
# from loginName instead of the canonical Nextcloud UID). Nextcloud's
450+
# user_ldap backend derives an internal UID from the LDAP UUID that differs
451+
# from the login (`alice`), which single-user / user_oidc / login-by-email
452+
# backends can't produce. Bootstrapped fresh on every `up` (no data volume),
453+
# so the custom LDIF is always applied.
454+
openldap:
455+
image: vegardit/openldap:2.6.10@sha256:f722617af8118ea9a740ebc138bdc188fd8d5e963c88e3eacf6e07e3456c88f4
456+
environment:
457+
- LDAP_INIT_ORG_DN=dc=example,dc=org
458+
- LDAP_INIT_ORG_NAME=Example Org
459+
- LDAP_INIT_ROOT_USER_PW=ldap_admin_pw
460+
# Plain LDAP on 389 is enough for the internal test network; skip the
461+
# self-signed LDAPS setup so user_ldap can connect without TLS fuss.
462+
- LDAP_LDAPS_ENABLED=false
463+
volumes:
464+
# Seed ou=people + the divergent `alice` user (evaluated on first launch;
465+
# no data volume, so it re-seeds fresh on every `up`).
466+
- ./ldap/bootstrap.ldif:/opt/ldifs/init_org_entries.ldif:ro
467+
ports:
468+
- 127.0.0.1:1389:389
469+
# Searching for the seeded `alice` entry (not just a port probe) means
470+
# "healthy" == "bootstrap LDIF applied" — the app-hook can rely on that.
471+
healthcheck:
472+
test:
473+
[
474+
"CMD",
475+
"ldapsearch",
476+
"-x",
477+
"-H",
478+
"ldap://localhost",
479+
"-b",
480+
"uid=alice,ou=people,dc=example,dc=org",
481+
"-D",
482+
"uid=admin,dc=example,dc=org",
483+
"-w",
484+
"ldap_admin_pw",
485+
]
486+
interval: 10s
487+
timeout: 5s
488+
retries: 30
489+
profiles:
490+
- ldap
491+
448492
volumes:
449493
nextcloud:
450494
db:

ldap/bootstrap.ldif

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Custom bootstrap entries for the OpenLDAP `ldap` compose profile.
2+
# Loaded by osixia/openldap into the seeded dc=example,dc=org tree.
3+
#
4+
# The single user `alice` is the divergent-principal fixture for reproducing
5+
# GH #980: her LDAP login is `alice`, but Nextcloud's user_ldap backend derives
6+
# a DIFFERENT canonical internal UID (from the UUID attribute), so DAV paths
7+
# built from the loginName (`/remote.php/dav/files/alice/`) miss her real home
8+
# at `/remote.php/dav/files/<internal-uid>/`.
9+
#
10+
# userPassword is a deterministic SSHA hash of "AlicePass123!" (salt "ncmcp980")
11+
# so this file stays reproducible across rebuilds.
12+
13+
dn: ou=people,dc=example,dc=org
14+
objectClass: organizationalUnit
15+
ou: people
16+
17+
dn: uid=alice,ou=people,dc=example,dc=org
18+
objectClass: inetOrgPerson
19+
objectClass: posixAccount
20+
cn: Alice Example
21+
sn: Example
22+
uid: alice
23+
uidNumber: 10001
24+
gidNumber: 10001
25+
homeDirectory: /home/alice
26+
mail: alice@ldap.example.org
27+
userPassword: {SSHA}PD7foplU4ZMYi7ENTdbi3i+sG79uY21jcDk4MA==

pyproject.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ markers = [
8080
"keycloak: OAuth tests that utilize keycloak external identity provider",
8181
"login_flow: Login Flow v2 integration tests (ADR-022)",
8282
"multi_user_basic: Multi-user BasicAuth pass-through tests (ADR-020)",
83+
"ldap: LDAP-backend tests reproducing divergent loginName/UID DAV paths (GH #980)",
8384
"postgres: Tests requiring the docker-compose postgres-test service (ADR-026)",
8485
"contract: Pact consumer/provider contract tests (ADR-029)",
8586
]

tests/server/ldap/__init__.py

Whitespace-only changes.

tests/server/ldap/conftest.py

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
"""Fixtures for the LDAP lane — reproduces GH #980 (divergent loginName/UID).
2+
3+
The `ldap` docker-compose profile runs an OpenLDAP server whose user `alice`
4+
logs in as `alice` but is mapped by Nextcloud's `user_ldap` backend to a
5+
canonical internal UID derived from the LDAP UUID (e.g.
6+
`c2c0a34c-09dd-1041-...`). That divergence (`loginName != UID`) is the exact
7+
shape of GH #980 that single-user, `user_oidc`, and login-by-email backends
8+
cannot produce — and, unlike login-by-email, the LDAP login is NOT a valid
9+
files-path alias, so `/remote.php/dav/files/alice/` genuinely misses the real
10+
home at `/remote.php/dav/files/<uid>/`.
11+
12+
These tests drive the **multi-user BasicAuth** MCP service (port 8003): the
13+
client sends `alice`'s LDAP credentials in the Authorization header, so the
14+
server builds DAV paths from the loginName `alice`. Without the #980 client fix
15+
(`BaseNextcloudClient._ensure_principal_id`) every DAV op targets the wrong,
16+
non-existent home and fails; with it, current-user-principal discovery resolves
17+
the real UID.
18+
"""
19+
20+
import base64
21+
from typing import Any, AsyncGenerator
22+
23+
import pytest
24+
from mcp import ClientSession
25+
26+
from tests.conftest import create_mcp_client_session
27+
28+
# The multi-user BasicAuth MCP service (ADR-020) — builds the Nextcloud client
29+
# per request from the BasicAuth username, i.e. the loginName `alice`.
30+
MULTI_USER_BASIC_MCP_URL = "http://localhost:8003/mcp"
31+
32+
# Divergent LDAP user seeded by ldap/bootstrap.ldif.
33+
LDAP_USERNAME = "alice"
34+
LDAP_PASSWORD = "AlicePass123!" # NOSONAR(S2068) - dev-only LDAP fixture credential
35+
36+
37+
@pytest.fixture
38+
async def nc_mcp_ldap_alice_client(
39+
anyio_backend,
40+
) -> AsyncGenerator[ClientSession, Any]:
41+
"""MCP session authenticated as the divergent LDAP user `alice` (port 8003).
42+
43+
Connects to the multi-user BasicAuth service with `alice`'s LDAP credentials
44+
so the server constructs DAV paths from her loginName (`alice`), not her
45+
canonical UID — the condition GH #980 fixes.
46+
"""
47+
credentials = base64.b64encode(f"{LDAP_USERNAME}:{LDAP_PASSWORD}".encode()).decode()
48+
async with create_mcp_client_session(
49+
url=MULTI_USER_BASIC_MCP_URL,
50+
headers={"Authorization": f"Basic {credentials}"},
51+
client_name="LDAP alice (multi-user-basic)",
52+
) as session:
53+
yield session

0 commit comments

Comments
 (0)