Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 31 additions & 2 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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: |
Expand Down Expand Up @@ -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
Expand Down
35 changes: 35 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
81 changes: 81 additions & 0 deletions app-hooks/post-installation/15-setup-ldap-backend.sh
Original file line number Diff line number Diff line change
@@ -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

Check failure on line 36 in app-hooks/post-installation/15-setup-ldap-backend.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.

See more on https://sonarcloud.io/project/issues?id=cbcoutinho_nextcloud-mcp-server&issues=AZ8foKLQqjSA4Nw6vw_0&open=AZ8foKLQqjSA4Nw6vw_0&pullRequest=997
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; }

Check warning on line 41 in app-hooks/post-installation/15-setup-ldap-backend.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Assign this positional parameter to a local variable.

See more on https://sonarcloud.io/project/issues?id=cbcoutinho_nextcloud-mcp-server&issues=AZ8foKLQqjSA4Nw6vw_1&open=AZ8foKLQqjSA4Nw6vw_1&pullRequest=997

Check warning on line 41 in app-hooks/post-installation/15-setup-ldap-backend.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Assign this positional parameter to a local variable.

See more on https://sonarcloud.io/project/issues?id=cbcoutinho_nextcloud-mcp-server&issues=AZ8foKLQqjSA4Nw6vw_2&open=AZ8foKLQqjSA4Nw6vw_2&pullRequest=997

# 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

Check failure on line 67 in app-hooks/post-installation/15-setup-ldap-backend.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Use '[[' instead of '[' for conditional tests. The '[[' construct is safer and more feature-rich.

See more on https://sonarcloud.io/project/issues?id=cbcoutinho_nextcloud-mcp-server&issues=AZ8foKLQqjSA4Nw6vw_3&open=AZ8foKLQqjSA4Nw6vw_3&pullRequest=997
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 "===================================================================="

Check warning on line 71 in app-hooks/post-installation/15-setup-ldap-backend.sh

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Define a constant instead of using the literal '====================================================================' 4 times.

See more on https://sonarcloud.io/project/issues?id=cbcoutinho_nextcloud-mcp-server&issues=AZ8foKLQqjSA4Nw6vw_4&open=AZ8foKLQqjSA4Nw6vw_4&pullRequest=997
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
44 changes: 44 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
27 changes: 27 additions & 0 deletions ldap/bootstrap.ldif
Original file line number Diff line number Diff line change
@@ -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/<internal-uid>/`.
#
# 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==
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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)",
]
Expand Down
Empty file added tests/server/ldap/__init__.py
Empty file.
53 changes: 53 additions & 0 deletions tests/server/ldap/conftest.py
Original file line number Diff line number Diff line change
@@ -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/<uid>/`.

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
Loading