This is an automated email from the ASF dual-hosted git repository. wu-sheng pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/skywalking-horizon-ui.git
commit 1afc29166379cb33f697826d69cca33d43f26e09 Author: Wu Sheng <[email protected]> AuthorDate: Thu May 21 17:25:18 2026 +0800 ldap: resolve groups with the service bind, not the user's credentials The login path searched groups bound as the just-authenticated user, but directories commonly deny regular users read access to the group subtree (OpenLDAP default ACLs), so every login silently collapsed to the `*` fallback role while the admin resolver (service bind) showed the correct roles — they disagreed. Group resolution now uses the service bind, consistent with the resolver. Verified against a test OpenLDAP: admin/operator/maintainer/viewer resolve their mapped roles. Adds the local-boot LDAP test config + seed (osixia OpenLDAP, role-named accounts mirroring horizon.local.yaml) and a SKILL.md test section. --- .claude/skills/local-boot/SKILL.md | 45 ++++++++++ .claude/skills/local-boot/horizon.ldap.yaml | 125 ++++++++++++++++++++++++++++ .claude/skills/local-boot/ldap-seed.ldif | 50 +++++++++++ apps/bff/src/user/ldap.ts | 25 ++++-- 4 files changed, 239 insertions(+), 6 deletions(-) diff --git a/.claude/skills/local-boot/SKILL.md b/.claude/skills/local-boot/SKILL.md index 62f528a..1c16348 100644 --- a/.claude/skills/local-boot/SKILL.md +++ b/.claude/skills/local-boot/SKILL.md @@ -98,6 +98,51 @@ When invoked as an agent and `OAP_PASSWORD` is not already set, ask the developer for it (do not guess, do not hardcode it into a file). The OAP network username is fixed as `admin` in `horizon.demo.yaml`. +## Boot against an LDAP directory (test) + +`horizon.ldap.yaml` points the BFF at a throwaway OpenLDAP seeded from +`ldap-seed.ldif`. Stand the directory up, seed it, then boot: + +```bash +REPO="$(git rev-parse --show-toplevel)" +docker rm -f horizon-ldap 2>/dev/null +docker run -d --name horizon-ldap -p 389:389 -p 636:636 \ + --env LDAP_ORGANISATION="Horizon Test" --env LDAP_DOMAIN="horizon.test" \ + --env LDAP_ADMIN_PASSWORD="admin" osixia/openldap:1.5.0 +# wait for slapd, then seed: +until docker exec horizon-ldap ldapwhoami -x -H ldap://localhost \ + -D "cn=admin,dc=horizon,dc=test" -w admin >/dev/null 2>&1; do sleep 1; done +docker cp "$REPO/.claude/skills/local-boot/ldap-seed.ldif" horizon-ldap:/tmp/seed.ldif +docker exec horizon-ldap ldapadd -x -H ldap://localhost \ + -D "cn=admin,dc=horizon,dc=test" -w admin -f /tmp/seed.ldif + +pkill -f "tsx watch src/server.ts" 2>/dev/null; pkill -f "tsx/dist/cli.mjs watch" 2>/dev/null +until ! lsof -nP -iTCP:8081 -sTCP:LISTEN >/dev/null 2>&1; do sleep 1; done +HORIZON_CONFIG="$REPO/.claude/skills/local-boot/horizon.ldap.yaml" \ + pnpm --filter @skywalking-horizon-ui/bff run dev & +``` + +Test accounts (seeded by `ldap-seed.ldif`). Login users are named after +their role and **password == username**, mirroring `horizon.local.yaml`: + +| Login | Group | Role | +|---|---|---| +| `admin` | cn=horizon-admin | admin | +| `operator` | cn=sre | operator | +| `maintainer` | cn=platform | maintainer | +| `viewer` | (none) | viewer (`*` fallback) | + +Directory bind account: `cn=admin,dc=horizon,dc=test` / `admin` (override via `LDAP_BIND_PW`). + +```bash +# verify a login resolves the expected role: +curl -s --noproxy '*' -H 'Content-Type: application/json' -X POST \ + http://127.0.0.1:8081/api/auth/login -d '{"username":"admin","password":"admin"}' +``` + +Note: group resolution runs on the **service bind**, not the user's +credentials (regular users usually can't read the group subtree). + ## Verify the BFF is healthy (no browser) ```bash diff --git a/.claude/skills/local-boot/horizon.ldap.yaml b/.claude/skills/local-boot/horizon.ldap.yaml new file mode 100644 index 0000000..6c9e0d2 --- /dev/null +++ b/.claude/skills/local-boot/horizon.ldap.yaml @@ -0,0 +1,125 @@ +# LDAP-backend boot config for the Horizon UI BFF. Authenticates against +# an external directory; Horizon stores no passwords. This file targets +# the throwaway test OpenLDAP described in SKILL.md ("Test the LDAP +# backend"): base dc=horizon,dc=test, users under ou=people, groups +# (groupOfNames) under ou=groups, seeded from ldap-seed.ldif. +# +# Test directory accounts (all seeded by ldap-seed.ldif). Login users +# are named after their role and password == username, mirroring +# horizon.local.yaml: +# directory admin (bindDn): cn=admin,dc=horizon,dc=test / admin +# login users (password == username): +# admin -> cn=horizon-admin -> role admin +# operator -> cn=sre -> role operator +# maintainer -> cn=platform -> role maintainer +# viewer -> (no group) -> role viewer (the "*" fallback) +# +# The directory bind password is read from ${LDAP_BIND_PW} (defaults to +# the test value `admin`); for a real directory, export LDAP_BIND_PW and +# never commit it. +# +# OAP still points at a local OAP so the rest of the app works. Boot via +# the local-boot skill with an ABSOLUTE HORIZON_CONFIG path. + +server: + host: 127.0.0.1 + port: 8081 + +oap: + queryUrl: http://localhost:12800 + adminUrl: http://localhost:12800 + zipkinUrl: http://localhost:9412/zipkin + timeoutMs: 15000 + +auth: + backend: ldap + ldap: + url: ldap://localhost:389 + bindDn: "cn=admin,dc=horizon,dc=test" + bindPassword: "${LDAP_BIND_PW:admin}" + userBaseDn: "ou=people,dc=horizon,dc=test" + userFilter: "(uid={username})" + displayNameAttr: cn + # This test directory uses groupOfNames groups without a memberOf + # overlay, so resolve membership by searching ou=groups for the + # user's DN. Switch to `memberOf` against directories that populate it. + groupStrategy: search + groupBaseDn: "ou=groups,dc=horizon,dc=test" + memberAttr: member + timeoutMs: 5000 + tlsInsecure: false + groupMappings: + - { group: "cn=horizon-admin,ou=groups,dc=horizon,dc=test", role: admin } + - { group: "cn=sre,ou=groups,dc=horizon,dc=test", role: operator } + - { group: "cn=platform,ou=groups,dc=horizon,dc=test", role: maintainer } + - { group: "*", role: viewer } + +rbac: + enabled: true + roles: + viewer: + - "metrics:read" + - "alarms:read" + - "traces:read" + - "logs:read" + - "topology:read" + - "profile:read" + - "overview:read" + maintainer: + - "metrics:read" + - "alarms:read" + - "traces:read" + - "logs:read" + - "topology:read" + - "profile:read" + - "overview:read" + - "cluster:read" + - "inspect:read" + - "ttl:read" + - "config:read" + operator: + - "metrics:read" + - "alarms:read" + - "traces:read" + - "logs:read" + - "topology:read" + - "profile:read" + - "cluster:read" + - "inspect:read" + - "ttl:read" + - "config:read" + - "overview:read" + - "overview:write" + - "setup:read" + - "setup:write" + - "dashboard:read" + - "dashboard:write" + - "alarm-setup:read" + - "alarm-setup:write" + - "alarm-rule:read" + - "alarm-rule:write" + - "rule:read" + - "rule:write" + - "rule:write:structural" + - "rule:delete" + - "rule:debug" + - "live-debug:read" + - "live-debug:write" + - "profile:enable" + admin: + - "*" + landingByRole: + viewer: / + maintainer: /operate/cluster + operator: / + admin: / + +session: + ttlMinutes: 60 + cookieName: horizon_sid + cookieSecure: false + +audit: { file: ./horizon-audit.jsonl } +setup: { file: ./horizon-setup.json } +alarms: { file: ./horizon-alarms.json } +debugLog: { enabled: false, file: ./horizon-wire.jsonl, maxBodyChars: 8192, redactAuthHeaders: true } diff --git a/.claude/skills/local-boot/ldap-seed.ldif b/.claude/skills/local-boot/ldap-seed.ldif new file mode 100644 index 0000000..f6714fa --- /dev/null +++ b/.claude/skills/local-boot/ldap-seed.ldif @@ -0,0 +1,50 @@ +dn: ou=people,dc=horizon,dc=test +objectClass: organizationalUnit +ou: people + +dn: ou=groups,dc=horizon,dc=test +objectClass: organizationalUnit +ou: groups + +dn: uid=admin,ou=people,dc=horizon,dc=test +objectClass: inetOrgPerson +uid: admin +cn: Admin User +sn: Admin +userPassword: admin + +dn: uid=operator,ou=people,dc=horizon,dc=test +objectClass: inetOrgPerson +uid: operator +cn: Operator User +sn: Operator +userPassword: operator + +dn: uid=maintainer,ou=people,dc=horizon,dc=test +objectClass: inetOrgPerson +uid: maintainer +cn: Maintainer User +sn: Maintainer +userPassword: maintainer + +dn: uid=viewer,ou=people,dc=horizon,dc=test +objectClass: inetOrgPerson +uid: viewer +cn: Viewer User +sn: Viewer +userPassword: viewer + +dn: cn=horizon-admin,ou=groups,dc=horizon,dc=test +objectClass: groupOfNames +cn: horizon-admin +member: uid=admin,ou=people,dc=horizon,dc=test + +dn: cn=sre,ou=groups,dc=horizon,dc=test +objectClass: groupOfNames +cn: sre +member: uid=operator,ou=people,dc=horizon,dc=test + +dn: cn=platform,ou=groups,dc=horizon,dc=test +objectClass: groupOfNames +cn: platform +member: uid=maintainer,ou=people,dc=horizon,dc=test diff --git a/apps/bff/src/user/ldap.ts b/apps/bff/src/user/ldap.ts index 36981dc..b018d51 100644 --- a/apps/bff/src/user/ldap.ts +++ b/apps/bff/src/user/ldap.ts @@ -153,6 +153,10 @@ export async function verifyLdapCredentials( username: string, password: string, ): Promise<LdapVerified | null> { + // Service-bound client: used for the user search AND (for the `search` + // group strategy) the group lookup. Kept open until after group + // resolution — see the note below on why groups are NOT resolved with + // the user's own credentials. const searchClient = clientFor(cfg); let user: FoundUser | null = null; try { @@ -165,32 +169,41 @@ export async function verifyLdapCredentials( await safeUnbind(searchClient); return null; } - await safeUnbind(searchClient); - if (!user) return null; + if (!user) { + await safeUnbind(searchClient); + return null; + } - // The actual password check — bind as the user. + // The actual password check — bind as the user on a separate client. const userClient = clientFor(cfg); try { await userClient.bind(user.dn, password); } catch { // Wrong password / locked account / disabled user → bind 49. await safeUnbind(userClient); + await safeUnbind(searchClient); return null; } + await safeUnbind(userClient); - // Resolve group memberships using the credentials we already have. + // Resolve group memberships with the SERVICE bind, not the user's + // credentials: directories commonly deny regular users read access to + // the group subtree (OpenLDAP's default ACLs hide it), which would + // silently yield zero groups and collapse every login to the `*` + // fallback role. Using the service bind also keeps login consistent + // with the Auth Status username resolver (resolveLdapUser). let groups: string[] = []; try { if (cfg.groupStrategy === 'memberOf') { groups = user.memberOfFromEntry; } else { - groups = await searchGroupsByMember(userClient, cfg, user.dn); + groups = await searchGroupsByMember(searchClient, cfg, user.dn); } } catch (err) { logger.warn({ err: errMsg(err), dn: user.dn }, 'ldap group resolution failed'); groups = []; } - await safeUnbind(userClient); + await safeUnbind(searchClient); const roles = mapGroupsToRoles(cfg, groups); if (roles.length === 0) {
